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

POC: attribute redactions with slog #3085

Closed
wants to merge 6 commits into from
Closed

Conversation

AustinAbro321
Copy link
Contributor

@AustinAbro321 AustinAbro321 commented Oct 9, 2024

Description

This is a POC of how attribute redactions might look with slog.

This changes two places where Zarf prints. The first is in the function debugPrintZarfState. Here is an example of what that function would print now.

state.ZarfAppliance=true state.distro=k3s state.architecture=amd64 state.storageClass=standard state.agentTLS.ca=REDACTED state.agentTLS.cert=REDACTED state.agentTLS.key=REDACTED state.gitServer.pushUsername=friend state.gitServer.pushPassword=**sanitized** state.gitServer.pullUsername=other-friend state.gitServer.pullPassword=**sanitized** state.gitServer.address="" state.registryInfo.pushUsername=friend state.registryInfo.pushPassword=**sanitized** state.registryInfo.pullUsername=other-friend state.registryInfo.pullPassword=**sanitized** state.registryInfo.address="" state.registryInfo.nodePort=0 state.registryInfo.secret=**sanitized** state.artifactServer.pushUsername="" state.artifactServer.pushToken=**sanitized** state.artifactServer.address=whatever

The second is in the function PrintCredentialUpdates which now may look like

msg="Artifact info" "URL Address.old"=whatever "URL Address.new"=cooler-address "Push Username"="(unchanged) cool-guy" "Push Token.old"=REDACTED "Push Token.new"=REDACTED

While this is clean in that any time any of the objects are printed we don't have to remember to redact, it does force us to implement LogValue functions on every object that includes a ZarfPassword sub object. This will also require us to update the object functions when we want something new on them

There is a lot of noise in the PR since I changed types. Relevant files are:

  • src/pkg/message/credentials.go
  • src/pkg/cluster/state.go
  • src/types/k8s.go

Checklist before merging

Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Copy link

netlify bot commented Oct 9, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit fc9bf6e
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6706ab159b5b2e0008f527b8

Signed-off-by: Austin Abro <[email protected]>
Copy link

codecov bot commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/internal/packager/template/template.go 60.60% <ø> (ø)
src/pkg/cluster/state.go 61.04% <100.00%> (-2.06%) ⬇️
src/pkg/message/credentials.go 0.00% <ø> (ø)
src/pkg/packager/interactive.go 0.00% <ø> (ø)
src/types/k8s.go 81.05% <100.00%> (+14.38%) ⬆️

Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
Signed-off-by: Austin Abro <[email protected]>
@mkcp
Copy link
Contributor

mkcp commented Oct 9, 2024

Nice work investigating this! I like that this offers another concrete benefit for elevating random values into their own types.

@AustinAbro321
Copy link
Contributor Author

Decided we're not going to use this strategy for now. Zarf already has sanitation logic in place, this doesn't offer enough benefit to switch that out.

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