Skip to content

Commit be3bcd3

Browse files
author
thejasn
committed
UPSTREAM: 2587: added EnableIPTargetType feature to controller
review changes and tests Signed-off-by: thejasn <[email protected]>
1 parent f175872 commit be3bcd3

11 files changed

+162
-20
lines changed

‎controllers/ingress/group_controller.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"sigs.k8s.io/aws-load-balancer-controller/controllers/ingress/eventhandlers"
1515
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
1616
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws"
17-
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
17+
cfg "sigs.k8s.io/aws-load-balancer-controller/pkg/config"
1818
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy"
1919
elbv2deploy "sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2"
2020
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking"
@@ -44,7 +44,7 @@ const (
4444
func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder,
4545
finalizerManager k8s.FinalizerManager, networkingSGManager networkingpkg.SecurityGroupManager,
4646
networkingSGReconciler networkingpkg.SecurityGroupReconciler, subnetsResolver networkingpkg.SubnetsResolver,
47-
config config.ControllerConfig, backendSGProvider networkingpkg.BackendSGProvider, logger logr.Logger) *groupReconciler {
47+
config cfg.ControllerConfig, backendSGProvider networkingpkg.BackendSGProvider, logger logr.Logger) *groupReconciler {
4848

4949
annotationParser := annotations.NewSuffixAnnotationParser(annotations.AnnotationPrefixIngress)
5050
authConfigBuilder := ingress.NewDefaultAuthConfigBuilder(annotationParser)
@@ -57,7 +57,7 @@ func NewGroupReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder
5757
annotationParser, subnetsResolver,
5858
authConfigBuilder, enhancedBackendBuilder, trackingProvider, elbv2TaggingManager,
5959
cloud.VpcID(), config.ClusterName, config.DefaultTags, config.ExternalManagedTags,
60-
config.DefaultSSLPolicy, backendSGProvider, config.EnableBackendSecurityGroup, config.DisableRestrictedSGRules, logger)
60+
config.DefaultSSLPolicy, backendSGProvider, config.EnableBackendSecurityGroup, config.DisableRestrictedSGRules, config.FeatureGates.Enabled(cfg.EnableIPTargetType), logger)
6161
stackMarshaller := deploy.NewDefaultStackMarshaller()
6262
stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler,
6363
config, ingressTagPrefix, logger)

‎controllers/service/service_controller.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"sigs.k8s.io/aws-load-balancer-controller/controllers/service/eventhandlers"
1111
"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
1212
"sigs.k8s.io/aws-load-balancer-controller/pkg/aws"
13-
"sigs.k8s.io/aws-load-balancer-controller/pkg/config"
13+
cfg "sigs.k8s.io/aws-load-balancer-controller/pkg/config"
1414
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy"
1515
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/elbv2"
1616
"sigs.k8s.io/aws-load-balancer-controller/pkg/deploy/tracking"
@@ -36,14 +36,14 @@ const (
3636
func NewServiceReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorder record.EventRecorder,
3737
finalizerManager k8s.FinalizerManager, networkingSGManager networking.SecurityGroupManager,
3838
networkingSGReconciler networking.SecurityGroupReconciler, subnetsResolver networking.SubnetsResolver,
39-
vpcInfoProvider networking.VPCInfoProvider, config config.ControllerConfig, logger logr.Logger) *serviceReconciler {
39+
vpcInfoProvider networking.VPCInfoProvider, config cfg.ControllerConfig, logger logr.Logger) *serviceReconciler {
4040

4141
annotationParser := annotations.NewSuffixAnnotationParser(serviceAnnotationPrefix)
4242
trackingProvider := tracking.NewDefaultProvider(serviceTagPrefix, config.ClusterName)
4343
elbv2TaggingManager := elbv2.NewDefaultTaggingManager(cloud.ELBV2(), cloud.VpcID(), config.FeatureGates, logger)
4444
serviceUtils := service.NewServiceUtils(annotationParser, serviceFinalizer, config.ServiceConfig.LoadBalancerClass, config.FeatureGates)
4545
modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcInfoProvider, cloud.VpcID(), trackingProvider,
46-
elbv2TaggingManager, config.ClusterName, config.DefaultTags, config.ExternalManagedTags, config.DefaultSSLPolicy, serviceUtils)
46+
elbv2TaggingManager, config.ClusterName, config.DefaultTags, config.ExternalManagedTags, config.DefaultSSLPolicy, config.FeatureGates.Enabled(cfg.EnableIPTargetType), serviceUtils)
4747
stackMarshaller := deploy.NewDefaultStackMarshaller()
4848
stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, config, serviceTagPrefix, logger)
4949
return &serviceReconciler{

‎docs/deploy/configurations.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,4 @@ They are a set of kye=value pairs that describe AWS load balance controller feat
145145
| WeightedTargetGroups | string | true | Enable or disable weighted target groups |
146146
| ServiceTypeLoadBalancerOnly | string | false | If enabled, controller will be limited to reconciling service of type `LoadBalancer`|
147147
| EnableServiceController | string | true | Toggles support for `Service` type resources. |
148+
| EnableIPTargetType | string | true | Used to toggle support for target-type `ip` across `Ingress` and `Service` type resources. |

‎pkg/config/feature_gates.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const (
1515
ServiceTypeLoadBalancerOnly Feature = "ServiceTypeLoadBalancerOnly"
1616
EndpointsFailOpen Feature = "EndpointsFailOpen"
1717
EnableServiceController Feature = "EnableServiceController"
18+
EnableIPTargetType Feature = "EnableIPTargetType"
1819
)
1920

2021
type FeatureGates interface {
@@ -47,6 +48,7 @@ func NewFeatureGates() FeatureGates {
4748
ServiceTypeLoadBalancerOnly: false,
4849
EndpointsFailOpen: false,
4950
EnableServiceController: true,
51+
EnableIPTargetType: true,
5052
},
5153
}
5254
}

