diff --git a/pkg/service/model_build_listener.go b/pkg/service/model_build_listener.go index 747f9cff0..d320fe5ea 100644 --- a/pkg/service/model_build_listener.go +++ b/pkg/service/model_build_listener.go @@ -14,9 +14,13 @@ import ( ) func (t *defaultModelBuildTask) buildListeners(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error { - cfg := t.buildListenerConfig(ctx) + cfg, err := t.buildListenerConfig(ctx) + if err != nil { + return err + } + for _, port := range t.service.Spec.Ports { - _, err := t.buildListener(ctx, port, cfg, scheme) + _, err := t.buildListener(ctx, port, *cfg, scheme) if err != nil { return err } @@ -115,10 +119,43 @@ func (t *defaultModelBuildTask) buildListenerCertificates(_ context.Context) []e return certificates } -func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) sets.String { +func validateTLSPortsSet(rawTLSPorts []string, ports []corev1.ServicePort) error { + unusedPorts := make([]string, 0) + + for _, tlsPort := range rawTLSPorts { + isPortUsed := false + for _, portObj := range ports { + if portObj.Name == tlsPort || strconv.Itoa(int(portObj.Port)) == tlsPort { + isPortUsed = true + break + } + } + + if !isPortUsed { + unusedPorts = append(unusedPorts, tlsPort) + } + } + + if len(unusedPorts) > 0 { + unusedPortErr := errors.Errorf("Unused port in ssl-ports annotation %v", unusedPorts) + return unusedPortErr + } + + return nil +} + +func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) (sets.String, error) { var rawTLSPorts []string + _ = t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLPorts, &rawTLSPorts, t.service.Annotations) - return sets.NewString(rawTLSPorts...) + + err := validateTLSPortsSet(rawTLSPorts, t.service.Spec.Ports) + + if err != nil { + return nil, err + } + + return sets.NewString(rawTLSPorts...), nil } func (t *defaultModelBuildTask) buildBackendProtocol(_ context.Context) string { @@ -154,18 +191,22 @@ type listenerConfig struct { backendProtocol string } -func (t *defaultModelBuildTask) buildListenerConfig(ctx context.Context) listenerConfig { +func (t *defaultModelBuildTask) buildListenerConfig(ctx context.Context) (*listenerConfig, error) { certificates := t.buildListenerCertificates(ctx) - tlsPortsSet := t.buildTLSPortsSet(ctx) + tlsPortsSet, err := t.buildTLSPortsSet(ctx) + if err != nil { + return nil, err + } + backendProtocol := t.buildBackendProtocol(ctx) sslPolicy := t.buildSSLNegotiationPolicy(ctx) - return listenerConfig{ + return &listenerConfig{ certificates: certificates, tlsPortsSet: tlsPortsSet, sslPolicy: sslPolicy, backendProtocol: backendProtocol, - } + }, nil } func (t *defaultModelBuildTask) buildListenerTags(ctx context.Context) (map[string]string, error) { diff --git a/pkg/service/model_build_listener_test.go b/pkg/service/model_build_listener_test.go index 3b1afe035..25d0ac76c 100644 --- a/pkg/service/model_build_listener_test.go +++ b/pkg/service/model_build_listener_test.go @@ -4,9 +4,12 @@ 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" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/aws-load-balancer-controller/pkg/annotations" elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2" ) @@ -88,3 +91,94 @@ func Test_defaultModelBuilderTask_buildListenerALPNPolicy(t *testing.T) { }) } } + +func Test_defaultModelBuilderTask_buildListenerConfig(t *testing.T) { + tests := []struct { + name string + svc *corev1.Service + wantErr error + want *listenerConfig + }{ + { + name: "Service with unused ports in the ssl-ports annotation, Unused ports provided", + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-ssl-ports": "80, 85, 90, arbitrary-name", + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: 80, + TargetPort: intstr.FromInt(80), + Protocol: corev1.ProtocolTCP, + NodePort: 31223, + }, + { + Name: "alt2", + Port: 83, + TargetPort: intstr.FromInt(8883), + Protocol: corev1.ProtocolTCP, + NodePort: 32323, + }, + }, + }, + }, + wantErr: errors.New("Unused port in ssl-ports annotation [85 90 arbitrary-name]"), + }, + { + name: "Service with unused ports in the ssl-ports annotation, No unused ports", + svc: &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "service.beta.kubernetes.io/aws-load-balancer-ssl-ports": "83", + }, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + Ports: []corev1.ServicePort{ + { + Name: "http", + Port: 80, + TargetPort: intstr.FromInt(80), + Protocol: corev1.ProtocolTCP, + NodePort: 31223, + }, + { + Name: "alt2", + Port: 83, + TargetPort: intstr.FromInt(8883), + Protocol: corev1.ProtocolTCP, + NodePort: 32323, + }, + }, + }, + }, + want: &listenerConfig{ + certificates: ([]elbv2model.Certificate)(nil), + tlsPortsSet: sets.NewString("83"), + sslPolicy: new(string), + backendProtocol: "", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parser := annotations.NewSuffixAnnotationParser("service.beta.kubernetes.io") + builder := &defaultModelBuildTask{ + annotationParser: parser, + service: tt.svc, + } + got, err := builder.buildListenerConfig(context.Background()) + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.Equal(t, tt.want, got) + } + }) + } +} diff --git a/test/e2e/service/nlb_ip_target_test.go b/test/e2e/service/nlb_ip_target_test.go index e492e89b8..0a7708d64 100644 --- a/test/e2e/service/nlb_ip_target_test.go +++ b/test/e2e/service/nlb_ip_target_test.go @@ -217,6 +217,18 @@ var _ = Describe("k8s service reconciled by the aws load balancer", func() { TargetPort: intstr.FromInt(80), Protocol: corev1.ProtocolTCP, }, + { + Port: 443, + Name: "https", + TargetPort: intstr.FromInt(443), + Protocol: corev1.ProtocolTCP, + }, + { + Port: 333, + Name: "arbitrary-port", + TargetPort: intstr.FromInt(333), + Protocol: corev1.ProtocolTCP, + }, }, }, } @@ -245,10 +257,14 @@ var _ = Describe("k8s service reconciled by the aws load balancer", func() { Scheme: "internet-facing", TargetType: "ip", Listeners: map[string]string{ - "80": "TLS", + "80": "TLS", + "443": "TLS", + "333": "TLS", }, TargetGroups: map[string]string{ - "80": "TCP", + "80": "TCP", + "443": "TCP", + "333": "TCP", }, NumTargets: int(numReplicas), }) @@ -272,10 +288,14 @@ var _ = Describe("k8s service reconciled by the aws load balancer", func() { Scheme: "internet-facing", TargetType: "ip", Listeners: map[string]string{ - "80": "TCP", + "80": "TCP", + "443": "TLS", + "333": "TLS", }, TargetGroups: map[string]string{ - "80": "TCP", + "80": "TCP", + "443": "TCP", + "333": "TCP", }, NumTargets: int(numReplicas), })