Merge pull request #17430 from h3poteto/iss-17250/additional-security-groups

Re-enable additionalSecurityGroups for bastion LB

k8s/crds/kops.k8s.io_clusters.yaml

@@ -6386,7 +6386,6 @@ spec:
                       loadBalancer:
                         properties:
                           additionalSecurityGroups:
-                            description: AdditionalSecurityGroups is unused
                             items:
                               type: string
                             type: array

pkg/apis/kops/bastion.go

@@ -24,6 +24,7 @@ type BastionSpec struct {
 }
 
 type BastionLoadBalancerSpec struct {
+	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
 	// Type of load balancer to create, it can be Public or Internal.
 	Type LoadBalancerType `json:"type,omitempty"`
 }

pkg/apis/kops/v1alpha2/bastion.go

@@ -25,8 +25,6 @@ type BastionSpec struct {
 }
 
 type BastionLoadBalancerSpec struct {
-	// AdditionalSecurityGroups is unused
-	// +k8s:conversion-gen=false
 	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
 	// Type of load balancer to create, it can be Public or Internal.
 	Type LoadBalancerType `json:"type,omitempty"`

pkg/apis/kops/v1alpha2/zz_generated.conversion.go

@@ -1742,7 +1742,7 @@ func Convert_kops_AzureSpec_To_v1alpha2_AzureSpec(in *kops.AzureSpec, out *Azure
 }
 
 func autoConvert_v1alpha2_BastionLoadBalancerSpec_To_kops_BastionLoadBalancerSpec(in *BastionLoadBalancerSpec, out *kops.BastionLoadBalancerSpec, s conversion.Scope) error {
-	// INFO: in.AdditionalSecurityGroups opted out of conversion generation
+	out.AdditionalSecurityGroups = in.AdditionalSecurityGroups
 	out.Type = kops.LoadBalancerType(in.Type)
 	return nil
 }
@@ -1753,6 +1753,7 @@ func Convert_v1alpha2_BastionLoadBalancerSpec_To_kops_BastionLoadBalancerSpec(in
 }
 
 func autoConvert_kops_BastionLoadBalancerSpec_To_v1alpha2_BastionLoadBalancerSpec(in *kops.BastionLoadBalancerSpec, out *BastionLoadBalancerSpec, s conversion.Scope) error {
+	out.AdditionalSecurityGroups = in.AdditionalSecurityGroups
 	out.Type = LoadBalancerType(in.Type)
 	return nil
 }

pkg/apis/kops/v1alpha3/bastion.go

@@ -24,6 +24,7 @@ type BastionSpec struct {
 }
 
 type BastionLoadBalancerSpec struct {
+	AdditionalSecurityGroups []string `json:"additionalSecurityGroups,omitempty"`
 	// Type of load balancer to create, it can be Public or Internal.
 	Type LoadBalancerType `json:"type,omitempty"`
 }

pkg/apis/kops/v1alpha3/zz_generated.conversion.go

@@ -1918,6 +1918,7 @@ func Convert_kops_AzureSpec_To_v1alpha3_AzureSpec(in *kops.AzureSpec, out *Azure
 }
 
 func autoConvert_v1alpha3_BastionLoadBalancerSpec_To_kops_BastionLoadBalancerSpec(in *BastionLoadBalancerSpec, out *kops.BastionLoadBalancerSpec, s conversion.Scope) error {
+	out.AdditionalSecurityGroups = in.AdditionalSecurityGroups
 	out.Type = kops.LoadBalancerType(in.Type)
 	return nil
 }
@@ -1928,6 +1929,7 @@ func Convert_v1alpha3_BastionLoadBalancerSpec_To_kops_BastionLoadBalancerSpec(in
 }
 
 func autoConvert_kops_BastionLoadBalancerSpec_To_v1alpha3_BastionLoadBalancerSpec(in *kops.BastionLoadBalancerSpec, out *BastionLoadBalancerSpec, s conversion.Scope) error {
+	out.AdditionalSecurityGroups = in.AdditionalSecurityGroups
 	out.Type = LoadBalancerType(in.Type)
 	return nil
 }

pkg/apis/kops/v1alpha3/zz_generated.deepcopy.go

@@ -423,6 +423,11 @@ func (in *AzureSpec) DeepCopy() *AzureSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *BastionLoadBalancerSpec) DeepCopyInto(out *BastionLoadBalancerSpec) {
 	*out = *in
+	if in.AdditionalSecurityGroups != nil {
+		in, out := &in.AdditionalSecurityGroups, &out.AdditionalSecurityGroups
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	return
 }
 
@@ -442,7 +447,7 @@ func (in *BastionSpec) DeepCopyInto(out *BastionSpec) {
 	if in.LoadBalancer != nil {
 		in, out := &in.LoadBalancer, &out.LoadBalancer
 		*out = new(BastionLoadBalancerSpec)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	return
 }

pkg/apis/kops/zz_generated.deepcopy.go

@@ -422,6 +422,11 @@ func (in *AzureSpec) DeepCopy() *AzureSpec {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *BastionLoadBalancerSpec) DeepCopyInto(out *BastionLoadBalancerSpec) {
 	*out = *in
+	if in.AdditionalSecurityGroups != nil {
+		in, out := &in.AdditionalSecurityGroups, &out.AdditionalSecurityGroups
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	return
 }
 
@@ -441,7 +446,7 @@ func (in *BastionSpec) DeepCopyInto(out *BastionSpec) {
 	if in.LoadBalancer != nil {
 		in, out := &in.LoadBalancer, &out.LoadBalancer
 		*out = new(BastionLoadBalancerSpec)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	return
 }

pkg/model/awsmodel/bastion.go

@@ -393,6 +393,20 @@ func (b *BastionModelBuilder) Build(c *fi.CloudupModelBuilderContext) error {
 
 		c.AddTask(tg)
 
+		// Add additional security groups to the NLB
+		if b.Cluster.Spec.Networking.Topology != nil && b.Cluster.Spec.Networking.Topology.Bastion != nil && b.Cluster.Spec.Networking.Topology.Bastion.LoadBalancer != nil && b.Cluster.Spec.Networking.Topology.Bastion.LoadBalancer.AdditionalSecurityGroups != nil {
+			for _, id := range b.Cluster.Spec.Networking.Topology.Bastion.LoadBalancer.AdditionalSecurityGroups {
+				t := &awstasks.SecurityGroup{
+					Name:      fi.PtrTo(id),
+					Lifecycle: b.SecurityLifecycle,
+					ID:        fi.PtrTo(id),
+					Shared:    fi.PtrTo(true),
+				}
+				c.EnsureTask(t)
+				nlb.SecurityGroups = append(nlb.SecurityGroups, t)
+			}
+		}
+
 		c.AddTask(nlb)
 	}
 

tests/integration/update_cluster/bastionadditional_user-data/data/aws_s3_object_cluster-completed.spec_content

@@ -209,6 +209,8 @@ spec:
     zone: us-test-1a
   topology:
     bastion:
-      loadBalancer: {}
+      loadBalancer:
+        additionalSecurityGroups:
+        - sg-exampleid
     dns:
       type: Public

tests/integration/update_cluster/bastionadditional_user-data/kubernetes.tf

@@ -773,7 +773,7 @@ resource "aws_lb" "bastion-bastionuserdata-example-com" {
   internal                         = false
   load_balancer_type               = "network"
   name                             = "bastion-bastionuserdata-e-4grhsv"
-  security_groups                  = [aws_security_group.bastion-elb-bastionuserdata-example-com.id]
+  security_groups                  = ["sg-exampleid", aws_security_group.bastion-elb-bastionuserdata-example-com.id]
   subnet_mapping {
     subnet_id = aws_subnet.utility-us-test-1a-bastionuserdata-example-com.id
   }