‎pkg/ingress/model_build_target_group.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,9 @@ func (t *defaultModelBuildTask) buildTargetGroupTargetType(_ context.Context, sv
214214
case string(elbv2model.TargetTypeInstance):
215215
return elbv2model.TargetTypeInstance, nil
216216
case string(elbv2model.TargetTypeIP):
217+
if !t.enableIPTargetType {
218+
return "", errors.Errorf("unsupported targetType: %v when EnableIPTargetType is %v", rawTargetType, t.enableIPTargetType)
219+
}
217220
return elbv2model.TargetTypeIP, nil
218221
default:
219222
return "", errors.Errorf("unknown targetType: %v", rawTargetType)

‎pkg/ingress/model_builder.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
4040
authConfigBuilder AuthConfigBuilder, enhancedBackendBuilder EnhancedBackendBuilder,
4141
trackingProvider tracking.Provider, elbv2TaggingManager elbv2deploy.TaggingManager,
4242
vpcID string, clusterName string, defaultTags map[string]string, externalManagedTags []string, defaultSSLPolicy string,
43-
backendSGProvider networkingpkg.BackendSGProvider, enableBackendSG bool, disableRestrictedSGRules bool, logger logr.Logger) *defaultModelBuilder {
43+
backendSGProvider networkingpkg.BackendSGProvider, enableBackendSG bool, disableRestrictedSGRules bool, enableIPTargetType bool, logger logr.Logger) *defaultModelBuilder {
4444
certDiscovery := NewACMCertDiscovery(acmClient, logger)
4545
ruleOptimizer := NewDefaultRuleOptimizer(logger)
4646
return &defaultModelBuilder{
@@ -63,6 +63,7 @@ func NewDefaultModelBuilder(k8sClient client.Client, eventRecorder record.EventR
6363
defaultSSLPolicy: defaultSSLPolicy,
6464
enableBackendSG: enableBackendSG,
6565
disableRestrictedSGRules: disableRestrictedSGRules,
66+
enableIPTargetType: enableIPTargetType,
6667
logger: logger,
6768
}
6869
}
@@ -92,6 +93,7 @@ type defaultModelBuilder struct {
9293
defaultSSLPolicy string
9394
enableBackendSG bool
9495
disableRestrictedSGRules bool
96+
enableIPTargetType bool
9597

9698
logger logr.Logger
9799
}
@@ -117,6 +119,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, ingGroup Group) (core.S
117119
logger: b.logger,
118120
enableBackendSG: b.enableBackendSG,
119121
disableRestrictedSGRules: b.disableRestrictedSGRules,
122+
enableIPTargetType: b.enableIPTargetType,
120123

121124
ingGroup: ingGroup,
122125
stack: stack,
@@ -172,6 +175,7 @@ type defaultModelBuildTask struct {
172175
backendSGIDToken core.StringToken
173176
enableBackendSG bool
174177
disableRestrictedSGRules bool
178+
enableIPTargetType bool
175179

176180
defaultTags map[string]string
177181
externalManagedTags sets.String

‎pkg/ingress/model_builder_test.go

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,13 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
194194
}
195195

196196
tests := []struct {
197-
name string
198-
env env
199-
args args
200-
fields fields
201-
wantStackJSON string
202-
wantErr error
197+
name string
198+
env env
199+
enableIPTargetType *bool
200+
args args
201+
fields fields
202+
wantStackJSON string
203+
wantErr error
203204
}{
204205
{
205206
name: "Ingress - vanilla internal",
@@ -3611,6 +3612,59 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
36113612
},
36123613
wantErr: errors.New("ingress: ns-1/ing-1: unsupported IPv6 configuration, lb not dual-stack"),
36133614
},
3615+
{
3616+
name: "target type IP with enableIPTargetType set to false",
3617+
env: env{
3618+
svcs: []*corev1.Service{svcWithNamedTargetPort},
3619+
},
3620+
enableIPTargetType: awssdk.Bool(false),
3621+
fields: fields{
3622+
resolveViaDiscoveryCalls: []resolveViaDiscoveryCall{resolveViaDiscoveryCallForInternalLB},
3623+
listLoadBalancersCalls: []listLoadBalancersCall{listLoadBalancerCallForEmptyLB},
3624+
enableBackendSG: true,
3625+
},
3626+
args: args{
3627+
ingGroup: Group{
3628+
ID: GroupID{Namespace: "ns-1", Name: "ing-1"},
3629+
Members: []ClassifiedIngress{
3630+
{
3631+
Ing: &networking.Ingress{ObjectMeta: metav1.ObjectMeta{
3632+
Namespace: "ns-1",
3633+
Name: "ing-1",
3634+
Annotations: map[string]string{
3635+
"alb.ingress.kubernetes.io/target-type": "ip",
3636+
},
3637+
},
3638+
Spec: networking.IngressSpec{
3639+
Rules: []networking.IngressRule{
3640+
{
3641+
IngressRuleValue: networking.IngressRuleValue{
3642+
HTTP: &networking.HTTPIngressRuleValue{
3643+
Paths: []networking.HTTPIngressPath{
3644+
{
3645+
Path: "/",
3646+
Backend: networking.IngressBackend{
3647+
Service: &networking.IngressServiceBackend{
3648+
Name: svcWithNamedTargetPort.Name,
3649+
Port: networking.ServiceBackendPort{
3650+
Name: "https",
3651+
},
3652+
},
3653+
},
3654+
},
3655+
},
3656+
},
3657+
},
3658+
},
3659+
},
3660+
},
3661+
},
3662+
},
3663+
},
3664+
},
3665+
},
3666+
wantErr: errors.New("ingress: ns-1/ing-1: unsupported targetType: ip when EnableIPTargetType is false"),
3667+
},
36143668
{
36153669
name: "target type IP with named target port",
36163670
env: env{
@@ -3900,6 +3954,12 @@ func Test_defaultModelBuilder_Build(t *testing.T) {
39003954
defaultSSLPolicy: "ELBSecurityPolicy-2016-08",
39013955
}
39023956

3957+
if tt.enableIPTargetType == nil {
3958+
b.enableIPTargetType = true
3959+
} else {
3960+
b.enableIPTargetType = *tt.enableIPTargetType
3961+
}
3962+
39033963
gotStack, _, _, err := b.Build(context.Background(), tt.args.ingGroup)
39043964
if tt.wantErr != nil {
39053965
assert.EqualError(t, err, tt.wantErr.Error())

‎pkg/service/model_build_target_group.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,9 @@ func (t *defaultModelBuildTask) buildTargetType(_ context.Context, port corev1.S
339339
var lbTargetType string
340340
lbTargetType = string(t.defaultTargetType)
341341
_ = t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixTargetType, &lbTargetType, t.service.Annotations)
342+
if lbTargetType == LoadBalancerTargetTypeIP && !t.enableIPTargetType {
343+
return "", errors.Errorf("unsupported targetType: %v when EnableIPTargetType is %v", lbTargetType, t.enableIPTargetType)
344+
}
342345
if lbType == LoadBalancerTypeNLBIP || lbTargetType == LoadBalancerTargetTypeIP {
343346
return elbv2model.TargetTypeIP, nil
344347
}

‎pkg/service/model_build_target_group_test.go

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,12 +1159,12 @@ func Test_defaultModelBuilder_buildPreserveClientIPFlag(t *testing.T) {
11591159
}
11601160

11611161
func Test_defaultModelBuilder_buildTargetType(t *testing.T) {
1162-
11631162
tests := []struct {
1164-
testName string
1165-
svc *corev1.Service
1166-
want elbv2.TargetType
1167-
wantErr error
1163+
testName string
1164+
svc *corev1.Service
1165+
want elbv2.TargetType
1166+
enableIPTargetType *bool
1167+
wantErr error
11681168
}{
11691169
{
11701170
testName: "empty annotation",
@@ -1247,6 +1247,29 @@ func Test_defaultModelBuilder_buildTargetType(t *testing.T) {
12471247
},
12481248
want: elbv2.TargetTypeIP,
12491249
},
1250+
{
1251+
testName: "enableIPTargetType is false, target ip",
1252+
svc: &corev1.Service{
1253+
ObjectMeta: metav1.ObjectMeta{
1254+
Annotations: map[string]string{
1255+
"service.beta.kubernetes.io/aws-load-balancer-type": "external",
1256+
"service.beta.kubernetes.io/aws-load-balancer-nlb-target-type": "ip",
1257+
},
1258+
},
1259+
Spec: corev1.ServiceSpec{
1260+
Ports: []corev1.ServicePort{
1261+
{
1262+
Name: "http",
1263+
Port: 80,
1264+
TargetPort: intstr.FromInt(80),
1265+
Protocol: corev1.ProtocolTCP,
1266+
},
1267+
},
1268+
},
1269+
},
1270+
enableIPTargetType: aws.Bool(false),
1271+
wantErr: errors.New("unsupported targetType: ip when EnableIPTargetType is false"),
1272+
},
12501273
{
12511274
testName: "external, ClusterIP with target type instance",
12521275
svc: &corev1.Service{
@@ -1337,6 +1360,11 @@ func Test_defaultModelBuilder_buildTargetType(t *testing.T) {
13371360
service: tt.svc,
13381361
defaultTargetType: LoadBalancerTargetTypeInstance,
13391362
}
1363+
if tt.enableIPTargetType == nil {
1364+
builder.enableIPTargetType = true
1365+
} else {
1366+
builder.enableIPTargetType = *tt.enableIPTargetType
1367+
}
13401368
got, err := builder.buildTargetType(context.Background(), tt.svc.Spec.Ports[0])
13411369
if tt.wantErr != nil {
13421370
assert.EqualError(t, err, tt.wantErr.Error())

‎pkg/service/model_builder.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ type ModelBuilder interface {
3636
func NewDefaultModelBuilder(annotationParser annotations.Parser, subnetsResolver networking.SubnetsResolver,
3737
vpcInfoProvider networking.VPCInfoProvider, vpcID string, trackingProvider tracking.Provider,
3838
elbv2TaggingManager elbv2deploy.TaggingManager, clusterName string, defaultTags map[string]string,
39-
externalManagedTags []string, defaultSSLPolicy string, serviceUtils ServiceUtils) *defaultModelBuilder {
39+
externalManagedTags []string, defaultSSLPolicy string, enableIPTargetType bool, serviceUtils ServiceUtils) *defaultModelBuilder {
4040
return &defaultModelBuilder{
4141
annotationParser: annotationParser,
4242
subnetsResolver: subnetsResolver,
@@ -49,6 +49,7 @@ func NewDefaultModelBuilder(annotationParser annotations.Parser, subnetsResolver
4949
defaultTags: defaultTags,
5050
externalManagedTags: sets.NewString(externalManagedTags...),
5151
defaultSSLPolicy: defaultSSLPolicy,
52+
enableIPTargetType: enableIPTargetType,
5253
}
5354
}
5455

@@ -67,6 +68,7 @@ type defaultModelBuilder struct {
6768
defaultTags map[string]string
6869
externalManagedTags sets.String
6970
defaultSSLPolicy string
71+
enableIPTargetType bool
7072
}
7173

7274
func (b *defaultModelBuilder) Build(ctx context.Context, service *corev1.Service) (core.Stack, *elbv2model.LoadBalancer, error) {
@@ -80,6 +82,7 @@ func (b *defaultModelBuilder) Build(ctx context.Context, service *corev1.Service
8082
trackingProvider: b.trackingProvider,
8183
elbv2TaggingManager: b.elbv2TaggingManager,
8284
serviceUtils: b.serviceUtils,
85+
enableIPTargetType: b.enableIPTargetType,
8386

8487
service: service,
8588
stack: stack,
@@ -129,6 +132,7 @@ type defaultModelBuildTask struct {
129132
trackingProvider tracking.Provider
130133
elbv2TaggingManager elbv2deploy.TaggingManager
131134
serviceUtils ServiceUtils
135+
enableIPTargetType bool
132136

133137
service *corev1.Service
134138

‎pkg/service/model_builder_test.go

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
101101
resolveViaNameOrIDSliceCalls []resolveViaNameOrIDSliceCall
102102
listLoadBalancerCalls []listLoadBalancerCall
103103
fetchVPCInfoCalls []fetchVPCInfoCall
104+
enableIPTargetType *bool
104105
svc *corev1.Service
105106
wantError bool
106107
wantValue string
@@ -1960,6 +1961,36 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
19601961
}
19611962
`,
19621963
},
1964+
{
1965+
testName: "service with enableIPTargetType set to false and type IP",
1966+
enableIPTargetType: aws.Bool(false),
1967+
svc: &corev1.Service{
1968+
ObjectMeta: metav1.ObjectMeta{
1969+
Name: "traffic-local",
1970+
Namespace: "default",
1971+
Annotations: map[string]string{
1972+
"service.beta.kubernetes.io/aws-load-balancer-type": "external",
1973+
"service.beta.kubernetes.io/aws-load-balancer-nlb-target-type": "ip",
1974+
},
1975+
},
1976+
Spec: corev1.ServiceSpec{
1977+
Type: corev1.ServiceTypeLoadBalancer,
1978+
Selector: map[string]string{"app": "hello"},
1979+
IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol},
1980+
Ports: []corev1.ServicePort{
1981+
{
1982+
Port: 80,
1983+
TargetPort: intstr.FromInt(80),
1984+
Protocol: corev1.ProtocolTCP,
1985+
NodePort: 32332,
1986+
},
1987+
},
1988+
},
1989+
},
1990+
resolveViaDiscoveryCalls: []resolveViaDiscoveryCall{resolveViaDiscoveryCallForOneSubnet},
1991+
listLoadBalancerCalls: []listLoadBalancerCall{listLoadBalancerCallForEmptyLB},
1992+
wantError: true,
1993+
},
19631994
{
19641995
testName: "list load balancers error",
19651996
svc: &corev1.Service{
@@ -2733,8 +2764,14 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) {
27332764
vpcInfoProvider.EXPECT().FetchVPCInfo(gomock.Any(), gomock.Any(), gomock.Any()).Return(call.wantVPCInfo, call.err).AnyTimes()
27342765
}
27352766
serviceUtils := NewServiceUtils(annotationParser, "service.k8s.aws/resources", "service.k8s.aws/nlb", featureGates)
2767+
var enableIPTargetType bool
2768+
if tt.enableIPTargetType == nil {
2769+
enableIPTargetType = true
2770+
} else {
2771+
enableIPTargetType = *tt.enableIPTargetType
2772+
}
27362773
builder := NewDefaultModelBuilder(annotationParser, subnetsResolver, vpcInfoProvider, "vpc-xxx", trackingProvider, elbv2TaggingManager,
2737-
"my-cluster", nil, nil, "ELBSecurityPolicy-2016-08", serviceUtils)
2774+
"my-cluster", nil, nil, "ELBSecurityPolicy-2016-08", enableIPTargetType, serviceUtils)
27382775
ctx := context.Background()
27392776
stack, _, err := builder.Build(ctx, tt.svc)
27402777
if tt.wantError {

0 commit comments

Comments
 (0)