Skip to content

Commit b3a5a35

Browse files
Alexander LahutsinLahutsin
Alexander Lahutsin
authored andcommitted
patch-fix-1
1 parent 57472c4 commit b3a5a35

File tree

4 files changed

+170
-21
lines changed

4 files changed

+170
-21
lines changed

go.mod

+2-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ require (
66
github.com/aws/aws-sdk-go v1.55.5
77
github.com/aws/aws-sdk-go-v2 v1.32.6
88
github.com/aws/aws-sdk-go-v2/config v1.27.27
9+
github.com/aws/aws-sdk-go-v2/credentials v1.17.27
910
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.11
1011
github.com/aws/aws-sdk-go-v2/service/acm v1.28.4
1112
github.com/aws/aws-sdk-go-v2/service/appmesh v1.27.7
@@ -14,6 +15,7 @@ require (
1415
github.com/aws/aws-sdk-go-v2/service/resourcegroupstaggingapi v1.23.3
1516
github.com/aws/aws-sdk-go-v2/service/servicediscovery v1.31.7
1617
github.com/aws/aws-sdk-go-v2/service/shield v1.27.3
18+
github.com/aws/aws-sdk-go-v2/service/sts v1.30.3
1719
github.com/aws/aws-sdk-go-v2/service/wafregional v1.23.3
1820
github.com/aws/aws-sdk-go-v2/service/wafv2 v1.51.4
1921
github.com/aws/smithy-go v1.22.1
@@ -56,16 +58,13 @@ require (
5658
github.com/ajg/form v1.5.1 // indirect
5759
github.com/andybalholm/brotli v1.0.4 // indirect
5860
github.com/asaskevich/govalidator v0.0.0-20200428143746-21a406dcc535 // indirect
59-
github.com/aws/aws-sdk-go-v2/credentials v1.17.27 // indirect
6061
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.25 // indirect
6162
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.25 // indirect
6263
github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
63-
github.com/aws/aws-sdk-go-v2/service/iam v1.36.3 // indirect
6464
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3 // indirect
6565
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.17 // indirect
6666
github.com/aws/aws-sdk-go-v2/service/sso v1.22.4 // indirect
6767
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.26.4 // indirect
68-
github.com/aws/aws-sdk-go-v2/service/sts v1.30.3 // indirect
6968
github.com/beorn7/perks v1.0.1 // indirect
7069
github.com/cespare/xxhash/v2 v2.3.0 // indirect
7170
github.com/chai2010/gettext-go v1.0.2 // indirect

go.sum

-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ github.com/aws/aws-sdk-go-v2/service/ec2 v1.173.0 h1:ta62lid9JkIpKZtZZXSj6rP2AqY
6060
github.com/aws/aws-sdk-go-v2/service/ec2 v1.173.0/go.mod h1:o6QDjdVKpP5EF0dp/VlvqckzuSDATr1rLdHt3A5m0YY=
6161
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.43.1 h1:L9Wt9zgtoYKIlaeFTy+EztGjL4oaXBBGtVXA+jaeYko=
6262
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.43.1/go.mod h1:yxzLdxt7bVGvIOPYIKFtiaJCJnx2ChlIIvlhW4QgI6M=
63-
github.com/aws/aws-sdk-go-v2/service/iam v1.36.3/go.mod h1:HSvujsK8xeEHMIB18oMXjSfqaN9cVqpo/MtHJIksQRk=
6463
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3 h1:dT3MqvGhSoaIhRseqw2I0yH81l7wiR2vjs57O51EAm8=
6564
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.3/go.mod h1:GlAeCkHwugxdHaueRr4nhPuY+WW+gR8UjlcqzPr1SPI=
6665
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.17 h1:HGErhhrxZlQ044RiM+WdoZxp0p+EGM62y3L6pwA4olE=

pkg/deploy/shield/protection_manager.go

+39-15
Original file line numberDiff line numberDiff line change
@@ -88,22 +88,46 @@ func (m *defaultProtectionManager) CreateProtection(ctx context.Context, resourc
8888
}
8989

9090
func (m *defaultProtectionManager) DeleteProtection(ctx context.Context, resourceARN string, protectionID string) error {
91-
req := &shieldsdk.DeleteProtectionInput{
92-
ProtectionId: awssdk.String(protectionID),
93-
}
94-
m.logger.Info("disabling shield protection",
95-
"resourceARN", resourceARN,
96-
"protectionID", protectionID)
97-
_, err := m.shieldClient.DeleteProtectionWithContext(ctx, req)
98-
if err != nil {
99-
return err
100-
}
101-
m.logger.Info("disabled shield protection",
102-
"resourceARN", resourceARN)
91+
req := &shieldsdk.DeleteProtectionInput{
92+
ProtectionId: awssdk.String(protectionID),
93+
}
94+
m.logger.Info("disabling shield protection",
95+
"resourceARN", resourceARN,
96+
"protectionID", protectionID)
97+
_, err := m.shieldClient.DeleteProtectionWithContext(ctx, req)
98+
if err != nil {
99+
return err
100+
}
101+
m.logger.Info("disabled shield protection",
102+
"resourceARN", resourceARN)
103+
104+
// Remove the protection info from the cache
105+
m.protectionInfoByResourceARNCache.Delete(resourceARN)
106+
107+
// Verify that the protection resource is deleted
108+
err = m.verifyProtectionDeleted(ctx, protectionID)
109+
if err != nil {
110+
return err
111+
}
112+
113+
return nil
114+
}
103115

104-
var protectionInfo *ProtectionInfo
105-
m.protectionInfoByResourceARNCache.Set(resourceARN, protectionInfo, m.protectionInfoByResourceARNCacheTTL)
106-
return nil
116+
func (m *defaultProtectionManager) verifyProtectionDeleted(ctx context.Context, protectionID string) error {
117+
req := &shieldsdk.DescribeProtectionInput{
118+
ProtectionId: awssdk.String(protectionID),
119+
}
120+
_, err := m.shieldClient.DescribeProtectionWithContext(ctx, req)
121+
if err != nil {
122+
var resourceNotFoundException *shieldtypes.ResourceNotFoundException
123+
if errors.As(err, &resourceNotFoundException) {
124+
// Protection resource is successfully deleted
125+
return nil
126+
}
127+
return err
128+
}
129+
// Protection resource still exists
130+
return errors.New("protection resource still exists")
107131
}
108132

109133
func (m *defaultProtectionManager) GetProtection(ctx context.Context, resourceARN string) (*ProtectionInfo, error) {

pkg/deploy/shield/protection_manager_test.go

+129-2
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,19 @@ package shield
22

33
import (
44
"context"
5-
shieldtypes "github.com/aws/aws-sdk-go-v2/service/shield/types"
65
"testing"
76
"time"
87

9-
shieldsdk "github.com/aws/aws-sdk-go-v2/service/shield"
8+
shieldtypes "github.com/aws/aws-sdk-go-v2/service/shield/types"
9+
"github.com/aws/aws-sdk-go-v2/aws"
1010
"github.com/go-logr/logr"
1111
"github.com/golang/mock/gomock"
1212
"github.com/pkg/errors"
1313
"github.com/stretchr/testify/assert"
1414
"k8s.io/apimachinery/pkg/util/cache"
1515
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
1616
"sigs.k8s.io/controller-runtime/pkg/log"
17+
shieldsdk "github.com/aws/aws-sdk-go-v2/service/shield"
1718
)
1819

1920
func Test_defaultProtectionManager_IsSubscribed(t *testing.T) {
@@ -169,3 +170,129 @@ func Test_defaultProtectionManager_IsSubscribed(t *testing.T) {
169170
})
170171
}
171172
}
173+
174+
func Test_defaultProtectionManager_DeleteProtection(t *testing.T) {
175+
type deleteProtectionCall struct {
176+
req *shieldsdk.DeleteProtectionInput
177+
resp *shieldsdk.DeleteProtectionOutput
178+
err error
179+
}
180+
type describeProtectionCall struct {
181+
req *shieldsdk.DescribeProtectionInput
182+
resp *shieldsdk.DescribeProtectionOutput
183+
err error
184+
}
185+
type fields struct {
186+
deleteProtectionCalls []deleteProtectionCall
187+
describeProtectionCalls []describeProtectionCall
188+
protectionInfoByResourceARNCacheTTL time.Duration
189+
}
190+
type testCase struct {
191+
resourceARN string
192+
protectionID string
193+
wantErr error
194+
}
195+
tests := []struct {
196+
name string
197+
fields fields
198+
testCases []testCase
199+
}{
200+
{
201+
name: "delete protection successfully",
202+
fields: fields{
203+
deleteProtectionCalls: []deleteProtectionCall{
204+
{
205+
req: &shieldsdk.DeleteProtectionInput{ProtectionId: aws.String("protection-id")},
206+
resp: &shieldsdk.DeleteProtectionOutput{},
207+
},
208+
},
209+
describeProtectionCalls: []describeProtectionCall{
210+
{
211+
req: &shieldsdk.DescribeProtectionInput{ProtectionId: aws.String("protection-id")},
212+
err: &shieldtypes.ResourceNotFoundException{},
213+
},
214+
},
215+
protectionInfoByResourceARNCacheTTL: 10 * time.Minute,
216+
},
217+
testCases: []testCase{
218+
{
219+
resourceARN: "resource-arn",
220+
protectionID: "protection-id",
221+
},
222+
},
223+
},
224+
{
225+
name: "delete protection fails",
226+
fields: fields{
227+
deleteProtectionCalls: []deleteProtectionCall{
228+
{
229+
req: &shieldsdk.DeleteProtectionInput{ProtectionId: aws.String("protection-id")},
230+
err: errors.New("some aws api error"),
231+
},
232+
},
233+
protectionInfoByResourceARNCacheTTL: 10 * time.Minute,
234+
},
235+
testCases: []testCase{
236+
{
237+
resourceARN: "resource-arn",
238+
protectionID: "protection-id",
239+
wantErr: errors.New("some aws api error"),
240+
},
241+
},
242+
},
243+
{
244+
name: "protection still exists after deletion",
245+
fields: fields{
246+
deleteProtectionCalls: []deleteProtectionCall{
247+
{
248+
req: &shieldsdk.DeleteProtectionInput{ProtectionId: aws.String("protection-id")},
249+
resp: &shieldsdk.DeleteProtectionOutput{},
250+
},
251+
},
252+
describeProtectionCalls: []describeProtectionCall{
253+
{
254+
req: &shieldsdk.DescribeProtectionInput{ProtectionId: aws.String("protection-id")},
255+
resp: &shieldsdk.DescribeProtectionOutput{Protection: &shieldtypes.Protection{}},
256+
},
257+
},
258+
protectionInfoByResourceARNCacheTTL: 10 * time.Minute,
259+
},
260+
testCases: []testCase{
261+
{
262+
resourceARN: "resource-arn",
263+
protectionID: "protection-id",
264+
wantErr: errors.New("protection resource still exists"),
265+
},
266+
},
267+
},
268+
}
269+
for _, tt := range tests {
270+
t.Run(tt.name, func(t *testing.T) {
271+
ctrl := gomock.NewController(t)
272+
defer ctrl.Finish()
273+
274+
shieldClient := services.NewMockShield(ctrl)
275+
for _, call := range tt.fields.deleteProtectionCalls {
276+
shieldClient.EXPECT().DeleteProtectionWithContext(gomock.Any(), call.req).Return(call.resp, call.err)
277+
}
278+
for _, call := range tt.fields.describeProtectionCalls {
279+
shieldClient.EXPECT().DescribeProtectionWithContext(gomock.Any(), call.req).Return(call.resp, call.err)
280+
}
281+
282+
m := &defaultProtectionManager{
283+
shieldClient: shieldClient,
284+
logger: logr.New(&log.NullLogSink{}),
285+
protectionInfoByResourceARNCache: cache.NewExpiring(),
286+
protectionInfoByResourceARNCacheTTL: tt.fields.protectionInfoByResourceARNCacheTTL,
287+
}
288+
for _, testCase := range tt.testCases {
289+
err := m.DeleteProtection(context.Background(), testCase.resourceARN, testCase.protectionID)
290+
if testCase.wantErr != nil {
291+
assert.EqualError(t, err, testCase.wantErr.Error())
292+
} else {
293+
assert.NoError(t, err)
294+
}
295+
}
296+
})
297+
}
298+
}

0 commit comments

Comments
 (0)