From 4ab10e19857f09428c90356714d304a54ad8d34a Mon Sep 17 00:00:00 2001 From: Sean McGrail Date: Tue, 12 Jan 2021 15:42:21 -0800 Subject: [PATCH] Change aws.Config#Retryer to be a value provider. --- aws/config.go | 11 ++++++++--- aws/retry/example_test.go | 8 +++++--- aws/retry/internal/mock/client.go | 2 +- aws/retry/internal/mock/config.go | 2 +- config/load_options.go | 10 +++++----- config/provider.go | 4 ++-- config/resolve_credentials.go | 13 +++++++++---- feature/ec2/imds/api_client.go | 5 ++++- .../apigateway/internal/customizations/unit_test.go | 4 +++- service/ec2/internal/customizations/unit_test.go | 4 +++- .../customizations/custom_error_deser_test.go | 4 +++- .../internal/customizations/sanitizeurl_test.go | 4 +++- service/s3/internal/customizations/presign_test.go | 4 +++- service/s3/internal/customizations/unit_test.go | 4 +++- 14 files changed, 53 insertions(+), 26 deletions(-) diff --git a/aws/config.go b/aws/config.go index f2c977e058a..481aa156381 100644 --- a/aws/config.go +++ b/aws/config.go @@ -42,10 +42,15 @@ type Config struct { // service and region Please see the `aws.EndpointResolver` documentation on usage. EndpointResolver EndpointResolver - // Retryer guides how HTTP requests should be retried in case of - // recoverable failures. When nil the API client will use a default + // Retryer is a function that provides a Retryer implementation. A Retryer guides how HTTP requests should be + // retried in case of recoverable failures. When nil the API client will use a default // retryer. - Retryer Retryer + // + // In general, the provider function should return a new instance of a Retyer if you are attempting + // to provide a consistent Retryer configuration across all clients. This will ensure that each client will be + // provided a new instance of the Retryer implementation, and will avoid issues such as sharing the same retry token + // bucket across services. + Retryer func() Retryer // ConfigSources are the sources that were used to construct the Config. // Allows for additional configuration to be loaded by clients. diff --git a/aws/retry/example_test.go b/aws/retry/example_test.go index b3b042d522c..d135559c858 100644 --- a/aws/retry/example_test.go +++ b/aws/retry/example_test.go @@ -14,9 +14,11 @@ import ( ) func Example_overrideForAllClients() { - custom := retry.AddWithMaxBackoffDelay(retry.NewStandard(), time.Second*5) - - cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRetryer(custom)) + cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRetryer(func() aws.Retryer { + // Generally you will always want to return new instance of a Retryer. This will avoid a global rate limit + // bucket being shared between across all service clients. + return retry.AddWithMaxBackoffDelay(retry.NewStandard(), time.Second*5) + })) if err != nil { log.Fatal(err) return diff --git a/aws/retry/internal/mock/client.go b/aws/retry/internal/mock/client.go index 1df971b449e..33821dedb6a 100644 --- a/aws/retry/internal/mock/client.go +++ b/aws/retry/internal/mock/client.go @@ -11,7 +11,7 @@ type Options struct { Retryer aws.Retryer } -// Options is a mock Client +// Client is a mock service client type Client struct{} // GetObjectInput is mock input diff --git a/aws/retry/internal/mock/config.go b/aws/retry/internal/mock/config.go index 1c3f8f45734..88f0ac377bd 100644 --- a/aws/retry/internal/mock/config.go +++ b/aws/retry/internal/mock/config.go @@ -12,6 +12,6 @@ func LoadDefaultConfig(context.Context, ...func()) (cfg aws.Config, err error) { } // WithRetryer is a mock for config.WithRetryer -func WithRetryer(v aws.Retryer) (f func()) { +func WithRetryer(v func() aws.Retryer) (f func()) { return f } diff --git a/config/load_options.go b/config/load_options.go index c296033271c..bd12a1fde61 100644 --- a/config/load_options.go +++ b/config/load_options.go @@ -33,9 +33,9 @@ type LoadOptions struct { // service and region Please see the `aws.EndpointResolver` documentation on usage. EndpointResolver aws.EndpointResolver - // Retryer guides how HTTP requests should be retried in case of - // recoverable failures. - Retryer aws.Retryer + // Retryer is a function that provides a Retryer implementation. A Retryer guides how HTTP requests should be + // retried in case of recoverable failures. + Retryer func() aws.Retryer // APIOptions provides the set of middleware mutations modify how the API // client requests will be handled. This is useful for adding additional @@ -477,7 +477,7 @@ func WithAPIOptions(v []func(*middleware.Stack) error) LoadOptionsFunc { } } -func (o LoadOptions) getRetryer(ctx context.Context) (aws.Retryer, bool, error) { +func (o LoadOptions) getRetryer(ctx context.Context) (func() aws.Retryer, bool, error) { if o.Retryer == nil { return nil, false, nil } @@ -489,7 +489,7 @@ func (o LoadOptions) getRetryer(ctx context.Context) (aws.Retryer, bool, error) // that sets Retryer on LoadOptions. If Retryer is set to nil, the // Retryer value is ignored. If multiple WithRetryer calls are // made, the last call overrides the previous call values. -func WithRetryer(v aws.Retryer) LoadOptionsFunc { +func WithRetryer(v func() aws.Retryer) LoadOptionsFunc { return func(o *LoadOptions) error { o.Retryer = v return nil diff --git a/config/provider.go b/config/provider.go index cd282923517..68406c6b881 100644 --- a/config/provider.go +++ b/config/provider.go @@ -373,10 +373,10 @@ func getClientLogMode(ctx context.Context, configs configs) (m aws.ClientLogMode // retryProvider is an configuration provider for custom Retryer. type retryProvider interface { - getRetryer(ctx context.Context) (aws.Retryer, bool, error) + getRetryer(ctx context.Context) (func() aws.Retryer, bool, error) } -func getRetryer(ctx context.Context, configs configs) (v aws.Retryer, found bool, err error) { +func getRetryer(ctx context.Context, configs configs) (v func() aws.Retryer, found bool, err error) { for _, c := range configs { if p, ok := c.(retryProvider); ok { v, found, err = p.getRetryer(ctx) diff --git a/config/resolve_credentials.go b/config/resolve_credentials.go index ddadc54ef11..35b48c4391c 100644 --- a/config/resolve_credentials.go +++ b/config/resolve_credentials.go @@ -202,7 +202,9 @@ func resolveHTTPCredProvider(ctx context.Context, cfg *aws.Config, url, authToke options.AuthorizationToken = authToken } options.APIOptions = cfg.APIOptions - options.Retryer = cfg.Retryer + if cfg.Retryer != nil { + options.Retryer = cfg.Retryer() + } }, } @@ -258,10 +260,13 @@ func resolveEC2RoleCredentials(ctx context.Context, cfg *aws.Config, configs con optFns = append(optFns, func(o *ec2rolecreds.Options) { // Only define a client from config if not already defined. if o.Client != nil { - o.Client = imds.New(imds.Options{ + options := imds.Options{ HTTPClient: cfg.HTTPClient, - Retryer: cfg.Retryer, - }) + } + if cfg.Retryer != nil { + options.Retryer = cfg.Retryer() + } + o.Client = imds.New(options) } }) diff --git a/feature/ec2/imds/api_client.go b/feature/ec2/imds/api_client.go index 4eb73c979f9..9d512f857d5 100644 --- a/feature/ec2/imds/api_client.go +++ b/feature/ec2/imds/api_client.go @@ -101,7 +101,10 @@ func NewFromConfig(cfg aws.Config, optFns ...func(*Options)) *Client { opts := Options{ APIOptions: append([]func(*middleware.Stack) error{}, cfg.APIOptions...), HTTPClient: cfg.HTTPClient, - Retryer: cfg.Retryer, + } + + if cfg.Retryer != nil { + opts.Retryer = cfg.Retryer() } return New(opts, optFns...) diff --git a/service/apigateway/internal/customizations/unit_test.go b/service/apigateway/internal/customizations/unit_test.go index 6c13c4a46a6..86eb219b3d0 100644 --- a/service/apigateway/internal/customizations/unit_test.go +++ b/service/apigateway/internal/customizations/unit_test.go @@ -51,7 +51,9 @@ func Test_EmptyResponse(t *testing.T) { SigningName: "apigateway", }, nil }), - Retryer: aws.NopRetryer{}, + Retryer: func() aws.Retryer { + return aws.NopRetryer{} + }, } client := apigateway.NewFromConfig(cfg) diff --git a/service/ec2/internal/customizations/unit_test.go b/service/ec2/internal/customizations/unit_test.go index bacb89b4f7c..e8bbfae7bcf 100644 --- a/service/ec2/internal/customizations/unit_test.go +++ b/service/ec2/internal/customizations/unit_test.go @@ -51,7 +51,9 @@ func Test_EmptyResponse(t *testing.T) { SigningName: "ec2", }, nil }), - Retryer: aws.NopRetryer{}, + Retryer: func() aws.Retryer { + return aws.NopRetryer{} + }, } client := ec2.NewFromConfig(cfg) diff --git a/service/route53/internal/customizations/custom_error_deser_test.go b/service/route53/internal/customizations/custom_error_deser_test.go index de976a6e824..4e269fbdfec 100644 --- a/service/route53/internal/customizations/custom_error_deser_test.go +++ b/service/route53/internal/customizations/custom_error_deser_test.go @@ -80,7 +80,9 @@ func TestCustomErrorDeserialization(t *testing.T) { SigningName: "route53", }, nil }), - Retryer: aws.NopRetryer{}, + Retryer: func() aws.Retryer { + return aws.NopRetryer{} + }, }) resp, err := svc.ChangeResourceRecordSets(context.Background(), &route53.ChangeResourceRecordSetsInput{ ChangeBatch: &types.ChangeBatch{ diff --git a/service/route53/internal/customizations/sanitizeurl_test.go b/service/route53/internal/customizations/sanitizeurl_test.go index 0aea417a029..5d275203cd8 100644 --- a/service/route53/internal/customizations/sanitizeurl_test.go +++ b/service/route53/internal/customizations/sanitizeurl_test.go @@ -37,7 +37,9 @@ func TestSanitizeURLMiddleware(t *testing.T) { t.Run(name, func(t *testing.T) { cfg := aws.Config{ Credentials: unit.StubCredentialsProvider{}, - Retryer: aws.NopRetryer{}, + Retryer: func() aws.Retryer { + return aws.NopRetryer{} + }, Region: "mock-region", } diff --git a/service/s3/internal/customizations/presign_test.go b/service/s3/internal/customizations/presign_test.go index d97d4495f4b..e5b63d67556 100644 --- a/service/s3/internal/customizations/presign_test.go +++ b/service/s3/internal/customizations/presign_test.go @@ -133,7 +133,9 @@ func TestPutObject_PresignURL(t *testing.T) { cfg := aws.Config{ Region: "us-west-2", Credentials: unit.StubCredentialsProvider{}, - Retryer: aws.NopRetryer{}, + Retryer: func() aws.Retryer { + return aws.NopRetryer{} + }, } presignClient := s3.NewPresignClient(s3.NewFromConfig(cfg), func(options *s3.PresignOptions) { options = &c.options diff --git a/service/s3/internal/customizations/unit_test.go b/service/s3/internal/customizations/unit_test.go index 4bf401c1f70..41b3e15805a 100644 --- a/service/s3/internal/customizations/unit_test.go +++ b/service/s3/internal/customizations/unit_test.go @@ -51,7 +51,9 @@ func Test_EmptyResponse(t *testing.T) { SigningName: "s3", }, nil }), - Retryer: aws.NopRetryer{}, + Retryer: func() aws.Retryer { + return aws.NopRetryer{} + }, } client := s3.NewFromConfig(cfg, func(options *s3.Options) {