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

Mask the Kubernetes Secrets data from dry-run and apply logs #420

Merged
merged 2 commits into from
Sep 9, 2021

Conversation

michalschott
Copy link
Contributor

@michalschott michalschott commented Sep 2, 2021

Different approach to fix #419

@michalschott michalschott changed the title [WIP] Redact secret data. Redact secret data. Sep 2, 2021
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalschott this is good start, can you please write some tests to ensure we don't replace all the lines in the error string? For a Kustomization that includes an invalid Secret, ConfigMap and Deployment only the Secret data should be replaced.

@michalschott
Copy link
Contributor Author

michalschott commented Sep 3, 2021

Hey,

Just pushed an update / rebase. Now output looks like this:

{"level":"error","ts":"2021-09-03T12:32:09.349+0200","logger":"controller.kustomization","msg":"Reconciliation failed after 265.930013ms, next try in 15s","reconciler group":"kustomize.toolkit.fluxcd.io","reconciler kind":"Kustomization","name":"backend","namespace":"default","revision":"master/b241d5f983ace900e699cf32b398ebdf24d9a5d9","error":"apply failed: Error from server (BadRequest): error when creating \"934440a6-3b77-4597-bfdd-895d0629c44d.yaml\": Secret in version \"v1\" cannot be handled as a Secret: v1.Secret.Data:  [ ** REDACTED ** ]\n"}
{"level":"error","ts":"2021-09-03T12:32:55.272+0200","logger":"controller.kustomization","msg":"Reconciliation failed after 215.382556ms, next try in 15s","reconciler group":"kustomize.toolkit.fluxcd.io","reconciler kind":"Kustomization","name":"backend","namespace":"default","revision":"master/7cb4e2d4a9f361fd2db37e5e4805bbc192717348","error":"apply failed: Error from server (BadRequest): error when creating \"934440a6-3b77-4597-bfdd-895d0629c44d.yaml\": Secret in version \"v1\" cannot be handled as a Secret: v1.Secret.StringData:  [ ** REDACTED ** ]\n|..., bigger context ...|et-2\",\"namespace\":\"default\"},\"stringData\":{ [ ** REDACTED ** ] }\n|...\n"}

Having tests would be useful indeed, will see what can I do.

UPDATE:
Added unit test for stripSensitiveData function.

Signed-off-by: Michal Schott <michal.schott@onegini.com>
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michalschott malformed secrets will not pass validation, can you please use stripSensitiveData for the validation error also?

Signed-off-by: Michal Schott <michal.schott@onegini.com>
@michalschott
Copy link
Contributor Author

@michalschott malformed secrets will not pass validation, can you please use stripSensitiveData for the validation error also?

Good spot, actioned.

@stefanprodan stefanprodan changed the title Redact secret data. Mask the Kubernetes Secrets data from dry-run and apply logs Sep 9, 2021
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks @michalschott 🏅

@@ -77,3 +79,14 @@ func containsString(slice []string, s string) bool {
}
return false
}

func stripSensitiveData(err error) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not important for this PR, as we are foreseeing that it will be refactored soon (or handled differently), but in cases like this I would pick maskSensitiveData over the current function name.

As a strip would literally take all data out, instead of replacing (= masking) it.

@stefanprodan stefanprodan merged commit 9838b77 into fluxcd:main Sep 9, 2021
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.

Secrets included in log messages and notifications
3 participants