diff --git a/README.md b/README.md index 48027ae7..119f5824 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,19 @@ secrets in SSM Parameter Store, an AWS service for storing secrets. For detailed info about using chamber, please read [The Right Way To Manage Secrets](https://aws.amazon.com/blogs/mt/the-right-way-to-store-secrets-using-parameter-store/) +## v3.0 Breaking Changes + +_Version 3.0 has not yet been released. Changes described here are forward-looking._ + +* **Use of the SSM Parameter Store's path-based API is now required.** Support + added in v2.0 to avoid it has been removed. The `CHAMBER_NO_PATHS` environment + variable no longer has any effect. You must migrate to the new storage format + using the instructions below. +* **The `--min-throttle-delay` option no longer has any effect.** Support for + specifying a minimum throttle delay has been removed from the underlying AWS + SDK with no direct replacement. Instead, set the new `--retry-mode` option to + "adaptive" to use an experimental model that accounts for throttling errors. + ## v2.0 Breaking Changes Starting with version 2.0, chamber uses parameter store's path based API by default. diff --git a/cmd/root.go b/cmd/root.go index ae5bcf6b..5221697f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/aws/aws-sdk-go-v2/aws" analytics "github.com/segmentio/analytics-go/v3" "github.com/segmentio/chamber/v2/store" "github.com/spf13/cobra" @@ -22,9 +23,11 @@ var ( validServiceFormatWithLabel = regexp.MustCompile(`^[\w\-\.\:]+$`) validServicePathFormatWithLabel = regexp.MustCompile(`^[\w\-\.]+((\/[\w\-\.]+)+(\:[\w\-\.]+)*)?$`) - verbose bool - numRetries int + verbose bool + numRetries int + // Deprecated: Use retryMode instead. minThrottleDelay time.Duration + retryMode string chamberVersion string // one of *Backend consts backend string @@ -74,7 +77,12 @@ var RootCmd = &cobra.Command{ func init() { RootCmd.PersistentFlags().IntVarP(&numRetries, "retries", "r", DefaultNumRetries, "For SSM or Secrets Manager, the number of retries we'll make before giving up; AKA $CHAMBER_RETRIES") - RootCmd.PersistentFlags().DurationVarP(&minThrottleDelay, "min-throttle-delay", "", store.DefaultMinThrottleDelay, "For SSM, minimal delay before retrying throttled requests. Default 500ms.") + RootCmd.PersistentFlags().DurationVarP(&minThrottleDelay, "min-throttle-delay", "", 0, "DEPRECATED and no longer has any effect. Use retry-mode instead") + RootCmd.PersistentFlags().StringVarP(&retryMode, "retry-mode", "", store.DefaultRetryMode.String(), + `For SSM, the model used to retry requests + `+aws.RetryModeStandard.String()+` + `+aws.RetryModeAdaptive.String(), + ) RootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "", false, "Print more information to STDOUT") RootCmd.PersistentFlags().StringVarP(&backendFlag, "backend", "b", "ssm", `Backend to use; AKA $CHAMBER_SECRET_BACKEND @@ -213,7 +221,11 @@ func getSecretStore() (store.Store, error) { return nil, errors.New("Unable to use --kms-key-alias with this backend. Use CHAMBER_KMS_KEY_ALIAS instead.") } - s, err = store.NewSSMStoreWithMinThrottleDelay(numRetries, minThrottleDelay) + parsedRetryMode, err := aws.ParseRetryMode(retryMode) + if err != nil { + return nil, fmt.Errorf("Invalid retry mode %s", retryMode) + } + s, err = store.NewSSMStoreWithRetryMode(numRetries, parsedRetryMode) default: return nil, fmt.Errorf("invalid backend `%s`", backend) } diff --git a/store/s3store.go b/store/s3store.go index 3a83af1f..13bbca74 100644 --- a/store/s3store.go +++ b/store/s3store.go @@ -56,7 +56,7 @@ type S3Store struct { } func NewS3StoreWithBucket(numRetries int, bucket string) (*S3Store, error) { - config, _, err := getConfig(numRetries) + config, _, err := getConfig(numRetries, aws.RetryModeStandard) if err != nil { return nil, err } diff --git a/store/s3storeKMS.go b/store/s3storeKMS.go index 416e8d76..abdca4e2 100644 --- a/store/s3storeKMS.go +++ b/store/s3storeKMS.go @@ -42,7 +42,7 @@ type S3KMSStore struct { } func NewS3KMSStore(numRetries int, bucket string, kmsKeyAlias string) (*S3KMSStore, error) { - config, _, err := getConfig(numRetries) + config, _, err := getConfig(numRetries, aws.RetryModeStandard) if err != nil { return nil, err } diff --git a/store/secretsmanagerstore.go b/store/secretsmanagerstore.go index 51488196..85018468 100644 --- a/store/secretsmanagerstore.go +++ b/store/secretsmanagerstore.go @@ -81,7 +81,7 @@ type SecretsManagerStore struct { // NewSecretsManagerStore creates a new SecretsManagerStore func NewSecretsManagerStore(numRetries int) (*SecretsManagerStore, error) { - cfg, _, err := getConfig(numRetries) + cfg, _, err := getConfig(numRetries, aws.RetryModeStandard) if err != nil { return nil, err } diff --git a/store/secretsmanagerstore_test.go b/store/secretsmanagerstore_test.go index 2d3d32ef..c52eaa65 100644 --- a/store/secretsmanagerstore_test.go +++ b/store/secretsmanagerstore_test.go @@ -121,7 +121,7 @@ func mockGetCallerIdentity(_ *sts.GetCallerIdentityInput) (*sts.GetCallerIdentit func NewTestSecretsManagerStore(secrets map[string]mockSecret, outputs map[string]secretsmanager.DescribeSecretOutput) *SecretsManagerStore { return &SecretsManagerStore{ - svc: &apiSecretsManagerMock{ + svc: &apiSecretsManagerMock{ CreateSecretFunc: func(ctx context.Context, params *secretsmanager.CreateSecretInput, optFns ...func(*secretsmanager.Options)) (*secretsmanager.CreateSecretOutput, error) { return mockCreateSecret(params, secrets) }, diff --git a/store/shared.go b/store/shared.go index 2f6be6a8..1a5e30f6 100644 --- a/store/shared.go +++ b/store/shared.go @@ -14,12 +14,12 @@ const ( CustomSSMEndpointEnvVar = "CHAMBER_AWS_SSM_ENDPOINT" ) -func getConfig(numRetries int) (aws.Config, string, error) { +func getConfig(numRetries int, retryMode aws.RetryMode) (aws.Config, string, error) { endpointResolver := func(service, region string, options ...interface{}) (aws.Endpoint, error) { customSsmEndpoint, ok := os.LookupEnv(CustomSSMEndpointEnvVar) if ok { return aws.Endpoint{ - URL: customSsmEndpoint, + URL: customSsmEndpoint, Source: aws.EndpointSourceCustom, }, nil } @@ -35,6 +35,7 @@ func getConfig(numRetries int) (aws.Config, string, error) { cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion(region), config.WithRetryMaxAttempts(numRetries), + config.WithRetryMode(retryMode), config.WithEndpointResolverWithOptions(aws.EndpointResolverWithOptionsFunc(endpointResolver)), ) if err != nil { diff --git a/store/shared_test.go b/store/shared_test.go new file mode 100644 index 00000000..6093a226 --- /dev/null +++ b/store/shared_test.go @@ -0,0 +1,39 @@ +package store + +import ( + "os" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/assert" +) + +func TestGetConfig(t *testing.T) { + originalEndpoint := os.Getenv(CustomSSMEndpointEnvVar) + os.Setenv(CustomSSMEndpointEnvVar, "https://example.com/custom-endpoint") + if originalEndpoint != "" { + defer os.Setenv(CustomSSMEndpointEnvVar, originalEndpoint) + } else { + defer os.Unsetenv(CustomSSMEndpointEnvVar) + } + + originalRegion := os.Getenv(RegionEnvVar) + os.Setenv(RegionEnvVar, "us-west-2") + if originalRegion != "" { + defer os.Setenv(RegionEnvVar, originalRegion) + } else { + defer os.Unsetenv(RegionEnvVar) + } + + config, region, err := getConfig(3, aws.RetryModeStandard) + + assert.NoError(t, err) + assert.Equal(t, "us-west-2", region) + + endpoint, err := config.EndpointResolverWithOptions.ResolveEndpoint("ssm", "us-west-2") + assert.Equal(t, "https://example.com/custom-endpoint", endpoint.URL) + assert.Equal(t, aws.EndpointSourceCustom, endpoint.Source) + + assert.Equal(t, 3, config.RetryMaxAttempts) + assert.Equal(t, aws.RetryModeStandard, config.RetryMode) +} diff --git a/store/ssmstore.go b/store/ssmstore.go index 1c007f82..0bf1822d 100644 --- a/store/ssmstore.go +++ b/store/ssmstore.go @@ -18,9 +18,8 @@ const ( // DefaultKeyID is the default alias for the KMS key used to encrypt/decrypt secrets DefaultKeyID = "alias/parameter_store_key" - // DefaultMinThrottleDelay is the default delay before retrying throttled requests - // DefaultMinThrottleDelay = client.DefaultRetryerMinThrottleDelay - DefaultMinThrottleDelay = 0 + // DefaultRetryMode is the default retry mode for AWS SDK configurations. + DefaultRetryMode = aws.RetryModeStandard ) // validPathKeyFormat is the format that is expected for key names inside parameter store @@ -47,28 +46,30 @@ type SSMStore struct { // NewSSMStore creates a new SSMStore func NewSSMStore(numRetries int) (*SSMStore, error) { - return ssmStoreUsingRetryer(numRetries, DefaultMinThrottleDelay) + return ssmStoreUsingRetryer(numRetries, DefaultRetryMode) } // NewSSMStoreWithMinThrottleDelay creates a new SSMStore with the aws sdk max retries and min throttle delay are configured. +// +// Deprecated: The AWS SDK no longer supports specifying a minimum throttle delay. Instead, use +// NewSSMStoreWithRetryMode. func NewSSMStoreWithMinThrottleDelay(numRetries int, minThrottleDelay time.Duration) (*SSMStore, error) { - return ssmStoreUsingRetryer(numRetries, minThrottleDelay) + return ssmStoreUsingRetryer(numRetries, DefaultRetryMode) } -func ssmStoreUsingRetryer(numRetries int, minThrottleDelay time.Duration) (*SSMStore, error) { - cfg, _, err := getConfig(numRetries) +// NewSSMStoreWithRetryMode creates a new SSMStore, configuring the underlying AWS SDK with the +// given maximum number of retries and retry mode. +func NewSSMStoreWithRetryMode(numRetries int, retryMode aws.RetryMode) (*SSMStore, error) { + return ssmStoreUsingRetryer(numRetries, retryMode) +} + +func ssmStoreUsingRetryer(numRetries int, retryMode aws.RetryMode) (*SSMStore, error) { + cfg, _, err := getConfig(numRetries, retryMode) if err != nil { return nil, err } - // FIXME minThrottleDelay is ignored - // retryer := retry.NewStandard( - // func(o *retry.StandardOptions) { - // o.MaxAttempts = numRetries - // }, - // ) - usePaths := true _, ok := os.LookupEnv("CHAMBER_NO_PATHS") if ok { diff --git a/store/ssmstore_test.go b/store/ssmstore_test.go index 43221df6..3690bed8 100644 --- a/store/ssmstore_test.go +++ b/store/ssmstore_test.go @@ -270,19 +270,19 @@ func NewTestSSMStore(parameters map[string]mockParameter, usePaths bool) *SSMSto return mockDeleteParameter(params, parameters) }, DescribeParametersFunc: func(ctx context.Context, params *ssm.DescribeParametersInput, optFns ...func(*ssm.Options)) (*ssm.DescribeParametersOutput, error) { - return mockDescribeParameters(params, parameters); + return mockDescribeParameters(params, parameters) }, GetParameterHistoryFunc: func(ctx context.Context, params *ssm.GetParameterHistoryInput, optFns ...func(*ssm.Options)) (*ssm.GetParameterHistoryOutput, error) { - return mockGetParameterHistory(params, parameters); + return mockGetParameterHistory(params, parameters) }, GetParametersFunc: func(ctx context.Context, params *ssm.GetParametersInput, optFns ...func(*ssm.Options)) (*ssm.GetParametersOutput, error) { - return mockGetParameters(params, parameters); + return mockGetParameters(params, parameters) }, GetParametersByPathFunc: func(ctx context.Context, params *ssm.GetParametersByPathInput, optFns ...func(*ssm.Options)) (*ssm.GetParametersByPathOutput, error) { - return mockGetParametersByPath(params, parameters); + return mockGetParametersByPath(params, parameters) }, PutParameterFunc: func(ctx context.Context, params *ssm.PutParameterInput, optFns ...func(*ssm.Options)) (*ssm.PutParameterOutput, error) { - return mockPutParameter(params, parameters); + return mockPutParameter(params, parameters) }, }, } @@ -330,24 +330,22 @@ func TestNewSSMStore(t *testing.T) { assert.ErrorAs(t, err, ¬FoundError) }) - // FIXME minThrottleDelay is ignored - // t.Run("Should set aws sdk min throttle delay to default", func(t *testing.T) { - // s, err := NewSSMStore(1) - // assert.Nil(t, err) - // assert.Equal(t, DefaultMinThrottleDelay, s.svc.(*ssm.SSM).Config.Retryer.(client.DefaultRetryer).MinThrottleDelay) - // }) + t.Run("Should set AWS SDK retry mode to default", func(t *testing.T) { + s, err := NewSSMStore(1) + assert.Nil(t, err) + assert.Equal(t, DefaultRetryMode, s.config.RetryMode) + }) } -// FIXME minThrottleDelay is ignored -// func TestNewSSMStoreMinThrottleDelay(t *testing.T) { -// t.Run("Should configure aws sdk retryer - num max retries and min throttle delay", func(t *testing.T) { -// s, err := NewSSMStoreWithMinThrottleDelay(2, time.Duration(1000)*time.Millisecond) -// assert.Nil(t, err) -// assert.Equal(t, 2, s.config.Retryer().MaxAttempts()) -// assert.Equal(t, time.Duration(1000)*time.Millisecond, s.svc.(*ssm.SSM).Config.Retryer.(client.DefaultRetryer).MinThrottleDelay) -// }) -// } +func TestNewSSMStoreWithRetryMode(t *testing.T) { + t.Run("Should configure AWS SDK max attempts and retry mode", func(t *testing.T) { + s, err := NewSSMStoreWithRetryMode(2, aws.RetryModeAdaptive) + assert.Nil(t, err) + assert.Equal(t, 2, s.config.RetryMaxAttempts) + assert.Equal(t, aws.RetryModeAdaptive, s.config.RetryMode) + }) +} func TestWrite(t *testing.T) { parameters := map[string]mockParameter{}