-
Notifications
You must be signed in to change notification settings - Fork 502
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
use google/safetext for yaml templating. #3122
Conversation
Welcome @hoskeri! |
Hi @hoskeri. Thanks for your PR. I'm waiting for a kubernetes 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 |
Thanks for the PR. cc @saschagrunert @jeremyrickard @xmudrii @Verolop @puerco @ameukam @kubernetes/release-managers |
I'm also a bit on the fence with this change. On one side I like the additional security (preventing yaml injections), but on the other side it does not seem to be an official google project (but in the google org?). Kind is using it since the merge of kubernetes-sigs/kind#2911 |
@saschagrunert If Kind is using i guess we can follow and use it as well @hoskeri can you rebase and fix the conflicts? thanks |
548954f
to
d58b1af
Compare
/retest |
that failed cause of a timeout reading the git repo? /test pull-release-integration-test |
/hold need to fix some lints errors @hoskeri |
Done, thanks! The tests are now passing. |
I think `krel` is used only with trusted inputs. However, this should be a safe drop in replacement that blocks yaml injection attacks via package metadata templates.
d58b1af
to
4543046
Compare
4543046
to
842ec8c
Compare
fixed! |
842ec8c
to
aa6a4bf
Compare
/retest |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hoskeri, saschagrunert, xmudrii The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@cpanato I think the hold can be removed: I've fixed the linter errors. |
/hold cancel |
I think
krel
is used only with trusted inputs.However, this should be a safe drop in replacement that blocks yaml injection attacks via package metadata templates.
/kind cleanup
What this PR does / why we need it:
use google/safetext (a drop-in replacement for text/template) to generate yaml.
This prevents certain yaml injection attacks.
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?