Skip to content

Commit 10b7d70

Browse files
kishorjTimothy-Dougherty
kishorj
authored and
Timothy-Dougherty
committed
refactor custom endpoint resolver (kubernetes-sigs#2270)
1 parent 21e4365 commit 10b7d70

File tree

7 files changed

+27
-224
lines changed

7 files changed

+27
-224
lines changed

‎main.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
"sigs.k8s.io/aws-load-balancer-controller/controllers/ingress"
3232
"sigs.k8s.io/aws-load-balancer-controller/controllers/service"
3333
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws"
34-
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/endpoints"
3534
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/throttle"
3635
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
3736
"sigs.k8s.io/aws-load-balancer-controller/pkg/inject"
@@ -173,7 +172,6 @@ func loadControllerConfig() (config.ControllerConfig, error) {
173172
controllerCFG := config.ControllerConfig{
174173
AWSConfig: aws.CloudConfig{
175174
ThrottleConfig: defaultAWSThrottleCFG,
176-
AWSEndpointResolver: &endpoints.AWSEndpointResolver{},
177175
},
178176
}
179177

‎pkg/aws/cloud.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/pkg/errors"
88
"github.com/prometheus/client_golang/prometheus"
99
"os"
10+
epresolver "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/endpoints"
1011
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/metrics"
1112
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services"
1213
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/throttle"
@@ -43,7 +44,8 @@ type Cloud interface {
4344

4445
// NewCloud constructs new Cloud implementation.
4546
func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud, error) {
46-
metadataCFG := aws.NewConfig().WithEndpointResolver(cfg.AWSEndpointResolver)
47+
endpointsResolver := epresolver.NewResolver(cfg.AWSEndpoints)
48+
metadataCFG := aws.NewConfig().WithEndpointResolver(endpointsResolver)
4749
metadataSess := session.Must(session.NewSession(metadataCFG))
4850
metadata := services.NewEC2Metadata(metadataSess)
4951
if len(cfg.VpcID) == 0 {
@@ -69,7 +71,7 @@ func NewCloud(cfg CloudConfig, metricsRegisterer prometheus.Registerer) (Cloud,
6971
}
7072
cfg.Region = region
7173
}
72-
awsCFG := aws.NewConfig().WithRegion(cfg.Region).WithSTSRegionalEndpoint(endpoints.RegionalSTSEndpoint).WithMaxRetries(cfg.MaxRetries).WithEndpointResolver(cfg.AWSEndpointResolver)
74+
awsCFG := aws.NewConfig().WithRegion(cfg.Region).WithSTSRegionalEndpoint(endpoints.RegionalSTSEndpoint).WithMaxRetries(cfg.MaxRetries).WithEndpointResolver(endpointsResolver)
7375
sess := session.Must(session.NewSession(awsCFG))
7476
injectUserAgent(&sess.Handlers)
7577

‎pkg/aws/cloud_config.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package aws
22

33
import (
44
"github.com/spf13/pflag"
5-
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/endpoints"
65
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/throttle"
76
)
87

@@ -30,14 +29,14 @@ type CloudConfig struct {
3029
// Max retries configuration for AWS APIs
3130
MaxRetries int
3231

33-
// AWS endpoint configuration
34-
AWSEndpointResolver *endpoints.AWSEndpointResolver
32+
// AWS endpoints configuration
33+
AWSEndpoints map[string]string
3534
}
3635

3736
func (cfg *CloudConfig) BindFlags(fs *pflag.FlagSet) {
3837
fs.StringVar(&cfg.Region, flagAWSRegion, defaultRegion, "AWS Region for the kubernetes cluster")
3938
fs.Var(cfg.ThrottleConfig, flagAWSAPIThrottle, "throttle settings for AWS APIs, format: serviceID1:operationRegex1=rate:burst,serviceID2:operationRegex2=rate:burst")
4039
fs.StringVar(&cfg.VpcID, flagAWSVpcID, defaultVpcID, "AWS VPC ID for the Kubernetes cluster")
4140
fs.IntVar(&cfg.MaxRetries, flagAWSMaxRetries, defaultAPIMaxRetries, "Maximum retries for AWS APIs")
42-
fs.Var(cfg.AWSEndpointResolver, flagAWSAPIEndpoints, "Custom AWS endpoint configuration, format: serviceID1=URL1,serviceID2=URL2")
41+
fs.StringToStringVar(&cfg.AWSEndpoints, flagAWSAPIEndpoints, nil, "Custom AWS endpoint configuration, format: serviceID1=URL1,serviceID2=URL2")
4342
}

‎pkg/aws/endpoints/resolver.go

Lines changed: 11 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,30 @@
11
package endpoints
22

33
import (
4-
"fmt"
5-
"net/url"
6-
"sort"
7-
"strings"
8-
94
awsendpoints "github.com/aws/aws-sdk-go/aws/endpoints"
10-
"github.com/pkg/errors"
11-
"github.com/spf13/pflag"
125
)
136

14-
var _ pflag.Value = &AWSEndpointResolver{}
15-
16-
// AWSEndpointResolver is an AWS endpoints.Resolver that allows to customize AWS API endpoints.
17-
// It can be configured using the following format "${AWSServiceID}=${URL}"
18-
// e.g. "ec2=https://ec2.domain.com,elasticloadbalancing=https://elbv2.domain.com"
19-
type AWSEndpointResolver struct {
20-
configuration map[string]string
21-
}
22-
23-
func (c *AWSEndpointResolver) String() string {
24-
if c == nil {
25-
return ""
26-
}
27-
28-
var configs []string
29-
var serviceIDs []string
30-
for serviceID := range c.configuration {
31-
serviceIDs = append(serviceIDs, serviceID)
32-
}
33-
sort.Strings(serviceIDs)
34-
for _, serviceID := range serviceIDs {
35-
configs = append(configs, fmt.Sprintf("%s=%s", serviceID, c.configuration[serviceID]))
7+
func NewResolver(configuration map[string]string) *resolver {
8+
return &resolver{
9+
configuration: configuration,
3610
}
37-
return strings.Join(configs, ",")
3811
}
3912

40-
func (c *AWSEndpointResolver) Set(val string) error {
41-
configurationOverride := make(map[string]string)
13+
var _ awsendpoints.Resolver = &resolver{}
4214

43-
if val != "" {
44-
configPairs := strings.Split(val, ",")
45-
for _, pair := range configPairs {
46-
kv := strings.Split(pair, "=")
47-
if len(kv) != 2 {
48-
return errors.Errorf("%s must be formatted as serviceID=URL", pair)
49-
}
50-
serviceID := kv[0]
51-
urlStr := kv[1]
52-
url, err := url.Parse(urlStr)
53-
if err != nil {
54-
return errors.Errorf("%s must be a valid url", urlStr)
55-
}
56-
if !url.IsAbs() {
57-
return errors.Errorf("%s must be an absolute url", urlStr)
58-
}
59-
configurationOverride[serviceID] = url.String()
60-
}
61-
}
62-
63-
if c.configuration == nil {
64-
c.configuration = make(map[string]string)
65-
}
66-
for k, v := range configurationOverride {
67-
c.configuration[k] = v
68-
}
69-
return nil
70-
}
71-
72-
func (c *AWSEndpointResolver) Type() string {
73-
return "awsEndpointResolver"
15+
// resolver is an AWS endpoints.Resolver that allows to customize AWS API endpoints.
16+
// It can be configured using the following format "${AWSServiceID}=${URL}"
17+
// e.g. "ec2=https://ec2.domain.com,elasticloadbalancing=https://elbv2.domain.com"
18+
type resolver struct {
19+
configuration map[string]string
7420
}
7521

76-
func (c *AWSEndpointResolver) EndpointFor(service, region string, opts ...func(*awsendpoints.Options)) (awsendpoints.ResolvedEndpoint, error) {
22+
func (c *resolver) EndpointFor(service, region string, opts ...func(*awsendpoints.Options)) (awsendpoints.ResolvedEndpoint, error) {
7723
customEndpoint := c.configuration[service]
7824
if len(customEndpoint) != 0 {
7925
return awsendpoints.ResolvedEndpoint{
8026
URL: customEndpoint,
8127
}, nil
8228
}
8329
return awsendpoints.DefaultResolver().EndpointFor(service, region, opts...)
84-
}
30+
}

‎pkg/aws/endpoints/resolver_test.go

Lines changed: 3 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -4,155 +4,15 @@ import (
44
"testing"
55

66
awsendpoints "github.com/aws/aws-sdk-go/aws/endpoints"
7-
"github.com/pkg/errors"
87
"github.com/stretchr/testify/assert"
98
)
109

11-
func TestAWSEndpointResolver_String(t *testing.T) {
12-
type fields struct {
13-
configuration map[string]string
14-
}
15-
tests := []struct {
16-
name string
17-
fields fields
18-
want string
19-
}{
20-
{
21-
name: "non-empty configuration",
22-
fields: fields{
23-
configuration: map[string]string{
24-
awsendpoints.Ec2ServiceID: "https://ec2.domain.com",
25-
awsendpoints.ElasticloadbalancingServiceID: "https://elbv2.domain.com",
26-
},
27-
},
28-
want: "ec2=https://ec2.domain.com,elasticloadbalancing=https://elbv2.domain.com",
29-
},
30-
{
31-
name: "nil configuration",
32-
fields: fields{
33-
configuration: nil,
34-
},
35-
want: "",
36-
},
37-
{
38-
name: "empty configuration",
39-
fields: fields{
40-
configuration: nil,
41-
},
42-
want: "",
43-
},
44-
}
45-
for _, tt := range tests {
46-
t.Run(tt.name, func(t *testing.T) {
47-
c := &AWSEndpointResolver{
48-
configuration: tt.fields.configuration,
49-
}
50-
got := c.String()
51-
assert.Equal(t, tt.want, got)
52-
})
53-
}
54-
}
55-
56-
func TestAWSEndpointResolver_Set(t *testing.T) {
57-
type fields struct {
58-
configuration map[string]string
59-
}
60-
type args struct {
61-
val string
62-
}
63-
tests := []struct {
64-
name string
65-
fields fields
66-
args args
67-
want AWSEndpointResolver
68-
wantErr error
69-
}{
70-
{
71-
name: "when default value is nil",
72-
fields: fields{
73-
configuration: nil,
74-
},
75-
args: args{
76-
val: "ec2=https://ec2.domain.com,elasticloadbalancing=https://elbv2.domain.com",
77-
},
78-
want: AWSEndpointResolver{
79-
configuration: map[string]string{
80-
awsendpoints.Ec2ServiceID: "https://ec2.domain.com",
81-
awsendpoints.ElasticloadbalancingServiceID: "https://elbv2.domain.com",
82-
},
83-
},
84-
},
85-
{
86-
name: "when val is empty",
87-
fields: fields{
88-
configuration: map[string]string{},
89-
},
90-
args: args{
91-
val: "",
92-
},
93-
want: AWSEndpointResolver{
94-
configuration: map[string]string{},
95-
},
96-
},
97-
{
98-
name: "when val is not valid format - case 1",
99-
fields: fields{
100-
configuration: map[string]string{},
101-
},
102-
args: args{
103-
val: "a=b=c",
104-
},
105-
wantErr: errors.Errorf("a=b=c must be formatted as serviceID=URL"),
106-
},
107-
{
108-
name: "when url is not absolute",
109-
fields: fields{
110-
configuration: map[string]string{},
111-
},
112-
args: args{
113-
val: "a=/relative/url",
114-
},
115-
wantErr: errors.Errorf("/relative/url must be an absolute url"),
116-
},
117-
{
118-
name: "when url is invalid",
119-
fields: fields{
120-
configuration: map[string]string{},
121-
},
122-
args: args{
123-
val: "a=invalid\turl",
124-
},
125-
wantErr: errors.Errorf("invalid\turl must be a valid url"),
126-
},
127-
}
128-
for _, tt := range tests {
129-
t.Run(tt.name, func(t *testing.T) {
130-
c := &AWSEndpointResolver{
131-
configuration: tt.fields.configuration,
132-
}
133-
err := c.Set(tt.args.val)
134-
if tt.wantErr != nil {
135-
assert.EqualError(t, err, tt.wantErr.Error())
136-
} else {
137-
assert.NoError(t, err)
138-
assert.Equal(t, tt.want, *c)
139-
}
140-
})
141-
}
142-
}
143-
144-
func TestAWSEndpointResolver_Type(t *testing.T) {
145-
c := &AWSEndpointResolver{}
146-
got := c.Type()
147-
assert.Equal(t, "awsEndpointResolver", got)
148-
}
149-
15010
func TestAWSEndpointResolver_EndpointFor(t *testing.T) {
15111
configuration := map[string]string{
152-
awsendpoints.Ec2ServiceID: "https://ec2.domain.com",
12+
awsendpoints.Ec2ServiceID: "https://ec2.domain.com",
15313
awsendpoints.ElasticloadbalancingServiceID: "https://elbv2.domain.com",
15414
}
155-
c := &AWSEndpointResolver{
15+
c := &resolver{
15616
configuration: configuration,
15717
}
15818

@@ -185,7 +45,7 @@ func TestAWSEndpointResolver_EndpointFor(t *testing.T) {
18545
want: nil,
18646
},
18747
}
188-
48+
18949
for _, tt := range tests {
19050
t.Run(tt.name, func(t *testing.T) {
19151
res, err := c.EndpointFor(tt.args.val, testRegion)

‎pkg/targetgroupbinding/networking_manager_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -878,8 +878,8 @@ func Test_defaultNetworkingManager_computeRestrictedIngressPermissionsPerSG(t *t
878878
{
879879
Permission: ec2sdk.IpPermission{
880880
IpProtocol: awssdk.String("tcp"),
881-
FromPort: nil,
882-
ToPort: nil,
881+
FromPort: nil,
882+
ToPort: nil,
883883
UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{
884884
{GroupId: awssdk.String("group-1")},
885885
},
@@ -914,8 +914,8 @@ func Test_defaultNetworkingManager_computeRestrictedIngressPermissionsPerSG(t *t
914914
{
915915
Permission: ec2sdk.IpPermission{
916916
IpProtocol: awssdk.String("tcp"),
917-
FromPort: nil,
918-
ToPort: nil,
917+
FromPort: nil,
918+
ToPort: nil,
919919
UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{
920920
{GroupId: awssdk.String("group-1")},
921921
},
@@ -928,8 +928,8 @@ func Test_defaultNetworkingManager_computeRestrictedIngressPermissionsPerSG(t *t
928928
{
929929
Permission: ec2sdk.IpPermission{
930930
IpProtocol: awssdk.String("tcp"),
931-
FromPort: nil,
932-
ToPort: nil,
931+
FromPort: nil,
932+
ToPort: nil,
933933
UserIdGroupPairs: []*ec2sdk.UserIdGroupPair{
934934
{GroupId: awssdk.String("group-2")},
935935
},

‎test/framework/framework.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"k8s.io/client-go/rest"
88
elbv2api "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1"
99
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws"
10-
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/endpoints"
1110
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws/throttle"
1211
"sigs.k8s.io/aws-load-balancer-controller/test/framework/controller"
1312
"sigs.k8s.io/aws-load-balancer-controller/test/framework/helm"
@@ -77,7 +76,6 @@ func InitFramework() (*Framework, error) {
7776
VpcID: globalOptions.AWSVPCID,
7877
MaxRetries: 3,
7978
ThrottleConfig: throttle.NewDefaultServiceOperationsThrottleConfig(),
80-
AWSEndpointResolver: &endpoints.AWSEndpointResolver{},
8179
}, nil)
8280
if err != nil {
8381
return nil, err

0 commit comments

Comments
 (0)