fix(secret/vault): update strategy to refresh token

[jbrockopp]

Closes go-vela/community#473

This change resolves a problem with the token expiring using HashiCorp Vault as an engine for secrets in Vela.

At this time, the behavior appears specifically related to using the AWS authentication method with Vault.

Currently, the code used to renew the Vault token spawns an infinite for loop:

server/secret/vault/refresh.go

Lines 117 to 132 in 0b72c15

	for {
	time.Sleep(c.config.TokenDuration)
	// token refresh may fail since the allowable refresh
	// timeframe varies depending on the auth method
	_, err := c.Vault.Auth().Token().RenewSelf(int(c.TTL / time.Second))
	// fall back to obtaining a new token if the refresh fails
	if err != nil {
	err = c.initialize()
	}
	if err != nil {
	c.Logger.Errorf("failed to refresh vault token: %s", err)
	} else {
	c.Logger.Trace("successfully refreshed vault token")
	}
	}

Problem 1

The first issue with this is the usage of an empty time to live (TTL) for the token returned from Vault:

server/secret/vault/refresh.go

Line 121 in 0b72c15

_, err := c.Vault.Auth().Token().RenewSelf(int(c.TTL / time.Second))

Previously, we passed c.TTL to the Vault RenewSelf() call but no code sets c.TTL so that value is always 0s.

We've removed both the c.TTL variable as well as the call to RenewSelf() since they are not needed.

Problem 2

The second issue with this is not handling when the TTL for the token is less than the configured renewal duration.

The current code has no checks to verify the token TTL is less than the renewal duration.

The problem this leads to is the token will expire (due to the TTL), before being renewed leading to a 403 error.

Now, we'll always refresh the token after waiting the configured renewal duration:

server/secret/vault/refresh.go

Lines 121 to 133 in 525707a

	for {
	c.Logger.Tracef("sleeping for configured vault token duration %v", c.config.TokenDuration)
	// sleep for the configured token duration before refreshing the token
	time.Sleep(c.config.TokenDuration)
	// reinitialize the client to refresh the token
	err := c.initialize()
	if err != nil {
	c.Logger.Errorf("failed to refresh vault token: %s", err)
	} else {
	c.Logger.Trace("successfully refreshed vault token")
	}
	}

Tech Debt

Accompanying the above changes, I added/updated some of the logging around this for future troubleshooting.

Also, a previous change was introduced to add metadata fields to secrets:

• feat(secrets)!: adding new metadata to secrets #526
• feat(secrets): add secret metadata types#206

However, I noticed that those fields weren't applied to secrets in Vault so I added support for that:

server/secret/vault/vault.go

Lines 238 to 272 in 6b33471

	// set created_at if found in Vault secret
	v, ok = data["created_at"]
	if ok {
	createdAt, ok := v.(int64)
	if ok {
	s.SetCreatedAt(createdAt)
	}
	}
	// set created_by if found in Vault secret
	v, ok = data["created_by"]
	if ok {
	createdBy, ok := v.(string)
	if ok {
	s.SetCreatedBy(createdBy)
	}
	}
	// set updated_at if found in Vault secret
	v, ok = data["updated_at"]
	if ok {
	updatedAt, ok := v.(int64)
	if ok {
	s.SetUpdatedAt(updatedAt)
	}
	}
	// set updated_by if found in Vault secret
	v, ok = data["updated_by"]
	if ok {
	updatedBy, ok := v.(string)
	if ok {
	s.SetUpdatedBy(updatedBy)
	}
	}

server/secret/vault/vault.go

Lines 328 to 346 in 6b33471

	// set created_at if found in Vela secret
	if s.GetCreatedAt() > 0 {
	vault.Data["created_at"] = s.GetCreatedAt()
	}
	// set created_by if found in Vela secret
	if len(s.GetCreatedBy()) > 0 {
	vault.Data["created_by"] = s.GetCreatedBy()
	}
	// set updated_at if found in Vela secret
	if s.GetUpdatedAt() > 0 {
	vault.Data["updated_at"] = s.GetUpdatedAt()
	}
	// set updated_by if found in Vela secret
	if len(s.GetUpdatedBy()) > 0 {
	vault.Data["updated_by"] = s.GetUpdatedBy()
	}

[codecov]

Codecov Report

Merging #570 (b07e2a7) into master (ff6764a) will increase coverage by 0.19%.
The diff coverage is 86.79%.

@@            Coverage Diff             @@
##           master     #570      +/-   ##
==========================================
+ Coverage   54.23%   54.43%   +0.19%     
==========================================
  Files         181      181              
  Lines       15093    15131      +38     
==========================================
+ Hits         8186     8236      +50     
+ Misses       6590     6578      -12     
  Partials      317      317              

Impacted Files	Coverage Δ
secret/vault/refresh.go	57.47% <60.00%> (+15.11%)	⬆️
secret/vault/vault.go	96.41% <97.36%> (+0.81%)	⬆️