DNM/SPLAT-2113: spike aws/CIO/IngressNLBSecurityGroup support BYO SG for NLB

docs/service_controller.md

@@ -17,6 +17,7 @@ The service controller is responsible for watch for service and node object chan
 | service.beta.kubernetes.io/aws-load-balancer-cross-zone-load-balancing-enabled | [true\|false]                       | -   | With cross-zone load balancing, each load balancer node for your Classic Load Balancer distributes requests evenly across the registered instances in all enabled Availability Zones. If cross-zone load balancing is disabled, each load balancer node distributes requests evenly across the registered instances in its Availability Zone only. |
 | service.beta.kubernetes.io/aws-load-balancer-extra-security-groups             | Comma-separated list                | -   | Specifies additional security groups to be added to ELB.    |
 | service.beta.kubernetes.io/aws-load-balancer-security-groups                   | Comma-separated list                | -   | Specifies the security groups to be added to ELB. Differently from the annotation "service.beta.kubernetes.io/aws-load-balancer-extra-security-groups", this replaces all other security groups previously assigned to the ELB. |
+| service.beta.kubernetes.io/aws-load-balancer-manage-security-group             | Bool                | -   | Indicates that the controller creates and manages the lifecycle of a Security Group when creating a Network Load Balancer (NLB). This is evaluated only when the service Load Balancer type `nlb` is created. You can not specify security group when using this option. |
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-healthy-threshold     | [2-10]                              | -   | Specifies the number of successive successful health checks required for a backend to be considered healthy for traffic. For NLB, healthy-threshold and unhealthy-threshold must be equal. |
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-interval              | [5-300]                             | 30  | Specifies, in seconds, the interval between health checks. |
 | service.beta.kubernetes.io/aws-load-balancer-healthcheck-timeout               | [2-60]                              | 5   | The amount of time to wait when receiving a response from the health check, in seconds. |

pkg/providers/v1/aws_loadbalancer.go

@@ -141,7 +141,7 @@ func getKeyValuePropertiesFromAnnotation(annotations map[string]string, annotati
 }
 
 // ensureLoadBalancerv2 ensures a v2 load balancer is created
