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

Diff secrets #5197

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Diff secrets #5197

wants to merge 3 commits into from

Conversation

bkreitch
Copy link
Contributor

@bkreitch bkreitch commented Feb 17, 2025

This PR will add --decrypt-secrets flag to decrypt SOPS secrets in diff command so the changes could be detected also in values:

► Secret/default/podinfo-token-77t89m9b67 drifted

data.token
  ± value change
    - *** (before)
    + *** (after)

Changes in metadata like replicator annotations, etc are also detected since decrypted secrets are compared in full and not only by keys. The output of diff command is still being sanitized by ssa package, so the actual values are not shown. Build command is not affected and it still masks secrets in the output.

It uses the key from decryption.secretRef of the Kustomization and depends on decrypt package from kustomize-controller being exported or maybe moved to pkg repo? So till then it won't compile.

I've tried it with age, gpg and hcvault keys/tokens. Currently diff command will give an error if --decrypt-secrets flag is used but SOPS secret cannot be decrypted. This could be changed to still pass encrypted secret with addition of another flag like --strict-decrypt to control the behavior.

There is potential issue with GPG keys since they are being imported into temporary directory on disk and if application is killed the key may remain there. I can think of two options to approach this. First is to add HasGPG() function that will return true if there is GPG key in SOPS secret and then application will refuse to proceed with --decrypt-secrets flag. Or add an option to filter out GPG keys in decryptor.ImportKeys() before calling d.gnuPGHome.Import().

WDYT?

@bkreitch bkreitch marked this pull request as ready for review February 19, 2025 16:30
Signed-off-by: Boris Kreitchman <[email protected]>
Signed-off-by: Boris Kreitchman <[email protected]>
Signed-off-by: Boris Kreitchman <[email protected]>
@stefanprodan
Copy link
Member

It uses the key from decryption.secretRef of the Kustomization and depends on decrypt package

We can't have the CLI import a controller main package, we need to extract the decryptor to fluxcd/pkg, refactor kustomize-controller then we can use it here. This will need to wait for Flux 2.6 if we decide we want this.

There is potential issue with GPG keys since they are being imported into temporary directory on disk

Storing the GPG keys on disk would result in a CVE in Flux, I'm not for doing this. Also what happens with KMS encrypted secrets?

@bkreitch
Copy link
Contributor Author

Also what happens with KMS encrypted secrets?

@stefanprodan With AWS KMS - if user running diff has access to the KMS key (via role or creds in env vars or .aws/credentials) then secret is decrypted and compared. Or if there are AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY in sops.aws-kms field of secret in decryption.secretRef.name then these are used instead. So there shouldn't be issues here. I suppose the situation is similar with GCP and Azure, but I can check if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants