-
Notifications
You must be signed in to change notification settings - Fork 782
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
allow vault_pki renewal interval to be configured by VaultLeaseRenewalThreshold #1908
allow vault_pki renewal interval to be configured by VaultLeaseRenewalThreshold #1908
Conversation
Hey! Thanks for this contribution. I'm not currently on duty to review community PRs, but I've alerted the person who is and we should get eyes on this soon once they wrap up what they're currently looking at. I've really appreciated the high quality PRs you've been putting up recently, and how quick you've been to respond to feedback, and I'm super excited to get all of this merged. If one of your PRs ever goes too long without attention or love, feel free to tag me directly and I'll try and make sure someone gets eyes on it. Once we get this merged, we'll work on getting a new consul-template version and getting it updated in Vault Agent, too. |
There is also a failing test that might be worth checking if it's caused by these changes! |
It looks like these are unrelated and I confirmed they pass locally. The first time the test only failed in enterprise because the race? test took too long. In the latest version its mad about TestFileQuery_Fetch/fires_changes which im not sure i touched |
dependency/vault_pki.go
Outdated
// 87-93% of the lifespan and adding it to the start | ||
rotationTime := start.Add(time.Millisecond * time.Duration(((lifespanMilliseconds*9)/10)+(lifespanMilliseconds*int64(r.Intn(6)-3))/100)) | ||
// calculate the 'time the certificate should be rotated' by figuring out -3% | ||
// - 2% + VaultLeaseRenewalThreshold of the lifespan and adding it to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably update this comment to "-3, +3"
closes #1646