-
Notifications
You must be signed in to change notification settings - Fork 186
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
Add support for generating files from command output #223
base: master
Are you sure you want to change the base?
Conversation
Hello @jekkel, thanks for the project! Does this feature make sense to you? Would it be possible to integrate it into the upcoming release? Best, |
if content_type == CONTENT_TYPE_BASE64_BINARY: | ||
file_data = base64.b64decode(content) | ||
else: | ||
file_data = content | ||
|
||
if full_filename.endswith(".url"): | ||
if full_filename.endswith(".url") and not remove: |
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.
I think this change breaks removal as the returned filename would still contain the suffix .url
which is not a part of the filename. To avoid running the request (and the script further down) on removals only setting file_data
should be skipped, not filename
.
filename = full_filename[:-4] | ||
file_data = request(file_data, "GET", enable_5xx).text | ||
elif full_filename.endswith(".command") and not remove: |
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.
see comment above
rand.sh: | | ||
#!/bin/sh | ||
dd if=/dev/random bs=4 count=1 | hexdump -v -e '/1 "%02X"' | ||
random.txt.command: '/tmp/rand.sh' |
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.
I think this example is only reliable if the key-values of a configmap are actually sorted when accessed through the python client. Otherwise occasionally the command is executed before the shell script is stored. Could you double check this?
@@ -48,6 +48,7 @@ metadata: | |||
``` | |||
|
|||
If the filename ends with `.url` suffix, the content will be processed as a URL which the target file contents will be downloaded from. | |||
If the filename ends with `.command` suffix, the content will be processed as a shell command which will be executed. Stdout of the command will be stored in the file. |
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.
Could you add a small example (as yaml block) showing the 3 different variants here?
In addition we should add some words under which circumstances the script gets executed (and the http request gets made). I believe this is now "on creation and each (even unrelated) update of the containing configmap or secret".
@@ -119,6 +121,8 @@ jobs: | |||
echo -n "This absolutely exists" | diff - /tmp/absolute.txt && | |||
echo -n "This relatively exists" | diff - /tmp/relative.txt && | |||
[ ! -f /tmp/500.txt ] && echo "No 5xx file created" && | |||
echo -n "This generated by script" | diff - /tmp/command-output.txt && |
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.
Given the example script this assertions is likely to fail (as the script generates random hex text, doesn't it?)
@antonu17 Thanks for your contribution. The feature seems reasonably useful :) As executing arbitrary commands is sort of dangerous and the actual commands are now sourced from "user content" (assuming a multi-team multi-tenant k8s cluster I think we should add a feature toggle (default disabled) which can be enabled from the sidecar-administrator if deemed safe. I can imagine situations where the sidecar admin doesn't want to openly allow arbitrary commands to be executed in his pod. |
Thanks for your feedback, @jekkel. I will address all the points in a few days! |
Add support for generating files from the shell command output.
I run into a situation where I need to pull a web page and process it before storing it into a file. So the
.url
-suffix feature doesn't work for me.The idea came to implement
.command
-suffix so I can have config map keys likefile_name.command: 'curl <my_url> | sed <my_transformation>'
.