Merge pull request #4866 from r4f4/sg-second-lb

🐛 fix: create ingress rules from all load balancers

Author: k8s-ci-robot

pkg/cloud/scope/managedcontrolplane.go

@@ -430,6 +430,11 @@ func (s *ManagedControlPlaneScope) ControlPlaneLoadBalancer() *infrav1.AWSLoadBa
 	return nil
 }
 
+// ControlPlaneLoadBalancers returns the AWSLoadBalancerSpecs.
+func (s *ManagedControlPlaneScope) ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec {
+	return nil
+}
+
 // Partition returns the cluster partition.
 func (s *ManagedControlPlaneScope) Partition() string {
 	if s.ControlPlane.Spec.Partition == "" {

pkg/cloud/scope/sg.go

@@ -44,6 +44,7 @@ type SGScope interface {
 	Bastion() *infrav1.Bastion
 
 	// ControlPlaneLoadBalancer returns the load balancer settings that are requested.
+	// Deprecated: Use ControlPlaneLoadBalancers()
 	ControlPlaneLoadBalancer() *infrav1.AWSLoadBalancerSpec
 
 	// SetNatGatewaysIPs sets the Nat Gateways Public IPs.
@@ -54,4 +55,8 @@ type SGScope interface {
 
 	// AdditionalControlPlaneIngressRules returns the additional ingress rules for the control plane security group.
 	AdditionalControlPlaneIngressRules() []infrav1.IngressRule
+
+	// ControlPlaneLoadBalancers returns both the ControlPlaneLoadBalancer and SecondaryControlPlaneLoadBalancer AWSLoadBalancerSpecs.
+	// The control plane load balancers should always be returned in the above order.
+	ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec
 }

pkg/cloud/services/securitygroup/securitygroups.go

@@ -660,10 +660,15 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
 		rulesToApply := customIngressRules.Difference(kubeletRules)
 		return append(kubeletRules, rulesToApply...), nil
 	case infrav1.SecurityGroupLB:
+		rules := infrav1.IngressRules{}
+		allowedNLBTraffic := false
 		// We hand this group off to the in-cluster cloud provider, so these rules aren't used
 		// Except if the load balancer type is NLB, and we have an AWS Cluster in which case we
 		// need to open port 6443 to the NLB traffic and health check inside the VPC.
-		if s.scope.ControlPlaneLoadBalancer() != nil && s.scope.ControlPlaneLoadBalancer().LoadBalancerType == infrav1.LoadBalancerTypeNLB {
+		for _, lb := range s.scope.ControlPlaneLoadBalancers() {
+			if lb == nil || lb.LoadBalancerType != infrav1.LoadBalancerTypeNLB {
+				continue
+			}
 			var (
 				ipv4CidrBlocks []string
 				ipv6CidrBlocks []string
@@ -673,25 +678,26 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
 			if s.scope.VPC().IsIPv6Enabled() {
 				ipv6CidrBlocks = []string{s.scope.VPC().IPv6.CidrBlock}
 			}
-			if s.scope.ControlPlaneLoadBalancer().PreserveClientIP {
+			if lb.PreserveClientIP {
 				ipv4CidrBlocks = []string{services.AnyIPv4CidrBlock}
 				if s.scope.VPC().IsIPv6Enabled() {
 					ipv6CidrBlocks = []string{services.AnyIPv6CidrBlock}
 				}
 			}
 
-			rules := infrav1.IngressRules{
-				{
+			if !allowedNLBTraffic {
+				rules = append(rules, infrav1.IngressRule{
 					Description:    "Allow NLB traffic to the control plane instances.",
 					Protocol:       infrav1.SecurityGroupProtocolTCP,
 					FromPort:       int64(s.scope.APIServerPort()),
 					ToPort:         int64(s.scope.APIServerPort()),
 					CidrBlocks:     ipv4CidrBlocks,
 					IPv6CidrBlocks: ipv6CidrBlocks,
-				},
+				})
+				allowedNLBTraffic = true
 			}
 
-			for _, ln := range s.scope.ControlPlaneLoadBalancer().AdditionalListeners {
+			for _, ln := range lb.AdditionalListeners {
 				rules = append(rules, infrav1.IngressRule{
 					Description:    fmt.Sprintf("Allow NLB traffic to the control plane instances on port %d.", ln.Port),
 					Protocol:       infrav1.SecurityGroupProtocolTCP,
@@ -701,10 +707,8 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
 					IPv6CidrBlocks: ipv6CidrBlocks,
 				})
 			}
-
-			return rules, nil
 		}
-		return infrav1.IngressRules{}, nil
+		return rules, nil
 	}
 
 	return nil, errors.Errorf("Cannot determine ingress rules for unknown security group role %q", role)
@@ -915,8 +919,14 @@ func (s *Service) getIngressRulesToAllowKubeletToAccessTheControlPlaneLB() infra
 // getControlPlaneLBIngressRules returns the ingress rules for the control plane LB.
 // We allow all traffic when no other rules are defined.
 func (s *Service) getControlPlaneLBIngressRules() infrav1.IngressRules {
-	if s.scope.ControlPlaneLoadBalancer() != nil && len(s.scope.ControlPlaneLoadBalancer().IngressRules) > 0 {
-		return s.scope.ControlPlaneLoadBalancer().IngressRules
+	ingressRules := infrav1.IngressRules{}
+	for _, lb := range s.scope.ControlPlaneLoadBalancers() {
+		if lb != nil && len(lb.IngressRules) > 0 {
+			ingressRules = append(ingressRules, lb.IngressRules...)
+		}
+	}
+	if len(ingressRules) > 0 {
+		return ingressRules
 	}
 
 	// If no custom ingress rules have been defined we allow all traffic so that the MC can access the WC API

pkg/cloud/services/securitygroup/securitygroups_test.go

@@ -1317,6 +1317,64 @@ func TestControlPlaneLoadBalancerIngressRules(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "defined rules are used when using internal and external LB",
+			awsCluster: &infrav1.AWSCluster{
+				Spec: infrav1.AWSClusterSpec{
+					ControlPlaneLoadBalancer: &infrav1.AWSLoadBalancerSpec{
+						IngressRules: []infrav1.IngressRule{
+							{
+								Description: "My custom ingress rule",
+								Protocol:    infrav1.SecurityGroupProtocolTCP,
+								FromPort:    1234,
+								ToPort:      1234,
+								CidrBlocks:  []string{"172.126.1.1/0"},
+							},
+						},
+						Scheme: &infrav1.ELBSchemeInternal,
+					},
+					SecondaryControlPlaneLoadBalancer: &infrav1.AWSLoadBalancerSpec{
+						IngressRules: []infrav1.IngressRule{
+							{
+								Description: "Another custom ingress rule",
+								Protocol:    infrav1.SecurityGroupProtocolTCP,
+								FromPort:    2345,
+								ToPort:      2345,
+								CidrBlocks:  []string{"0.0.0.0/0"},
+							},
+						},
+					},
+					NetworkSpec: infrav1.NetworkSpec{
+						VPC: infrav1.VPCSpec{
+							CidrBlock: "10.0.0.0/16",
+						},
+					},
+				},
+			},
+			expectedIngresRules: infrav1.IngressRules{
+				infrav1.IngressRule{
+					Description: "Kubernetes API",
+					Protocol:    infrav1.SecurityGroupProtocolTCP,
+					FromPort:    6443,
+					ToPort:      6443,
+					CidrBlocks:  []string{"10.0.0.0/16"},
+				},
+				infrav1.IngressRule{
+					Description: "My custom ingress rule",
+					Protocol:    infrav1.SecurityGroupProtocolTCP,
+					FromPort:    1234,
+					ToPort:      1234,
+					CidrBlocks:  []string{"172.126.1.1/0"},
+				},
+				infrav1.IngressRule{
+					Description: "Another custom ingress rule",
+					Protocol:    infrav1.SecurityGroupProtocolTCP,
+					FromPort:    2345,
+					ToPort:      2345,
+					CidrBlocks:  []string{"0.0.0.0/0"},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {