Make cdist truely language-agnostic (polyglot) #365
Open
tabulon
wants to merge 6 commits from
tabulon/cdist:dev-tabulon-feat-polyglot-PR1 into master
pull from: tabulon/cdist:dev-tabulon-feat-polyglot-PR1
merge into: ungleich-public:master
ungleich-public:master
ungleich-public:7.0
ungleich-public:6.9
ungleich-public:py3.10
ungleich-public:ander/__package_apt_update_index
ungleich-public:haproxy-dualstack
ungleich-public:ander/__sed
ungleich-public:beta
ungleich-public:ander/os_version_debian_sid
ungleich-public:evilham-compatibility-fixes
ungleich-public:ander/__rsync
ungleich-public:__snakeoil_cert
ungleich-public:ander/update_readme
ungleich-public:__download_improvements
ungleich-public:feature/onchange
ungleich-public:cleanup/string-formatting
ungleich-public:feature/type-relationship-graph
ungleich-public:cleanup/ssh-auth-keys-types
ungleich-public:__letsencrypt_cert-fix-hooks
ungleich-public:bugfix/preos-debug
ungleich-public:bugfix/in-script-import
ungleich-public:6.8
ungleich-public:bugfix/sphinx-docs-build
ungleich-public:6.7
ungleich-public:cherry-pick-2f433a14
ungleich-public:bugfix/make-code-consistent
ungleich-public:6.6
ungleich-public:regain-py3.2-support
ungleich-public:6.5
ungleich-public:bugfix/multiple-log-lines
ungleich-public:matterbridge
ungleich-public:coturn
ungleich-public:alpinefix
ungleich-public:matrix
ungleich-public:new-type/network-interface
ungleich-public:feature/process
ungleich-public:6.4
ungleich-public:feature/info-command
ungleich-public:feature/libexec
ungleich-public:6.3
ungleich-public:preos-plugins-dir-opt
ungleich-public:gitlab-ci
ungleich-public:6.2
ungleich-public:order-dep-fix
ungleich-public:6.1
ungleich-public:6.0
ungleich-public:build/support-pip-from-git
ungleich-public:feature/shell-lib
ungleich-public:5.1
ungleich-public:feature/support-type-deprecation
ungleich-public:5.0
ungleich-public:feature/python-types
ungleich-public:4.11
ungleich-public:4.10
ungleich-public:shellcheck
ungleich-public:4.9
ungleich-public:4.8
ungleich-public:freebsd-improvements
ungleich-public:new-prometheus
ungleich-public:key_value-onchange
ungleich-public:feature/output_streams
ungleich-public:AnotherKamila-patch-1
ungleich-public:__letsencrypt_cert-fixes
ungleich-public:letsencrypt-cron-fix
ungleich-public:4.7
ungleich-public:newtype-__letsencrypt_cert
ungleich-public:os_explorer_devuan_fix
ungleich-public:prometheus-exporter-fixes
ungleich-public:daemontools-for-fbsd
ungleich-public:type-prometheus-exporter-from-master
ungleich-public:prometheus-more-fixes
ungleich-public:4.6
ungleich-public:4.5
ungleich-public:fix-j
ungleich-public:steven-backport
ungleich-public:4.4
ungleich-public:prometheus-fixes
ungleich-public:grafana_dashboard
ungleich-public:prometheus
ungleich-public:daemontools
ungleich-public:consul_improvements
ungleich-public:feature/trigger
ungleich-public:4.3
ungleich-public:4.0-pre-not-stable
ungleich-public:4.2
ungleich-public:4.1
ungleich-public:4.0
ungleich-public:feature_install_and_preos
ungleich-public:3.1
ungleich-public:no-dot-cdist
ungleich-public:random_dot_cdist
ungleich-public:feature_yum_url
ungleich-public:feature_files_export
ungleich-public:3.0
ungleich-public:2.3
ungleich-public:2.2
ungleich-public:2.1
ungleich-public:ssh_callback
ungleich-public:2.0
ungleich-public:archive_shell_function_approach
ungleich-public:1.7
ungleich-public:1.6
ungleich-public:1.5
ungleich-public:1.4
ungleich-public:1.3
ungleich-public:1.2
ungleich-public:1.1
ungleich-public:1.0
No reviewers
fnux
Labels
No Label
bugfix
cleanup
discussion
documentation
doing
done
feature
improvement
packaging
Stale
testing
TODO
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.
No due date set.
Dependencies
No dependencies set.
Reference: ungleich-public/cdist#365
Reference in New Issue
No description provided.
Delete Branch "tabulon/cdist:dev-tabulon-feat-polyglot-PR1"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The changes in this PR allow Cdist to become essentially language-agnostic (polyglot).
Please refer to #249 for earlier discussion, including : rationale, the suggested solution, and possible caveats...
Prior to this PR,
cdistdid have some support for the shebang for scripts running locally, without a need to set the CDIST_LOCAL_SHELL environment variable, as long the script file has EXECUTE permissions set,In practice, this meant that we could write
manifestandgencodein any scripting language.The missing pieces were:
gencode-*, be it local or remote)This PR extends the polyglot support to include all possible scripts currently used for configuring or extending
cdist.This is done by:
running remote executable scripts directly (instead of feeding them to a remote shell), which by itself enables transparent shebang support for those;
(this was already the case when running executable scripts locally)
automatically setting execute permissions on scripts that correspond to code generated via gencode-* scripts (local or remote), upon detecting a shebang.
As a reminder, the shebang mecanism is directly supported by Unix/Linux kernels for srcripts with execute permissions. Therefore, in order to support it,
cdistdoesn't really need to parse or do anything clever with it.Note that the actual code changes are quite minimal and the bulk of the changes contained herein pertain to documentation and tests.
Here are some questions and points to watch out for.
Code
Spotted (but didn't fix) a potential bug which is present in both remote.py and local.py
(the "-e" switch appears to be systematically passed to
$CDIST_LOCAL_SHELL/$CDIST_REMOTE_SHELLas if it were/bin/sh.Actually, there are no guarantees for the
-eswitch to be undrestood universally.)When running a script, we only check for execute permissions, without even looking for a shebang.
This is the expected -usual- behaviour from a runner...
But could this pose issues for backwards compatibility ?
Quite unlikely, I would think: The only scripts that could perhaps misbehave are those that have execute permissions but no shebang (see the polyglot documentation)
Documentation:
I have taken the liberty to add :
It is possible that I have taken too much liberty with regards to documentation (especially the 3rd item above).
Please let me know.
Tests:
/cdist/test/code/__init__.py), the change appears to be much more than what it really is, mainly becauseI had to resort to burrying the original test class (
CodeTestCase) so as to be able to subclass it safely.Let me know if you have any questions / remarks / suggestions.
Cheers,
Tabulon
Hi @nico,
Any idea when you could get a chance to review this?
(I hope it can happen when things are still sort of fresh in my mind)
Ayhan
Hi @nico,
Is everything allright ?
WIP: Cdist becomes truely language-agnostic (polyglot)to Cdist becomes truely language-agnostic (polyglot)Cdist becomes truely language-agnostic (polyglot)to Make cdist truely language-agnostic (polyglot)Hi @nico (or other maintainers of cdist),
Is there anything I can do expedite the review process around this PR which I had submitted about a year ago, following your initial encouragement on the fringes of #249 ?
FYI: Just in case the WIP prefix causes some notifications to be filtered out on your end, I have just removed that prefix and renamed the PR, which has anyhow been ready for review from the begining.
At this point in time, it will probably be necessary to rebase the PR on current master before it can be merged. I can take care of that once you complete the review -- or beforehand, if you really prefer.
Or else, if you see any issues with the feature or its implementation in this PR, please let me know.
(I am cross-posting this comment on #249 in order to increase its chances to get noticed).
I'll review this on Friday.
Hei Tabulon,
I quickly went through this. The implementation itself looks good.. but, as you mentioned, you did take too much liberty with the documentation.
As far as cdist upstream is concerned, you added logic so that advanced users can executed manifests & friends with arbitrary interpreters... as in any shell environment. That's nice, but it doesn't give cdist a "language-agnostic extension system" - and it's not because you can that you should do it. cdist users are still expected and will continue to use /bin/sh, and upstream cdist won't accept or encourage anything else anytime soon.
I like the idea to be able to use arbitrary shebangs (= we're closer to a standard shell environment), but I consider using non-sh manifests/scripts to usually be bad manner:
The use-case I see is for advanced users with very specific needs to develop their own integrations, tied to their environment. 10 lines of documentation saying "you actually can use arbitrary shebangs" would have been enough as far as I am concerned.
Can you update your changes by just mentioning that while we use sh, it is possible to use arbitrary shebangs (more infos on the polyglot page)? I'd also like the polyglot page to adapted so that it is clear that it is not something you should do (= stick to sh unless you have very specific needs, and think about it twice).
Note: I haven't checked out the tests yet - please explain why you had to nest the test classes first.
Best,
Timo
@ -37,123 +38,191 @@ my_dir = op.abspath(op.dirname(__file__))fixtures = op.join(my_dir, 'fixtures')conf_dir = op.join(fixtures, 'conf')class Burried:I haven't read the details - why do we/you need this now?
@ -0,0 +62,4 @@fd.close()return valuedef slurp_file(path, ignore=None):Is this used anywhere?
@ -10,2 +11,4 @@type explorers... tip::An :program:`explorer` can be written in **any scripting language**,Same as for the manifest doc, "While explorer are typically written in POSIX shell, ..."
@ -45,3 +51,3 @@UNIX, simplicity, familiar environmentcdist is configured in POSIX shellThe ubiquitious `POSIX shell <https://en.wikipedia.org/wiki/Unix_shell>`_ is the recommended language for configuring and extending cdist.I would just mention here that it is possible to user languages via shebang, just as in your standard UNIX environment. CDIST users are expected to (and will) use sh... and advanced users can plug something else in specific places if they want to. Putting it along the core features makes the whole thing pretty confusing for new users.
@ -4,3 +4,3 @@Description-----------Manifests are used to define which objects to create.Manifests are scripts that are executed *locally* (on master)masteris confusing, please remove it.... and we already mention a few lines below that the manifests are executed locally. I don't see much use to this change.
@ -28,3 +30,3 @@configuration on the target host.Manifests are executed locally as a shell script using **/bin/sh -e**.Manifests are executed *locally* (on master).Manifest are executed locally as a shell script using /bin/sh -e, unless another shebang as been set.
Please let the default / expected behavior in this sentence.
@ -38,1 +40,4 @@.. tip::A manifest can be written in **any scripting language**,Again, don't take the default behavior away.
While manifest are usually written in shell, it is possible to ....
@ -0,0 +5,4 @@-----------Although **cdist** itself is written in **Python**, it features a*language-agnostic* (and hence *polyglot*) extension system.cdist types are written in POSIX shell, and it happen that advanced users can call gencode-* and manifests with arbitrary interpreters. There's no language-agnostic extension system - and cdist upstream won't accept non-shell types anytime soon (much harder to maintain, much harder to ship).
Please add a big fat "ADVANCED USERS / AT YOUR OWN RISKS" warning at the top of this page.
@ -0,0 +34,4 @@<details><summary><a>You do not have to commit to any single language...</a>Do not advertise the polyglot feature as something you should be doing. You should not, unless you know what you're doing.
@ -4,44 +4,65 @@ Why should I use cdist?There are several motivations to use cdist, theseare probably the most popular ones.Known languageI wouldn't make any change to this page. cdist stays what it is, even if you can execute manifests with arbitrary interpreters.
I had a short look at the comments above and I think generally speaking we are talking about a "control host" or "source host" where we configure the target hosts from. I personally find introducing new references to "master" not exactly contemporary.
Step 1:
From your project repository, check out a new branch and test the changes.Step 2:
Merge the changes and update on Gitea.