diff --git a/go.mod b/go.mod index 2346312e90..1cae1c91b9 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/aws/aws-sdk-go v1.55.5 github.com/aws/aws-sdk-go-v2 v1.32.6 github.com/aws/aws-sdk-go-v2/config v1.27.27 + github.com/aws/aws-sdk-go-v2/credentials v1.17.27 github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.11 github.com/aws/aws-sdk-go-v2/service/acm v1.28.4 github.com/aws/aws-sdk-go-v2/service/appmesh v1.27.7 @@ -14,6 +15,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi v1.23.3 github.com/aws/aws-sdk-go-v2/service/servicediscovery v1.31.7 github.com/aws/aws-sdk-go-v2/service/shield v1.27.3 + github.com/aws/aws-sdk-go-v2/service/sts v1.30.3 github.com/aws/aws-sdk-go-v2/service/wafregional v1.23.3 github.com/aws/aws-sdk-go-v2/service/wafv2 v1.51.4 github.com/aws/smithy-go v1.22.1 @@ -56,16 +58,13 @@ require ( github.com/ajg/form v1.5.1 // indirect github.com/andybalholm/brotli v1.0.4 // indirect github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535 // indirect - github.com/aws/aws-sdk-go-v2/credentials v1.17.27 // indirect github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 // indirect github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25 // indirect github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect - github.com/aws/aws-sdk-go-v2/service/iam v1.36.3 // indirect github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3 // indirect github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.17 // indirect github.com/aws/aws-sdk-go-v2/service/sso v1.22.4 // indirect github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.4 // indirect - github.com/aws/aws-sdk-go-v2/service/sts v1.30.3 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/chai2010/gettext-go v1.0.2 // indirect diff --git a/go.sum b/go.sum index 6676d7940c..292ee96ce6 100644 --- a/go.sum +++ b/go.sum @@ -60,7 +60,6 @@ github.com/aws/aws-sdk-go-v2/service/ec2 v1.173.0 h1:ta62lid9JkIpKZtZZXSj6rP2AqY github.com/aws/aws-sdk-go-v2/service/ec2 v1.173.0/go.mod h1:o6QDjdVKpP5EF0dp/VlvqckzuSDATr1rLdHt3A5m0YY= github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.43.1 h1:L9Wt9zgtoYKIlaeFTy+EztGjL4oaXBBGtVXA+jaeYko= github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.43.1/go.mod h1:yxzLdxt7bVGvIOPYIKFtiaJCJnx2ChlIIvlhW4QgI6M= -github.com/aws/aws-sdk-go-v2/service/iam v1.36.3/go.mod h1:HSvujsK8xeEHMIB18oMXjSfqaN9cVqpo/MtHJIksQRk= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3 h1:dT3MqvGhSoaIhRseqw2I0yH81l7wiR2vjs57O51EAm8= github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3/go.mod h1:GlAeCkHwugxdHaueRr4nhPuY+WW+gR8UjlcqzPr1SPI= github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.17 h1:HGErhhrxZlQ044RiM+WdoZxp0p+EGM62y3L6pwA4olE= diff --git a/pkg/deploy/shield/protection_manager.go b/pkg/deploy/shield/protection_manager.go index b8266fbad7..3741e6eb01 100644 --- a/pkg/deploy/shield/protection_manager.go +++ b/pkg/deploy/shield/protection_manager.go @@ -88,22 +88,46 @@ func (m *defaultProtectionManager) CreateProtection(ctx context.Context, resourc } func (m *defaultProtectionManager) DeleteProtection(ctx context.Context, resourceARN string, protectionID string) error { - req := &shieldsdk.DeleteProtectionInput{ - ProtectionId: awssdk.String(protectionID), - } - m.logger.Info("disabling shield protection", - "resourceARN", resourceARN, - "protectionID", protectionID) - _, err := m.shieldClient.DeleteProtectionWithContext(ctx, req) - if err != nil { - return err - } - m.logger.Info("disabled shield protection", - "resourceARN", resourceARN) + req := &shieldsdk.DeleteProtectionInput{ + ProtectionId: awssdk.String(protectionID), + } + m.logger.Info("disabling shield protection", + "resourceARN", resourceARN, + "protectionID", protectionID) + _, err := m.shieldClient.DeleteProtectionWithContext(ctx, req) + if err != nil { + return err + } + m.logger.Info("disabled shield protection", + "resourceARN", resourceARN) + + // Remove the protection info from the cache + m.protectionInfoByResourceARNCache.Delete(resourceARN) + + // Verify that the protection resource is deleted + err = m.verifyProtectionDeleted(ctx, protectionID) + if err != nil { + return err + } + + return nil +} - var protectionInfo *ProtectionInfo - m.protectionInfoByResourceARNCache.Set(resourceARN, protectionInfo, m.protectionInfoByResourceARNCacheTTL) - return nil +func (m *defaultProtectionManager) verifyProtectionDeleted(ctx context.Context, protectionID string) error { + req := &shieldsdk.DescribeProtectionInput{ + ProtectionId: awssdk.String(protectionID), + } + _, err := m.shieldClient.DescribeProtectionWithContext(ctx, req) + if err != nil { + var resourceNotFoundException *shieldtypes.ResourceNotFoundException + if errors.As(err, &resourceNotFoundException) { + // Protection resource is successfully deleted + return nil + } + return err + } + // Protection resource still exists + return errors.New("protection resource still exists") } func (m *defaultProtectionManager) GetProtection(ctx context.Context, resourceARN string) (*ProtectionInfo, error) { diff --git a/pkg/deploy/shield/protection_manager_test.go b/pkg/deploy/shield/protection_manager_test.go index 255f1e7c02..798fe651f5 100644 --- a/pkg/deploy/shield/protection_manager_test.go +++ b/pkg/deploy/shield/protection_manager_test.go @@ -2,11 +2,11 @@ package shield import ( "context" - shieldtypes "github.com/aws/aws-sdk-go-v2/service/shield/types" "testing" "time" - shieldsdk "github.com/aws/aws-sdk-go-v2/service/shield" + shieldtypes "github.com/aws/aws-sdk-go-v2/service/shield/types" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/go-logr/logr" "github.com/golang/mock/gomock" "github.com/pkg/errors" @@ -14,6 +14,7 @@ import ( "k8s.io/apimachinery/pkg/util/cache" "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services" "sigs.k8s.io/controller-runtime/pkg/log" + shieldsdk "github.com/aws/aws-sdk-go-v2/service/shield" ) func Test_defaultProtectionManager_IsSubscribed(t *testing.T) { @@ -169,3 +170,129 @@ func Test_defaultProtectionManager_IsSubscribed(t *testing.T) { }) } } + +func Test_defaultProtectionManager_DeleteProtection(t *testing.T) { + type deleteProtectionCall struct { + req *shieldsdk.DeleteProtectionInput + resp *shieldsdk.DeleteProtectionOutput + err error + } + type describeProtectionCall struct { + req *shieldsdk.DescribeProtectionInput + resp *shieldsdk.DescribeProtectionOutput + err error + } + type fields struct { + deleteProtectionCalls []deleteProtectionCall + describeProtectionCalls []describeProtectionCall + protectionInfoByResourceARNCacheTTL time.Duration + } + type testCase struct { + resourceARN string + protectionID string + wantErr error + } + tests := []struct { + name string + fields fields + testCases []testCase + }{ + { + name: "delete protection successfully", + fields: fields{ + deleteProtectionCalls: []deleteProtectionCall{ + { + req: &shieldsdk.DeleteProtectionInput{ProtectionId: aws.String("protection-id")}, + resp: &shieldsdk.DeleteProtectionOutput{}, + }, + }, + describeProtectionCalls: []describeProtectionCall{ + { + req: &shieldsdk.DescribeProtectionInput{ProtectionId: aws.String("protection-id")}, + err: &shieldtypes.ResourceNotFoundException{}, + }, + }, + protectionInfoByResourceARNCacheTTL: 10 * time.Minute, + }, + testCases: []testCase{ + { + resourceARN: "resource-arn", + protectionID: "protection-id", + }, + }, + }, + { + name: "delete protection fails", + fields: fields{ + deleteProtectionCalls: []deleteProtectionCall{ + { + req: &shieldsdk.DeleteProtectionInput{ProtectionId: aws.String("protection-id")}, + err: errors.New("some aws api error"), + }, + }, + protectionInfoByResourceARNCacheTTL: 10 * time.Minute, + }, + testCases: []testCase{ + { + resourceARN: "resource-arn", + protectionID: "protection-id", + wantErr: errors.New("some aws api error"), + }, + }, + }, + { + name: "protection still exists after deletion", + fields: fields{ + deleteProtectionCalls: []deleteProtectionCall{ + { + req: &shieldsdk.DeleteProtectionInput{ProtectionId: aws.String("protection-id")}, + resp: &shieldsdk.DeleteProtectionOutput{}, + }, + }, + describeProtectionCalls: []describeProtectionCall{ + { + req: &shieldsdk.DescribeProtectionInput{ProtectionId: aws.String("protection-id")}, + resp: &shieldsdk.DescribeProtectionOutput{Protection: &shieldtypes.Protection{}}, + }, + }, + protectionInfoByResourceARNCacheTTL: 10 * time.Minute, + }, + testCases: []testCase{ + { + resourceARN: "resource-arn", + protectionID: "protection-id", + wantErr: errors.New("protection resource still exists"), + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + shieldClient := services.NewMockShield(ctrl) + for _, call := range tt.fields.deleteProtectionCalls { + shieldClient.EXPECT().DeleteProtectionWithContext(gomock.Any(), call.req).Return(call.resp, call.err) + } + for _, call := range tt.fields.describeProtectionCalls { + shieldClient.EXPECT().DescribeProtectionWithContext(gomock.Any(), call.req).Return(call.resp, call.err) + } + + m := &defaultProtectionManager{ + shieldClient: shieldClient, + logger: logr.New(&log.NullLogSink{}), + protectionInfoByResourceARNCache: cache.NewExpiring(), + protectionInfoByResourceARNCacheTTL: tt.fields.protectionInfoByResourceARNCacheTTL, + } + for _, testCase := range tt.testCases { + err := m.DeleteProtection(context.Background(), testCase.resourceARN, testCase.protectionID) + if testCase.wantErr != nil { + assert.EqualError(t, err, testCase.wantErr.Error()) + } else { + assert.NoError(t, err) + } + } + }) + } +}