Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ linters:
allow:
- $gostd
- github.com/BurntSushi/toml
- github.com/cenkalti/backoff/v4
- golang.org/x/sys/unix
- golang.org/x/exp/maps
- golang.org/x/text/cases
Expand Down
27 changes: 23 additions & 4 deletions dependency/vault_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,23 @@ var (
onceVaultDefaultLeaseDuration sync.Once
VaultLeaseRenewalThreshold float64
onceVaultLeaseRenewalThreshold sync.Once

// VaultTTLZeroMaxRetries is the maximum number of retries for TTL=0 cases
// Set to 0 for unlimited retries. Default: 0 (unlimited)
VaultTTLZeroMaxRetries = 0

// VaultTTLZeroMaxBackoff is the maximum backoff time for TTL=0 retries
// Default: 5 minutes
VaultTTLZeroMaxBackoff = 5 * time.Minute
)

// VaultTTLZeroError is returned when a rotating secret has TTL=0
type VaultTTLZeroError struct{}

func (e *VaultTTLZeroError) Error() string {
return "vault rotating secret returned ttl=0, will retry"
}

// Secret is the structure returned for every secret within Vault.
type Secret struct {
// The request ID that generated this response
Expand Down Expand Up @@ -110,8 +125,8 @@ func renewSecret(clients *ClientSet, d renewer) error {
}

// leaseCheckWait accepts a secret and returns the recommended amount of
// time to sleep.
func leaseCheckWait(s *Secret) time.Duration {
// time to sleep. Returns an error if TTL=0 to trigger retry mechanism.
func leaseCheckWait(s *Secret) (time.Duration, error) {
// Handle whether this is an auth or a regular secret.
base := s.LeaseDuration
if s.Auth != nil && s.Auth.LeaseDuration > 0 {
Expand Down Expand Up @@ -143,11 +158,15 @@ func leaseCheckWait(s *Secret) time.Duration {
var rotatingSecret bool
if _, ok := s.Data["rotation_period"]; ok && s.LeaseID == "" {
if ttlInterface, ok := s.Data["ttl"]; ok {
if ttlData, err := ttlInterface.(json.Number).Int64(); err == nil {
if ttlData, err := ttlInterface.(json.Number).Int64(); err == nil && ttlData > 0 {
log.Printf("[DEBUG] Found rotation_period and set lease duration to %d seconds", ttlData)
// Add a second for cushion
base = int(ttlData) + 1
rotatingSecret = true
} else if err == nil && ttlData == 0 {
// TTL is 0, return error to trigger retry mechanism
log.Printf("[DEBUG] Found rotation_period with ttl=0, returning error to trigger retry")
return 0, &VaultTTLZeroError{}
}
}
}
Expand Down Expand Up @@ -186,7 +205,7 @@ func leaseCheckWait(s *Secret) time.Duration {
sleep = sleep * finalFraction
}

return time.Duration(sleep)
return time.Duration(sleep), nil
}

// printVaultWarnings prints warnings for a given dependency.
Expand Down
83 changes: 59 additions & 24 deletions dependency/vault_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,21 @@ func init() {

func TestVaultRenewDuration(t *testing.T) {
renewable := Secret{LeaseDuration: 100, Renewable: true}
renewableDur := leaseCheckWait(&renewable).Seconds()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm curious why the .Seconds() was dropped here? That seems to have forced a lot of changes unrelated to the goal of this PR that I'm not sure are necessary?

At this point I'd say either set this back to .Seconds(), or apply the suggestions I made to the rest of TestVaultRenewDuration():

Copy link
Copy Markdown
Contributor Author

@aswanidutt aswanidutt Feb 26, 2026

Choose a reason for hiding this comment

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

.Seconds() dropped because now the leaseCheckWait is returning multiple values not just duration but also error. updated with your suggestions to the rest of TestVaultRenewDuration():

if renewableDur < 16 || renewableDur >= 34 {
t.Fatalf("renewable duration is not within 1/6 to 1/3 of lease duration: %f", renewableDur)
renewableDur, err := leaseCheckWait(&renewable)
if err != nil {
t.Fatal(err)
}
if renewableDur.Seconds() < 16 || renewableDur.Seconds() >= 34 {
t.Fatalf("renewable duration is not within 1/6 to 1/3 of lease duration: %f", renewableDur.Seconds())
}

nonRenewable := Secret{LeaseDuration: 100}
nonRenewableDur := leaseCheckWait(&nonRenewable).Seconds()
if nonRenewableDur < 80 || nonRenewableDur > 95 {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableDur)
nonRenewableDur, err := leaseCheckWait(&nonRenewable)
if err != nil {
t.Fatal(err)
}
if nonRenewableDur.Seconds() < 80 || nonRenewableDur.Seconds() > 95 {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableDur.Seconds())
}

data := map[string]interface{}{
Expand All @@ -37,11 +43,14 @@ func TestVaultRenewDuration(t *testing.T) {
}

nonRenewableRotated := Secret{LeaseDuration: 100, Data: data}
nonRenewableRotatedDur := leaseCheckWait(&nonRenewableRotated).Seconds()
nonRenewableRotatedDur, err := leaseCheckWait(&nonRenewableRotated)
if err != nil {
t.Fatal(err)
}

// We expect a 1 second cushion
if nonRenewableRotatedDur != 31 {
t.Fatalf("renewable duration is not 31: %f", nonRenewableRotatedDur)
if nonRenewableRotatedDur.Seconds() != 31 {
t.Fatalf("renewable duration is not 31: %f", nonRenewableRotatedDur.Seconds())
}

data = map[string]interface{}{
Expand All @@ -50,11 +59,25 @@ func TestVaultRenewDuration(t *testing.T) {
}

nonRenewableRotated = Secret{LeaseDuration: 100, Data: data}
nonRenewableRotatedDur = leaseCheckWait(&nonRenewableRotated).Seconds()
nonRenewableRotatedDur, err = leaseCheckWait(&nonRenewableRotated)
if err != nil {
t.Fatal(err)
}

// We expect a 1 second cushion
if nonRenewableRotatedDur != 6 {
t.Fatalf("renewable duration is not 6: %f", nonRenewableRotatedDur)
if nonRenewableRotatedDur.Seconds() != 6 {
t.Fatalf("renewable duration is not 6: %f", nonRenewableRotatedDur.Seconds())
}

// Test TTL=0 case - should return error
data = map[string]interface{}{
"rotation_period": json.Number("30"),
"ttl": json.Number("0"),
}
nonRenewableRotatedZero := Secret{LeaseDuration: 100, Data: data}
_, err = leaseCheckWait(&nonRenewableRotatedZero)
if _, ok := err.(*VaultTTLZeroError); !ok {
t.Fatalf("expected VaultTTLZeroError for ttl=0, got: %v", err)
}

rawExpiration := time.Now().Unix() + 100
Expand All @@ -66,9 +89,12 @@ func TestVaultRenewDuration(t *testing.T) {
}

nonRenewableCert := Secret{LeaseDuration: 100, Data: data}
nonRenewableCertDur := leaseCheckWait(&nonRenewableCert).Seconds()
if nonRenewableCertDur < 80 || nonRenewableCertDur > 95 {
t.Fatalf("non renewable certificate duration is not within 80%% to 95%%: %f", nonRenewableCertDur)
nonRenewableCertDur, err := leaseCheckWait(&nonRenewableCert)
if err != nil {
t.Fatal(err)
}
if nonRenewableCertDur.Seconds() < 80 || nonRenewableCertDur.Seconds() > 95 {
t.Fatalf("non renewable certificate duration is not within 80%% to 95%%: %f", nonRenewableCertDur.Seconds())
}

t.Run("secret ID handling", func(t *testing.T) {
Expand All @@ -80,10 +106,13 @@ func TestVaultRenewDuration(t *testing.T) {
}

nonRenewableSecretID := Secret{LeaseDuration: 100, Data: data}
nonRenewableSecretIDDur := leaseCheckWait(&nonRenewableSecretID).Seconds()
nonRenewableSecretIDDur, err := leaseCheckWait(&nonRenewableSecretID)
if err != nil {
t.Fatal(err)
}

if nonRenewableSecretIDDur < 0.80*(60+1) || nonRenewableSecretIDDur > 0.95*(60+1) {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableSecretIDDur)
if nonRenewableSecretIDDur.Seconds() < 0.80*(60+1) || nonRenewableSecretIDDur.Seconds() > 0.95*(60+1) {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableSecretIDDur.Seconds())
}
})

Expand All @@ -96,10 +125,13 @@ func TestVaultRenewDuration(t *testing.T) {
}

nonRenewableSecretID := Secret{LeaseDuration: leaseDuration, Data: data}
nonRenewableSecretIDDur := leaseCheckWait(&nonRenewableSecretID).Seconds()
nonRenewableSecretIDDur, err := leaseCheckWait(&nonRenewableSecretID)
if err != nil {
t.Fatal(err)
}

if nonRenewableSecretIDDur < 0.80*(leaseDuration+1) || nonRenewableSecretIDDur > 0.95*(leaseDuration+1) {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableSecretIDDur)
if nonRenewableSecretIDDur.Seconds() < 0.80*(leaseDuration+1) || nonRenewableSecretIDDur.Seconds() > 0.95*(leaseDuration+1) {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableSecretIDDur.Seconds())
}
})

Expand All @@ -111,10 +143,13 @@ func TestVaultRenewDuration(t *testing.T) {
}

nonRenewableSecretID := Secret{LeaseDuration: leaseDuration, Data: data}
nonRenewableSecretIDDur := leaseCheckWait(&nonRenewableSecretID).Seconds()
nonRenewableSecretIDDur, err := leaseCheckWait(&nonRenewableSecretID)
if err != nil {
t.Fatal(err)
}

if nonRenewableSecretIDDur < 0.80*(leaseDuration+1) || nonRenewableSecretIDDur > 0.95*(leaseDuration+1) {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableSecretIDDur)
if nonRenewableSecretIDDur.Seconds() < 0.80*(leaseDuration+1) || nonRenewableSecretIDDur.Seconds() > 0.95*(leaseDuration+1) {
t.Fatalf("renewable duration is not within 80%% to 95%% of lease duration: %f", nonRenewableSecretIDDur.Seconds())
}
})
})
Expand Down
6 changes: 5 additions & 1 deletion dependency/vault_read.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,11 @@ func (d *VaultReadQuery) Fetch(clients *ClientSet, opts *QueryOptions,
}

if !vaultSecretRenewable(d.secret) {
dur := leaseCheckWait(d.secret)
dur, err := leaseCheckWait(d.secret)
if err != nil {
// TTL=0 case - return error to trigger retry mechanism
return nil, nil, err
}
log.Printf("[TRACE] %s: non-renewable secret, set sleep for %s", d, dur)
d.sleepCh <- dur
}
Expand Down
6 changes: 5 additions & 1 deletion dependency/vault_write.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ func (d *VaultWriteQuery) Fetch(clients *ClientSet, opts *QueryOptions,
d.secret = transformSecret(vaultSecret)

if !vaultSecretRenewable(d.secret) {
dur := leaseCheckWait(d.secret)
dur, err := leaseCheckWait(d.secret)
if err != nil {
// TTL=0 case - return error to trigger retry mechanism
return nil, nil, err
}
log.Printf("[TRACE] %s: non-renewable secret, set sleep for %s", d, dur)
d.sleepCh <- dur
}
Expand Down
Loading
Loading