Repository: crossplane/provider-aws (go)

Issue:
Unneccessary API call ListTagsForResource in EKS Addons controller
### What happened?

The current controller implementation for EKS Addons uses `ListTagsForResource` to list the tags for API controller. This is not necessary since the tags are already provided through `DescribeAddon` which is called anyway.

The referred line: https://github.com/crossplane/provider-aws/blob/96a15aa76c5060da9769605acb4a3824ec521d0f/pkg/controller/eks/addon/setup.go#L138

### What environment did it happen in?

Current master (96a15aa76c5060da9769605acb4a3824ec521d0f)

Pull request:
fix(eks-addon): Replace ListTagsForResource with DescribeAddon to save one API call
### Description of your changes

Fixes #1067 

This removes the `ListTagsForResource` call from the `Addon` controller in `isUpToDate` and replaces it with `DescribeAddon` in `postUpdate`.

Unfortunately AWS does not include a `Tag` field in `UpdateAddon` so the `postUpdate` method cannot be further simplified.

I have:

- [x] Read and followed Crossplane's [contribution process].
- [x] Run `make reviewable test` to ensure this PR is ready for review.

### How has this code been tested

Manually

[contribution process]: https://git.io/fj2m9


Patch:
diff --git a/apis/v1beta1/providerconfig_types.go b/apis/v1beta1/providerconfig_types.go
index 08eb10d00..d1c25af5d 100644
--- a/apis/v1beta1/providerconfig_types.go
+++ b/apis/v1beta1/providerconfig_types.go
@@ -31,6 +31,10 @@ type ProviderConfigSpec struct {
 	// +optional
 	AssumeRoleARN *string `json:"assumeRoleARN,omitempty"`
 
+	// ExternalID is the external ID used when assuming role.
+	// +optional
+	ExternalID *string `json:"externalID,omitempty"`
+
 	// Endpoint is where you can override the default endpoint configuration
 	// of AWS calls made by the provider.
 	// +optional
diff --git a/apis/v1beta1/zz_generated.deepcopy.go b/apis/v1beta1/zz_generated.deepcopy.go
index 63f48b77e..5fb733709 100644
--- a/apis/v1beta1/zz_generated.deepcopy.go
+++ b/apis/v1beta1/zz_generated.deepcopy.go
@@ -153,6 +153,11 @@ func (in *ProviderConfigSpec) DeepCopyInto(out *ProviderConfigSpec) {
 		*out = new(string)
 		**out = **in
 	}
+	if in.ExternalID != nil {
+		in, out := &in.ExternalID, &out.ExternalID
+		*out = new(string)
+		**out = **in
+	}
 	if in.Endpoint != nil {
 		in, out := &in.Endpoint, &out.Endpoint
 		*out = new(EndpointConfig)
diff --git a/package/crds/aws.crossplane.io_providerconfigs.yaml b/package/crds/aws.crossplane.io_providerconfigs.yaml
index f05bfca87..a8f971b01 100644
--- a/package/crds/aws.crossplane.io_providerconfigs.yaml
+++ b/package/crds/aws.crossplane.io_providerconfigs.yaml
@@ -207,6 +207,9 @@ spec:
                 required:
                 - url
                 type: object
+              externalID:
+                description: ExternalID is the external ID used when assuming role.
+                type: string
             required:
             - credentials
             type: object
diff --git a/pkg/clients/aws.go b/pkg/clients/aws.go
index 1b0721ec6..ff0faeb5d 100644
--- a/pkg/clients/aws.go
+++ b/pkg/clients/aws.go
@@ -298,7 +298,11 @@ func UseProviderSecretAssumeRole(ctx context.Context, data []byte, profile, regi
 	}))
 
 	stsSvc := sts.NewFromConfig(config)
