Skip to content

Commit

Permalink
client/setec: plumb contexts to dynamic lookup methods of Store (#93)
Browse files Browse the repository at this point in the history
Previously, we reused the lifecycle context for dynamic lookups.  Instead, make
those methods (LookupSecret, LookupWatcher, and Fields.Apply)) take their own
context and update the tests accordingly.

This is a breaking change to the store package interface.
  • Loading branch information
creachadair authored Feb 10, 2024
1 parent b84de74 commit 70d1ccd
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 25 deletions.
13 changes: 7 additions & 6 deletions client/setec/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package setec

import (
"context"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -121,11 +122,11 @@ func (f *Fields) Secrets() []string {
// Note: When applying secrets to struct fields from an existing Store, the
// AllowLookup option of the Store must be enabled, or else Apply will report
// an error for any field that refers to a secret not already available.
func (f *Fields) Apply(s *Store) error {
func (f *Fields) Apply(ctx context.Context, s *Store) error {
var errs []error
for _, fi := range f.fields {
fullName := path.Join(f.prefix, fi.secretName)
if err := fi.apply(s, fullName); err != nil {
if err := fi.apply(ctx, s, fullName); err != nil {
errs = append(errs, fmt.Errorf("apply %q to field %q: %w", fullName, fi.fieldName, err))
}
}
Expand All @@ -146,25 +147,25 @@ type fieldInfo struct {
//
// If f.isJSON is true, the data are unmarshaled as JSON.
// Otherwise, the data are converted to the target type and copied.
func (f fieldInfo) apply(s *Store, fullName string) error {
func (f fieldInfo) apply(ctx context.Context, s *Store, fullName string) error {
if f.isJSON {
v, err := s.LookupSecret(fullName)
v, err := s.LookupSecret(ctx, fullName)
if err != nil {
return err
}
return json.Unmarshal(v.Get(), f.value.Interface())
}

if f.vtype == watcherType {
w, err := s.LookupWatcher(fullName)
w, err := s.LookupWatcher(ctx, fullName)
if err != nil {
return err
}
f.value.Elem().Set(reflect.ValueOf(w))
return nil
}

v, err := s.LookupSecret(fullName)
v, err := s.LookupSecret(ctx, fullName)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion client/setec/fields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestFields(t *testing.T) {
}

// Check that we can apply values.
if err := f.Apply(st); err != nil {
if err := f.Apply(context.Background(), st); err != nil {
t.Errorf("Apply failed; %v", err)
}

Expand Down
25 changes: 14 additions & 11 deletions client/setec/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func NewStore(ctx context.Context, cfg StoreConfig) (*Store, error) {

// Plumb secrets in to struct fields, if necessary.
for _, fi := range structs {
if err := fi.Apply(s); err != nil {
if err := fi.Apply(ctx, s); err != nil {
return nil, fmt.Errorf("apply secrets to struct: %w", err)
}
}
Expand Down Expand Up @@ -331,25 +331,28 @@ func (s *Store) secretLocked(name string) Secret {
// latest active version of the secret from the service and either adds it to
// the collection or reports an error. LookupSecret does not automatically
// retry in case of errors.
func (s *Store) LookupSecret(name string) (Secret, error) {
func (s *Store) LookupSecret(ctx context.Context, name string) (Secret, error) {
if f := s.Secret(name); f != nil {
return f, nil
} else if !s.allowLookup {
return nil, errors.New("lookup is not enabled")
}
return s.lookupSecretInternal(name)
return s.lookupSecretInternal(ctx, name)
}

// lookupSecretInternal fetches the specified secret from the service and,
// if successful, installs it into the active set.
// The caller must not hold the s.active lock; the call to the service is
// performed outside the lock to avoid stalling other readers.
func (s *Store) lookupSecretInternal(name string) (Secret, error) {
// Impose a loose deadline so requests do not stall forever if the
// infrastructure is farkakte.
getCtx, cancel := context.WithTimeout(s.ctx, 5*time.Minute)
defer cancel()
sv, err := s.client.Get(getCtx, name)
func (s *Store) lookupSecretInternal(ctx context.Context, name string) (Secret, error) {
// Impose a loose deadline if ctx doesn't already have one, so requests do
// not stall forever if the infrastructure is farkakte.
if _, ok := ctx.Deadline(); !ok {
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(s.ctx, 5*time.Minute)
defer cancel()
}
sv, err := s.client.Get(ctx, name)
if err != nil {
return nil, fmt.Errorf("lookup %q: %w", name, err)
}
Expand Down Expand Up @@ -383,7 +386,7 @@ func (s *Store) Watcher(name string) Watcher {
// latest active version of the secret from the service and either adds it to
// the collection or reports an error.
// LookupWatcher does not automatically retry in case of errors.
func (s *Store) LookupWatcher(name string) (Watcher, error) {
func (s *Store) LookupWatcher(ctx context.Context, name string) (Watcher, error) {
s.active.Lock()
defer s.active.Unlock()
var secret Secret
Expand All @@ -397,7 +400,7 @@ func (s *Store) LookupWatcher(name string) (Watcher, error) {
got, err := func() (Secret, error) {
s.active.Unlock() // NOTE: This order is intended.
defer s.active.Lock()
return s.lookupSecretInternal(name)
return s.lookupSecretInternal(ctx, name)
}()
if err != nil {
return Watcher{}, err
Expand Down
14 changes: 7 additions & 7 deletions client/setec/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ func TestStore(t *testing.T) {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if s, err := st.LookupSecret("bravo"); err == nil {
if s, err := st.LookupSecret(ctx, "bravo"); err == nil {
t.Errorf("Lookup(bravo): got %q, want error", s.Get())
}
if w, err := st.LookupWatcher("bravo"); err == nil {
if w, err := st.LookupWatcher(ctx, "bravo"); err == nil {
t.Errorf("Lookup(brav0: got %q, want error", w.Get())
}
})
Expand Down Expand Up @@ -440,21 +440,21 @@ func TestLookup(t *testing.T) {
}

// Case 3: We can look up a secret for "green".
if s, err := st.LookupSecret("green"); err != nil {
if s, err := st.LookupSecret(ctx, "green"); err != nil {
t.Errorf("Lookup(green): unexepcted error: %v", err)
} else if got, want := string(s.Get()), "eggs and ham"; got != want {
t.Errorf("Lookup(green): got %q, want %q", got, want)
}

// Case 4: We can look up a watcher for "blue".
if w, err := st.LookupWatcher("blue"); err != nil {
if w, err := st.LookupWatcher(ctx, "blue"); err != nil {
t.Errorf("Lookup(blue): unexpected error: %v", err)
} else if got, want := string(w.Get()), "dolphins"; got != want {
t.Errorf("Lookup(blue): got %q, want %q", got, want)
}

// Case 5: We still can't lookup a non-existent secret.
if s, err := st.LookupSecret("orange"); err == nil {
if s, err := st.LookupSecret(ctx, "orange"); err == nil {
t.Errorf("Lookup(orange): got %q, want error", s.Get())
} else {
t.Logf("Lookup(orange) correctly failed: %v", err)
Expand Down Expand Up @@ -496,11 +496,11 @@ func TestCacheExpiry(t *testing.T) {
defer st.Close()

advance(25 * time.Second)
if _, err := st.LookupSecret("plum"); err != nil {
if _, err := st.LookupSecret(ctx, "plum"); err != nil {
t.Fatalf("Lookup(plum): unexpected error: %v", err)
}
advance(25 * time.Second)
if _, err := st.LookupSecret("cherry"); err != nil {
if _, err := st.LookupSecret(ctx, "cherry"); err != nil {
t.Fatalf("Lookup(cherry): unexpected error: %v", err)
}
})
Expand Down

0 comments on commit 70d1ccd

Please sign in to comment.