[release-4.7] Bug 1954097: Annotate services of type LoadBalancer with user tags (AWS only)

go.mod

@@ -18,7 +18,7 @@ require (
 	github.com/imdario/mergo v0.3.10 // indirect
 	github.com/kevinburke/go-bindata v3.11.0+incompatible
 	github.com/onsi/ginkgo v1.14.0 // indirect
-	github.com/openshift/api v0.0.0-20201201210054-c6debb38648f
+	github.com/openshift/api v0.0.0-20210428205234-a8389931bee7
 	github.com/openshift/library-go v0.0.0-20200423123937-d1360419413d
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.7.1

pkg/operator/controller/ingress/load_balancer_service.go

@@ -3,6 +3,7 @@ package ingress
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -22,6 +23,13 @@ import (
 )
 
 const (
+	// awsLBAdditionalResourceTags is a comma separated list of
+	// Key=Value pairs that are additionally recorded on
+	// load balancer resources and security groups.
+	//
+	// https://kubernetes.io/docs/concepts/services-networking/service/#aws-load-balancer-additional-resource-tags
+	awsLBAdditionalResourceTags = "service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags"
+
 	// awsLBProxyProtocolAnnotation is used to enable the PROXY protocol on any
 	// AWS load balancer services created.
 	//
@@ -222,6 +230,19 @@ func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef
 			} else {
 				service.Annotations[awsLBHealthCheckIntervalAnnotation] = awsLBHealthCheckIntervalDefault
 			}
+
+			if platform.AWS != nil && len(platform.AWS.ResourceTags) > 0 {
+				var additionalTags []string
+				for _, userTag := range platform.AWS.ResourceTags {
+					if len(userTag.Key) > 0 {
+						additionalTags = append(additionalTags, userTag.Key+"="+userTag.Value)
+					}
+				}
+				if len(additionalTags) > 0 {
+					service.Annotations[awsLBAdditionalResourceTags] = strings.Join(additionalTags, ",")
+				}
+			}
+
 			// Set the load balancer for AWS to be as aggressive as Azure (2 fail @ 5s interval, 2 healthy)
 			service.Annotations[awsLBHealthCheckTimeoutAnnotation] = awsLBHealthCheckTimeoutDefault
 			service.Annotations[awsLBHealthCheckUnhealthyThresholdAnnotation] = awsLBHealthCheckUnhealthyThresholdDefault

pkg/operator/controller/ingress/load_balancer_service_test.go

@@ -2,9 +2,10 @@ package ingress
 
 import (
 	"fmt"
-	corev1 "k8s.io/api/core/v1"
 	"testing"
 
+	corev1 "k8s.io/api/core/v1"
+
 	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 
@@ -14,12 +15,14 @@ import (
 
 func TestDesiredLoadBalancerService(t *testing.T) {
 	testCases := []struct {
-		description  string
-		platform     configv1.PlatformType
-		strategyType operatorv1.EndpointPublishingStrategyType
-		lbStrategy   operatorv1.LoadBalancerStrategy
-		proxyNeeded  bool
-		expect       bool
+		description          string
+		platform             configv1.PlatformType
+		strategyType         operatorv1.EndpointPublishingStrategyType
+		lbStrategy           operatorv1.LoadBalancerStrategy
+		proxyNeeded          bool
+		expect               bool
+		platformStatus       configv1.PlatformStatus
+		expectedResourceTags string
 	}{
 		{
 			description:  "external classic load balancer with scope for aws platform",
@@ -31,6 +34,30 @@ func TestDesiredLoadBalancerService(t *testing.T) {
 			proxyNeeded: true,
 			expect:      true,
 		},
+		{
+			description:  "external classic load balancer with scope for aws platform and custom user tags",
+			platform:     configv1.AWSPlatformType,
+			strategyType: operatorv1.LoadBalancerServiceStrategyType,
+			lbStrategy: operatorv1.LoadBalancerStrategy{
+				Scope: operatorv1.ExternalLoadBalancer,
+			},
+			proxyNeeded: true,
+			expect:      true,
+			platformStatus: configv1.PlatformStatus{
+				AWS: &configv1.AWSPlatformStatus{
+					ResourceTags: []configv1.AWSResourceTag{{
+						Key:   "classic-load-balancer-key-with-value",
+						Value: "100",
+					}, {
+						Key:   "classic-load-balancer-key-with-empty-value",
+						Value: "",
+					}, {
+						Value: "classic-load-balancer-value-without-key",
+					}},
+				},
+			},
+			expectedResourceTags: "classic-load-balancer-key-with-value=100,classic-load-balancer-key-with-empty-value=",
+		},
 		{
 			description:  "external classic load balancer without scope for aws platform",
 			platform:     configv1.AWSPlatformType,
@@ -86,6 +113,37 @@ func TestDesiredLoadBalancerService(t *testing.T) {
 			proxyNeeded: false,
 			expect:      true,
 		},
+		{
+			description:  "external network load balancer with scope for aws platform and custom user tags",
+			platform:     configv1.AWSPlatformType,
+			strategyType: operatorv1.LoadBalancerServiceStrategyType,
+			lbStrategy: operatorv1.LoadBalancerStrategy{
+				Scope: operatorv1.ExternalLoadBalancer,
+				ProviderParameters: &operatorv1.ProviderLoadBalancerParameters{
+					Type: operatorv1.AWSLoadBalancerProvider,
+					AWS: &operatorv1.AWSLoadBalancerParameters{
+						Type: operatorv1.AWSNetworkLoadBalancer,
+					},
+				},
+			},
+			proxyNeeded: false,
+			expect:      true,
+
+			platformStatus: configv1.PlatformStatus{
+				AWS: &configv1.AWSPlatformStatus{
+					ResourceTags: []configv1.AWSResourceTag{{
+						Key:   "network-load-balancer-key-with-value",
+						Value: "200",
+					}, {
+						Key:   "network-load-balancer-key-with-empty-value",
+						Value: "",
+					}, {
+						Value: "network-load-balancer-value-without-key",
+					}},
+				},
+			},
+			expectedResourceTags: "network-load-balancer-key-with-value=200,network-load-balancer-key-with-empty-value=",
+		},
 		{
 			description:  "nodePort service for aws platform",
 			platform:     configv1.AWSPlatformType,
@@ -189,11 +247,10 @@ func TestDesiredLoadBalancerService(t *testing.T) {
 		}
 		infraConfig := &configv1.Infrastructure{
 			Status: configv1.InfrastructureStatus{
-				PlatformStatus: &configv1.PlatformStatus{
-					Type: tc.platform,
-				},
+				PlatformStatus: &tc.platformStatus,
 			},
 		}
+		infraConfig.Status.PlatformStatus.Type = tc.platform
 
 		proxyNeeded, err := IsProxyProtocolNeeded(ic, infraConfig.Status.PlatformStatus)
 		switch {
@@ -235,13 +292,31 @@ func TestDesiredLoadBalancerService(t *testing.T) {
 				classicLB := tc.lbStrategy.ProviderParameters == nil || tc.lbStrategy.ProviderParameters.AWS.Type == operatorv1.AWSClassicLoadBalancer
 				switch {
 				case classicLB:
+					if len(tc.expectedResourceTags) > 0 {
+						if err := checkServiceHasAnnotation(svc, awsLBAdditionalResourceTags, true, tc.expectedResourceTags); err != nil {
+							t.Errorf("annotation check for test %q failed: %v, unexpected value", tc.description, err)
+						}
+					} else {
+						if err := checkServiceHasAnnotation(svc, awsLBAdditionalResourceTags, false, ""); err == nil {
+							t.Errorf("annotation check for test %q failed; unexpected annotation %s", tc.description, awsLBAdditionalResourceTags)
+						}
+					}
 					if err := checkServiceHasAnnotation(svc, awsLBHealthCheckIntervalAnnotation, true, awsLBHealthCheckIntervalDefault); err != nil {
 						t.Errorf("annotation check for test %q failed: %v", tc.description, err)
 					}
 					if err := checkServiceHasAnnotation(svc, awsLBProxyProtocolAnnotation, true, "*"); err != nil {
 						t.Errorf("annotation check for test %q failed: %v", tc.description, err)
 					}
 				case tc.lbStrategy.ProviderParameters.AWS.Type == operatorv1.AWSNetworkLoadBalancer:
+					if len(tc.expectedResourceTags) > 0 {
+						if err := checkServiceHasAnnotation(svc, awsLBAdditionalResourceTags, true, tc.expectedResourceTags); err != nil {
+							t.Errorf("annotation check for test %q failed: %v, unexpected value", tc.description, err)
+						}
+					} else {
+						if err := checkServiceHasAnnotation(svc, awsLBAdditionalResourceTags, false, ""); err == nil {
+							t.Errorf("annotation check for test %q failed; unexpected annotation %s", tc.description, awsLBAdditionalResourceTags)
+						}
+					}
 					if err := checkServiceHasAnnotation(svc, awsLBHealthCheckIntervalAnnotation, true, awsLBHealthCheckIntervalNLB); err != nil {
 						t.Errorf("annotation check for test %q failed: %v", tc.description, err)
 					}