-	stsAssume := stscreds.NewAssumeRoleProvider(stsSvc, StringValue(pc.Spec.AssumeRoleARN))
+	stsAssume := stscreds.NewAssumeRoleProvider(
+		stsSvc,
+		StringValue(pc.Spec.AssumeRoleARN),
+		func(opt *stscreds.AssumeRoleOptions) { opt.ExternalID = pc.Spec.ExternalID },
+	)
 	config.Credentials = aws.NewCredentialsCache(stsAssume)
 
 	return &config, err
@@ -320,6 +324,7 @@ func UsePodServiceAccountAssumeRole(ctx context.Context, _ []byte, _, region str
 			stscreds.NewAssumeRoleProvider(
 				stsclient,
 				StringValue(pc.Spec.AssumeRoleARN),
+				func(opt *stscreds.AssumeRoleOptions) { opt.ExternalID = pc.Spec.ExternalID },
 			)),
 		),
 	)
@@ -411,7 +416,11 @@ func UseProviderSecretV1AssumeRole(ctx context.Context, data []byte, pc *v1beta1
 	}
 
 	stsSvc := sts.NewFromConfig(config)
-	stsAssume := stscreds.NewAssumeRoleProvider(stsSvc, StringValue(pc.Spec.AssumeRoleARN))
+	stsAssume := stscreds.NewAssumeRoleProvider(
+		stsSvc,
+		StringValue(pc.Spec.AssumeRoleARN),
+		func(opt *stscreds.AssumeRoleOptions) { opt.ExternalID = pc.Spec.ExternalID },
+	)
 	config.Credentials = aws.NewCredentialsCache(stsAssume)
 
 	v2creds, err := config.Credentials.Retrieve(ctx)
@@ -475,6 +484,7 @@ func UsePodServiceAccountV1AssumeRole(ctx context.Context, _ []byte, pc *v1beta1
 			stscreds.NewAssumeRoleProvider(
 				stsclient,
 				StringValue(pc.Spec.AssumeRoleARN),
+				func(opt *stscreds.AssumeRoleOptions) { opt.ExternalID = pc.Spec.ExternalID },
 			)),
 		),
 	)
