diff --git a/api/lifetime_watcher.go b/api/lifetime_watcher.go index f775dfb15..f06263526 100644 --- a/api/lifetime_watcher.go +++ b/api/lifetime_watcher.go @@ -113,7 +113,9 @@ type LifetimeWatcherInput struct { // The new TTL, in seconds, that should be set on the lease. The TTL set // here may or may not be honored by the vault server, based on Vault - // configuration or any associated max TTL values. + // configuration or any associated max TTL values. If specified, the + // minimum of this value and the remaining lease duration will be used + // for grace period calculations. Increment int // RenewBehavior controls what happens when a renewal errors or the @@ -257,7 +259,7 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool, initialTime := time.Now() priorDuration := time.Duration(initLeaseDuration) * time.Second - r.calculateGrace(priorDuration) + r.calculateGrace(priorDuration, time.Duration(r.increment)*time.Second) var errorBackoff backoff.BackOff for { @@ -345,7 +347,7 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool, // extending. Once it stops extending, we've hit the max and need to // rely on the grace duration. if remainingLeaseDuration > priorDuration { - r.calculateGrace(remainingLeaseDuration) + r.calculateGrace(remainingLeaseDuration, time.Duration(r.increment)*time.Second) } priorDuration = remainingLeaseDuration @@ -373,16 +375,21 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool, } } -// calculateGrace calculates the grace period based on a reasonable set of -// assumptions given the total lease time; it also adds some jitter to not have -// clients be in sync. -func (r *LifetimeWatcher) calculateGrace(leaseDuration time.Duration) { - if leaseDuration <= 0 { +// calculateGrace calculates the grace period based on the minimum of the +// remaining lease duration and the token increment value; it also adds some +// jitter to not have clients be in sync. +func (r *LifetimeWatcher) calculateGrace(leaseDuration, increment time.Duration) { + minDuration := leaseDuration + if minDuration > increment && increment > 0 { + minDuration = increment + } + + if minDuration <= 0 { r.grace = 0 return } - leaseNanos := float64(leaseDuration.Nanoseconds()) + leaseNanos := float64(minDuration.Nanoseconds()) jitterMax := 0.1 * leaseNanos // For a given lease duration, we want to allow 80-90% of that to elapse, diff --git a/api/renewer_test.go b/api/renewer_test.go index 3fa9ed8e2..2a5d5745b 100644 --- a/api/renewer_test.go +++ b/api/renewer_test.go @@ -24,28 +24,28 @@ func TestRenewer_NewRenewer(t *testing.T) { err bool }{ { - "nil", - nil, - nil, - true, + name: "nil", + i: nil, + e: nil, + err: true, }, { - "missing_secret", - &RenewerInput{ + name: "missing_secret", + i: &RenewerInput{ Secret: nil, }, - nil, - true, + e: nil, + err: true, }, { - "default_grace", - &RenewerInput{ + name: "default_grace", + i: &RenewerInput{ Secret: &Secret{}, }, - &Renewer{ + e: &Renewer{ secret: &Secret{}, }, - false, + err: false, }, } @@ -98,67 +98,78 @@ func TestLifetimeWatcher(t *testing.T) { expectRenewal bool }{ { - time.Second, - "no_error", - 60, - 60, - func(_ string, _ int) (*Secret, error) { + maxTestTime: time.Second, + name: "no_error", + leaseDurationSeconds: 60, + incrementSeconds: 60, + renew: func(_ string, _ int) (*Secret, error) { return renewedSecret, nil }, - nil, - true, + expectError: nil, + expectRenewal: true, }, { - 5 * time.Second, - "one_error", - 15, - 15, - func(_ string, _ int) (*Secret, error) { + maxTestTime: time.Second, + name: "short_increment_duration", + leaseDurationSeconds: 60, + incrementSeconds: 10, + renew: func(_ string, _ int) (*Secret, error) { + return renewedSecret, nil + }, + expectError: nil, + expectRenewal: true, + }, + { + maxTestTime: 5 * time.Second, + name: "one_error", + leaseDurationSeconds: 15, + incrementSeconds: 15, + renew: func(_ string, _ int) (*Secret, error) { if caseOneErrorCount == 0 { caseOneErrorCount++ return nil, fmt.Errorf("renew failure") } return renewedSecret, nil }, - nil, - true, + expectError: nil, + expectRenewal: true, }, { - 15 * time.Second, - "many_errors", - 15, - 15, - func(_ string, _ int) (*Secret, error) { + maxTestTime: 15 * time.Second, + name: "many_errors", + leaseDurationSeconds: 15, + incrementSeconds: 15, + renew: func(_ string, _ int) (*Secret, error) { if caseManyErrorsCount == 3 { return renewedSecret, nil } caseManyErrorsCount++ return nil, fmt.Errorf("renew failure") }, - nil, - true, + expectError: nil, + expectRenewal: true, }, { - 15 * time.Second, - "only_errors", - 15, - 15, - func(_ string, _ int) (*Secret, error) { + maxTestTime: 15 * time.Second, + name: "only_errors", + leaseDurationSeconds: 15, + incrementSeconds: 15, + renew: func(_ string, _ int) (*Secret, error) { return nil, fmt.Errorf("renew failure") }, - nil, - false, + expectError: nil, + expectRenewal: false, }, { - 15 * time.Second, - "negative_lease_duration", - -15, - 15, - func(_ string, _ int) (*Secret, error) { + maxTestTime: 15 * time.Second, + name: "negative_lease_duration", + leaseDurationSeconds: -15, + incrementSeconds: 15, + renew: func(_ string, _ int) (*Secret, error) { return renewedSecret, nil }, - nil, - true, + expectError: nil, + expectRenewal: true, }, } diff --git a/changelog/14836.txt b/changelog/14836.txt new file mode 100644 index 000000000..6ee8d54a2 --- /dev/null +++ b/changelog/14836.txt @@ -0,0 +1,3 @@ +```release-note:bug +api: Respect increment value in grace period calculations in LifetimeWatcher +```