Skip to content
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

Template environment substitution (e.g. from Flux) breaks Kubernetes Secrets #687

Open
xvzf opened this issue Mar 30, 2022 · 9 comments · May be fixed by #731
Open

Template environment substitution (e.g. from Flux) breaks Kubernetes Secrets #687

xvzf opened this issue Mar 30, 2022 · 9 comments · May be fixed by #731
Labels
kind/enhancement Improve something existing

Comments

@xvzf
Copy link
Contributor

xvzf commented Mar 30, 2022

We're using Flux Substitution in combination with tk export in order to pass in some externally managed references and template secrets out of other secrets

e.g. the tanka rendered manifest

apiVersion: v1
kind: Secret
metadata:
  name: test
stringData:
  TEST_INT: ${SOME_INT}

with the variable SOME_INT="1234" would be substituted to

apiVersion: v1
kind: Secret
metadata:
  name: test
stringData:
  TEST_INT: 1234

which is not a valid Kubernetes secret. With Kustomize I used to prevent this by quoting those references:

apiVersion: v1
kind: Secret
metadata:
  name: test
stringData:
  TEST_INT: "${SOME_INT}"

Is this something worth exploring Tanka or an edge case you don't want to support? As mitigation we're now setting SOME_INT="'1234'" which is not very elegant :-)

@Duologic
Copy link
Member

Do you have some jsonnet example that i can use to reproduce?

@xvzf
Copy link
Contributor Author

xvzf commented Mar 30, 2022

sure!

local k = (import 'k.libsonnet');

{
  secret:
    k.core.v1.secret.new('test', {})
    + k.core.v1.secret.withStringData({
      TEST_INT: '${SOME_INT}',
    }),
}

renders to:

apiVersion: v1
data: {}
kind: Secret
metadata:
  name: test
  namespace: default
stringData:
  TEST_INT: ${SOME_INT}
type: Opaque

@sh0rez
Copy link
Member

sh0rez commented Mar 30, 2022

so this is really the yaml marshaler deciding it does not need to quote this, right?

seems like there is a way to enforce quoting: go-yaml/yaml#556

not sure how practical it is though, it might require traversing every single manifest looking for such replacement directives. in case it turns out too heavy for tanka, it might be feasible to handle it in a separate binary that gets the piped input from tanka though

@xvzf
Copy link
Contributor Author

xvzf commented Mar 30, 2022

@sh0rez exactly, I've been thinking about a simple sed ... in the CD step but ended up not liking this solution. Since it might be a common issue I've been thinking about a CLI flag to enforce quoting (at least for Secrets & ConfigMaps).

@xvzf
Copy link
Contributor Author

xvzf commented Mar 30, 2022

I'll create a PR the following days

@stale
Copy link

stale bot commented Apr 29, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 29, 2022
@ghostsquad
Copy link

Probably not stale. 30 days is aggressive. It sometimes takes me months to circle back to a OSS PR.

@stale stale bot removed the stale label May 2, 2022
@xvzf
Copy link
Contributor Author

xvzf commented Jul 14, 2022

Still not stale, did not find the time yet!

xvzf added a commit to xvzf/tanka that referenced this issue Jul 14, 2022
fixes grafana#687

Signed-off-by: Matthias Riegler <me@xvzf.tech>
xvzf added a commit to xvzf/tanka that referenced this issue Jul 14, 2022
fixes grafana#687

Signed-off-by: Matthias Riegler <me@xvzf.tech>
@xvzf
Copy link
Contributor Author

xvzf commented Jul 14, 2022

PR is up @sh0rez

Would be great if you can test it on your code-bases as well. I bet they are generating a bit more than ours

@zerok zerok added the kind/enhancement Improve something existing label May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improve something existing
Projects
Status: In discussion
Development

Successfully merging a pull request may close this issue.

5 participants