Skip to content

Commit

Permalink
Properly forward (or specifically don't) sys calls that result in rea…
Browse files Browse the repository at this point in the history
…d only errors (#4129)

Prior to this policy writes against a performance secondary would not
succeed because the read-only error was swallowed by handleError. In
addition to fixing this, it adds a similar function that explicitly
doesn't trigger forwarding. This is useful for things that are local to
the secondary such as raw operations and lease management.
  • Loading branch information
jefferai authored Mar 18, 2018
1 parent c01e098 commit f3656e8
Showing 1 changed file with 27 additions and 9 deletions.
36 changes: 27 additions & 9 deletions vault/logical_system.go
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,7 @@ func (b *SystemBackend) handleTidyLeases(ctx context.Context, req *logical.Reque
err := b.Core.expiration.Tidy()
if err != nil {
b.Backend.Logger().Error("sys: failed to tidy leases", "error", err)
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
return nil, err
}
Expand Down Expand Up @@ -1591,6 +1591,24 @@ func (b *SystemBackend) handleMount(ctx context.Context, req *logical.Request, d
// used to intercept an HTTPCodedError so it goes back to callee
func handleError(
err error) (*logical.Response, error) {
if strings.Contains(err.Error(), logical.ErrReadOnly.Error()) {
return logical.ErrorResponse(err.Error()), err
}
switch err.(type) {
case logical.HTTPCodedError:
return logical.ErrorResponse(err.Error()), err
default:
return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
}
}

// Performs a similar function to handleError, but upon seeing a ReadOnlyError
// will actually strip it out to prevent forwarding
func handleErrorNoReadOnlyForward(
err error) (*logical.Response, error) {
if strings.Contains(err.Error(), logical.ErrReadOnly.Error()) {
return nil, fmt.Errorf("operation could not be completed as storage is read-only")
}
switch err.(type) {
case logical.HTTPCodedError:
return logical.ErrorResponse(err.Error()), err
Expand Down Expand Up @@ -1950,7 +1968,7 @@ func (b *SystemBackend) handleLeaseLookupList(ctx context.Context, req *logical.
keys, err := b.Core.expiration.idView.List(ctx, prefix)
if err != nil {
b.Backend.Logger().Error("sys: error listing leases", "prefix", prefix, "error", err)
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
return logical.ListResponse(keys), nil
}
Expand All @@ -1975,7 +1993,7 @@ func (b *SystemBackend) handleRenew(ctx context.Context, req *logical.Request, d
resp, err := b.Core.expiration.Renew(leaseID, increment)
if err != nil {
b.Backend.Logger().Error("sys: lease renewal failed", "lease_id", leaseID, "error", err)
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
return resp, err
}
Expand All @@ -1995,7 +2013,7 @@ func (b *SystemBackend) handleRevoke(ctx context.Context, req *logical.Request,
// Invoke the expiration manager directly
if err := b.Core.expiration.Revoke(leaseID); err != nil {
b.Backend.Logger().Error("sys: lease revocation failed", "lease_id", leaseID, "error", err)
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
return nil, nil
}
Expand Down Expand Up @@ -2025,7 +2043,7 @@ func (b *SystemBackend) handleRevokePrefixCommon(
}
if err != nil {
b.Backend.Logger().Error("sys: revoke prefix failed", "prefix", prefix, "error", err)
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
return nil, nil
}
Expand Down Expand Up @@ -2499,7 +2517,7 @@ func (b *SystemBackend) handleRawRead(ctx context.Context, req *logical.Request,

entry, err := b.Core.barrier.Get(ctx, path)
if err != nil {
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
if entry == nil {
return nil, nil
Expand All @@ -2511,7 +2529,7 @@ func (b *SystemBackend) handleRawRead(ctx context.Context, req *logical.Request,
// will be nil.
outputBytes, _, err := compressutil.Decompress(entry.Value)
if err != nil {
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}

// `outputBytes` is nil if the input is uncompressed. In that case set it to the original input.
Expand Down Expand Up @@ -2563,7 +2581,7 @@ func (b *SystemBackend) handleRawDelete(ctx context.Context, req *logical.Reques
}

if err := b.Core.barrier.Delete(ctx, path); err != nil {
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
return nil, nil
}
Expand All @@ -2585,7 +2603,7 @@ func (b *SystemBackend) handleRawList(ctx context.Context, req *logical.Request,

keys, err := b.Core.barrier.List(ctx, path)
if err != nil {
return handleError(err)
return handleErrorNoReadOnlyForward(err)
}
return logical.ListResponse(keys), nil
}
Expand Down

0 comments on commit f3656e8

Please sign in to comment.