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
agent: tolerate partial restore failure from persistent cache #12718
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,3 @@ | ||
+```release-note:improvement | ||
+auth/kubernetes: validate JWT against the provided role on alias look ahead operations | ||
+``` | ||
|
||
```release-note:improvement | ||
auth/kubernetes: validate JWT against the provided role on alias look ahead operations | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:improvement | ||
agent/cache: tolerate partial restore failure from persistent cache | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import ( | |
"time" | ||
|
||
hclog "github.com/hashicorp/go-hclog" | ||
"github.com/hashicorp/go-multierror" | ||
"github.com/hashicorp/go-secure-stdlib/base62" | ||
"github.com/hashicorp/vault/api" | ||
"github.com/hashicorp/vault/command/agent/cache/cacheboltdb" | ||
|
@@ -969,56 +970,68 @@ func (c *LeaseCache) Flush() error { | |
// tokens first, since restoring a lease's renewal context and watcher requires | ||
// looking up the token in the cachememdb. | ||
func (c *LeaseCache) Restore(ctx context.Context, storage *cacheboltdb.BoltStorage) error { | ||
var errors *multierror.Error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be worth looking into down the road for how we define custom errors. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which bit of the guide are you referring to? The only bit I see that refers to multiple errors is wrapping, and that's not appropriate here because the errors we're collecting at this level are "peers" rather than forming part of a stack. In the case that there are errors, we want to log a message about what failed and optionally continue (if |
||
|
||
// Process tokens first | ||
tokens, err := storage.GetByType(ctx, cacheboltdb.TokenType) | ||
if err != nil { | ||
return err | ||
} | ||
if err := c.restoreTokens(tokens); err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
} else { | ||
if err := c.restoreTokens(tokens); err != nil { | ||
errors = multierror.Append(errors, err) | ||
} | ||
} | ||
|
||
// Then process auth leases | ||
authLeases, err := storage.GetByType(ctx, cacheboltdb.AuthLeaseType) | ||
if err != nil { | ||
return err | ||
} | ||
if err := c.restoreLeases(authLeases); err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
} else { | ||
if err := c.restoreLeases(authLeases); err != nil { | ||
errors = multierror.Append(errors, err) | ||
} | ||
} | ||
|
||
// Then process secret leases | ||
secretLeases, err := storage.GetByType(ctx, cacheboltdb.SecretLeaseType) | ||
if err != nil { | ||
return err | ||
} | ||
if err := c.restoreLeases(secretLeases); err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
} else { | ||
if err := c.restoreLeases(secretLeases); err != nil { | ||
errors = multierror.Append(errors, err) | ||
} | ||
} | ||
|
||
return nil | ||
return errors.ErrorOrNil() | ||
} | ||
|
||
func (c *LeaseCache) restoreTokens(tokens [][]byte) error { | ||
var errors *multierror.Error | ||
|
||
for _, token := range tokens { | ||
newIndex, err := cachememdb.Deserialize(token) | ||
if err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
continue | ||
} | ||
newIndex.RenewCtxInfo = c.createCtxInfo(nil) | ||
if err := c.db.Set(newIndex); err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
continue | ||
} | ||
c.logger.Trace("restored token", "id", newIndex.ID) | ||
} | ||
return nil | ||
return errors.ErrorOrNil() | ||
} | ||
|
||
func (c *LeaseCache) restoreLeases(leases [][]byte) error { | ||
var errors *multierror.Error | ||
|
||
for _, lease := range leases { | ||
newIndex, err := cachememdb.Deserialize(lease) | ||
if err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
continue | ||
} | ||
|
||
// Check if this lease has already expired | ||
|
@@ -1031,14 +1044,17 @@ func (c *LeaseCache) restoreLeases(leases [][]byte) error { | |
} | ||
|
||
if err := c.restoreLeaseRenewCtx(newIndex); err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
continue | ||
} | ||
if err := c.db.Set(newIndex); err != nil { | ||
return err | ||
errors = multierror.Append(errors, err) | ||
continue | ||
} | ||
c.logger.Trace("restored lease", "id", newIndex.ID, "path", newIndex.RequestPath) | ||
} | ||
return nil | ||
|
||
tomhjp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return errors.ErrorOrNil() | ||
} | ||
|
||
// restoreLeaseRenewCtx re-creates a RenewCtx for an index object and starts | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing a small syntax error in the previous changelog