diff --git a/pkg/controller/eks/addon/setup.go b/pkg/controller/eks/addon/setup.go
index db75a04c8..df1102cb7 100644
--- a/pkg/controller/eks/addon/setup.go
+++ b/pkg/controller/eks/addon/setup.go
@@ -43,13 +43,8 @@ import (
 const (
 	errNotEKSCluster    = "managed resource is not an EKS cluster custom resource"
 	errKubeUpdateFailed = "cannot update EKS cluster custom resource"
-	errListTags         = "cannot list tags"
 	errTagResource      = "cannot tag resource"
 	errUntagResource    = "cannot untag resource"
-
-	statusCreating = "CREATING"
-	statusActive   = "ACTIVE"
-	statusDeleting = "DELETING"
 )
 
 // SetupAddon adds a controller that reconciles Clusters.
@@ -104,12 +99,16 @@ func postObserve(_ context.Context, cr *v1alpha1.Addon, _ *awseks.DescribeAddonO
 	}
 
 	switch awsclients.StringValue(cr.Status.AtProvider.Status) {
-	case statusCreating:
+	case awseks.AddonStatusCreating:
 		cr.SetConditions(xpv1.Creating())
-	case statusDeleting:
+	case awseks.AddonStatusDeleting:
 		cr.SetConditions(xpv1.Deleting())
-	case statusActive:
+	case awseks.AddonStatusActive:
 		cr.SetConditions(xpv1.Available())
+	case awseks.AddonStatusUpdating:
+		cr.SetConditions(xpv1.Available().WithMessage(awseks.AddonStatusUpdating))
+		// Prevent Update() call during update state - which will fail.
+		obs.ResourceUpToDate = true
 	default:
 		cr.SetConditions(xpv1.Unavailable())
 	}
@@ -124,25 +123,14 @@ func lateInitialize(spec *v1alpha1.AddonParameters, resp *awseks.DescribeAddonOu
 }
 
 func (h *hooks) isUpToDate(cr *v1alpha1.Addon, resp *awseks.DescribeAddonOutput) (bool, error) {
-	if resp.Addon == nil {
-		return false, nil
-	}
-
 	switch {
-	case resp.Addon == nil:
-	case cr.Spec.ForProvider.AddonVersion != nil && awsclients.StringValue(cr.Spec.ForProvider.AddonVersion) != awsclients.StringValue(resp.Addon.AddonVersion):
-	case cr.Spec.ForProvider.ServiceAccountRoleARN != nil && awsclients.StringValue(cr.Spec.ForProvider.ServiceAccountRoleARN) != awsclients.StringValue(resp.Addon.ServiceAccountRoleArn):
+	case resp.Addon == nil,
+		cr.Spec.ForProvider.AddonVersion != nil && awsclients.StringValue(cr.Spec.ForProvider.AddonVersion) != awsclients.StringValue(resp.Addon.AddonVersion),
+		cr.Spec.ForProvider.ServiceAccountRoleARN != nil && awsclients.StringValue(cr.Spec.ForProvider.ServiceAccountRoleARN) != awsclients.StringValue(resp.Addon.ServiceAccountRoleArn):
 		return false, nil
 	}
 
-	tags, err := h.client.ListTagsForResource(&awseks.ListTagsForResourceInput{
-		ResourceArn: awsclients.String(meta.GetExternalName(cr)),
-	})
-	if err != nil {
-		return false, errors.Wrap(err, errListTags)
-	}
-	add, remove := awsclients.DiffTagsMapPtr(cr.Spec.ForProvider.Tags, tags.Tags)
-
+	add, remove := awsclients.DiffTagsMapPtr(cr.Spec.ForProvider.Tags, resp.Addon.Tags)
 	return len(add) == 0 && len(remove) == 0, nil
 }
 
@@ -156,14 +144,17 @@ func (h *hooks) postUpdate(ctx context.Context, cr *v1alpha1.Addon, resp *awseks
 		return managed.ExternalUpdate{}, err
 	}
 
-	tags, err := h.client.ListTagsForResource(&awseks.ListTagsForResourceInput{
-		ResourceArn: awsclients.String(meta.GetExternalName(cr)),
+	// Tag update needs to separate because UpdateAddon does not include tags (for unknown reason).
+
+	desc, err := h.client.DescribeAddonWithContext(ctx, &awseks.DescribeAddonInput{
+		AddonName:   cr.Spec.ForProvider.AddonName,
+		ClusterName: cr.Spec.ForProvider.ClusterName,
 	})
-	if err != nil {
-		return managed.ExternalUpdate{}, errors.Wrap(err, errListTags)
+	if err != nil || desc.Addon == nil {
+		return managed.ExternalUpdate{}, errors.Wrap(err, errDescribe)
 	}
-	add, remove := awsclients.DiffTagsMapPtr(cr.Spec.ForProvider.Tags, tags.Tags)
 
+	add, remove := awsclients.DiffTagsMapPtr(cr.Spec.ForProvider.Tags, desc.Addon.Tags)
 	if len(add) > 0 {
 		_, err := h.client.TagResourceWithContext(ctx, &awseks.TagResourceInput{
 			ResourceArn: awsclients.String(meta.GetExternalName(cr)),


Test patch:
diff --git a/pkg/controller/eks/addon/setup_test.go b/pkg/controller/eks/addon/setup_test.go
index fe0be9575..2f6a9e9b2 100644
--- a/pkg/controller/eks/addon/setup_test.go
+++ b/pkg/controller/eks/addon/setup_test.go
@@ -25,7 +25,6 @@ import (
 	"github.com/pkg/errors"
 
 	awseks "github.com/aws/aws-sdk-go/service/eks"
-	"github.com/aws/aws-sdk-go/service/eks/eksiface"
 
 	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
 	"github.com/crossplane/crossplane-runtime/pkg/meta"
@@ -51,8 +50,10 @@ var (
 	errBoom                   = errors.New("boom")
 )
 
+type mockClientFn func(t *testing.T) *mockeksiface.MockEKSAPI
+
 type args struct {
-	eks eksiface.EKSAPI
+	eks mockClientFn
 	cr  *v1alpha1.Addon
 }
 
@@ -74,13 +75,15 @@ func withStatus(s v1alpha1.AddonObservation) AddonModifier {
 	return func(r *v1alpha1.Addon) { r.Status.AtProvider = s }
 }
 
-type mockClientModifier func(*mockeksiface.MockEKSAPI)
+type mockClientModifier func(me *mockeksiface.MockEKSAPI)
 
-func mockClient(t *testing.T, mod mockClientModifier) *mockeksiface.MockEKSAPI {
-	ctrl := gomock.NewController(t)
-	mock := mockeksiface.NewMockEKSAPI(ctrl)
-	mod(mock)
-	return mock
+func mockClient(m mockClientModifier) mockClientFn {
+	return func(t *testing.T) *mockeksiface.MockEKSAPI {
+		ctrl := gomock.NewController(t)
+		mock := mockeksiface.NewMockEKSAPI(ctrl)
+		m(mock)
+		return mock
+	}
 }
 
 func addon(m ...AddonModifier) *v1alpha1.Addon {
@@ -104,7 +107,7 @@ func TestObserve(t *testing.T) {
 	}{
 		"SuccessfulAvailable": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						DescribeAddonWithContext(
 							context.Background(),
@@ -112,16 +115,9 @@ func TestObserve(t *testing.T) {
 						).
 						Return(&awseks.DescribeAddonOutput{
 							Addon: &awseks.Addon{
-								Status: awsclient.String(statusActive),
+								Status: awsclient.String(awseks.AddonStatusActive),
 							},
 						}, nil)
-					me.EXPECT().
-						ListTagsForResource(&awseks.ListTagsForResourceInput{
-							ResourceArn: &testExternalName,
-						}).
-						Return(&awseks.ListTagsForResourceOutput{
-							Tags: map[string]*string{},
-						}, nil)
 				}),
 				cr: addon(
 					withExternalName(testExternalName),
@@ -132,7 +128,7 @@ func TestObserve(t *testing.T) {
 					withExternalName(testExternalName),
 					withConditions(xpv1.Available()),
 					withStatus(v1alpha1.AddonObservation{
-						Status: awsclient.String(statusActive),
+						Status: awsclient.String(awseks.AddonStatusActive),
 					}),
 				),
 				result: managed.ExternalObservation{
@@ -144,7 +140,7 @@ func TestObserve(t *testing.T) {
 		},
 		"FailedDescribeRequest": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						DescribeAddonWithContext(
 							context.Background(),
@@ -163,35 +159,9 @@ func TestObserve(t *testing.T) {
 				err: awsclient.Wrap(errBoom, errDescribe),
 			},
 		},
-		"FailedListTagsForResource": {
-			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
-					me.EXPECT().
-						DescribeAddonWithContext(
-							context.Background(),
-							&awseks.DescribeAddonInput{},
-						).
-						Return(&awseks.DescribeAddonOutput{Addon: &awseks.Addon{}}, nil)
-					me.EXPECT().
-						ListTagsForResource(&awseks.ListTagsForResourceInput{
-							ResourceArn: &testExternalName,
-						}).
-						Return(nil, errBoom)
-				}),
-				cr: addon(
-					withExternalName(testExternalName),
-				),
-			},
-			want: want{
-				cr: addon(
-					withExternalName(testExternalName),
-				),
-				err: awsclient.Wrap(errors.Wrap(errBoom, errListTags), "isUpToDate check failed"),
-			},
-		},
 		"LateInitSuccess": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						DescribeAddonWithContext(
 							context.Background(),
@@ -200,16 +170,9 @@ func TestObserve(t *testing.T) {
 						Return(&awseks.DescribeAddonOutput{
 							Addon: &awseks.Addon{
 								ServiceAccountRoleArn: &testServiceAccountRoleArn,
-								Status:                awsclient.String(statusActive),
+								Status:                awsclient.String(awseks.AddonStatusActive),
 							},
 						}, nil)
-					me.EXPECT().
-						ListTagsForResource(&awseks.ListTagsForResourceInput{
-							ResourceArn: &testExternalName,
-						}).
-						Return(&awseks.ListTagsForResourceOutput{
-							Tags: map[string]*string{},
-						}, nil)
 				}),
 				cr: addon(
 					withExternalName(testExternalName),
@@ -225,7 +188,7 @@ func TestObserve(t *testing.T) {
 						},
 					),
 					withStatus(v1alpha1.AddonObservation{
-						Status: awsclient.String(statusActive),
+						Status: awsclient.String(awseks.AddonStatusActive),
 					}),
 				),
 				result: managed.ExternalObservation{
@@ -239,7 +202,7 @@ func TestObserve(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			e := newExternal(nil, tc.eks, []option{setupHooks})
+			e := newExternal(nil, tc.eks(t), []option{setupHooks})
 			o, err := e.Observe(context.Background(), tc.args.cr)
 
 			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
@@ -268,7 +231,7 @@ func TestCreate(t *testing.T) {
 	}{
 		"Successful": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						CreateAddonWithContext(
 							context.Background(),
@@ -320,7 +283,7 @@ func TestCreate(t *testing.T) {
 		},
 		"FailedRequest": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						CreateAddonWithContext(
 							context.Background(),
@@ -366,7 +329,7 @@ func TestCreate(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			e := newExternal(nil, tc.eks, []option{setupHooks})
+			e := newExternal(nil, tc.eks(t), []option{setupHooks})
 			o, err := e.Create(context.Background(), tc.args.cr)
 
 			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
@@ -395,7 +358,7 @@ func TestUpdate(t *testing.T) {
 	}{
 		"Successful": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						UpdateAddonWithContext(
 							context.Background(),
@@ -409,12 +372,18 @@ func TestUpdate(t *testing.T) {
 						).
 						Return(&awseks.UpdateAddonOutput{}, nil)
 					me.EXPECT().
-						ListTagsForResource(&awseks.ListTagsForResourceInput{
-							ResourceArn: &testExternalName,
-						}).
-						Return(&awseks.ListTagsForResourceOutput{
-							Tags: map[string]*string{
-								testOtherTagKey: &testOtherTagValue,
+						DescribeAddonWithContext(
+							context.Background(),
+							&awseks.DescribeAddonInput{
+								AddonName:   &testAddonName,
+								ClusterName: &testClusterName,
+							},
+						).
+						Return(&awseks.DescribeAddonOutput{
+							Addon: &awseks.Addon{
+								Tags: map[string]*string{
+									testOtherTagKey: &testOtherTagValue,
+								},
 							},
 						}, nil)
 					me.EXPECT().
@@ -479,7 +448,7 @@ func TestUpdate(t *testing.T) {
 		},
 		"FailedUpdateRequest": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						UpdateAddonWithContext(
 							context.Background(),
@@ -522,9 +491,9 @@ func TestUpdate(t *testing.T) {
 				err: awsclient.Wrap(errBoom, errUpdate),
 			},
 		},
-		"FailedListTagsRequest": {
+		"FailedDescribeAddon": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						UpdateAddonWithContext(
 							context.Background(),
@@ -538,9 +507,13 @@ func TestUpdate(t *testing.T) {
 						).
 						Return(&awseks.UpdateAddonOutput{}, nil)
 					me.EXPECT().
-						ListTagsForResource(&awseks.ListTagsForResourceInput{
-							ResourceArn: &testExternalName,
-						}).
+						DescribeAddonWithContext(
+							context.Background(),
+							&awseks.DescribeAddonInput{
+								AddonName:   &testAddonName,
+								ClusterName: &testClusterName,
+							},
+						).
 						Return(nil, errBoom)
 				}),
 				cr: addon(
@@ -569,12 +542,12 @@ func TestUpdate(t *testing.T) {
 						},
 					}),
 				),
-				err: awsclient.Wrap(errBoom, errListTags),
+				err: awsclient.Wrap(errBoom, errDescribe),
 			},
 		},
 		"FailedTagResource": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						UpdateAddonWithContext(
 							context.Background(),
@@ -588,12 +561,18 @@ func TestUpdate(t *testing.T) {
 						).
 						Return(&awseks.UpdateAddonOutput{}, nil)
 					me.EXPECT().
-						ListTagsForResource(&awseks.ListTagsForResourceInput{
-							ResourceArn: &testExternalName,
-						}).
-						Return(&awseks.ListTagsForResourceOutput{
-							Tags: map[string]*string{
-								testOtherTagKey: &testOtherTagValue,
+						DescribeAddonWithContext(
+							context.Background(),
+							&awseks.DescribeAddonInput{
+								AddonName:   &testAddonName,
+								ClusterName: &testClusterName,
+							},
+						).
+						Return(&awseks.DescribeAddonOutput{
+							Addon: &awseks.Addon{
+								Tags: map[string]*string{
+									testOtherTagKey: &testOtherTagValue,
+								},
 							},
 						}, nil)
 					me.EXPECT().
@@ -650,7 +629,7 @@ func TestUpdate(t *testing.T) {
 		},
 		"UntagResource": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						UpdateAddonWithContext(
 							context.Background(),
@@ -664,12 +643,18 @@ func TestUpdate(t *testing.T) {
 						).
 						Return(&awseks.UpdateAddonOutput{}, nil)
 					me.EXPECT().
-						ListTagsForResource(&awseks.ListTagsForResourceInput{
-							ResourceArn: &testExternalName,
-						}).
-						Return(&awseks.ListTagsForResourceOutput{
-							Tags: map[string]*string{
-								testOtherTagKey: &testOtherTagValue,
+						DescribeAddonWithContext(
+							context.Background(),
+							&awseks.DescribeAddonInput{
+								AddonName:   &testAddonName,
+								ClusterName: &testClusterName,
+							},
+						).
+						Return(&awseks.DescribeAddonOutput{
+							Addon: &awseks.Addon{
+								Tags: map[string]*string{
+									testOtherTagKey: &testOtherTagValue,
+								},
 							},
 						}, nil)
 					me.EXPECT().
@@ -736,7 +721,7 @@ func TestUpdate(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			e := newExternal(nil, tc.eks, []option{setupHooks})
+			e := newExternal(nil, tc.eks(t), []option{setupHooks})
 			o, err := e.Update(context.Background(), tc.args.cr)
 
 			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
@@ -764,7 +749,7 @@ func TestDelete(t *testing.T) {
 	}{
 		"Successful": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						DeleteAddonWithContext(
 							context.Background(),
@@ -798,7 +783,7 @@ func TestDelete(t *testing.T) {
 		},
 		"FailedRequest": {
 			args: args{
-				eks: mockClient(t, func(me *mockeksiface.MockEKSAPI) {
+				eks: mockClient(func(me *mockeksiface.MockEKSAPI) {
 					me.EXPECT().
 						DeleteAddonWithContext(
 							context.Background(),
@@ -835,7 +820,7 @@ func TestDelete(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			e := newExternal(nil, tc.eks, []option{setupHooks})
+			e := newExternal(nil, tc.eks(t), []option{setupHooks})
 			err := e.Delete(context.Background(), tc.args.cr)
 
 			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {