Skip to content

Commit 7b15fbd

Browse files
Update ELB and ELBV2 packages to AWS SDK Go V2 (#1157)
1 parent 73d2386 commit 7b15fbd

File tree

12 files changed

+1171
-919
lines changed

12 files changed

+1171
-919
lines changed

go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ require (
99
github.com/aws/aws-sdk-go-v2/config v1.29.14
1010
github.com/aws/aws-sdk-go-v2/service/ecr v1.36.2
1111
github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.27.2
12+
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing v1.29.3
13+
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.45.2
1214
github.com/mitchellh/hashstructure/v2 v2.0.2
1315
github.com/onsi/ginkgo/v2 v2.23.0
1416
github.com/onsi/gomega v1.36.2

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ github.com/aws/aws-sdk-go-v2/service/ecr v1.36.2 h1:VDQaVwGOokbd3VUbHF+wupiffdrb
3030
github.com/aws/aws-sdk-go-v2/service/ecr v1.36.2/go.mod h1:lvUlMghKYmSxSfv0vU7pdU/8jSY+s0zpG8xXhaGKCw0=
3131
github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.27.2 h1:Zru9Iy2JPM5+uRnFnoqeOZzi8JIVIHJ0ua6JdeDHcyg=
3232
github.com/aws/aws-sdk-go-v2/service/ecrpublic v1.27.2/go.mod h1:PtQC3XjutCYFCn1+i8+wtpDaXvEK+vXF2gyLIKAmh4A=
33+
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing v1.29.3 h1:DpyV8LeDf0y7iDaGZ3h1Y+Nh5IaBOR+xj44vVgEEegY=
34+
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing v1.29.3/go.mod h1:H232HdqVlSUoqy0cMJYW1TKjcxvGFGFZ20xQG8fOAPw=
35+
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.45.2 h1:vX70Z4lNSr7XsioU0uJq5yvxgI50sB66MvD+V/3buS4=
36+
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.45.2/go.mod h1:xnCC3vFBfOKpU6PcsCKL2ktgBTZfOwTGxj6V8/X3IS4=
3337
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 h1:eAh2A4b5IzM/lum78bZ590jy36+d/aFLgKF/4Vd1xPE=
3438
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3/go.mod h1:0yKJC/kb8sAnmlYa6Zs3QVYqaC8ug2AbnNChv5Ox3uA=
3539
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 h1:dM9/92u2F1JbDaGooxTq18wmmFzbJRfXfVfy96/1CXM=

pkg/controllers/tagging/tagging_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ func (tc *Controller) untagEc2Instance(ctx context.Context, node *taggingControl
383383

384384
var err error
385385
if tc.batchingEnabled {
386-
err = tc.cloud.UntagResourceBatch(context.TODO(), string(instanceID), tc.tags)
386+
err = tc.cloud.UntagResourceBatch(ctx, string(instanceID), tc.tags)
387387
} else {
388388
err = tc.cloud.UntagResource(ctx, string(instanceID), tc.tags)
389389
}

pkg/providers/v1/aws.go

Lines changed: 153 additions & 144 deletions
Large diffs are not rendered by default.

pkg/providers/v1/aws_fakes.go

Lines changed: 55 additions & 75 deletions
Large diffs are not rendered by default.

pkg/providers/v1/aws_loadbalancer.go

Lines changed: 272 additions & 232 deletions
Large diffs are not rendered by default.

pkg/providers/v1/aws_loadbalancer_test.go

Lines changed: 162 additions & 168 deletions
Large diffs are not rendered by default.

pkg/providers/v1/aws_sdk.go

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,14 @@ import (
2727
awsConfig "github.com/aws/aws-sdk-go-v2/config"
2828
stscredsv2 "github.com/aws/aws-sdk-go-v2/credentials/stscreds"
2929
"github.com/aws/aws-sdk-go-v2/service/ec2"
30+
elb "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing"
31+
elbv2 "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
3032
"github.com/aws/aws-sdk-go/aws"
3133
"github.com/aws/aws-sdk-go/aws/credentials"
3234
"github.com/aws/aws-sdk-go/aws/ec2metadata"
3335
"github.com/aws/aws-sdk-go/aws/request"
3436
"github.com/aws/aws-sdk-go/aws/session"
35-
"github.com/aws/aws-sdk-go/service/elb"
36-
"github.com/aws/aws-sdk-go/service/elbv2"
37+
3738
"github.com/aws/aws-sdk-go/service/kms"
3839

3940
smithymiddleware "github.com/aws/smithy-go/middleware"
@@ -174,10 +175,9 @@ func (p *awsSDKProvider) Compute(ctx context.Context, regionName string, assumeR
174175
o.Retryer = &customRetryer{
175176
retry.NewStandard(),
176177
}
177-
})
178-
opts = append(opts, func(o *ec2.Options) {
179178
o.EndpointResolverV2 = p.cfg.GetCustomEC2Resolver()
180179
})
180+
181181
ec2Client := ec2.NewFromConfig(cfg, opts...)
182182

183183
ec2 := &awsSdkEC2{
@@ -186,45 +186,54 @@ func (p *awsSDKProvider) Compute(ctx context.Context, regionName string, assumeR
186186
return ec2, nil
187187
}
188188

189-
func (p *awsSDKProvider) LoadBalancing(regionName string) (ELB, error) {
190-
awsConfig := &aws.Config{
191-
Region: &regionName,
192-
Credentials: p.creds,
189+
func (p *awsSDKProvider) LoadBalancing(ctx context.Context, regionName string, assumeRoleProvider *stscredsv2.AssumeRoleProvider) (ELB, error) {
190+
cfg, err := awsConfig.LoadDefaultConfig(ctx, awsConfig.WithDefaultsMode(awsv2.DefaultsModeInRegion),
191+
awsConfig.WithRegion(regionName),
192+
)
193+
if assumeRoleProvider != nil {
194+
cfg.Credentials = awsv2.NewCredentialsCache(assumeRoleProvider)
193195
}
194-
awsConfig = awsConfig.WithCredentialsChainVerboseErrors(true).
195-
WithEndpointResolver(p.cfg.GetResolver())
196-
sess, err := session.NewSessionWithOptions(session.Options{
197-
Config: *awsConfig,
198-
SharedConfigState: session.SharedConfigEnable,
199-
})
200196
if err != nil {
201-
return nil, fmt.Errorf("unable to initialize AWS session: %v", err)
197+
return nil, fmt.Errorf("unable to initialize AWS config: %v", err)
202198
}
203-
elbClient := elb.New(sess)
204-
p.AddHandlers(regionName, &elbClient.Handlers)
199+
200+
p.AddHandlersV2(ctx, regionName, &cfg)
201+
var opts []func(*elb.Options) = p.cfg.GetELBEndpointOpts(regionName)
202+
opts = append(opts, func(o *elb.Options) {
203+
o.Retryer = &customRetryer{
204+
retry.NewStandard(),
205+
}
206+
o.EndpointResolverV2 = p.cfg.GetCustomELBResolver()
207+
})
208+
209+
elbClient := elb.NewFromConfig(cfg, opts...)
205210

206211
return elbClient, nil
207212
}
208213

209-
func (p *awsSDKProvider) LoadBalancingV2(regionName string) (ELBV2, error) {
210-
awsConfig := &aws.Config{
211-
Region: &regionName,
212-
Credentials: p.creds,
214+
func (p *awsSDKProvider) LoadBalancingV2(ctx context.Context, regionName string, assumeRoleProvider *stscredsv2.AssumeRoleProvider) (ELBV2, error) {
215+
cfg, err := awsConfig.LoadDefaultConfig(ctx, awsConfig.WithDefaultsMode(awsv2.DefaultsModeInRegion),
216+
awsConfig.WithRegion(regionName),
217+
)
218+
if assumeRoleProvider != nil {
219+
cfg.Credentials = awsv2.NewCredentialsCache(assumeRoleProvider)
213220
}
214-
awsConfig = awsConfig.WithCredentialsChainVerboseErrors(true).
215-
WithEndpointResolver(p.cfg.GetResolver())
216-
sess, err := session.NewSessionWithOptions(session.Options{
217-
Config: *awsConfig,
218-
SharedConfigState: session.SharedConfigEnable,
219-
})
220221
if err != nil {
221-
return nil, fmt.Errorf("unable to initialize AWS session: %v", err)
222+
return nil, fmt.Errorf("unable to initialize AWS config: %v", err)
222223
}
223-
elbClient := elbv2.New(sess)
224224

225-
p.AddHandlers(regionName, &elbClient.Handlers)
225+
p.AddHandlersV2(ctx, regionName, &cfg)
226+
var opts []func(*elbv2.Options) = p.cfg.GetELBV2EndpointOpts(regionName)
227+
opts = append(opts, func(o *elbv2.Options) {
228+
o.Retryer = &customRetryer{
229+
retry.NewStandard(),
230+
}
231+
o.EndpointResolverV2 = p.cfg.GetCustomELBV2Resolver()
232+
})
233+
234+
elbv2Client := elbv2.NewFromConfig(cfg, opts...)
226235

227-
return elbClient, nil
236+
return elbv2Client, nil
228237
}
229238

230239
func (p *awsSDKProvider) Metadata() (config.EC2Metadata, error) {

pkg/providers/v1/aws_sdk_test.go

Lines changed: 119 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,23 @@ import (
88
"testing"
99

1010
"github.com/aws/aws-sdk-go-v2/service/ec2"
11+
elb "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancing"
12+
elbv2 "github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2"
13+
1114
"github.com/stretchr/testify/assert"
1215
"k8s.io/cloud-provider-aws/pkg/providers/v1/config"
1316
)
1417

1518
// Given an override, a custom endpoint should be used when making API requests
16-
func TestComputeEndpointOverride(t *testing.T) {
19+
func TestClientsEndpointOverride(t *testing.T) {
1720
usedCustomEndpoint := false
21+
// Dummy server that sets usedCustomEndpoint when called
1822
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1923
usedCustomEndpoint = true
2024
}))
2125

26+
// Pass in the test server URL through a CloudConfig, which is used by each client's custom endpoint
27+
// resolver to override the URL for a request (see EC2Resolver.ResolveEndpoint for an example)
2228
cfgWithServiceOverride := config.CloudConfig{
2329
ServiceOverride: map[string]*struct {
2430
Service string
@@ -29,7 +35,21 @@ func TestComputeEndpointOverride(t *testing.T) {
2935
SigningName string
3036
}{
3137
"1": {
32-
Service: "EC2",
38+
Service: ec2.ServiceID,
39+
Region: "us-west-2",
40+
URL: testServer.URL,
41+
SigningRegion: "signingRegion",
42+
SigningName: "signingName",
43+
},
44+
"2": {
45+
Service: elb.ServiceID,
46+
Region: "us-west-2",
47+
URL: testServer.URL,
48+
SigningRegion: "signingRegion",
49+
SigningName: "signingName",
50+
},
51+
"3": {
52+
Service: elbv2.ServiceID,
3353
Region: "us-west-2",
3454
URL: testServer.URL,
3555
SigningRegion: "signingRegion",
@@ -42,17 +62,35 @@ func TestComputeEndpointOverride(t *testing.T) {
4262
regionDelayers: make(map[string]*CrossRequestRetryDelay),
4363
}
4464

65+
// EC2 Client
4566
ec2Client, err := mockProvider.Compute(context.TODO(), "us-west-2", nil)
4667
if err != nil {
4768
t.Errorf("error creating client, %v", err)
4869
}
4970
_, err = ec2Client.DescribeInstances(context.TODO(), &ec2.DescribeInstancesInput{})
5071
assert.True(t, usedCustomEndpoint == true, "custom endpoint was not used for EC2 Client")
72+
73+
usedCustomEndpoint = false // reset boolean flag for next request
74+
elbClient, err := mockProvider.LoadBalancing(context.TODO(), "us-west-2", nil)
75+
if err != nil {
76+
t.Errorf("error creating client, %v", err)
77+
}
78+
_, err = elbClient.DescribeLoadBalancers(context.TODO(), &elb.DescribeLoadBalancersInput{})
79+
assert.True(t, usedCustomEndpoint == true, "custom endpoint was not used for ELB Client")
80+
81+
usedCustomEndpoint = false
82+
elbv2Client, err := mockProvider.LoadBalancingV2(context.TODO(), "us-west-2", nil)
83+
if err != nil {
84+
t.Errorf("error creating client, %v", err)
85+
}
86+
_, err = elbv2Client.DescribeLoadBalancers(context.TODO(), &elbv2.DescribeLoadBalancersInput{})
87+
assert.True(t, usedCustomEndpoint == true, "custom endpoint was not used for ELBV2 Client")
5188
}
5289

53-
// When a nonRetryableError is thrown, an API request should not be retried
54-
func TestComputeNoRetry(t *testing.T) {
90+
// Test whether SDK clients refrain from retrying an API request when given a nonRetryableError.
91+
func TestClientsNoRetry(t *testing.T) {
5592
attemptCount := 0
93+
// Dummy server that counts attempts and returns a nonRetryableError
5694
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
5795
attemptCount++
5896
w.Header().Set("Content-Type", "text/xml")
@@ -74,6 +112,7 @@ func TestComputeNoRetry(t *testing.T) {
74112
}))
75113
defer testServer.Close()
76114

115+
// Override service endpoints with dummy server URL
77116
cfgWithServiceOverride := config.CloudConfig{
78117
ServiceOverride: map[string]*struct {
79118
Service string
@@ -84,7 +123,21 @@ func TestComputeNoRetry(t *testing.T) {
84123
SigningName string
85124
}{
86125
"1": {
87-
Service: "EC2",
126+
Service: ec2.ServiceID,
127+
Region: "us-west-2",
128+
URL: testServer.URL,
129+
SigningRegion: "signingRegion",
130+
SigningName: "signingName",
131+
},
132+
"2": {
133+
Service: elb.ServiceID,
134+
Region: "us-west-2",
135+
URL: testServer.URL,
136+
SigningRegion: "signingRegion",
137+
SigningName: "signingName",
138+
},
139+
"3": {
140+
Service: elbv2.ServiceID,
88141
Region: "us-west-2",
89142
URL: testServer.URL,
90143
SigningRegion: "signingRegion",
@@ -97,23 +150,45 @@ func TestComputeNoRetry(t *testing.T) {
97150
regionDelayers: make(map[string]*CrossRequestRetryDelay),
98151
}
99152

153+
// EC2 Client
100154
ec2Client, err := mockProvider.Compute(context.TODO(), "us-west-2", nil)
101155
if err != nil {
102156
t.Errorf("error creating client, %v", err)
103157
}
104158
_, err = ec2Client.DescribeInstances(context.TODO(), &ec2.DescribeInstancesInput{})
105-
assert.True(t, attemptCount == 1, fmt.Sprintf("expected an attempt count of 1, got %d", attemptCount))
159+
// Ensure that only 1 attempt was made, signifying no retries
160+
assert.True(t, attemptCount == 1, fmt.Sprintf("expected an attempt count of 1 for EC2 client, got %d", attemptCount))
161+
162+
// ELB Client
163+
attemptCount = 0 // reset attempt count for next request
164+
elbClient, err := mockProvider.LoadBalancing(context.TODO(), "us-west-2", nil)
165+
if err != nil {
166+
t.Errorf("error creating client, %v", err)
167+
}
168+
_, err = elbClient.DescribeLoadBalancers(context.TODO(), &elb.DescribeLoadBalancersInput{})
169+
assert.True(t, attemptCount == 1, fmt.Sprintf("expected an attempt count of 1 for ELB client, got %d", attemptCount))
170+
171+
// ELBV2 Client
172+
attemptCount = 0
173+
elbv2Client, err := mockProvider.LoadBalancingV2(context.TODO(), "us-west-2", nil)
174+
if err != nil {
175+
t.Errorf("error creating client, %v", err)
176+
}
177+
_, err = elbv2Client.DescribeLoadBalancers(context.TODO(), &elbv2.DescribeLoadBalancersInput{})
178+
assert.True(t, attemptCount == 1, fmt.Sprintf("expected an attempt count of 1 for ELBV2 client, got %d", attemptCount))
106179
}
107180

108-
// When a retryable error is thrown, an API request should be retried
109-
func TestComputeWithRetry(t *testing.T) {
181+
// Test whether SDK clients retry an API request when given a retryable error code.
182+
func TestClientsWithRetry(t *testing.T) {
110183
attemptCount := 0
184+
// Dummy server that counts attempts and returns a retryable error
111185
testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
112186
attemptCount++
113187
// 500 status codes are retried by SDK (see https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/aws/retry)
114188
http.Error(w, "RequestTimeout", 500)
115189
}))
116190

191+
// Override service endpoints with dummy server URL
117192
cfgWithServiceOverride := config.CloudConfig{
118193
ServiceOverride: map[string]*struct {
119194
Service string
@@ -124,7 +199,21 @@ func TestComputeWithRetry(t *testing.T) {
124199
SigningName string
125200
}{
126201
"1": {
127-
Service: "EC2",
202+
Service: ec2.ServiceID,
203+
Region: "us-west-2",
204+
URL: testServer.URL,
205+
SigningRegion: "signingRegion",
206+
SigningName: "signingName",
207+
},
208+
"2": {
209+
Service: elb.ServiceID,
210+
Region: "us-west-2",
211+
URL: testServer.URL,
212+
SigningRegion: "signingRegion",
213+
SigningName: "signingName",
214+
},
215+
"3": {
216+
Service: elbv2.ServiceID,
128217
Region: "us-west-2",
129218
URL: testServer.URL,
130219
SigningRegion: "signingRegion",
@@ -137,10 +226,30 @@ func TestComputeWithRetry(t *testing.T) {
137226
regionDelayers: make(map[string]*CrossRequestRetryDelay),
138227
}
139228

229+
// EC2 Client
140230
ec2Client, err := mockProvider.Compute(context.TODO(), "us-west-2", nil)
141231
if err != nil {
142232
t.Errorf("error creating client, %v", err)
143233
}
144234
_, err = ec2Client.DescribeInstances(context.TODO(), &ec2.DescribeInstancesInput{})
145-
assert.True(t, attemptCount > 1, fmt.Sprintf("expected an attempt count >1, got %d", attemptCount))
235+
// Ensure that more than 1 attempt was made, signifying retries
236+
assert.True(t, attemptCount > 1, fmt.Sprintf("expected an attempt count of >1 for EC2 client, got %d", attemptCount))
237+
238+
// ELB Client
239+
attemptCount = 0 // Reset the attempt count before the next request
240+
elbClient, err := mockProvider.LoadBalancing(context.TODO(), "us-west-2", nil)
241+
if err != nil {
242+
t.Errorf("error creating client, %v", err)
243+
}
244+
_, err = elbClient.DescribeLoadBalancers(context.TODO(), &elb.DescribeLoadBalancersInput{})
245+
assert.True(t, attemptCount > 1, fmt.Sprintf("expected an attempt count of >1 for ELB client, got %d", attemptCount))
246+
247+
// ELBV2 Client
248+
attemptCount = 0
249+
elbv2Client, err := mockProvider.LoadBalancingV2(context.TODO(), "us-west-2", nil)
250+
if err != nil {
251+
t.Errorf("error creating client, %v", err)
252+
}
253+
_, err = elbv2Client.DescribeLoadBalancers(context.TODO(), &elbv2.DescribeLoadBalancersInput{})
254+
assert.True(t, attemptCount > 1, fmt.Sprintf("expected an attempt count of >1 for ELB client, got %d", attemptCount))
146255
}

0 commit comments

Comments
 (0)