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

Update secret version hashing algorithm #198

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

tomhjp
Copy link
Contributor

@tomhjp tomhjp commented Mar 24, 2023

Updates the version generation algorithm to include an HMAC key. On each mount request, the provider will try to read the HMAC key from a Kubernetes secret and race to create it if not found. This ensures each provider produces consistent versions, and also makes recovering from unexpected errors easy (an admin just deletes the secret) without introducing the complexity and overhead of leader elections.

The PR also makes generating versions best-effort. If we can't use an HMAC key, we log a warning and revert to our pre-1.2.0 behaviour of not reporting a version at all, as consistency and reliability seem much more important than accurate version reporting. If versions thrash about unnecessarily it will cause lots of thrashing for any systems that observe the version.

Sorry for the size. I also made a feature to specify custom metadata for the created secret in 4b422ed, but felt it wasn't quite straightforward enough so took that feature out for the initial PR to keep the size lower. The idea was to replicate the pod's metadata, but there are certain pod-specific labels you'd want to filter out like controller-revision-hash and pod-template-generation. I'll try to refine that idea and follow up in another PR.

@tomhjp tomhjp requested review from swenson and tvoran March 24, 2023 14:23
Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

LGTM; one suggestion

internal/provider/provider.go Outdated Show resolved Hide resolved
Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

The reactor usage in the tests is really nice!

main.go Outdated Show resolved Hide resolved
@tomhjp tomhjp merged commit b0ab1fe into main Mar 30, 2023
@tomhjp tomhjp deleted the vault-14736/version-hash-algorithm-update branch March 30, 2023 13:56
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.

3 participants