-
Notifications
You must be signed in to change notification settings - Fork 685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Shell quote support for Jinja macros #10524
Conversation
Quote a string to safely use as in a POSIX shell. Handling quoting shell is quite hard when there is complex macro workflow. This filter allows to quote where ever variable is used. Name mirrors respective feature in ansible. usage: foo={{{ FOO | quote }}} If FOO requires additional quotes they are added, if not, they are not. Result can be like: foo=bar or foo='"bar"' Implemented using python shlex quote https://docs.python.org/3/library/shlex.html#shlex.quote
Hi @maage. Thanks for your PR. I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
Do you have an example where this could be used? |
It should be used anywhere where there is shellscript and it is not absolutely sure jinja value does not have shell metacharacters. From 10-bash.jinja bash_ensure_pam_module_options start, set used jinja variables as shell variables using quote and then use them as normal shell variables.
This pattern would clearly split parts of shell scripts where there is first part to make all jinja parameters as shell parameters and then rest of the script is just pure shellscript. For somewhat complex parts that is not so. But simple macros like And essentially any string that is not meant to be regexp but used in regexp context in shell script should be handled like:
But that would require also standardizing s delim character in sed parts etc and then ensuring it is also quoted in escape_regex. Also I think shell scripts should use
to catch any typos in variable names. Another example from: shared/templates/pam_options/ansible.template
But anyways, above should be rewritten using command + argv, example below. Also what is lacking is same stuff with ansible Like: shared/templates/rsyslog_logfiles_attributes_modify/ansible.template
Though above should be rewritten as awk / sed and use command, then there is no need to have quote. Just like this should be like:
In most cases ansible shell: tasks should be converted to one command task and then pure ansible to parse the output. |
Added documentation fragments for this and other filters. These could be improved though. I hope this was okay. |
Code Climate has analyzed commit 2a9ae08 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 71.4% (50% is the threshold). This pull request will bring the total coverage in the repository to 52.4% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Description:
Quote a string to safely use as in a POSIX shell.
Handling quoting shell is quite hard when there is complex macro
workflow. This filter allows to quote where ever variable is used.
Name mirrors respective feature in ansible.
I also sorted jinja filters alphabetically for them to be neat.
Rationale:
When ever I look at bash remediations, there is multiple ways things can go sideways if variables contain shell metacharacters. There is sometimes extra checks.
If this feature is used, it provides clear security benefit.
Review Hints:
For now there is no test suite or any template that uses this.