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

Call VDS callbacks on VaultAuth and VaultConnection changes #739

Merged
merged 2 commits into from May 14, 2024

Conversation

benashz
Copy link
Collaborator

@benashz benashz commented May 10, 2024

During some internal work we discovered that when updating a VaultAuth instance, any associated Vault Clients are pruned from the cache without notifying the VaultDynamicSecrets controller. This could potentially lead to premature revocation of secret leases, causing service disruptions for applications that rely on valid secret data.

Upon update to a VaultAuth or VaultClient instance, the CachingClientFactory will send any pruned Clients to its callbackHandler channel. Upon deletion of either resource, the Client will be removed from the cache and no callbacks will be called.

  • Update integration tests to cover this case.

@benashz benashz requested a review from a team as a code owner May 10, 2024 18:20
@benashz benashz added the bug Something isn't working label May 10, 2024
@benashz benashz added this to the v0.6.1 milestone May 10, 2024
@benashz benashz changed the title Call VDS Callbacks on VaultAuth and VaultConnection changes Call VDS callbacks on VaultAuth and VaultConnection changes May 10, 2024
@benashz benashz force-pushed the VAULT-27003/fix-vaultAuth-changes-lead-to-lease-expiry branch from 4cf9e0d to 1aaefc6 Compare May 10, 2024 18:24
@thyton
Copy link
Contributor

thyton commented May 13, 2024

LGTM. Let me know when the update for integration tests is in. I can continue the review then.

@benashz benashz requested a review from thyton May 13, 2024 16:39
@benashz
Copy link
Collaborator Author

benashz commented May 13, 2024

LGTM. Let me know when the update for integration tests is in. I can continue the review then.

Updated with bb331e3

@benashz benashz requested a review from tvoran May 14, 2024 10:38
Copy link
Contributor

@thyton thyton left a comment

Choose a reason for hiding this comment

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

LGTM. Nice tests!

@benashz benashz merged commit cd8dee6 into main May 14, 2024
38 checks passed
@benashz benashz deleted the VAULT-27003/fix-vaultAuth-changes-lead-to-lease-expiry branch May 14, 2024 13:45
@benashz benashz modified the milestones: v0.6.1, v0.7.0 May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants