Skip to content

Commit

Permalink
refactor!: Properly support contexts
Browse files Browse the repository at this point in the history
The new AWS SDK uses contexts for all API functions and for loading the
SDK configuration. When chamber was migrated to the SDK, all of them
were set to `context.TODO()`. This commit removes those temporary values
and threads contexts properly throughout the codebase, starting from the
chamber commands.

BREAKING CHANGE: This commit changes the signature of many exported
functions to add a context argument.
  • Loading branch information
bhavanki committed Jun 5, 2024
1 parent e70bb44 commit 3367239
Show file tree
Hide file tree
Showing 24 changed files with 289 additions and 271 deletions.
4 changes: 2 additions & 2 deletions cmd/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func delete(cmd *cobra.Command, args []string) error {
Set("backend", backend),
})
}
secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 54 in cmd/delete.go

View check run for this annotation

Codecov / codecov/patch

cmd/delete.go#L54

Added line #L54 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
Expand All @@ -60,5 +60,5 @@ func delete(cmd *cobra.Command, args []string) error {
Key: key,
}

return secretStore.Delete(secretId)
return secretStore.Delete(cmd.Context(), secretId)

Check warning on line 63 in cmd/delete.go

View check run for this annotation

Codecov / codecov/patch

cmd/delete.go#L63

Added line #L63 was not covered by tests
}
4 changes: 2 additions & 2 deletions cmd/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ func exportEnv(cmd *cobra.Command, args []string) ([]string, error) {
return nil, fmt.Errorf("Failed to validate service: %w", err)
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 67 in cmd/env.go

View check run for this annotation

Codecov / codecov/patch

cmd/env.go#L67

Added line #L67 was not covered by tests
if err != nil {
return nil, fmt.Errorf("Failed to get secret store: %w", err)
}

rawSecrets, err := secretStore.ListRaw(service)
rawSecrets, err := secretStore.ListRaw(cmd.Context(), service)

Check warning on line 72 in cmd/env.go

View check run for this annotation

Codecov / codecov/patch

cmd/env.go#L72

Added line #L72 was not covered by tests
if err != nil {
return nil, fmt.Errorf("Failed to list store contents: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func execRun(cmd *cobra.Command, args []string) error {
}
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 95 in cmd/exec.go

View check run for this annotation

Codecov / codecov/patch

cmd/exec.go#L95

Added line #L95 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
Expand All @@ -108,7 +108,7 @@ func execRun(cmd *cobra.Command, args []string) error {
}
var err error
env = environ.Environ(os.Environ())
err = env.LoadStrict(secretStore, strictValue, pristine, services...)
err = env.LoadStrict(cmd.Context(), secretStore, strictValue, pristine, services...)

Check warning on line 111 in cmd/exec.go

View check run for this annotation

Codecov / codecov/patch

cmd/exec.go#L111

Added line #L111 was not covered by tests
if err != nil {
return err
}
Expand All @@ -120,7 +120,7 @@ func execRun(cmd *cobra.Command, args []string) error {
collisions := make([]string, 0)
var err error
// TODO: these interfaces should look the same as Strict*, so move pristine in there
err = env.Load(secretStore, service, &collisions)
err = env.Load(cmd.Context(), secretStore, service, &collisions)

Check warning on line 123 in cmd/exec.go

View check run for this annotation

Codecov / codecov/patch

cmd/exec.go#L123

Added line #L123 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to list store contents: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func runExport(cmd *cobra.Command, args []string) error {
})
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 56 in cmd/export.go

View check run for this annotation

Codecov / codecov/patch

cmd/export.go#L56

Added line #L56 was not covered by tests
if err != nil {
return err
}
Expand All @@ -64,7 +64,7 @@ func runExport(cmd *cobra.Command, args []string) error {
return fmt.Errorf("Failed to validate service %s: %w", service, err)
}

rawSecrets, err := secretStore.ListRaw(service)
rawSecrets, err := secretStore.ListRaw(cmd.Context(), service)

Check warning on line 67 in cmd/export.go

View check run for this annotation

Codecov / codecov/patch

cmd/export.go#L67

Added line #L67 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to list store contents for service %s: %w", service, err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,18 @@ func find(cmd *cobra.Command, args []string) error {
includeSecrets = true
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 42 in cmd/find.go

View check run for this annotation

Codecov / codecov/patch

cmd/find.go#L42

Added line #L42 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
services, err := secretStore.ListServices(blankService, includeSecrets)
services, err := secretStore.ListServices(cmd.Context(), blankService, includeSecrets)

Check warning on line 46 in cmd/find.go

View check run for this annotation

Codecov / codecov/patch

cmd/find.go#L46

Added line #L46 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to list store contents: %w", err)
}

