Browse Source

retryer implementation to handle throttling exceptions on AWS (#1331)

* awsretryer implemented for AWS providers
Stanislaw Scherban 3 years ago
parent
commit
eb8e614755

+ 1 - 1
docs/snippets/full-secret-store.yaml

@@ -14,7 +14,7 @@ spec:
   # You can specify retry settings for the http connection
   # these fields allow you to set a maxRetries before failure, and
   # an interval between the retries.
-  # Current supported providers: IBM
+  # Current supported providers: AWS, IBM
   retrySettings:
     maxRetries: 5
     retryInterval: "10s"

+ 2 - 2
pkg/provider/aws/parameterstore/parameterstore.go

@@ -53,10 +53,10 @@ const (
 )
 
 // New constructs a ParameterStore Provider that is specific to a store.
-func New(sess *session.Session) (*ParameterStore, error) {
+func New(sess *session.Session, cfg *aws.Config) (*ParameterStore, error) {
 	return &ParameterStore{
 		sess:   sess,
-		client: ssm.New(sess),
+		client: ssm.New(sess, cfg),
 	}, nil
 }
 

+ 37 - 2
pkg/provider/aws/provider.go

@@ -17,8 +17,12 @@ package aws
 import (
 	"context"
 	"fmt"
+	"time"
 
+	"github.com/aws/aws-sdk-go/aws"
+	awsclient "github.com/aws/aws-sdk-go/aws/client"
 	"github.com/aws/aws-sdk-go/aws/endpoints"
+	"github.com/aws/aws-sdk-go/aws/request"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
@@ -39,6 +43,7 @@ const (
 	errUnableCreateSession    = "unable to create session: %w"
 	errUnknownProviderService = "unknown AWS Provider Service: %s"
 	errRegionNotFound         = "region not found: %s"
+	errInitAWSProvider        = "unable to initialize aws provider: %s"
 )
 
 // NewClient constructs a new secrets client based on the provided store.
@@ -98,17 +103,47 @@ func newClient(ctx context.Context, store esv1beta1.GenericStore, kube client.Cl
 	if err != nil {
 		return nil, err
 	}
+	if store == nil {
+		return nil, fmt.Errorf(errInitAWSProvider, "nil store")
+	}
+	storeSpec := store.GetSpec()
+	var cfg *aws.Config
 
 	sess, err := awsauth.New(ctx, store, kube, namespace, assumeRoler, awsauth.DefaultJWTProvider)
 	if err != nil {
 		return nil, fmt.Errorf(errUnableCreateSession, err)
 	}
 
+	// Setup retry options, if present in storeSpec
+	if storeSpec.RetrySettings != nil {
+		var retryAmount int
+		var retryDuration time.Duration
+
+		if storeSpec.RetrySettings.MaxRetries != nil {
+			retryAmount = int(*storeSpec.RetrySettings.MaxRetries)
+		} else {
+			retryAmount = 3
+		}
+
+		if storeSpec.RetrySettings.RetryInterval != nil {
+			retryDuration, err = time.ParseDuration(*storeSpec.RetrySettings.RetryInterval)
+		}
+		if err != nil {
+			return nil, fmt.Errorf(errInitAWSProvider, err)
+		}
+		awsRetryer := awsclient.DefaultRetryer{
+			NumMaxRetries:    retryAmount,
+			MinRetryDelay:    retryDuration,
+			MaxThrottleDelay: 120 * time.Second,
+		}
+		cfg = request.WithRetryer(aws.NewConfig(), awsRetryer)
+	}
+
 	switch prov.Service {
 	case esv1beta1.AWSServiceSecretsManager:
-		return secretsmanager.New(sess)
+		return secretsmanager.New(sess, cfg)
 	case esv1beta1.AWSServiceParameterStore:
-		return parameterstore.New(sess)
+		return parameterstore.New(sess, cfg)
 	}
 	return nil, fmt.Errorf(errUnknownProviderService, prov.Service)
 }

+ 72 - 0
pkg/provider/aws/provider_test.go

@@ -16,13 +16,20 @@ package aws
 
 import (
 	"context"
+	"fmt"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/session"
+	"github.com/aws/aws-sdk-go/service/sts/stsiface"
+	"github.com/crossplane/crossplane-runtime/pkg/test"
 	"github.com/stretchr/testify/assert"
+	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
+	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 	clientfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
@@ -343,3 +350,68 @@ func TestValidateStore(t *testing.T) {
 		})
 	}
 }
+
+func TestValidRetryInput(t *testing.T) {
+	invalid := "Invalid"
+	spec := &esv1beta1.SecretStore{
+		Spec: esv1beta1.SecretStoreSpec{
+			Provider: &esv1beta1.SecretStoreProvider{
+				AWS: &esv1beta1.AWSProvider{
+					Service: "ParameterStore",
+					Region:  validRegion,
+					Auth: esv1beta1.AWSAuth{
+						SecretRef: &esv1beta1.AWSAuthSecretRef{
+							SecretAccessKey: esmeta.SecretKeySelector{
+								Name:      "sak",
+								Namespace: pointer.String("OK"),
+								Key:       "sak",
+							},
+							AccessKeyID: esmeta.SecretKeySelector{
+								Name:      "ak",
+								Namespace: pointer.String("OK"),
+								Key:       "ak",
+							},
+						},
+					},
+				},
+			},
+			RetrySettings: &esv1beta1.SecretStoreRetrySettings{
+				RetryInterval: &invalid,
+			},
+		},
+	}
+
+	expected := fmt.Sprintf("unable to initialize aws provider: time: invalid duration %q", invalid)
+	ctx := context.TODO()
+
+	kube := &test.MockClient{
+		MockGet: test.NewMockGetFn(nil, func(obj kclient.Object) error {
+			if o, ok := obj.(*corev1.Secret); ok {
+				o.Data = map[string][]byte{
+					"sak": []byte("OK"),
+					"ak":  []byte("OK"),
+				}
+				return nil
+			}
+			return nil
+		}),
+	}
+
+	provider := func(*session.Session) stsiface.STSAPI { return nil }
+
+	_, err := newClient(ctx, spec, kube, "default", provider)
+
+	if !ErrorContains(err, expected) {
+		t.Errorf("CheckValidRetryInput unexpected error: %s, expected: '%s'", err.Error(), expected)
+	}
+}
+
+func ErrorContains(out error, want string) bool {
+	if out == nil {
+		return want == ""
+	}
+	if want == "" {
+		return false
+	}
+	return strings.Contains(out.Error(), want)
+}

+ 3 - 2
pkg/provider/aws/secretsmanager/secretsmanager.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/session"
 	awssm "github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/tidwall/gjson"
@@ -57,10 +58,10 @@ const (
 var log = ctrl.Log.WithName("provider").WithName("aws").WithName("secretsmanager")
 
 // New creates a new SecretsManager client.
-func New(sess *session.Session) (*SecretsManager, error) {
+func New(sess *session.Session, cfg *aws.Config) (*SecretsManager, error) {
 	return &SecretsManager{
 		sess:   sess,
-		client: awssm.New(sess),
+		client: awssm.New(sess, cfg),
 		cache:  make(map[string]*awssm.GetSecretValueOutput),
 	}, nil
 }