feat!: Switch from min throttle delay to retry mode

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.

cmd/root.go

@@ -9,6 +9,7 @@
 	"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 @@
 	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 @@
 
 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 @@
 			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)
 	}

store/s3store.go

@@ -69,7 +69,7 @@
 }
 
 func NewS3StoreWithBucket(numRetries int, bucket string) (*S3Store, error) {
-	config, _, err := getConfig(numRetries)
+	config, _, err := getConfig(numRetries, aws.RetryModeStandard)
 	if err != nil {
 		return nil, err
 	}

store/s3storeKMS.go

@@ -42,7 +42,7 @@
 }
 
 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
 	}

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
 	}

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)
 			},

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 {

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)
+}

store/ssmstore.go

@@ -18,9 +18,8 @@
 	// 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 @@
 
 // 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 {

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, &notFoundError)
 	})
 
-	// 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{}