-
Notifications
You must be signed in to change notification settings - Fork 187
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
decryptor: improve detection of in and out formats for Secret data fields #644
Conversation
This ensures the Secret field gets formatted back into JSON, instead of it being detected as binary output. Signed-off-by: Hidde Beydals <hello@hidde.co>
551c831
to
a7639c6
Compare
ca69fb6
to
83e3e33
Compare
case strings.HasSuffix(path, corev1.DockerConfigJsonKey): | ||
return formats.Json |
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.
Perhaps we should only look for this key when the Secret's "type" field is "kubernetes.io/dockerconfigjson."
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.
Think that with the current approach, it gives people some flexibility to e.g. have opaque Secrets with multiple .dockerconfigjson
files, for whatever reason that may be required. In addition to simplifying the code :-).
83e3e33
to
ddb277d
Compare
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.
Modulo the switch from var
to const
, this would have helped me when encrypting my Docker credentials. I used sops --encrypt --input-type=json first, but found that Flux couldn't interpret the decrypted content correctly. With this patch, it looks like that would have worked as expected.
ddb277d
to
3516d3f
Compare
This checks the base64 decoded bytes from a Secret field for any of the marker bytes, thereby allowing data to be encrypted into any format. Instead of the previous behavior which assumed it to either be YAML or JSON. Signed-off-by: Hidde Beydals <hello@hidde.co>
3516d3f
to
36df540
Compare
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.
LGTM
Thanks @hiddeco 🥇
This PR improves two things:
.dockerconfigjson
field gets formatted back into JSON, instead of it being detected as binary output.Test image (multi-arch):
docker.io/hiddeco/kustomize-controller:decryptor-detect-dockercfg-83e3e33@sha256:08016777ad75813681a3