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

Increase resilience to very low/high TTLs #1115

Merged
merged 3 commits into from Sep 12, 2019

Conversation

azdagron
Copy link
Member

When TTLs are very low (i.e. lower than ~2x the internal timers that check for expiration) then the agent or server can miss its chance for renewal. The current timers are set to 30 seconds, which makes it hard to use low TTLs for testing/demo purposes. This PR drops the timers down to 5 seconds.

When CA TTLs are very high, SPIRE server prepares and activates much to early. For example, if the CA TTL is one year, a new one will be prepared at six months and activated with two months remaining. This change caps the preparation/activation time to 30 and 7 days from expiration, respectively.

- Lowered TTL check related timers to 5 seconds.
- Capped preparation/activation lifetimes to 30 days and 7 days,
respectively

Signed-off-by: Andrew Harding <azdagron@gmail.com>
Copy link
Member

@amartinezfayo amartinezfayo left a comment

Choose a reason for hiding this comment

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

Thanks for this @azdagron!
Looks good. I have a small comment.

@@ -48,7 +48,7 @@ func New(c *Config) (*manager, error) {
}

if c.RotationInterval == 0 {
c.RotationInterval = 60 * time.Second
c.RotationInterval = 5 * time.Second
Copy link
Member

Choose a reason for hiding this comment

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

maybe we can export svid.defaultInterval and use it here?

@@ -17,7 +17,7 @@ import (
"github.com/sirupsen/logrus"
)

const defaultInterval = 60 * time.Second
const defaultInterval = 5 * time.Second
Copy link
Member

Choose a reason for hiding this comment

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

we may export this const to be used in the manager package?

…ence

Signed-off-by: Andrew Harding <azdagron@gmail.com>
Signed-off-by: Andrew Harding <azdagron@gmail.com>
Copy link
Member

@amartinezfayo amartinezfayo left a comment

Choose a reason for hiding this comment

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

🚀

@azdagron azdagron merged commit 8024c4d into spiffe:master Sep 12, 2019
@azdagron azdagron deleted the increase-ttl-resilience branch September 12, 2019 16:41
@azdagron azdagron mentioned this pull request Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants