From 2c207f9aba8ec41c2152f97953bbdaec3cd0b2a2 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Fri, 3 Feb 2023 18:00:58 -0800 Subject: [PATCH 1/2] Add SSLPolicy field to IngressClassParams --- .../elbv2/v1beta1/ingressclassparams_types.go | 4 + .../elbv2.k8s.aws_ingressclassparams.yaml | 4 + docs/guide/ingress/ingress_class.md | 5 + .../crds/crds.yaml | 4 + pkg/ingress/model_build_listener.go | 17 +- pkg/ingress/model_builder.go | 5 +- pkg/ingress/model_builder_test.go | 267 ++++++++++++++++++ pkg/service/model_build_listener_test.go | 10 +- 8 files changed, 302 insertions(+), 14 deletions(-) diff --git a/apis/elbv2/v1beta1/ingressclassparams_types.go b/apis/elbv2/v1beta1/ingressclassparams_types.go index ab7a12edf..e818061b0 100644 --- a/apis/elbv2/v1beta1/ingressclassparams_types.go +++ b/apis/elbv2/v1beta1/ingressclassparams_types.go @@ -99,6 +99,10 @@ type IngressClassParamsSpec struct { // +optional Scheme *LoadBalancerScheme `json:"scheme,omitempty"` + // SSLPolicy specifies the SSL Policy for all Ingresses that belong to IngressClass with this IngressClassParams. + // +optional + SSLPolicy string `json:"sslPolicy,omitEmpty"` + // Subnets defines the subnets for all Ingresses that belong to IngressClass with this IngressClassParams. // +optional Subnets *SubnetSelector `json:"subnets,omitempty"` diff --git a/config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml b/config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml index 9dfc88d8f..0cc5fb603 100644 --- a/config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml +++ b/config/crd/bases/elbv2.k8s.aws_ingressclassparams.yaml @@ -140,6 +140,10 @@ spec: - internal - internet-facing type: string + sslPolicy: + description: SSLPolicy specifies the SSL Policy for all Ingresses + that belong to IngressClass with this IngressClassParams. + type: string subnets: description: Subnets defines the subnets for all Ingresses that belong to IngressClass with this IngressClassParams. diff --git a/docs/guide/ingress/ingress_class.md b/docs/guide/ingress/ingress_class.md index 4b8ae5a2d..5f53c448d 100644 --- a/docs/guide/ingress/ingress_class.md +++ b/docs/guide/ingress/ingress_class.md @@ -135,6 +135,11 @@ Cluster administrators can use the `scheme` field to restrict the scheme for all 1. If `scheme` specified, all Ingresses with this IngressClass will have the specified scheme. 2. If `scheme` un-specified, Ingresses with this IngressClass can continue to use `alb.ingress.kubernetes.io/scheme annotation` to specify scheme. +#### spec.sslPolicy + +Cluster administrators can use the optional `sslPolicy` field to specify the SSL policy for the load balancers that belong to this IngressClass. +If the field is specified, LBC will ignore the `alb.ingress.kubernetes.io/ssl-policy annotation` annotation. + #### spec.subnets Cluster administrators can use the optional `subnets` field to specify the subnets for the load balancers that belong to this IngressClass. diff --git a/helm/aws-load-balancer-controller/crds/crds.yaml b/helm/aws-load-balancer-controller/crds/crds.yaml index 8e036901e..f8c45bd6b 100644 --- a/helm/aws-load-balancer-controller/crds/crds.yaml +++ b/helm/aws-load-balancer-controller/crds/crds.yaml @@ -139,6 +139,10 @@ spec: - internal - internet-facing type: string + sslPolicy: + description: SSLPolicy specifies the SSL Policy for all Ingresses + that belong to IngressClass with this IngressClassParams. + type: string subnets: description: Subnets defines the subnets for all Ingresses that belong to IngressClass with this IngressClassParams. diff --git a/pkg/ingress/model_build_listener.go b/pkg/ingress/model_build_listener.go index 8a99dc146..9d96166c1 100644 --- a/pkg/ingress/model_build_listener.go +++ b/pkg/ingress/model_build_listener.go @@ -104,15 +104,15 @@ type listenPortConfig struct { tlsCerts []string } -func (t *defaultModelBuildTask) computeIngressListenPortConfigByPort(ctx context.Context, ing *networking.Ingress) (map[int64]listenPortConfig, error) { - explicitTLSCertARNs := t.computeIngressExplicitTLSCertARNs(ctx, ing) +func (t *defaultModelBuildTask) computeIngressListenPortConfigByPort(ctx context.Context, ing *ClassifiedIngress) (map[int64]listenPortConfig, error) { + explicitTLSCertARNs := t.computeIngressExplicitTLSCertARNs(ctx, ing.Ing) explicitSSLPolicy := t.computeIngressExplicitSSLPolicy(ctx, ing) - inboundCIDRv4s, inboundCIDRV6s, err := t.computeIngressExplicitInboundCIDRs(ctx, ing) + inboundCIDRv4s, inboundCIDRV6s, err := t.computeIngressExplicitInboundCIDRs(ctx, ing.Ing) if err != nil { return nil, err } preferTLS := len(explicitTLSCertARNs) != 0 - listenPorts, err := t.computeIngressListenPorts(ctx, ing, preferTLS) + listenPorts, err := t.computeIngressListenPorts(ctx, ing.Ing, preferTLS) if err != nil { return nil, err } @@ -126,7 +126,7 @@ func (t *defaultModelBuildTask) computeIngressListenPortConfigByPort(ctx context } var inferredTLSCertARNs []string if containsHTTPSPort && len(explicitTLSCertARNs) == 0 { - inferredTLSCertARNs, err = t.computeIngressInferredTLSCertARNs(ctx, ing) + inferredTLSCertARNs, err = t.computeIngressInferredTLSCertARNs(ctx, ing.Ing) if err != nil { return nil, err } @@ -228,9 +228,12 @@ func (t *defaultModelBuildTask) computeIngressExplicitInboundCIDRs(_ context.Con return inboundCIDRv4s, inboundCIDRv6s, nil } -func (t *defaultModelBuildTask) computeIngressExplicitSSLPolicy(_ context.Context, ing *networking.Ingress) *string { +func (t *defaultModelBuildTask) computeIngressExplicitSSLPolicy(_ context.Context, ing *ClassifiedIngress) *string { var rawSSLPolicy string - if exists := t.annotationParser.ParseStringAnnotation(annotations.IngressSuffixSSLPolicy, &rawSSLPolicy, ing.Annotations); !exists { + if ing.IngClassConfig.IngClassParams != nil && ing.IngClassConfig.IngClassParams.Spec.SSLPolicy != "" { + return &ing.IngClassConfig.IngClassParams.Spec.SSLPolicy + } + if exists := t.annotationParser.ParseStringAnnotation(annotations.IngressSuffixSSLPolicy, &rawSSLPolicy, ing.Ing.Annotations); !exists { return nil } return &rawSSLPolicy diff --git a/pkg/ingress/model_builder.go b/pkg/ingress/model_builder.go index 36400252c..5f72b1411 100644 --- a/pkg/ingress/model_builder.go +++ b/pkg/ingress/model_builder.go @@ -2,6 +2,8 @@ package ingress import ( "context" + "strconv" + awssdk "github.com/aws/aws-sdk-go/aws" elbv2sdk "github.com/aws/aws-sdk-go/service/elbv2" "github.com/go-logr/logr" @@ -21,7 +23,6 @@ import ( elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" networkingpkg "sigs.k8s.io/aws-load-balancer-controller/pkg/networking" "sigs.k8s.io/controller-runtime/pkg/client" - "strconv" ) const ( @@ -227,7 +228,7 @@ func (t *defaultModelBuildTask) run(ctx context.Context) error { listenPortConfigsByPort := make(map[int64][]listenPortConfigWithIngress) for _, member := range t.ingGroup.Members { ingKey := k8s.NamespacedName(member.Ing) - listenPortConfigByPortForIngress, err := t.computeIngressListenPortConfigByPort(ctx, member.Ing) + listenPortConfigByPortForIngress, err := t.computeIngressListenPortConfigByPort(ctx, &member) if err != nil { return errors.Wrapf(err, "ingress: %v", ingKey.String()) } diff --git a/pkg/ingress/model_builder_test.go b/pkg/ingress/model_builder_test.go index 234562cb9..2474841ac 100644 --- a/pkg/ingress/model_builder_test.go +++ b/pkg/ingress/model_builder_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" clientgoscheme "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/record" + "sigs.k8s.io/aws-load-balancer-controller/apis/elbv2/v1beta1" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" "sigs.k8s.io/aws-load-balancer-controller/pkg/aws/services" "sigs.k8s.io/aws-load-balancer-controller/pkg/config" @@ -1473,6 +1474,272 @@ func Test_defaultModelBuilder_Build(t *testing.T) { "ns-1/ing-1-svc-3:https": null } } +}`, + }, + { + name: "Ingress - ssl-policy in IngressClassParams", + env: env{ + svcs: []*corev1.Service{ns_1_svc_1, ns_1_svc_2, ns_1_svc_3}, + }, + fields: fields{ + resolveViaDiscoveryCalls: []resolveViaDiscoveryCall{resolveViaDiscoveryCallForInternalLB}, + listLoadBalancersCalls: []listLoadBalancersCall{listLoadBalancerCallForEmptyLB}, + enableBackendSG: true, + }, + args: args{ + ingGroup: Group{ + ID: GroupID{Namespace: "ns-1", Name: "ing-1"}, + Members: []ClassifiedIngress{ + { + IngClassConfig: ClassConfiguration{ + IngClassParams: &v1beta1.IngressClassParams{ + Spec: v1beta1.IngressClassParamsSpec{ + SSLPolicy: "ingress-class-policy", + }, + }, + }, + Ing: &networking.Ingress{ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns-1", + Name: "ing-1", + Annotations: map[string]string{ + "alb.ingress.kubernetes.io/certificate-arn": "arn:aws:acm:us-east-1:9999999:certificate/11111111", + "alb.ingress.kubernetes.io/ssl-policy": "annotated-ssl-policy", + }, + }, + Spec: networking.IngressSpec{ + Rules: []networking.IngressRule{ + { + Host: "app-1.example.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + Path: "/svc-1", + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ns_1_svc_1.Name, + Port: networking.ServiceBackendPort{ + Name: "http", + }, + }, + }, + }, + { + Path: "/svc-2", + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ns_1_svc_2.Name, + Port: networking.ServiceBackendPort{ + Name: "http", + }, + }, + }, + }, + }, + }, + }, + }, + { + Host: "app-2.example.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + Path: "/svc-3", + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ns_1_svc_3.Name, + Port: networking.ServiceBackendPort{ + Name: "https", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantStackPatch: ` +{ + "resources": { + "AWS::EC2::SecurityGroup": { + "ManagedLBSecurityGroup": { + "spec": { + "ingress": [ + { + "fromPort": 443, + "ipProtocol": "tcp", + "ipRanges": [ + { + "cidrIP": "0.0.0.0/0" + } + ], + "toPort": 443 + } + ] + } + } + }, + "AWS::ElasticLoadBalancingV2::Listener": { + "443": { + "spec": { + "certificates": [ + { + "certificateARN": "arn:aws:acm:us-east-1:9999999:certificate/11111111" + } + ], + "defaultActions": [ + { + "fixedResponseConfig": { + "contentType": "text/plain", + "statusCode": "404" + }, + "type": "fixed-response" + } + ], + "loadBalancerARN": { + "$ref": "#/resources/AWS::ElasticLoadBalancingV2::LoadBalancer/LoadBalancer/status/loadBalancerARN" + }, + "port": 443, + "protocol": "HTTPS", + "sslPolicy": "ingress-class-policy" + } + }, + "80": null + }, + "AWS::ElasticLoadBalancingV2::ListenerRule": { + "443:1": { + "spec": { + "actions": [ + { + "forwardConfig": { + "targetGroups": [ + { + "targetGroupARN": { + "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/ns-1/ing-1-svc-1:http/status/targetGroupARN" + } + } + ] + }, + "type": "forward" + } + ], + "conditions": [ + { + "field": "host-header", + "hostHeaderConfig": { + "values": [ + "app-1.example.com" + ] + } + }, + { + "field": "path-pattern", + "pathPatternConfig": { + "values": [ + "/svc-1" + ] + } + } + ], + "listenerARN": { + "$ref": "#/resources/AWS::ElasticLoadBalancingV2::Listener/443/status/listenerARN" + }, + "priority": 1 + } + }, + "443:2": { + "spec": { + "actions": [ + { + "forwardConfig": { + "targetGroups": [ + { + "targetGroupARN": { + "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/ns-1/ing-1-svc-2:http/status/targetGroupARN" + } + } + ] + }, + "type": "forward" + } + ], + "conditions": [ + { + "field": "host-header", + "hostHeaderConfig": { + "values": [ + "app-1.example.com" + ] + } + }, + { + "field": "path-pattern", + "pathPatternConfig": { + "values": [ + "/svc-2" + ] + } + } + ], + "listenerARN": { + "$ref": "#/resources/AWS::ElasticLoadBalancingV2::Listener/443/status/listenerARN" + }, + "priority": 2 + } + }, + "443:3": { + "spec": { + "actions": [ + { + "forwardConfig": { + "targetGroups": [ + { + "targetGroupARN": { + "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/ns-1/ing-1-svc-3:https/status/targetGroupARN" + } + } + ] + }, + "type": "forward" + } + ], + "conditions": [ + { + "field": "host-header", + "hostHeaderConfig": { + "values": [ + "app-2.example.com" + ] + } + }, + { + "field": "path-pattern", + "pathPatternConfig": { + "values": [ + "/svc-3" + ] + } + } + ], + "listenerARN": { + "$ref": "#/resources/AWS::ElasticLoadBalancingV2::Listener/443/status/listenerARN" + }, + "priority": 3 + } + }, + "80:1": null, + "80:2": null, + "80:3": null + } + } }`, }, { diff --git a/pkg/service/model_build_listener_test.go b/pkg/service/model_build_listener_test.go index 416a99984..3b1afe035 100644 --- a/pkg/service/model_build_listener_test.go +++ b/pkg/service/model_build_listener_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -16,7 +15,7 @@ func Test_defaultModelBuilderTask_buildListenerALPNPolicy(t *testing.T) { tests := []struct { name string svc *corev1.Service - wantErr error + wantErr string want []string listenerProtocol elbv2model.Protocol targetProtocol elbv2model.Protocol @@ -66,7 +65,7 @@ func Test_defaultModelBuilderTask_buildListenerALPNPolicy(t *testing.T) { }, }, }, - wantErr: errors.New("invalid ALPN policy unknown, policy must be one of [None, HTTP1Only, HTTP2Only, HTTP2Optional, HTTP2Preferred]"), + wantErr: "invalid ALPN policy unknown, policy must be one of [None, HTTP1Only, HTTP2Only, HTTP2Optional, HTTP2Preferred]", listenerProtocol: elbv2model.ProtocolTLS, targetProtocol: elbv2model.ProtocolTLS, }, @@ -80,9 +79,10 @@ func Test_defaultModelBuilderTask_buildListenerALPNPolicy(t *testing.T) { service: tt.svc, } got, err := builder.buildListenerALPNPolicy(context.Background(), tt.listenerProtocol, tt.targetProtocol) - if tt.wantErr != nil { - assert.EqualError(t, err, tt.wantErr.Error()) + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) } else { + assert.NoError(t, err) assert.Equal(t, tt.want, got) } }) From aeef13eb89544899827054a5fde27d4d737cc512 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Wed, 22 Feb 2023 09:00:06 -0800 Subject: [PATCH 2/2] Fix typo Co-authored-by: Tim Bannister --- docs/guide/ingress/ingress_class.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide/ingress/ingress_class.md b/docs/guide/ingress/ingress_class.md index 5f53c448d..d14b8df16 100644 --- a/docs/guide/ingress/ingress_class.md +++ b/docs/guide/ingress/ingress_class.md @@ -138,7 +138,7 @@ Cluster administrators can use the `scheme` field to restrict the scheme for all #### spec.sslPolicy Cluster administrators can use the optional `sslPolicy` field to specify the SSL policy for the load balancers that belong to this IngressClass. -If the field is specified, LBC will ignore the `alb.ingress.kubernetes.io/ssl-policy annotation` annotation. +If the field is specified, LBC will ignore the `alb.ingress.kubernetes.io/ssl-policy` annotation. #### spec.subnets