Add support for specifying NLB target group attributes (kubernetes-sigs#1632)

* Refactor NLB model builder

* Support NLB target group attributes

* add comments, unexport constants

* address review comments

Author: kishorj

controllers/service/service_controller.go

@@ -17,7 +17,7 @@ import (
 	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/networking"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/runtime"
-	"sigs.k8s.io/aws-load-balancer-controller/pkg/service/nlb"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/service"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -37,7 +37,7 @@ func NewServiceReconciler(cloud aws.Cloud, k8sClient client.Client, eventRecorde
 	config config.ControllerConfig, logger logr.Logger) *serviceReconciler {
 
 	annotationParser := annotations.NewSuffixAnnotationParser(serviceAnnotationPrefix)
-	modelBuilder := nlb.NewDefaultModelBuilder(annotationParser, subnetsResolver, config.ClusterName)
+	modelBuilder := service.NewDefaultModelBuilder(annotationParser, subnetsResolver, config.ClusterName)
 	stackMarshaller := deploy.NewDefaultStackMarshaller()
 	stackDeployer := deploy.NewDefaultStackDeployer(cloud, k8sClient, networkingSGManager, networkingSGReconciler, config, serviceTagPrefix, logger)
 	return &serviceReconciler{
@@ -61,7 +61,7 @@ type serviceReconciler struct {
 	finalizerManager k8s.FinalizerManager
 	annotationParser annotations.Parser
 
-	modelBuilder    nlb.ModelBuilder
+	modelBuilder    service.ModelBuilder
 	stackMarshaller deploy.StackMarshaller
 	stackDeployer   deploy.StackDeployer
 	logger          logr.Logger

docs/guide/controller/pod_readiness_gate.md

@@ -36,6 +36,7 @@ Annotations:  <none>
 Status:       Active
 ```
 Once labelled, the controller will add the pod readiness gates config to all the pods created subsequently that meet all the following conditions
+
 * There exists a service matching the pod labels in the same namespace
 * There exists at least one target group binding that refers to the matching service
 * The target type is IP

docs/guide/service/annotations.md

@@ -13,7 +13,7 @@
 |--------------------------------------------------------------------------------|------------|---------------------------|------------------------|
 | service.beta.kubernetes.io/aws-load-balancer-type                              | string     |                           |                        |
 | service.beta.kubernetes.io/aws-load-balancer-internal                          | boolean    | false                     |                        |
-| service.beta.kubernetes.io/aws-load-balancer-proxy-protocol                    | string     |                           | Set to `"*"` to enable |
+| [service.beta.kubernetes.io/aws-load-balancer-proxy-protocol](#proxy-protocol-v2)                 | string     |                           | Set to `"*"` to enable |
 | service.beta.kubernetes.io/aws-load-balancer-access-log-enabled                | boolean    | false                     |                        |
 | service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-name         | string     |                           |                        |
 | service.beta.kubernetes.io/aws-load-balancer-access-log-s3-bucket-prefix       | string     |                           |                        |
@@ -30,4 +30,32 @@
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-protocol              | string     | TCP                       |                        |
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-port                  | string     | traffic-port              |                        |
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-path                  | string     | "/" for HTTP(S) protocols |                        |
-| service.beta.kubernetes.io/aws-load-balancer-eip-allocations                   | stringList |                           |                        |
+| service.beta.kubernetes.io/aws-load-balancer-eip-allocations                   | stringList |                           |                        |
+| [service.beta.kubernetes.io/aws-load-balancer-target-group-attributes](#target-group-attributes)  | stringMap  |                           |                        |
+
+## Resource attributes
+NLB target group attributes can be controlled via the following annotations:
+
+- <a name="proxy-protocol-v2">service.beta.kubernetes.io/aws-load-balancer-proxy-protocol</a> specifies whether to enable proxy protocol v2 on the target group.
+Set to '*' to enable proxy protocol v2. This annotation takes precedence over the annotation `service.beta.kubernetes.io/aws-load-balancer-target-group-attributes`
+for proxy protocol v2 configuration.
+
+    !!!note ""
+        The only valid value for this annotation is `*`.
+
+- <a name="target-group-attributes">`service.beta.kubernetes.io/aws-load-balancer-target-group-attributes`</a> specifies the
+[Target Group Attributes](https://docs.aws.amazon.com/elasticloadbalancing/latest/network/load-balancer-target-groups.html#target-group-attributes) to be configured.
+
+    !!!example
+        - set the deregistration delay to 120 seconds (available range is 0-3600 seconds)
+            ```
+            service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: deregistration_delay.timeout_seconds=120
+            ```
+        - enable source IP affinity
+            ```
+            service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: stickiness.enabled=true,stickiness.type=source_ip
+            ```
+        - enable proxy protocol version 2
+            ```
+            service.beta.kubernetes.io/aws-load-balancer-target-group-attributes: proxy_protocol_v2.enabled=true
+            ```

pkg/annotations/constants.go

@@ -64,4 +64,5 @@ const (
 	SvcLBSuffixHCPort                        = "aws-load-balancer-healthcheck-port"
 	SvcLBSuffixHCPath                        = "aws-load-balancer-healthcheck-path"
 	SvcLBSuffixEIPAllocations                = "aws-load-balancer-eip-allocations"
+	SvcLBSuffixTargetGroupAttributes         = "aws-load-balancer-target-group-attributes"
 )

pkg/ingress/model_build_listener_rules.go

@@ -2,8 +2,8 @@ package ingress
 
 import (
 	"context"
-	"github.com/pkg/errors"
 	"fmt"
+	"github.com/pkg/errors"
 	networking "k8s.io/api/networking/v1beta1"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/k8s"
 	"sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"

pkg/service/nlb/mock_visitor.go renamed to pkg/service/mock_visitor.go

@@ -1,4 +1,4 @@
-package nlb
+package service
 
 import "sigs.k8s.io/aws-load-balancer-controller/pkg/model/core"
 

pkg/service/model_build_listener.go

@@ -0,0 +1,134 @@
+package service
+
+import (
+	"context"
+	"fmt"
+	"github.com/aws/aws-sdk-go/aws"
+	corev1 "k8s.io/api/core/v1"
+	"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"
+	"strconv"
+)
+
+func (t *defaultModelBuildTask) buildListeners(ctx context.Context) error {
+	cfg := t.buildListenerConfig(ctx)
+	for _, port := range t.service.Spec.Ports {
+		_, err := t.buildListener(ctx, port, cfg)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (t *defaultModelBuildTask) buildListener(ctx context.Context, port corev1.ServicePort, cfg listenerConfig) (*elbv2model.Listener, error) {
+	lsSpec, err := t.buildListenerSpec(ctx, port, cfg)
+	if err != nil {
+		return nil, err
+	}
+	listenerResID := fmt.Sprintf("%v", port.Port)
+	ls := elbv2model.NewListener(t.stack, listenerResID, lsSpec)
+	return ls, nil
+}
+
+func (t *defaultModelBuildTask) buildListenerSpec(ctx context.Context, port corev1.ServicePort, cfg listenerConfig) (elbv2model.ListenerSpec, error) {
+	tgProtocol := elbv2model.Protocol(port.Protocol)
+	listenerProtocol := elbv2model.Protocol(port.Protocol)
+	if tgProtocol != elbv2model.ProtocolUDP && len(cfg.certificates) != 0 && (cfg.tlsPortsSet.Len() == 0 ||
+		cfg.tlsPortsSet.Has(port.Name) || cfg.tlsPortsSet.Has(strconv.Itoa(int(port.Port)))) {
+		if cfg.backendProtocol == "ssl" {
+			tgProtocol = elbv2model.ProtocolTLS
+		}
+		listenerProtocol = elbv2model.ProtocolTLS
+	}
+
+	targetGroup, err := t.buildTargetGroup(ctx, port, tgProtocol)
+	if err != nil {
+		return elbv2model.ListenerSpec{}, err
+	}
+
+	var sslPolicy *string
+	var certificates []elbv2model.Certificate
+	if listenerProtocol == elbv2model.ProtocolTLS {
+		sslPolicy = cfg.sslPolicy
+		certificates = cfg.certificates
+	}
+
+	defaultActions := t.buildListenerDefaultActions(ctx, targetGroup)
+	return elbv2model.ListenerSpec{
+		LoadBalancerARN: t.loadBalancer.LoadBalancerARN(),
+		Port:            int64(port.Port),
+		Protocol:        listenerProtocol,
+		Certificates:    certificates,
+		SSLPolicy:       sslPolicy,
+		DefaultActions:  defaultActions,
+	}, nil
+}
+
+func (t *defaultModelBuildTask) buildListenerDefaultActions(_ context.Context, targetGroup *elbv2model.TargetGroup) []elbv2model.Action {
+	return []elbv2model.Action{
+		{
+			Type: elbv2model.ActionTypeForward,
+			ForwardConfig: &elbv2model.ForwardActionConfig{
+				TargetGroups: []elbv2model.TargetGroupTuple{
+					{
+						TargetGroupARN: targetGroup.TargetGroupARN(),
+					},
+				},
+			},
+		},
+	}
+}
+
+func (t *defaultModelBuildTask) buildSSLNegotiationPolicy(_ context.Context) *string {
+	rawSslPolicyStr := ""
+	if exists := t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixSSLNegotiationPolicy, &rawSslPolicyStr, t.service.Annotations); exists {
+		return &rawSslPolicyStr
+	}
+	return nil
+}
+
+func (t *defaultModelBuildTask) buildListenerCertificates(_ context.Context) []elbv2model.Certificate {
+	var rawCertificateARNs []string
+	_ = t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLCertificate, &rawCertificateARNs, t.service.Annotations)
+
+	var certificates []elbv2model.Certificate
+	for _, cert := range rawCertificateARNs {
+		certificates = append(certificates, elbv2model.Certificate{CertificateARN: aws.String(cert)})
+	}
+	return certificates
+}
+
+func (t *defaultModelBuildTask) buildTLSPortsSet(_ context.Context) sets.String {
+	var rawTLSPorts []string
+	_ = t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixSSLPorts, &rawTLSPorts, t.service.Annotations)
+	return sets.NewString(rawTLSPorts...)
+}
+
+func (t *defaultModelBuildTask) buildBackendProtocol(_ context.Context) string {
+	rawBackendProtocol := ""
+	_ = t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixBEProtocol, &rawBackendProtocol, t.service.Annotations)
+	return rawBackendProtocol
+}
+
+type listenerConfig struct {
+	certificates    []elbv2model.Certificate
+	tlsPortsSet     sets.String
+	sslPolicy       *string
+	backendProtocol string
+}
+
+func (t *defaultModelBuildTask) buildListenerConfig(ctx context.Context) listenerConfig {
+	certificates := t.buildListenerCertificates(ctx)
+	tlsPortsSet := t.buildTLSPortsSet(ctx)
+	backendProtocol := t.buildBackendProtocol(ctx)
+	sslPolicy := t.buildSSLNegotiationPolicy(ctx)
+
+	return listenerConfig{
+		certificates:    certificates,
+		tlsPortsSet:     tlsPortsSet,
+		sslPolicy:       sslPolicy,
+		backendProtocol: backendProtocol,
+	}
+}

pkg/service/model_build_load_balancer.go

@@ -0,0 +1,156 @@
+package service
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/pkg/errors"
+	"regexp"
+	"sigs.k8s.io/aws-load-balancer-controller/pkg/annotations"
+	elbv2model "sigs.k8s.io/aws-load-balancer-controller/pkg/model/elbv2"
+	"strconv"
+)
+
+const (
+	lbAttrsAccessLogsS3Enabled           = "access_logs.s3.enabled"
+	lbAttrsAccessLogsS3Bucket            = "access_logs.s3.bucket"
+	lbAttrsAccessLogsS3Prefix            = "access_logs.s3.prefix"
+	lbAttrsLoadBalancingCrossZoneEnabled = "load_balancing.cross_zone.enabled"
+
+	resourceIDLoadBalancer = "LoadBalancer"
+)
+
+func (t *defaultModelBuildTask) buildLoadBalancer(ctx context.Context, scheme elbv2model.LoadBalancerScheme) error {
+	spec, err := t.buildLoadBalancerSpec(ctx, scheme)
+	if err != nil {
+		return err
+	}
+	t.loadBalancer = elbv2model.NewLoadBalancer(t.stack, resourceIDLoadBalancer, spec)
+	return nil
+}
+
+func (t *defaultModelBuildTask) buildLoadBalancerSpec(ctx context.Context, scheme elbv2model.LoadBalancerScheme) (elbv2model.LoadBalancerSpec, error) {
+	ipAddressType := elbv2model.IPAddressTypeIPV4
+	lbAttributes, err := t.buildLoadBalancerAttributes(ctx)
+	if err != nil {
+		return elbv2model.LoadBalancerSpec{}, err
+	}
+	tags, err := t.buildLoadBalancerTags(ctx)
+	if err != nil {
+		return elbv2model.LoadBalancerSpec{}, err
+	}
+	subnetMappings, err := t.buildSubnetMappings(ctx, t.ec2Subnets)
+	if err != nil {
+		return elbv2model.LoadBalancerSpec{}, err
+	}
+	name := t.buildLoadBalancerName(ctx, scheme)
+	spec := elbv2model.LoadBalancerSpec{
+		Name:                   name,
+		Type:                   elbv2model.LoadBalancerTypeNetwork,
+		Scheme:                 &scheme,
+		IPAddressType:          &ipAddressType,
+		SubnetMappings:         subnetMappings,
+		LoadBalancerAttributes: lbAttributes,
+		Tags:                   tags,
+	}
+	return spec, nil
+}
+
+func (t *defaultModelBuildTask) buildLoadBalancerScheme(_ context.Context) (elbv2model.LoadBalancerScheme, error) {
+	internal := false
+	if _, err := t.annotationParser.ParseBoolAnnotation(annotations.SvcLBSuffixInternal, &internal, t.service.Annotations); err != nil {
+		return "", err
+	}
+	if internal {
+		return elbv2model.LoadBalancerSchemeInternal, nil
+	}
+	return elbv2model.LoadBalancerSchemeInternetFacing, nil
+}
+
+func (t *defaultModelBuildTask) buildAdditionalResourceTags(_ context.Context) (map[string]string, error) {
+	tags := make(map[string]string)
+	_, err := t.annotationParser.ParseStringMapAnnotation(annotations.SvcLBSuffixAdditionalTags, &tags, t.service.Annotations)
+	if err != nil {
+		return nil, err
+	}
+	return tags, nil
+}
+
+func (t *defaultModelBuildTask) buildLoadBalancerTags(ctx context.Context) (map[string]string, error) {
+	return t.buildAdditionalResourceTags(ctx)
+}
+
+func (t *defaultModelBuildTask) buildSubnetMappings(_ context.Context, ec2Subnets []*ec2.Subnet) ([]elbv2model.SubnetMapping, error) {
+	var eipAllocation []string
+	eipConfigured := t.annotationParser.ParseStringSliceAnnotation(annotations.SvcLBSuffixEIPAllocations, &eipAllocation, t.service.Annotations)
+	if eipConfigured && len(eipAllocation) != len(ec2Subnets) {
+		return []elbv2model.SubnetMapping{}, errors.Errorf("number of EIP allocations (%d) and subnets (%d) must match", len(eipAllocation), len(ec2Subnets))
+	}
+	subnetMappings := make([]elbv2model.SubnetMapping, 0, len(ec2Subnets))
+	for idx, subnet := range ec2Subnets {
+		mapping := elbv2model.SubnetMapping{
+			SubnetID: aws.StringValue(subnet.SubnetId),
+		}
+		if idx < len(eipAllocation) {
+			mapping.AllocationID = aws.String(eipAllocation[idx])
+		}
+		subnetMappings = append(subnetMappings, mapping)
+	}
+	return subnetMappings, nil
+}
+
+func (t *defaultModelBuildTask) buildLoadBalancerAttributes(_ context.Context) ([]elbv2model.LoadBalancerAttribute, error) {
+	var attrs []elbv2model.LoadBalancerAttribute
+	accessLogEnabled := t.defaultAccessLogS3Enabled
+	bucketName := t.defaultAccessLogsS3Bucket
+	bucketPrefix := t.defaultAccessLogsS3Prefix
+	if _, err := t.annotationParser.ParseBoolAnnotation(annotations.SvcLBSuffixAccessLogEnabled, &accessLogEnabled, t.service.Annotations); err != nil {
+		return []elbv2model.LoadBalancerAttribute{}, err
+	}
+	if accessLogEnabled {
+		t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixAccessLogS3BucketName, &bucketName, t.service.Annotations)
+		t.annotationParser.ParseStringAnnotation(annotations.SvcLBSuffixAccessLogS3BucketPrefix, &bucketPrefix, t.service.Annotations)
+	}
+	crossZoneEnabled := t.defaultLoadBalancingCrossZoneEnabled
+	if _, err := t.annotationParser.ParseBoolAnnotation(annotations.SvcLBSuffixCrossZoneLoadBalancingEnabled, &crossZoneEnabled, t.service.Annotations); err != nil {
+		return []elbv2model.LoadBalancerAttribute{}, err
+	}
+
+	attrs = []elbv2model.LoadBalancerAttribute{
+		{
+			Key:   lbAttrsAccessLogsS3Enabled,
+			Value: strconv.FormatBool(accessLogEnabled),
+		},
+		{
+			Key:   lbAttrsAccessLogsS3Bucket,
+			Value: bucketName,
+		},
+		{
+			Key:   lbAttrsAccessLogsS3Prefix,
+			Value: bucketPrefix,
+		},
+		{
+			Key:   lbAttrsLoadBalancingCrossZoneEnabled,
+			Value: strconv.FormatBool(crossZoneEnabled),
+		},
+	}
+
+	return attrs, nil
+}
+
+var invalidLoadBalancerNamePattern = regexp.MustCompile("[[:^alnum:]]")
+
+func (t *defaultModelBuildTask) buildLoadBalancerName(_ context.Context, scheme elbv2model.LoadBalancerScheme) string {
+	uuidHash := sha256.New()
+	_, _ = uuidHash.Write([]byte(t.clusterName))
+	_, _ = uuidHash.Write([]byte(t.service.UID))
+	_, _ = uuidHash.Write([]byte(scheme))
+	uuid := hex.EncodeToString(uuidHash.Sum(nil))
+
+	sanitizedNamespace := invalidLoadBalancerNamePattern.ReplaceAllString(t.service.Namespace, "")
+	sanitizedName := invalidLoadBalancerNamePattern.ReplaceAllString(t.service.Name, "")
+	return fmt.Sprintf("k8s-%.8s-%.8s-%.10s", sanitizedNamespace, sanitizedName, uuid)
+}