if byValue {
for _, service := range services {
allSecrets, err := secretStore.List(service, true)
allSecrets, err := secretStore.List(cmd.Context(), service, true)

Check warning on line 53 in cmd/find.go

View check run for this annotation

Codecov / codecov/patch

cmd/find.go#L53

Added line #L53 was not covered by tests
if err == nil {
matches = append(matches, findValueMatch(allSecrets, findSecret)...)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func history(cmd *cobra.Command, args []string) error {
})
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 50 in cmd/history.go

View check run for this annotation

Codecov / codecov/patch

cmd/history.go#L50

Added line #L50 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
Expand All @@ -56,7 +56,7 @@ func history(cmd *cobra.Command, args []string) error {
Key: key,
}

events, err := secretStore.History(secretId)
events, err := secretStore.History(cmd.Context(), secretId)

Check warning on line 59 in cmd/history.go

View check run for this annotation

Codecov / codecov/patch

cmd/history.go#L59

Added line #L59 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get history: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func importRun(cmd *cobra.Command, args []string) error {
})
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 68 in cmd/import.go

View check run for this annotation

Codecov / codecov/patch

cmd/import.go#L68

Added line #L68 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
Expand All @@ -78,7 +78,7 @@ func importRun(cmd *cobra.Command, args []string) error {
Service: service,
Key: key,
}
if err := secretStore.Write(secretId, value); err != nil {
if err := secretStore.Write(cmd.Context(), secretId, value); err != nil {

Check warning on line 81 in cmd/import.go

View check run for this annotation

Codecov / codecov/patch

cmd/import.go#L81

Added line #L81 was not covered by tests
return fmt.Errorf("Failed to write secret: %w", err)
}
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/list-services.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ func listServices(cmd *cobra.Command, args []string) error {
service = utils.NormalizeService(args[0])

}
secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 37 in cmd/list-services.go

View check run for this annotation

Codecov / codecov/patch

cmd/list-services.go#L37

Added line #L37 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
secrets, err := secretStore.ListServices(service, includeSecretName)
secrets, err := secretStore.ListServices(cmd.Context(), service, includeSecretName)

Check warning on line 41 in cmd/list-services.go

View check run for this annotation

Codecov / codecov/patch

cmd/list-services.go#L41

Added line #L41 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to list store contents: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ func list(cmd *cobra.Command, args []string) error {
})
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 57 in cmd/list.go

View check run for this annotation

Codecov / codecov/patch

cmd/list.go#L57

Added line #L57 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
secrets, err := secretStore.List(service, withValues)
secrets, err := secretStore.List(cmd.Context(), service, withValues)

Check warning on line 61 in cmd/list.go

View check run for this annotation

Codecov / codecov/patch

cmd/list.go#L61

Added line #L61 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to list store contents: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func read(cmd *cobra.Command, args []string) error {
})
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 57 in cmd/read.go

View check run for this annotation

Codecov / codecov/patch

cmd/read.go#L57

Added line #L57 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
Expand All @@ -64,7 +64,7 @@ func read(cmd *cobra.Command, args []string) error {
Key: key,
}

secret, err := secretStore.Read(secretId, version)
secret, err := secretStore.Read(cmd.Context(), secretId, version)

Check warning on line 67 in cmd/read.go

View check run for this annotation

Codecov / codecov/patch

cmd/read.go#L67

Added line #L67 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to read: %w", err)
}
Expand Down
11 changes: 6 additions & 5 deletions cmd/root.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"context"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -133,7 +134,7 @@ func validateKey(key string) error {
return nil
}

