Fix a data race in the new autoseal health check (#13136)

* Move the ctx capture outside the goroutine to avoid a race

* refactor the toggleable wrapper to avoid races

* Move the capture back outside the goroutine

* defer
This commit is contained in:
Scott Miller 2021-11-12 15:58:46 -06:00 committed by GitHub
parent 1279413ea2
commit a5e55f6b05
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 26 additions and 14 deletions

View File

@ -555,7 +555,7 @@ func TestRaft_SnapshotAPI_MidstreamFailure(t *testing.T) {
// defer goleak.VerifyNone(t)
t.Parallel()
seal, errptr := vaultseal.NewToggleableTestSeal(nil)
seal, setErr := vaultseal.NewToggleableTestSeal(nil)
cluster := raftCluster(t, &RaftClusterOpts{
NumCores: 1,
Seal: vault.NewAutoSeal(seal),
@ -587,7 +587,7 @@ func TestRaft_SnapshotAPI_MidstreamFailure(t *testing.T) {
wg.Done()
}()
*errptr = errors.New("seal failure")
setErr(errors.New("seal failure"))
// Take a snapshot
err := leaderClient.Sys().RaftSnapshot(w)
w.Close()

View File

@ -2,6 +2,7 @@ package seal
import (
"context"
"sync"
"github.com/hashicorp/go-hclog"
wrapping "github.com/hashicorp/go-kms-wrapping"
@ -25,7 +26,7 @@ func NewTestSeal(opts *TestSealOpts) *Access {
}
}
func NewToggleableTestSeal(opts *TestSealOpts) (*Access, *error) {
func NewToggleableTestSeal(opts *TestSealOpts) (*Access, func(error)) {
if opts == nil {
opts = new(TestSealOpts)
}
@ -34,26 +35,37 @@ func NewToggleableTestSeal(opts *TestSealOpts) (*Access, *error) {
return &Access{
Wrapper: w,
OverriddenType: opts.Name,
}, &w.Error
}, w.SetError
}
type ToggleableWrapper struct {
wrapping.Wrapper
Error error
error error
l sync.RWMutex
}
func (t ToggleableWrapper) Encrypt(ctx context.Context, bytes []byte, bytes2 []byte) (*wrapping.EncryptedBlobInfo, error) {
if t.Error != nil {
return nil, t.Error
func (t *ToggleableWrapper) Encrypt(ctx context.Context, bytes []byte, bytes2 []byte) (*wrapping.EncryptedBlobInfo, error) {
t.l.RLock()
defer t.l.RUnlock()
if t.error != nil {
return nil, t.error
}
return t.Wrapper.Encrypt(ctx, bytes, bytes2)
}
func (t ToggleableWrapper) Decrypt(ctx context.Context, info *wrapping.EncryptedBlobInfo, bytes []byte) ([]byte, error) {
if t.Error != nil {
return nil, t.Error
t.l.RLock()
defer t.l.RUnlock()
if t.error != nil {
return nil, t.error
}
return t.Wrapper.Decrypt(ctx, info, bytes)
}
func (t *ToggleableWrapper) SetError(err error) {
t.l.Lock()
defer t.l.Unlock()
t.error = err
}
var _ wrapping.Wrapper = &ToggleableWrapper{}

View File

@ -524,9 +524,9 @@ func (d *autoSeal) StartHealthCheck() {
healthCheck := time.NewTicker(sealHealthTestIntervalNominal)
d.healthCheckStop = make(chan struct{})
healthCheckStop := d.healthCheckStop
ctx := d.core.activeContext
go func() {
ctx := d.core.activeContext
lastTestOk := true
lastSeenOk := time.Now()

View File

@ -177,7 +177,7 @@ func TestAutoSeal_HealthCheck(t *testing.T) {
metrics.NewGlobal(metricsConf, inmemSink)
core, _, _ := TestCoreUnsealed(t)
testSeal, testErr := seal.NewToggleableTestSeal(nil)
testSeal, setErr := seal.NewToggleableTestSeal(nil)
var encKeys []string
changeKey := func(key string) {
@ -196,8 +196,8 @@ func TestAutoSeal_HealthCheck(t *testing.T) {
sealHealthTestIntervalNominal = 10 * time.Millisecond
sealHealthTestIntervalUnhealthy = 10 * time.Millisecond
setErr(errors.New("disconnected"))
autoSeal.StartHealthCheck()
*testErr = errors.New("disconnected")
time.Sleep(50 * time.Millisecond)
@ -213,7 +213,7 @@ func TestAutoSeal_HealthCheck(t *testing.T) {
t.Fatalf("Expected value metric %s to be non-zero", asu)
}
}
*testErr = nil
setErr(nil)
time.Sleep(50 * time.Millisecond)
intervals = inmemSink.Data()
if len(intervals) == 1 {