diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index baadcfb..42d51c6 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/spf13/pflag" + "golang.org/x/sync/errgroup" ) // ErrSecretNotFound signals that a store has no entry for an app at all, as @@ -86,23 +87,48 @@ type layeredSecretResolver struct { override SecretResolver } -// Resolve returns the base values with the override's merged on top. +// Resolve queries the base and override stores concurrently and returns the base +// values with the override's merged on top. The two are independent round-trips, +// so overlapping them halves the per-app latency when both back onto a slow store +// (e.g. two 1Password vaults). A base failure is fatal; an override reporting +// ErrSecretNotFound means it doesn't cover this app, so it falls through to the +// base rather than cancelling it. func (l layeredSecretResolver) Resolve(ctx context.Context, names []string) (map[string]string, error) { - out, err := l.base.Resolve(ctx, names) - if err != nil { - return nil, err - } if l.override == nil { - return out, nil + return l.base.Resolve(ctx, names) } - ov, err := l.override.Resolve(ctx, names) - if err != nil { - if errors.Is(err, ErrSecretNotFound) { - return out, nil + + var baseVals, overrideVals map[string]string + g, ctx := errgroup.WithContext(ctx) + g.Go(func() error { + v, err := l.base.Resolve(ctx, names) + if err != nil { + return err + } + baseVals = v + return nil + }) + g.Go(func() error { + v, err := l.override.Resolve(ctx, names) + if err != nil { + // The override not covering this app is expected; swallow it here so it + // neither fails the group nor cancels the base query still in flight. + if errors.Is(err, ErrSecretNotFound) { + return nil + } + return fmt.Errorf("secret override resolver: %w", err) } - return nil, fmt.Errorf("secret override resolver: %w", err) + overrideVals = v + return nil + }) + if err := g.Wait(); err != nil { + return nil, err + } + + if overrideVals == nil { + return baseVals, nil } - return mergeOver(out, ov), nil + return mergeOver(baseVals, overrideVals), nil } // mergeOver returns a new map of base with override's entries layered on top so diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index b5a2e43..4303c93 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -7,6 +7,7 @@ import ( "fmt" "path/filepath" "testing" + "time" "github.com/spf13/pflag" "github.com/stretchr/testify/assert" @@ -165,6 +166,51 @@ func TestLayeredSecretResolverBaseNotFoundIsFatal(t *testing.T) { assert.ErrorIs(t, err, ErrSecretNotFound) } +// blockingResolver signals when its Resolve is entered and blocks until released, +// so a test can prove two resolvers run at the same time. +type blockingResolver struct { + entered chan struct{} + release chan struct{} + have map[string]string +} + +func (b blockingResolver) Resolve(context.Context, []string) (map[string]string, error) { + close(b.entered) + <-b.release + return b.have, nil +} + +// The base and override stores are queried concurrently: both are inside Resolve +// at the same time before either returns, rather than one waiting on the other. +func TestLayeredSecretResolverQueriesConcurrently(t *testing.T) { + base := blockingResolver{entered: make(chan struct{}), release: make(chan struct{}), have: map[string]string{"A": "base"}} + override := blockingResolver{entered: make(chan struct{}), release: make(chan struct{}), have: map[string]string{"B": "override"}} + l := layeredSecretResolver{base: base, override: override} + + done := make(chan map[string]string, 1) + go func() { + got, err := l.Resolve(context.Background(), []string{"A", "B"}) + require.NoError(t, err) + done <- got + }() + + // Neither goroutine can finish until released, so both entering proves overlap. + select { + case <-base.entered: + case <-time.After(time.Second): + t.Fatal("base resolver never started") + } + select { + case <-override.entered: + case <-time.After(time.Second): + t.Fatal("override resolver did not start while base was blocked") + } + close(base.release) + close(override.release) + + assert.Equal(t, map[string]string{"A": "base", "B": "override"}, <-done) +} + func TestLayeredSecretResolverNilOverrideIsBase(t *testing.T) { l := layeredSecretResolver{base: mapResolver{have: map[string]string{"A": "base"}}} got, err := l.Resolve(context.Background(), []string{"A"})