From b4e159080ed8a8757e6cbda1be83535b7db35834 Mon Sep 17 00:00:00 2001 From: Bill Havanki Date: Thu, 30 May 2024 13:28:41 -0400 Subject: [PATCH] feat!: Switch from min throttle delay to retry mode (#488) The v2 AWS SDK has no equivalent setting for a minimum delay for throttle errors. So, the delay has been replaced with the new retry mode configuration setting that is available. The "adaptive" mode accounts for throttle errors, although it is not configurable any further than that, and the mode itself is still experimental. The choice of retry mode still only applies to the SSM parameter store, but it could be extended to other implementations very easily. The `--min-throttle-delay` option remains in place so that existing scripted calls don't break, but the value no longer has any effect. The README is updated accordingly, mentioning the loss of min throttle delay as a breaking change. The removal of the `CHAMBER_NO_PATHS` avoiding of the path-based API for the SSM parameter store is also added as a breaking change, although that already happened when chamber moved to the v2 SDK. --- README.md | 13 +++++++++++ cmd/root.go | 20 ++++++++++++---- store/s3store.go | 2 +- store/s3storeKMS.go | 2 +- store/secretsmanagerstore.go | 2 +- store/secretsmanagerstore_test.go | 2 +- store/shared.go | 5 ++-- store/shared_test.go | 39 +++++++++++++++++++++++++++++++ store/ssmstore.go | 29 ++++++++++++----------- store/ssmstore_test.go | 38 ++++++++++++++---------------- 10 files changed, 108 insertions(+), 44 deletions(-) create mode 100644 store/shared_test.go 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{}