diff --git a/api/renewer.go b/api/renewer.go index b50cf814f376..7fd1de7db20d 100644 --- a/api/renewer.go +++ b/api/renewer.go @@ -13,9 +13,6 @@ var ( ErrRenewerNotRenewable = errors.New("secret is not renewable") ErrRenewerNoSecretData = errors.New("returned empty secret data") - // DefaultRenewerGrace is the default grace period - DefaultRenewerGrace = 15 * time.Second - // DefaultRenewerRenewBuffer is the default size of the buffer for renew // messages on the channel. DefaultRenewerRenewBuffer = 5 @@ -111,9 +108,6 @@ func (c *Client) NewRenewer(i *RenewerInput) (*Renewer, error) { } grace := i.Grace - if grace == 0 { - grace = DefaultRenewerGrace - } random := i.Rand if random == nil { @@ -184,6 +178,9 @@ func (r *Renewer) renewAuth() error { return ErrRenewerNotRenewable } + priorDuration := time.Duration(r.secret.Auth.LeaseDuration) * time.Second + r.calculateGrace(priorDuration) + client, token := r.client, r.secret.Auth.ClientToken for { @@ -216,13 +213,28 @@ func (r *Renewer) renewAuth() error { return ErrRenewerNotRenewable } - // Grab the lease duration and sleep duration - note that we grab the auth - // lease duration, not the secret lease duration. + // Grab the lease duration leaseDuration := time.Duration(renewal.Auth.LeaseDuration) * time.Second - sleepDuration := r.sleepDuration(leaseDuration) - // If we are within grace, return now. - if leaseDuration <= r.grace || sleepDuration <= r.grace { + // We keep evaluating a new grace period so long as the lease is + // extending. Once it stops extending, we've hit the max and need to + // rely on the grace duration. + if leaseDuration > priorDuration { + r.calculateGrace(leaseDuration) + } + priorDuration = leaseDuration + + // The sleep duration is set to 2/3 of the current lease duration plus + // 1/3 of the current grace period, which adds jitter. + sleepDuration := time.Duration(float64(leaseDuration.Nanoseconds())*2/3 + float64(r.grace.Nanoseconds())/3) + + // If we are within grace, return now; or, if the amount of time we + // would sleep would land us in the grace period. This helps with short + // tokens; for example, you don't want a current lease duration of 4 + // seconds, a grace period of 3 seconds, and end up sleeping for more + // than three of those seconds and having a very small budget of time + // to renew. + if leaseDuration <= r.grace || leaseDuration-sleepDuration <= r.grace { return nil } @@ -241,6 +253,9 @@ func (r *Renewer) renewLease() error { return ErrRenewerNotRenewable } + priorDuration := time.Duration(r.secret.LeaseDuration) * time.Second + r.calculateGrace(priorDuration) + client, leaseID := r.client, r.secret.LeaseID for { @@ -273,12 +288,28 @@ func (r *Renewer) renewLease() error { return ErrRenewerNotRenewable } - // Grab the lease duration and sleep duration + // Grab the lease duration leaseDuration := time.Duration(renewal.LeaseDuration) * time.Second - sleepDuration := r.sleepDuration(leaseDuration) - // If we are within grace, return now. - if leaseDuration <= r.grace || sleepDuration <= r.grace { + // We keep evaluating a new grace period so long as the lease is + // extending. Once it stops extending, we've hit the max and need to + // rely on the grace duration. + if leaseDuration > priorDuration { + r.calculateGrace(leaseDuration) + } + priorDuration = leaseDuration + + // The sleep duration is set to 2/3 of the current lease duration plus + // 1/3 of the current grace period, which adds jitter. + sleepDuration := time.Duration(float64(leaseDuration.Nanoseconds())*2/3 + float64(r.grace.Nanoseconds())/3) + + // If we are within grace, return now; or, if the amount of time we + // would sleep would land us in the grace period. This helps with short + // tokens; for example, you don't want a current lease duration of 4 + // seconds, a grace period of 3 seconds, and end up sleeping for more + // than three of those seconds and having a very small budget of time + // to renew. + if leaseDuration <= r.grace || leaseDuration-sleepDuration <= r.grace { return nil } @@ -307,3 +338,20 @@ func (r *Renewer) sleepDuration(base time.Duration) time.Duration { return time.Duration(sleep) } + +// 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 *Renewer) calculateGrace(leaseDuration time.Duration) { + if leaseDuration == 0 { + r.grace = 0 + return + } + + leaseNanos := float64(leaseDuration.Nanoseconds()) + jitterMax := 0.1 * leaseNanos + + // For a given lease duration, we want to allow 80-90% of that to elapse, + // so the remaining amount is the grace period + r.grace = time.Duration(jitterMax) + time.Duration(uint64(r.random.Int63())%uint64(jitterMax)) +} diff --git a/api/renewer_integration_test.go b/api/renewer_integration_test.go index 50a775e1261c..e3b83e93fbee 100644 --- a/api/renewer_integration_test.go +++ b/api/renewer_integration_test.go @@ -109,8 +109,8 @@ func TestRenewer_Renew(t *testing.T) { "creation_statements": `` + `CREATE ROLE "{{name}}" WITH LOGIN PASSWORD '{{password}}' VALID UNTIL '{{expiration}}';` + `GRANT SELECT ON ALL TABLES IN SCHEMA public TO "{{name}}";`, - "default_ttl": "1s", - "max_ttl": "3s", + "default_ttl": "5s", + "max_ttl": "10s", }); err != nil { t.Fatal(err) } @@ -139,22 +139,28 @@ func TestRenewer_Renew(t *testing.T) { if !renew.Secret.Renewable { t.Errorf("expected lease to be renewable: %#v", renew) } - if renew.Secret.LeaseDuration > 2 { - t.Errorf("expected lease to < 2s: %#v", renew) + if renew.Secret.LeaseDuration > 5 { + t.Errorf("expected lease to <= 5s: %#v", renew) } - case <-time.After(3 * time.Second): + case <-time.After(5 * time.Second): t.Errorf("no renewal") } - select { - case err := <-v.DoneCh(): - if err != nil { - t.Fatal(err) + outer: + for { + select { + case err := <-v.DoneCh(): + if err != nil { + t.Fatal(err) + } + break outer + case renew := <-v.RenewCh(): + t.Logf("renew called, remaining lease duration: %d", renew.Secret.LeaseDuration) + continue outer + case <-time.After(5 * time.Second): + t.Errorf("no data") + break outer } - case renew := <-v.RenewCh(): - t.Fatalf("should not have renewed (lease should be up): %#v", renew) - case <-time.After(3 * time.Second): - t.Errorf("no data") } }) @@ -205,15 +211,20 @@ func TestRenewer_Renew(t *testing.T) { t.Errorf("no renewal") } - select { - case err := <-v.DoneCh(): - if err != nil { - t.Fatal(err) + outer: + for { + select { + case err := <-v.DoneCh(): + if err != nil { + t.Fatal(err) + } + break outer + case <-v.RenewCh(): + continue outer + case <-time.After(3 * time.Second): + t.Errorf("no data") + break outer } - case renew := <-v.RenewCh(): - t.Fatalf("should not have renewed (lease should be up): %#v", renew) - case <-time.After(3 * time.Second): - t.Errorf("no data") } }) }) diff --git a/api/renewer_test.go b/api/renewer_test.go index 262484e0fa01..d0b828b451ed 100644 --- a/api/renewer_test.go +++ b/api/renewer_test.go @@ -41,7 +41,6 @@ func TestRenewer_NewRenewer(t *testing.T) { }, &Renewer{ secret: &Secret{}, - grace: DefaultRenewerGrace, }, false, },