Correct the casing of internet-facing ELB scheme

kubernetes-sigs · Oct 21, 2021 · 65f1525 · 65f1525
1 parent 6e79eba
commit 65f1525
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 15 deletions.
diff --git a/api/v1alpha3/awscluster_types.go b/api/v1alpha3/awscluster_types.go
@@ -144,9 +144,9 @@ type Bastion struct {
 
 // AWSLoadBalancerSpec defines the desired state of an AWS load balancer
 type AWSLoadBalancerSpec struct {
-	// Scheme sets the scheme of the load balancer (defaults to Internet-facing)
-	// +kubebuilder:default=Internet-facing
-	// +kubebuilder:validation:Enum=Internet-facing;internal
+	// Scheme sets the scheme of the load balancer (defaults to internet-facing)
+	// +kubebuilder:default=internet-facing
+	// +kubebuilder:validation:Enum=internet-facing;Internet-facing;internal
 	// +optional
 	Scheme *ClassicELBScheme `json:"scheme,omitempty"`
 

diff --git a/api/v1alpha3/awscluster_webhook.go b/api/v1alpha3/awscluster_webhook.go
@@ -73,20 +73,34 @@ func (r *AWSCluster) ValidateUpdate(old runtime.Object) error {
 		)
 	}
 
-	existingLoadBalancer := &AWSLoadBalancerSpec{}
 	newLoadBalancer := &AWSLoadBalancerSpec{}
 
-	if oldC.Spec.ControlPlaneLoadBalancer != nil {
-		existingLoadBalancer = oldC.Spec.ControlPlaneLoadBalancer.DeepCopy()
-	}
 	if r.Spec.ControlPlaneLoadBalancer != nil {
 		newLoadBalancer = r.Spec.ControlPlaneLoadBalancer.DeepCopy()
 	}
-	if !reflect.DeepEqual(existingLoadBalancer.Scheme, newLoadBalancer.Scheme) {
-		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"),
-				r.Spec.ControlPlaneLoadBalancer.Scheme, "field is immutable"),
-		)
+
+	if oldC.Spec.ControlPlaneLoadBalancer == nil {
+		// If old scheme was nil, the only value accepted here is the default value: internet-facing
+		if newLoadBalancer.Scheme != nil && newLoadBalancer.Scheme.String() != ClassicELBSchemeInternetFacing.String() {
+			allErrs = append(allErrs,
+				field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"),
+					r.Spec.ControlPlaneLoadBalancer.Scheme, "field is immutable, default value was set to internet-facing"),
+			)
+		}
+	}
+	if oldC.Spec.ControlPlaneLoadBalancer != nil {
+		existingLoadBalancer := oldC.Spec.ControlPlaneLoadBalancer.DeepCopy()
+
+		if !reflect.DeepEqual(existingLoadBalancer.Scheme, newLoadBalancer.Scheme) {
+			// Only allow changes from Internet-facing scheme to internet-facing.
+			if newLoadBalancer.Scheme == nil || !(existingLoadBalancer.Scheme.String() == ClassicELBSchemeIncorrectInternetFacing.String() &&
+				newLoadBalancer.Scheme.String() == ClassicELBSchemeInternetFacing.String()) {
+				allErrs = append(allErrs,
+					field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"),
+						r.Spec.ControlPlaneLoadBalancer.Scheme, "field is immutable"),
+				)
+			}
+		}
 	}
 
 	if !reflect.DeepEqual(oldC.Spec.ControlPlaneEndpoint, clusterv1.APIEndpoint{}) &&
@@ -126,6 +140,14 @@ func (r *AWSCluster) Default() {
 	SetDefaults_Bastion(&r.Spec.Bastion)
 	SetDefaults_NetworkSpec(&r.Spec.NetworkSpec)
 
+	if r.Spec.ControlPlaneLoadBalancer == nil {
+		// Default nil scheme to internet-facing.
+		r.Spec.ControlPlaneLoadBalancer = &AWSLoadBalancerSpec{Scheme: &ClassicELBSchemeInternetFacing}
+	} else if r.Spec.ControlPlaneLoadBalancer.Scheme != nil && r.Spec.ControlPlaneLoadBalancer.Scheme.String() == ClassicELBSchemeIncorrectInternetFacing.String() {
+		// 	If ELB scheme is set to Internet-facing due to an API bug in versions > v0.6.6, default it to internet-facing.
+		r.Spec.ControlPlaneLoadBalancer.Scheme = &ClassicELBSchemeInternetFacing
+	}
+
 	if r.Spec.IdentityRef == nil {
 		r.Spec.IdentityRef = &AWSIdentityReference{
 			Kind: ControllerIdentityKind,

diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go
@@ -89,13 +89,20 @@ type ClassicELBScheme string
 var (
 	// ClassicELBSchemeInternetFacing defines an internet-facing, publicly
 	// accessible AWS Classic ELB scheme
-	ClassicELBSchemeInternetFacing = ClassicELBScheme("Internet-facing")
+	ClassicELBSchemeInternetFacing = ClassicELBScheme("internet-facing")
 
 	// ClassicELBSchemeInternal defines an internal-only facing
 	// load balancer internal to an ELB.
 	ClassicELBSchemeInternal = ClassicELBScheme("internal")
+
+	// ClassicELBSchemeIncorrectInternetFacing was inaccurately used to define an internet-facing LB in v0.6 releases > v0.6.6 and v0.7.0 release.
+	ClassicELBSchemeIncorrectInternetFacing = ClassicELBScheme("Internet-facing")
 )
 
+func (e ClassicELBScheme) String() string {
+	return string(e)
+}
+
 // ClassicELBProtocol defines listener protocols for a classic load balancer.
 type ClassicELBProtocol string
 

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -539,10 +539,11 @@ spec:
                       to false."
                     type: boolean
                   scheme:
-                    default: Internet-facing
+                    default: internet-facing
                     description: Scheme sets the scheme of the load balancer (defaults
-                      to Internet-facing)
+                      to internet-facing)
                     enum:
+                    - internet-facing
                     - Internet-facing
                     - internal
                     type: string

diff --git a/pkg/cloud/services/elb/loadbalancer.go b/pkg/cloud/services/elb/loadbalancer.go
@@ -316,6 +316,14 @@ func (s *Service) getAPIServerClassicELBSpec() (*infrav1.ClassicELB, error) {
 	}
 	securityGroupIDs = append(securityGroupIDs, s.scope.SecurityGroups()[infrav1.SecurityGroupAPIServerLB].ID)
 
+	// If ELB scheme is set to Internet-facing due to an API bug in versions > v0.6.6, change it to internet-facing and patch.
+	if s.scope.ControlPlaneLoadBalancerScheme().String() == infrav1.ClassicELBSchemeIncorrectInternetFacing.String() {
+		s.scope.ControlPlaneLoadBalancer().Scheme = &infrav1.ClassicELBSchemeInternetFacing
+		if err := s.scope.PatchObject(); err != nil {
+			return nil, err
+		}
+	}
+
 	res := &infrav1.ClassicELB{
 		Name:   elbName,
 		Scheme: s.scope.ControlPlaneLoadBalancerScheme(),