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

LIfetime Watcher Ignores "increment" when computing grace period. #14638

Closed
vlaurenzano opened this issue Mar 22, 2022 · 2 comments · Fixed by #14836
Closed

LIfetime Watcher Ignores "increment" when computing grace period. #14638

vlaurenzano opened this issue Mar 22, 2022 · 2 comments · Fixed by #14836
Labels
bug Used to indicate a potential bug core/api core/lease devex Developer Experience

Comments

@vlaurenzano
Copy link

vlaurenzano commented Mar 22, 2022

Describe the bug
A clear and concise description of what the bug is.

In lifetime_watcher the grace period is calculated with the secret's lease duration, not the given "increment" value.

r.calculateGrace(priorDuration)

This means that for long lived leases, a client cannot submit it's own increment value that is less than the leases grace period without triggering and immediate return (without sleeping).

return nil

In my case, the initial lease duration is 12 hours, but I’m trying to check the secrete every 10 minutes (in case of a revocation). The grace period for 12 hours ends up being something near 2 hours which is greater than my increment value, forcing the return before the time.After call.

Expected behavior
Grace period should be calculated using the minimum value of the lease duration and the client increment value.

Additional context
Add any other context about the problem here.

Discussion forum: https://discuss.hashicorp.com/t/vault-sdk-lifetime-watcher-client-increment-and-secret-lease-duration/36850

If there is agreement from a maintainer I'm willing to make the update.

@swayne275 swayne275 added the bug Used to indicate a potential bug label Mar 22, 2022
@swayne275
Copy link
Contributor

Thanks for raising this! This seems like a bug. We'll queue it for investigation.

@averche
Copy link
Contributor

averche commented Apr 4, 2022

Hi @vlaurenzano,

Thank you very much for opening this issue! I was able to successfully reproduce it and have a fix ready in #14836 based on your suggestions! I will run it by a few more people who are more familiar with the code and the "increment" behavior before merging. Thank you again for contributing to HashiCorp!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug core/api core/lease devex Developer Experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants