Merge pull request #4438 from prometherion/issues/4437

✨ Supporting externally managed Control Plane
kubernetes-sigs · Feb 8, 2024 · 6193d17 · 6193d17
2 parents 6ffb575 + fc3cb07
commit 6193d17
Show file tree

Hide file tree

Showing 19 changed files with 448 additions and 47 deletions.
diff --git a/api/v1beta2/awscluster_types.go b/api/v1beta2/awscluster_types.go
@@ -167,10 +167,11 @@ type Bastion struct {
 type LoadBalancerType string
 
 var (
-	LoadBalancerTypeClassic = LoadBalancerType("classic")
-	LoadBalancerTypeELB     = LoadBalancerType("elb")
-	LoadBalancerTypeALB     = LoadBalancerType("alb")
-	LoadBalancerTypeNLB     = LoadBalancerType("nlb")
+	LoadBalancerTypeClassic  = LoadBalancerType("classic")
+	LoadBalancerTypeELB      = LoadBalancerType("elb")
+	LoadBalancerTypeALB      = LoadBalancerType("alb")
+	LoadBalancerTypeNLB      = LoadBalancerType("nlb")
+	LoadBalancerTypeDisabled = LoadBalancerType("disabled")
 )
 
 // AWSLoadBalancerSpec defines the desired state of an AWS load balancer.
@@ -229,7 +230,7 @@ type AWSLoadBalancerSpec struct {
 
 	// LoadBalancerType sets the type for a load balancer. The default type is classic.
 	// +kubebuilder:default=classic
-	// +kubebuilder:validation:Enum:=classic;elb;alb;nlb
+	// +kubebuilder:validation:Enum:=classic;elb;alb;nlb;disabled
 	LoadBalancerType LoadBalancerType `json:"loadBalancerType,omitempty"`
 
 	// DisableHostsRewrite disabled the hair pinning issue solution that adds the NLB's address as 127.0.0.1 to the hosts

diff --git a/api/v1beta2/awscluster_webhook.go b/api/v1beta2/awscluster_webhook.go
@@ -298,5 +298,49 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
 		}
 	}
 
+	if r.Spec.ControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeDisabled {
+		if r.Spec.ControlPlaneLoadBalancer.Name != nil {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "name"), r.Spec.ControlPlaneLoadBalancer.Name, "cannot configure a name if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if r.Spec.ControlPlaneLoadBalancer.CrossZoneLoadBalancing {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "crossZoneLoadBalancing"), r.Spec.ControlPlaneLoadBalancer.CrossZoneLoadBalancing, "cross-zone load balancing cannot be set if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if len(r.Spec.ControlPlaneLoadBalancer.Subnets) > 0 {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "subnets"), r.Spec.ControlPlaneLoadBalancer.Subnets, "subnets cannot be set if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol != nil {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "healthCheckProtocol"), r.Spec.ControlPlaneLoadBalancer.HealthCheckProtocol, "healthcheck protocol cannot be set if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if len(r.Spec.ControlPlaneLoadBalancer.AdditionalSecurityGroups) > 0 {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "additionalSecurityGroups"), r.Spec.ControlPlaneLoadBalancer.AdditionalSecurityGroups, "additional Security Groups cannot be set if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if len(r.Spec.ControlPlaneLoadBalancer.AdditionalListeners) > 0 {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "additionalListeners"), r.Spec.ControlPlaneLoadBalancer.AdditionalListeners, "cannot set additional listeners if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if len(r.Spec.ControlPlaneLoadBalancer.IngressRules) > 0 {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules, "ingress rules cannot be set if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if r.Spec.ControlPlaneLoadBalancer.PreserveClientIP {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "preserveClientIP"), r.Spec.ControlPlaneLoadBalancer.PreserveClientIP, "cannot preserve client IP if the LoadBalancer reconciliation is disabled"))
+		}
+
+		if r.Spec.ControlPlaneLoadBalancer.DisableHostsRewrite {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "disableHostsRewrite"), r.Spec.ControlPlaneLoadBalancer.DisableHostsRewrite, "cannot disable hosts rewrite if the LoadBalancer reconciliation is disabled"))
+		}
+	}
+
+	for _, rule := range r.Spec.ControlPlaneLoadBalancer.IngressRules {
+		if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) {
+			allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules, "CIDR blocks and security group IDs or security group roles cannot be used together"))
+		}
+	}
+
 	return allErrs
 }
diff --git a/api/v1beta2/awscluster_webhook_test.go b/api/v1beta2/awscluster_webhook_test.go
@@ -26,6 +26,7 @@ import (
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilfeature "k8s.io/component-base/featuregate/testing"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"sigs.k8s.io/cluster-api-provider-aws/v2/feature"
@@ -50,6 +51,126 @@ func TestAWSClusterValidateCreate(t *testing.T) {
 		wantErr bool
 		expect  func(g *WithT, res *AWSLoadBalancerSpec)
 	}{
+		{
+			name: "No options are allowed when LoadBalancer is disabled (name)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						LoadBalancerType: LoadBalancerTypeDisabled,
+						Name:             ptr.To("name"),
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (crossZoneLoadBalancing)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						CrossZoneLoadBalancing: true,
+						LoadBalancerType:       LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (subnets)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						Subnets:          []string{"foo", "bar"},
+						LoadBalancerType: LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (healthCheckProtocol)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						HealthCheckProtocol: &ELBProtocolTCP,
+						LoadBalancerType:    LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (additionalSecurityGroups)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						AdditionalSecurityGroups: []string{"foo", "bar"},
+						LoadBalancerType:         LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (additionalListeners)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						AdditionalListeners: []AdditionalListenerSpec{
+							{
+								Port:     6443,
+								Protocol: ELBProtocolTCP,
+							},
+						},
+						LoadBalancerType: LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (ingressRules)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						IngressRules: []IngressRule{
+							{
+								Description: "ingress rule",
+								Protocol:    SecurityGroupProtocolTCP,
+								FromPort:    6443,
+								ToPort:      6443,
+							},
+						},
+						LoadBalancerType: LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (disableHostsRewrite)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						DisableHostsRewrite: true,
+						LoadBalancerType:    LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "No options are allowed when LoadBalancer is disabled (preserveClientIP)",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
+						PreserveClientIP: true,
+						LoadBalancerType: LoadBalancerTypeDisabled,
+					},
+				},
+			},
+			wantErr: true,
+		},
 		// The SSHKeyName tests were moved to sshkeyname_test.go
 		{
 			name: "Supported schemes are 'internet-facing, Internet-facing, internal, or nil', rest will be rejected",

diff --git a/api/v1beta2/conditions_consts.go b/api/v1beta2/conditions_consts.go
@@ -125,6 +125,9 @@ const (
 	LoadBalancerReadyCondition clusterv1.ConditionType = "LoadBalancerReady"
 	// WaitForDNSNameReason used while waiting for a DNS name for the API server to be populated.
 	WaitForDNSNameReason = "WaitForDNSName"
+	// WaitForExternalControlPlaneEndpointReason is available when the AWS Cluster is waiting for an externally managed
+	// Load Balancer, such as an external Control Plane provider.
+	WaitForExternalControlPlaneEndpointReason = "WaitForExternalControlPlaneEndpoint"
 	// WaitForDNSNameResolveReason used while waiting for DNS name to resolve.
 	WaitForDNSNameResolveReason = "WaitForDNSNameResolve"
 	// LoadBalancerFailedReason used when an error occurs during load balancer reconciliation.

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -1109,6 +1109,7 @@ spec:
                     - elb
                     - alb
                     - nlb
+                    - disabled
                     type: string
                   name:
                     description: Name sets the name of the classic ELB load balancer.
@@ -1689,6 +1690,7 @@ spec:
                     - elb
                     - alb
                     - nlb
+                    - disabled
                     type: string
                   name:
                     description: Name sets the name of the classic ELB load balancer.

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml
@@ -707,6 +707,7 @@ spec:
                             - elb
                             - alb
                             - nlb
+                            - disabled
                             type: string
                           name:
                             description: Name sets the name of the classic ELB load
@@ -1316,6 +1317,7 @@ spec:
                             - elb
                             - alb
                             - nlb
+                            - disabled
                             type: string
                           name:
                             description: Name sets the name of the classic ELB load

diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -119,6 +119,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - controlplane.cluster.x-k8s.io
+  resources:
+  - '*'
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - controlplane.cluster.x-k8s.io
   resources:

diff --git a/controllers/awscluster_controller.go b/controllers/awscluster_controller.go
@@ -266,6 +266,45 @@ func (r *AWSClusterReconciler) reconcileDelete(ctx context.Context, clusterScope
 	return nil
 }
 
+func (r *AWSClusterReconciler) reconcileLoadBalancer(clusterScope *scope.ClusterScope, awsCluster *infrav1.AWSCluster) (*time.Duration, error) {
+	retryAfterDuration := 15 * time.Second
+	if clusterScope.AWSCluster.Spec.ControlPlaneLoadBalancer.LoadBalancerType == infrav1.LoadBalancerTypeDisabled {
+		clusterScope.Debug("load balancer reconciliation shifted to external provider, checking external endpoint")
+
+		return r.checkForExternalControlPlaneLoadBalancer(clusterScope, awsCluster), nil
+	}
+
+	elbService := r.getELBService(clusterScope)
+
+	if err := elbService.ReconcileLoadbalancers(); err != nil {
+		clusterScope.Error(err, "failed to reconcile load balancer")
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.LoadBalancerFailedReason, infrautilconditions.ErrorConditionAfterInit(clusterScope.ClusterObj()), err.Error())
+		return nil, err
+	}
+
+	if awsCluster.Status.Network.APIServerELB.DNSName == "" {
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameReason, clusterv1.ConditionSeverityInfo, "")
+		clusterScope.Info("Waiting on API server ELB DNS name")
+		return &retryAfterDuration, nil
+	}
+
+	clusterScope.Debug("Looking up IP address for DNS", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
+	if _, err := net.LookupIP(awsCluster.Status.Network.APIServerELB.DNSName); err != nil {
+		clusterScope.Error(err, "failed to get IP address for dns name", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameResolveReason, clusterv1.ConditionSeverityInfo, "")
+		clusterScope.Info("Waiting on API server ELB DNS name to resolve")
+		return &retryAfterDuration, nil
+	}
+	conditions.MarkTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
+
+	awsCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
+		Host: awsCluster.Status.Network.APIServerELB.DNSName,
+		Port: clusterScope.APIServerPort(),
+	}
+
+	return nil, nil
+}
+
 func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope) (reconcile.Result, error) {
 	clusterScope.Info("Reconciling AWSCluster")
 
@@ -280,7 +319,6 @@ func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope)
 	}
 
 	ec2Service := r.getEC2Service(clusterScope)
-	elbService := r.getELBService(clusterScope)
 	networkSvc := r.getNetworkService(*clusterScope)
 	sgService := r.getSecurityGroupService(*clusterScope)
 	s3Service := s3.NewService(clusterScope)
@@ -310,37 +348,17 @@ func (r *AWSClusterReconciler) reconcileNormal(clusterScope *scope.ClusterScope)
 		}
 	}
 
-	if err := elbService.ReconcileLoadbalancers(); err != nil {
-		clusterScope.Error(err, "failed to reconcile load balancer")
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.LoadBalancerFailedReason, infrautilconditions.ErrorConditionAfterInit(clusterScope.ClusterObj()), err.Error())
+	if requeueAfter, err := r.reconcileLoadBalancer(clusterScope, awsCluster); err != nil {
 		return reconcile.Result{}, err
+	} else if requeueAfter != nil {
+		return reconcile.Result{RequeueAfter: *requeueAfter}, err
 	}
 
 	if err := s3Service.ReconcileBucket(); err != nil {
 		conditions.MarkFalse(awsCluster, infrav1.S3BucketReadyCondition, infrav1.S3BucketFailedReason, clusterv1.ConditionSeverityError, err.Error())
 		return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile S3 Bucket for AWSCluster %s/%s", awsCluster.Namespace, awsCluster.Name)
 	}
 
-	if awsCluster.Status.Network.APIServerELB.DNSName == "" {
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameReason, clusterv1.ConditionSeverityInfo, "")
-		clusterScope.Info("Waiting on API server ELB DNS name")
-		return reconcile.Result{RequeueAfter: 15 * time.Second}, nil
-	}
-
-	clusterScope.Debug("looking up IP address for DNS", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
-	if _, err := net.LookupIP(awsCluster.Status.Network.APIServerELB.DNSName); err != nil {
-		clusterScope.Error(err, "failed to get IP address for dns name", "dns", awsCluster.Status.Network.APIServerELB.DNSName)
-		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForDNSNameResolveReason, clusterv1.ConditionSeverityInfo, "")
-		clusterScope.Info("Waiting on API server ELB DNS name to resolve")
-		return reconcile.Result{RequeueAfter: 15 * time.Second}, nil
-	}
-	conditions.MarkTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
-
-	awsCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
-		Host: awsCluster.Status.Network.APIServerELB.DNSName,
-		Port: clusterScope.APIServerPort(),
-	}
-
 	for _, subnet := range clusterScope.Subnets().FilterPrivate() {
 		found := false
 		for _, az := range awsCluster.Status.Network.APIServerELB.AvailabilityZones {
@@ -447,3 +465,29 @@ func (r *AWSClusterReconciler) requeueAWSClusterForUnpausedCluster(_ context.Con
 		}
 	}
 }
+
+func (r *AWSClusterReconciler) checkForExternalControlPlaneLoadBalancer(clusterScope *scope.ClusterScope, awsCluster *infrav1.AWSCluster) *time.Duration {
+	requeueAfterPeriod := 15 * time.Second
+
+	switch {
+	case len(awsCluster.Spec.ControlPlaneEndpoint.Host) == 0 && awsCluster.Spec.ControlPlaneEndpoint.Port == 0:
+		clusterScope.Info("AWSCluster control plane endpoint is still non-populated")
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForExternalControlPlaneEndpointReason, clusterv1.ConditionSeverityInfo, "")
+
+		return &requeueAfterPeriod
+	case len(awsCluster.Spec.ControlPlaneEndpoint.Host) == 0:
+		clusterScope.Info("AWSCluster control plane endpoint host is still non-populated")
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForExternalControlPlaneEndpointReason, clusterv1.ConditionSeverityInfo, "")
+
+		return &requeueAfterPeriod
+	case awsCluster.Spec.ControlPlaneEndpoint.Port == 0:
+		clusterScope.Info("AWSCluster control plane endpoint port is still non-populated")
+		conditions.MarkFalse(awsCluster, infrav1.LoadBalancerReadyCondition, infrav1.WaitForExternalControlPlaneEndpointReason, clusterv1.ConditionSeverityInfo, "")
+
+		return &requeueAfterPeriod
+	default:
+		conditions.MarkTrue(awsCluster, infrav1.LoadBalancerReadyCondition)
+
+		return nil
+	}
+}