-func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBalancerName string, mappings []nlbPortMapping, instanceIDs, discoveredSubnetIDs []string, internalELB bool, annotations map[string]string) (*elbv2.LoadBalancer, error) {
+func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBalancerName string, mappings []nlbPortMapping, instanceIDs, discoveredSubnetIDs []string, internalELB bool, annotations map[string]string, securityGroups []*string) (*elbv2.LoadBalancer, error) {
 	loadBalancer, err := c.describeLoadBalancerv2(loadBalancerName)
 	if err != nil {
 		return nil, err
@@ -177,6 +177,9 @@ func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBa
 		// TODO: What happens if we have more than one subnet per AZ?
 		createRequest.SubnetMappings = createSubnetMappings(discoveredSubnetIDs, allocationIDs)
 
+		// Enable provisioning NLB with security groups when the annotation(s) are set.
+		createRequest.SecurityGroups = securityGroups
+
 		for k, v := range tags {
 			createRequest.Tags = append(createRequest.Tags, &elbv2.Tag{
 				Key: aws.String(k), Value: aws.String(v),
@@ -402,6 +405,7 @@ func (c *Cloud) ensureLoadBalancerv2(namespacedName types.NamespacedName, loadBa
 func (c *Cloud) reconcileLBAttributes(loadBalancerArn string, annotations map[string]string) error {
 	desiredLoadBalancerAttributes := map[string]string{}
 
+	//REVIEW on Update
 	desiredLoadBalancerAttributes[lbAttrLoadBalancingCrossZoneEnabled] = "false"
 	crossZoneLoadBalancingEnabledAnnotation := annotations[ServiceAnnotationLoadBalancerCrossZoneLoadBalancingEnabled]
 	if crossZoneLoadBalancingEnabledAnnotation != "" {

pkg/providers/v1/aws_test.go

@@ -2063,7 +2063,7 @@ func TestLBExtraSecurityGroupsAnnotation(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			serviceName := types.NamespacedName{Namespace: "default", Name: "myservice"}
 
-			sgList, setupSg, err := c.buildELBSecurityGroupList(serviceName, "aid", test.annotations)
+			sgList, setupSg, err := c.buildELBSecurityGroupList(serviceName, "aid", test.annotations, false)
 			assert.NoError(t, err, "buildELBSecurityGroupList failed")
 			extraSGs := sgList[1:]
 			assert.True(t, sets.NewString(test.expectedSGs...).Equal(sets.NewString(extraSGs...)),
@@ -2097,7 +2097,7 @@ func TestLBSecurityGroupsAnnotation(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			serviceName := types.NamespacedName{Namespace: "default", Name: "myservice"}
 
-			sgList, setupSg, err := c.buildELBSecurityGroupList(serviceName, "aid", test.annotations)
+			sgList, setupSg, err := c.buildELBSecurityGroupList(serviceName, "aid", test.annotations, false)
 			assert.NoError(t, err, "buildELBSecurityGroupList failed")
 			assert.True(t, sets.NewString(test.expectedSGs...).Equal(sets.NewString(sgList...)),
 				"Security Groups expected=%q , returned=%q", test.expectedSGs, sgList)
@@ -2106,6 +2106,57 @@ func TestLBSecurityGroupsAnnotation(t *testing.T) {
 	}
 }
 
+func TestLBManagedSecurityGroupAnnotation(t *testing.T) {
+	awsServices := newMockedFakeAWSServices(TestClusterID)
+	c, _ := newAWSCloud(config.CloudConfig{}, awsServices)
+
+	loadBalancerName := "nlbsg"
+	managed := map[string]string{ServiceAnnotationLoadBalancerManagedSecurityGroup: "true"}
+	managedSG := "k8s-elb-" + loadBalancerName
+	unmanaged := map[string]string{ServiceAnnotationLoadBalancerManagedSecurityGroup: "false"}
+
+	tests := []struct {
+		name        string
+		annotations map[string]string
+		skipDefault bool
+		expectedSGs []string
+		wantSetupSG bool
+	}{
+		{
+			"No NLB Managed SG", map[string]string{}, false, []string{}, true,
+		},
+		{
+			"NLB Managed SG specified true skipDefault", managed, true, []string{}, false,
+		},
+		{
+			"NLB Managed SG specified true", managed, false, []string{managedSG}, true,
+		},
+		{
+			"NLB Managed SG specified false", unmanaged, true, []string{}, false,
+		},
+		{
+			"NLB Managed SG specified true with securityGroupLabels", managed, true, []string{}, false,
+		},
+		{
+			"NLB Managed SG specified true with extra securityGroupLabels", managed, true, []string{}, false,
+		},
+	}
+
+	awsServices.ec2.(*MockedFakeEC2).expectDescribeSecurityGroups(TestClusterID, managedSG)
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			serviceName := types.NamespacedName{Namespace: "default", Name: "myservice"}
+
+			sgList, setupSg, err := c.buildELBSecurityGroupList(serviceName, loadBalancerName, test.annotations, test.skipDefault)
+			assert.NoError(t, err, "buildELBSecurityGroupList failed")
+			assert.True(t, sets.NewString(test.expectedSGs...).Equal(sets.NewString(sgList...)),
+				"Security Groups expected=%q , returned=%q", test.expectedSGs, sgList)
+			assert.False(t, setupSg, "Security Groups Setup Permissions Flag expected=%t , returned=%t", test.wantSetupSG, setupSg)
+		})
+	}
+}
+
 // Test that we can add a load balancer tag
 func TestAddLoadBalancerTags(t *testing.T) {
 	loadBalancerName := "test-elb"

pkg/providers/v1/config/config.go

@@ -19,6 +19,9 @@ const (
 	ClusterServiceLoadBalancerHealthProbeModeServiceNodePort = "ServiceNodePort"
 )
 
+// NLBSecurityGroupEnabled indicates whether the service loadbalancer type NLB is created with a Security Group.
+type NLBSecurityGroupEnabled bool
+
 // CloudConfig wraps the settings for the AWS cloud provider.
 // NOTE: Cloud config files should follow the same Kubernetes deprecation policy as
 // flags or CLIs. Config fields should not change behavior in incompatible ways and
@@ -83,6 +86,10 @@ type CloudConfig struct {
 
 		// ClusterServiceSharedLoadBalancerHealthProbePath defines the target path of the shared health probe. Default to `/healthz`.
 		ClusterServiceSharedLoadBalancerHealthProbePath string `json:"clusterServiceSharedLoadBalancerHealthProbePath,omitempty" yaml:"clusterServiceSharedLoadBalancerHealthProbePath,omitempty"`
+
+		// NLBSecurityGroupEnabled determines if the service type loadbalancer NLB creates and manages
+		// the resource with a security group (default behavior Classic Load Balancer).
+		NLBSecurityGroupEnabled NLBSecurityGroupEnabled `json:"nlbSecurityGroupEnabled,omitempty" yaml:"nlbSecurityGroupEnabled,omitempty"`
 	}
 	// [ServiceOverride "1"]
 	//  Service = s3

tests/e2e/go.mod

@@ -3,6 +3,9 @@ module k8s.io/cloud-provider-aws/tests/e2e
 go 1.23.0
 
 require (
+	github.com/aws/aws-sdk-go-v2 v1.36.3
+	github.com/aws/aws-sdk-go-v2/config v1.29.14
+	github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.45.2
 	github.com/onsi/ginkgo/v2 v2.9.4
 	github.com/onsi/gomega v1.27.6
 	k8s.io/api v0.26.0
@@ -13,6 +16,17 @@ require (
 )
 
 require (
+	github.com/aws/aws-sdk-go-v2/credentials v1.17.67 // indirect
+	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3 // indirect
+	github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 // indirect
+	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 // indirect
+	github.com/aws/aws-sdk-go-v2/service/sso v1.25.3 // indirect
+	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1 // indirect
+	github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 // indirect
+	github.com/aws/smithy-go v1.22.2 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/blang/semver/v4 v4.0.0 // indirect
 	github.com/cenkalti/backoff/v4 v4.1.3 // indirect

tests/e2e/go.sum

@@ -42,6 +42,34 @@ github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk5
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
+github.com/aws/aws-sdk-go-v2 v1.36.3 h1:mJoei2CxPutQVxaATCzDUjcZEjVRdpsiiXi2o38yqWM=
+github.com/aws/aws-sdk-go-v2 v1.36.3/go.mod h1:LLXuLpgzEbD766Z5ECcRmi8AzSwfZItDtmABVkRLGzg=
+github.com/aws/aws-sdk-go-v2/config v1.29.14 h1:f+eEi/2cKCg9pqKBoAIwRGzVb70MRKqWX4dg1BDcSJM=
+github.com/aws/aws-sdk-go-v2/config v1.29.14/go.mod h1:wVPHWcIFv3WO89w0rE10gzf17ZYy+UVS1Geq8Iei34g=
+github.com/aws/aws-sdk-go-v2/credentials v1.17.67 h1:9KxtdcIA/5xPNQyZRgUSpYOE6j9Bc4+D7nZua0KGYOM=
+github.com/aws/aws-sdk-go-v2/credentials v1.17.67/go.mod h1:p3C44m+cfnbv763s52gCqrjaqyPikj9Sg47kUVaNZQQ=
+github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30 h1:x793wxmUWVDhshP8WW2mlnXuFrO4cOd3HLBroh1paFw=
+github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.30/go.mod h1:Jpne2tDnYiFascUEs2AWHJL9Yp7A5ZVy3TNyxaAjD6M=
+github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34 h1:ZK5jHhnrioRkUNOc+hOgQKlUL5JeC3S6JgLxtQ+Rm0Q=
+github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.34/go.mod h1:p4VfIceZokChbA9FzMbRGz5OV+lekcVtHlPKEO0gSZY=
+github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 h1:SZwFm17ZUNNg5Np0ioo/gq8Mn6u9w19Mri8DnJ15Jf0=
+github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34/go.mod h1:dFZsC0BLo346mvKQLWmoJxT+Sjp+qcVR1tRVHQGOH9Q=
+github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3 h1:bIqFDwgGXXN1Kpp99pDOdKMTTb5d2KyU5X/BZxjOkRo=
+github.com/aws/aws-sdk-go-v2/internal/ini v1.8.3/go.mod h1:H5O/EsxDWyU+LP/V8i5sm8cxoZgc2fdNR9bxlOFrQTo=
+github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.45.2 h1:vX70Z4lNSr7XsioU0uJq5yvxgI50sB66MvD+V/3buS4=
+github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.45.2/go.mod h1:xnCC3vFBfOKpU6PcsCKL2ktgBTZfOwTGxj6V8/X3IS4=
+github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 h1:eAh2A4b5IzM/lum78bZ590jy36+d/aFLgKF/4Vd1xPE=
+github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3/go.mod h1:0yKJC/kb8sAnmlYa6Zs3QVYqaC8ug2AbnNChv5Ox3uA=
+github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 h1:dM9/92u2F1JbDaGooxTq18wmmFzbJRfXfVfy96/1CXM=
+github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15/go.mod h1:SwFBy2vjtA0vZbjjaFtfN045boopadnoVPhu4Fv66vY=
+github.com/aws/aws-sdk-go-v2/service/sso v1.25.3 h1:1Gw+9ajCV1jogloEv1RRnvfRFia2cL6c9cuKV2Ps+G8=
+github.com/aws/aws-sdk-go-v2/service/sso v1.25.3/go.mod h1:qs4a9T5EMLl/Cajiw2TcbNt2UNo/Hqlyp+GiuG4CFDI=
+github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1 h1:hXmVKytPfTy5axZ+fYbR5d0cFmC3JvwLm5kM83luako=
+github.com/aws/aws-sdk-go-v2/service/ssooidc v1.30.1/go.mod h1:MlYRNmYu/fGPoxBQVvBYr9nyr948aY/WLUvwBMBJubs=
+github.com/aws/aws-sdk-go-v2/service/sts v1.33.19 h1:1XuUZ8mYJw9B6lzAkXhqHlJd/XvaX32evhproijJEZY=
+github.com/aws/aws-sdk-go-v2/service/sts v1.33.19/go.mod h1:cQnB8CUnxbMU82JvlqjKR2HBOm3fe9pWorWBza6MBJ4=
+github.com/aws/smithy-go v1.22.2 h1:6D9hW43xKFrRx/tXXfAlIZc4JI+yQe6snnWcQyxSyLQ=
+github.com/aws/smithy-go v1.22.2/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
 github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=