func getSecretStore() (store.Store, error) {
func getSecretStore(ctx context.Context) (store.Store, error) {

Check warning on line 137 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L137

Added line #L137 was not covered by tests
rootPflags := RootCmd.PersistentFlags()
if backendEnvVarValue := os.Getenv(BackendEnvVar); !rootPflags.Changed("backend") && backendEnvVarValue != "" {
backend = backendEnvVarValue
Expand Down Expand Up @@ -170,7 +171,7 @@ func getSecretStore() (store.Store, error) {
if bucket == "" {
return nil, errors.New("Must set bucket for s3 backend")
}
s, err = store.NewS3StoreWithBucket(numRetries, bucket)
s, err = store.NewS3StoreWithBucket(ctx, numRetries, bucket)

Check warning on line 174 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L174

Added line #L174 was not covered by tests
case S3KMSBackend:
var bucket string
if bucketEnvVarValue := os.Getenv(BucketEnvVar); !rootPflags.Changed("backend-s3-bucket") && bucketEnvVarValue != "" {
Expand All @@ -197,9 +198,9 @@ func getSecretStore() (store.Store, error) {
return nil, errors.New("Must set kmsKeyAlias for S3 KMS backend")
}

s, err = store.NewS3KMSStore(numRetries, bucket, kmsKeyAlias)
s, err = store.NewS3KMSStore(ctx, numRetries, bucket, kmsKeyAlias)

Check warning on line 201 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L201

Added line #L201 was not covered by tests
case SecretsManagerBackend:
s, err = store.NewSecretsManagerStore(numRetries)
s, err = store.NewSecretsManagerStore(ctx, numRetries)

Check warning on line 203 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L203

Added line #L203 was not covered by tests
case SSMBackend:
if kmsKeyAliasFlag != DefaultKMSKey {
return nil, errors.New("Unable to use --kms-key-alias with this backend. Use CHAMBER_KMS_KEY_ALIAS instead.")
Expand All @@ -209,7 +210,7 @@ func getSecretStore() (store.Store, error) {
if err != nil {
return nil, fmt.Errorf("Invalid retry mode %s", retryMode)
}
s, err = store.NewSSMStoreWithRetryMode(numRetries, parsedRetryMode)
s, err = store.NewSSMStoreWithRetryMode(ctx, numRetries, parsedRetryMode)

Check warning on line 213 in cmd/root.go

View check run for this annotation

Codecov / codecov/patch

cmd/root.go#L213

Added line #L213 was not covered by tests
default:
return nil, fmt.Errorf("invalid backend `%s`", backend)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func write(cmd *cobra.Command, args []string) error {
}
}

secretStore, err := getSecretStore()
secretStore, err := getSecretStore(cmd.Context())

Check warning on line 78 in cmd/write.go

View check run for this annotation

Codecov / codecov/patch

cmd/write.go#L78

Added line #L78 was not covered by tests
if err != nil {
return fmt.Errorf("Failed to get secret store: %w", err)
}
Expand All @@ -86,11 +86,11 @@ func write(cmd *cobra.Command, args []string) error {
}

if skipUnchanged {
currentSecret, err := secretStore.Read(secretId, -1)
currentSecret, err := secretStore.Read(cmd.Context(), secretId, -1)

Check warning on line 89 in cmd/write.go

View check run for this annotation

Codecov / codecov/patch

cmd/write.go#L89

Added line #L89 was not covered by tests
if err == nil && value == *currentSecret.Value {
return nil
}
}

return secretStore.Write(secretId, value)
return secretStore.Write(cmd.Context(), secretId, value)

Check warning on line 95 in cmd/write.go

View check run for this annotation

Codecov / codecov/patch

cmd/write.go#L95

Added line #L95 was not covered by tests
}
17 changes: 9 additions & 8 deletions environ/environ.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package environ

import (
"context"
"fmt"
"strings"

Expand Down Expand Up @@ -83,8 +84,8 @@ func normalizeEnvVarName(k string) string {

// load loads environment variables into e from s given a service
// collisions will be populated with any keys that get overwritten
func (e *Environ) load(s store.Store, service string, collisions *[]string) error {
rawSecrets, err := s.ListRaw(utils.NormalizeService(service))
func (e *Environ) load(ctx context.Context, s store.Store, service string, collisions *[]string) error {
rawSecrets, err := s.ListRaw(ctx, utils.NormalizeService(service))

Check warning on line 88 in environ/environ.go

View check run for this annotation

Codecov / codecov/patch

environ/environ.go#L87-L88

Added lines #L87 - L88 were not covered by tests
if err != nil {
return err
}
Expand All @@ -104,20 +105,20 @@ func (e *Environ) load(s store.Store, service string, collisions *[]string) erro

// Load loads environment variables into e from s given a service
// collisions will be populated with any keys that get overwritten
func (e *Environ) Load(s store.Store, service string, collisions *[]string) error {
return e.load(s, service, collisions)
func (e *Environ) Load(ctx context.Context, s store.Store, service string, collisions *[]string) error {
return e.load(ctx, s, service, collisions)

Check warning on line 109 in environ/environ.go

View check run for this annotation

Codecov / codecov/patch

environ/environ.go#L108-L109

Added lines #L108 - L109 were not covered by tests
}

// LoadStrict loads all services from s in strict mode: env vars in e with value equal to valueExpected
// are the only ones substituted. If there are any env vars in s that are also in e, but don't have their value
// set to valueExpected, this is an error.
func (e *Environ) LoadStrict(s store.Store, valueExpected string, pristine bool, services ...string) error {
return e.loadStrict(s, valueExpected, pristine, services...)
func (e *Environ) LoadStrict(ctx context.Context, s store.Store, valueExpected string, pristine bool, services ...string) error {
return e.loadStrict(ctx, s, valueExpected, pristine, services...)

Check warning on line 116 in environ/environ.go

View check run for this annotation

Codecov / codecov/patch

environ/environ.go#L115-L116

Added lines #L115 - L116 were not covered by tests
}

func (e *Environ) loadStrict(s store.Store, valueExpected string, pristine bool, services ...string) error {
func (e *Environ) loadStrict(ctx context.Context, s store.Store, valueExpected string, pristine bool, services ...string) error {

Check warning on line 119 in environ/environ.go

View check run for this annotation

Codecov / codecov/patch

environ/environ.go#L119

Added line #L119 was not covered by tests
for _, service := range services {
rawSecrets, err := s.ListRaw(utils.NormalizeService(service))
rawSecrets, err := s.ListRaw(ctx, utils.NormalizeService(service))

Check warning on line 121 in environ/environ.go

View check run for this annotation

Codecov / codecov/patch

environ/environ.go#L121

Added line #L121 was not covered by tests
if err != nil {
return err
}
Expand Down
26 changes: 14 additions & 12 deletions store/backendbenchmarks_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package store

import (
"context"
"flag"
"fmt"
"math/rand"
Expand Down Expand Up @@ -36,8 +37,9 @@ func RandStringRunes(n int) string {
}

func benchmarkStore(t *testing.T, store Store, services []string) {
setupStore(t, store, services)
defer cleanupStore(t, store, services)
ctx := context.Background()
setupStore(t, ctx, store, services)
defer cleanupStore(t, ctx, store, services)

concurrentExecs := []int{1, 10, 500, 1000}

Expand All @@ -47,7 +49,7 @@ func benchmarkStore(t *testing.T, store Store, services []string) {

for i := 0; i < concurrency; i++ {
wg.Add(1)
go emulateExec(t, &wg, store, services)
go emulateExec(t, ctx, &wg, store, services)

}
wg.Wait()
Expand All @@ -56,11 +58,11 @@ func benchmarkStore(t *testing.T, store Store, services []string) {
}
}

func emulateExec(t *testing.T, wg *sync.WaitGroup, s Store, services []string) error {
func emulateExec(t *testing.T, ctx context.Context, wg *sync.WaitGroup, s Store, services []string) error {
defer wg.Done()
// Exec calls ListRaw once per service specified
for _, service := range services {
_, err := s.ListRaw(service)
_, err := s.ListRaw(ctx, service)
if err != nil {
t.Logf("Failed to execute ListRaw: %s", err)
return err
Expand All @@ -73,15 +75,15 @@ func TestS3StoreConcurrency(t *testing.T) {
if !benchmarkEnabled {
t.SkipNow()
}
s, _ := NewS3StoreWithBucket(10, "chamber-test")
s, _ := NewS3StoreWithBucket(context.Background(), 10, "chamber-test")
benchmarkStore(t, s, []string{"foo"})
}

func TestSecretsManagerStoreConcurrency(t *testing.T) {
if !benchmarkEnabled {
t.SkipNow()
}
s, _ := NewSecretsManagerStore(10)
s, _ := NewSecretsManagerStore(context.Background(), 10)
benchmarkStore(t, s, []string{"foo"})
}

Expand All @@ -90,11 +92,11 @@ func TestSSMConcurrency(t *testing.T) {
t.SkipNow()
}

s, _ := NewSSMStore(10)
s, _ := NewSSMStore(context.Background(), 10)
benchmarkStore(t, s, []string{"foo"})
}

func setupStore(t *testing.T, store Store, services []string) {
func setupStore(t *testing.T, ctx context.Context, store Store, services []string) {
// populate the store for services listed
for _, service := range services {
for i := 0; i < KeysPerService; i++ {
Expand All @@ -104,12 +106,12 @@ func setupStore(t *testing.T, store Store, services []string) {
Key: key,
}

store.Write(id, RandStringRunes(100))
store.Write(ctx, id, RandStringRunes(100))
}
}
}

func cleanupStore(t *testing.T, store Store, services []string) {
func cleanupStore(t *testing.T, ctx context.Context, store Store, services []string) {
for _, service := range services {
for i := 0; i < KeysPerService; i++ {
key := fmt.Sprintf("var%d", i)
Expand All @@ -118,7 +120,7 @@ func cleanupStore(t *testing.T, store Store, services []string) {
Key: key,
}

store.Delete(id)
store.Delete(ctx, id)
}
}
}
Loading

0 comments on commit 3367239

Please sign in to comment.