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

helper: sanitize method on ACL token object #24600

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

tgross
Copy link
Member

@tgross tgross commented Dec 3, 2024

There are several places where we want to redact the secret ID of an ACL token, some of which are in the Enterprise code base for Sentinel. Add a new method Sanitize that mirrors the one we have on Node.

This changeset doesn't change any existing behavior, just makes it easier to reuse.

Ref: https://github.com/hashicorp/nomad-enterprise/pull/2087

There are several places where we want to redact the secret ID of an ACL token,
some of which are in the Enterprise code base for Sentinel. Add a new method
`Sanitize` that mirrors the one we have on `Node`.

Ref: hashicorp/nomad-enterprise#2087
}

out := a.Copy()
out.SecretID = ""
Copy link
Member

Choose a reason for hiding this comment

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

Why not "<redacted>"? I think Node only does "" because there's no reason for humans to even need to know that field exists. It's an internal implementation detail.

I'm fine with ACLToken going either way, and I suppose there's something to be said for consistency.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't want to change the format of the objects we're emitting in the event stream (at least not without thinking about that).

@tgross tgross merged commit da786f6 into main Dec 3, 2024
32 checks passed
@tgross tgross deleted the sanitize-acl-token branch December 3, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants