Merge pull request #2832 from sedefsavas/internetfacing-beta1

Fix for internet-facing ELB scheme

api/v1alpha3/awscluster_types.go

@@ -148,7 +148,7 @@ type Bastion struct {
 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
+	// +kubebuilder:validation:Enum=internet-facing;Internet-facing;internal
 	// +optional
 	Scheme *ClassicELBScheme `json:"scheme,omitempty"`
 

api/v1alpha3/types.go

@@ -96,6 +96,10 @@ var (
 	ClassicELBSchemeInternal = ClassicELBScheme("internal")
 )
 
+func (e ClassicELBScheme) String() string {
+	return string(e)
+}
+
 // ClassicELBProtocol defines listener protocols for a classic load balancer.
 type ClassicELBProtocol string
 

api/v1alpha4/awscluster_types.go

@@ -148,7 +148,7 @@ type Bastion struct {
 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
+	// +kubebuilder:validation:Enum=internet-facing;Internet-facing;internal
 	// +optional
 	Scheme *ClassicELBScheme `json:"scheme,omitempty"`
 

api/v1beta1/awscluster_webhook.go

@@ -95,10 +95,14 @@ func (r *AWSCluster) ValidateUpdate(old runtime.Object) error {
 		// If old scheme was not nil, the new scheme should be the same.
 		existingLoadBalancer := oldC.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"),
-			)
+			// Only allow changes from Internet-facing scheme to internet-facing.
+			if !(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"),
+				)
+			}
 		}
 	}
 

api/v1beta1/awscluster_webhook_test.go

@@ -19,12 +19,14 @@ package v1beta1
 import (
 	"context"
 	"testing"
+	"time"
 
 	. "github.com/onsi/gomega"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func TestAWSClusterDefault(t *testing.T) {
@@ -36,12 +38,50 @@ func TestAWSClusterDefault(t *testing.T) {
 }
 
 func TestAWSCluster_ValidateCreate(t *testing.T) {
+	unsupportedIncorrectScheme := ClassicELBScheme("any-other-scheme")
+
 	tests := []struct {
 		name    string
 		cluster *AWSCluster
 		wantErr bool
+		expect  func(t *testing.T, res *AWSLoadBalancerSpec)
 	}{
 		// The SSHKeyName tests were moved to sshkeyname_test.go
+		{
+			name: "Default nil scheme to `internet-facing`",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{},
+			},
+			expect: func(t *testing.T, res *AWSLoadBalancerSpec) {
+				if res.Scheme.String() != ClassicELBSchemeInternetFacing.String() {
+					t.Error("Expected internet-facing defaulting for nil loadbalancer schemes")
+				}
+			},
+			wantErr: false,
+		},
+		{
+			name: "Internet-facing ELB scheme is defaulted to internet-facing during creation",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{Scheme: &ClassicELBSchemeIncorrectInternetFacing},
+				},
+			},
+			expect: func(t *testing.T, res *AWSLoadBalancerSpec) {
+				if res.Scheme.String() != ClassicELBSchemeInternetFacing.String() {
+					t.Error("Expected internet-facing defaulting for supported incorrect scheme: Internet-facing")
+				}
+			},
+			wantErr: false,
+		},
+		{
+			name: "Supported schemes are 'internet-facing, Internet-facing, internal, or nil', rest will be rejected",
+			cluster: &AWSCluster{
+				Spec: AWSClusterSpec{
+					ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{Scheme: &unsupportedIncorrectScheme},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -54,6 +94,24 @@ func TestAWSCluster_ValidateCreate(t *testing.T) {
 			if err := testEnv.Create(ctx, cluster); (err != nil) != tt.wantErr {
 				t.Errorf("ValidateCreate() error = %v, wantErr %v", err, tt.wantErr)
 			}
+
+			if tt.wantErr {
+				return
+			}
+
+			c := &AWSCluster{}
+			key := client.ObjectKey{
+				Name:      cluster.Name,
+				Namespace: "default",
+			}
+
+			g := NewWithT(t)
+			g.Eventually(func() bool {
+				err := testEnv.Get(ctx, key, c)
+				return err == nil
+			}, 10*time.Second).Should(Equal(true))
+
+			tt.expect(t, c.Spec.ControlPlaneLoadBalancer)
 		})
 	}
 }

api/v1beta1/defaults.go

@@ -59,6 +59,13 @@ func SetDefaults_AWSClusterSpec(s *AWSClusterSpec) { //nolint:golint,stylecheck
 			Name: AWSClusterControllerIdentityName,
 		}
 	}
+
+	// 	If ELB scheme is set to Internet-facing due to an API bug in versions > v0.6.6 and v0.7.0, default it to internet-facing.
+	if s.ControlPlaneLoadBalancer == nil {
+		s.ControlPlaneLoadBalancer = &AWSLoadBalancerSpec{Scheme: &ClassicELBSchemeInternetFacing}
+	} else if s.ControlPlaneLoadBalancer.Scheme != nil && s.ControlPlaneLoadBalancer.Scheme.String() == ClassicELBSchemeIncorrectInternetFacing.String() {
+		s.ControlPlaneLoadBalancer.Scheme = &ClassicELBSchemeInternetFacing
+	}
 }
 
 // SetDefaults_Labels is used to default cluster scope resources for clusterctl move.

api/v1beta1/network_types.go

@@ -42,6 +42,9 @@ var (
 	// 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 {

config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml

@@ -143,6 +143,7 @@ spec:
                       to internet-facing)
                     enum:
                     - internet-facing
+                    - Internet-facing
                     - internal
                     type: string
                   subnets:
@@ -891,6 +892,7 @@ spec:
                       to internet-facing)
                     enum:
                     - internet-facing
+                    - Internet-facing
                     - internal
                     type: string
                   subnets:

config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml

@@ -131,6 +131,7 @@ spec:
                               (defaults to internet-facing)
                             enum:
                             - internet-facing
+                            - Internet-facing
                             - internal
                             type: string
                           subnets:

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 and v0.7.0, 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(),