Skip to content

Commit 4fbaf47

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2861 from sedefsavas/v1alpha4-internet-facing-fix
[Backport-v1alpha4] Correct the casing of internet-facing ELB scheme
2 parents 0fcf9d6 + fc97f91 commit 4fbaf47

File tree

9 files changed

+107
-19
lines changed

9 files changed

+107
-19
lines changed

‎api/v1alpha3/awscluster_types.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ type Bastion struct {
146146

147147
// AWSLoadBalancerSpec defines the desired state of an AWS load balancer.
148148
type AWSLoadBalancerSpec struct {
149-
// Scheme sets the scheme of the load balancer (defaults to Internet-facing)
150-
// +kubebuilder:default=Internet-facing
151-
// +kubebuilder:validation:Enum=Internet-facing;internal
149+
// Scheme sets the scheme of the load balancer (defaults to internet-facing)
150+
// +kubebuilder:default=internet-facing
151+
// +kubebuilder:validation:Enum=internet-facing;Internet-facing;internal
152152
// +optional
153153
Scheme *ClassicELBScheme `json:"scheme,omitempty"`
154154

‎api/v1alpha3/types.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,17 @@ type ClassicELBScheme string
8989
var (
9090
// ClassicELBSchemeInternetFacing defines an internet-facing, publicly
9191
// accessible AWS Classic ELB scheme.
92-
ClassicELBSchemeInternetFacing = ClassicELBScheme("Internet-facing")
92+
ClassicELBSchemeInternetFacing = ClassicELBScheme("internet-facing")
9393

9494
// ClassicELBSchemeInternal defines an internal-only facing
9595
// load balancer internal to an ELB.
9696
ClassicELBSchemeInternal = ClassicELBScheme("internal")
9797
)
9898

99+
func (e ClassicELBScheme) String() string {
100+
return string(e)
101+
}
102+
99103
// ClassicELBProtocol defines listener protocols for a classic load balancer.
100104
type ClassicELBProtocol string
101105

‎api/v1alpha4/awscluster_types.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ type Bastion struct {
146146

147147
// AWSLoadBalancerSpec defines the desired state of an AWS load balancer.
148148
type AWSLoadBalancerSpec struct {
149-
// Scheme sets the scheme of the load balancer (defaults to Internet-facing)
150-
// +kubebuilder:default=Internet-facing
151-
// +kubebuilder:validation:Enum=Internet-facing;internal
149+
// Scheme sets the scheme of the load balancer (defaults to internet-facing)
150+
// +kubebuilder:default=internet-facing
151+
// +kubebuilder:validation:Enum=internet-facing;Internet-facing;internal
152152
// +optional
153153
Scheme *ClassicELBScheme `json:"scheme,omitempty"`
154154

‎api/v1alpha4/awscluster_webhook.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func (r *AWSCluster) ValidateUpdate(old runtime.Object) error {
8484
}
8585

8686
if oldC.Spec.ControlPlaneLoadBalancer == nil {
87-
// If old scheme was nil, the only value accepted here is the default value: Internet-facing
87+
// If old scheme was nil, the only value accepted here is the default value: internet-facing
8888
if newLoadBalancer.Scheme != nil && newLoadBalancer.Scheme.String() != ClassicELBSchemeInternetFacing.String() {
8989
allErrs = append(allErrs,
9090
field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"),
@@ -95,10 +95,14 @@ func (r *AWSCluster) ValidateUpdate(old runtime.Object) error {
9595
// If old scheme was not nil, the new scheme should be the same.
9696
existingLoadBalancer := oldC.Spec.ControlPlaneLoadBalancer.DeepCopy()
9797
if !reflect.DeepEqual(existingLoadBalancer.Scheme, newLoadBalancer.Scheme) {
98-
allErrs = append(allErrs,
99-
field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"),
100-
r.Spec.ControlPlaneLoadBalancer.Scheme, "field is immutable"),
101-
)
98+
// Only allow changes from Internet-facing scheme to internet-facing.
99+
if newLoadBalancer.Scheme == nil || !(existingLoadBalancer.Scheme.String() == ClassicELBSchemeIncorrectInternetFacing.String() &&
100+
newLoadBalancer.Scheme.String() == ClassicELBSchemeInternetFacing.String()) {
101+
allErrs = append(allErrs,
102+
field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "scheme"),
103+
r.Spec.ControlPlaneLoadBalancer.Scheme, "field is immutable"),
104+
)
105+
}
102106
}
103107
}
104108

@@ -155,6 +159,13 @@ func SetDefaultsAWSClusterSpec(s *AWSClusterSpec) {
155159
SetDefaults_Bastion(&s.Bastion)
156160
SetDefaults_NetworkSpec(&s.NetworkSpec)
157161

162+
// 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.
163+
if s.ControlPlaneLoadBalancer == nil {
164+
s.ControlPlaneLoadBalancer = &AWSLoadBalancerSpec{Scheme: &ClassicELBSchemeInternetFacing}
165+
} else if s.ControlPlaneLoadBalancer.Scheme != nil && s.ControlPlaneLoadBalancer.Scheme.String() == ClassicELBSchemeIncorrectInternetFacing.String() {
166+
s.ControlPlaneLoadBalancer.Scheme = &ClassicELBSchemeInternetFacing
167+
}
168+
158169
if s.IdentityRef == nil {
159170
s.IdentityRef = &AWSIdentityReference{
160171
Kind: ControllerIdentityKind,

‎api/v1alpha4/awscluster_webhook_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ package v1alpha4
1919
import (
2020
"context"
2121
"testing"
22+
"time"
2223

2324
. "github.com/onsi/gomega"
2425

2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
2728
utildefaulting "sigs.k8s.io/cluster-api/util/defaulting"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
2830
)
2931

3032
func TestAWSClusterDefault(t *testing.T) {
@@ -36,12 +38,51 @@ func TestAWSClusterDefault(t *testing.T) {
3638
}
3739

3840
func TestAWSCluster_ValidateCreate(t *testing.T) {
41+
unsupportedIncorrectScheme := ClassicELBScheme("any-other-scheme")
42+
3943
tests := []struct {
4044
name string
4145
cluster *AWSCluster
4246
wantErr bool
47+
expect func(t *testing.T, res *AWSLoadBalancerSpec)
4348
}{
4449
// The SSHKeyName tests were moved to sshkeyname_test.go
50+
51+
{
52+
name: "Default nil scheme to `internet-facing`",
53+
cluster: &AWSCluster{
54+
Spec: AWSClusterSpec{},
55+
},
56+
expect: func(t *testing.T, res *AWSLoadBalancerSpec) {
57+
if res.Scheme.String() != ClassicELBSchemeInternetFacing.String() {
58+
t.Error("Expected internet-facing defaulting for nil loadbalancer schemes")
59+
}
60+
},
61+
wantErr: false,
62+
},
63+
{
64+
name: "Internet-facing ELB scheme is defaulted to internet-facing during creation",
65+
cluster: &AWSCluster{
66+
Spec: AWSClusterSpec{
67+
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{Scheme: &ClassicELBSchemeIncorrectInternetFacing},
68+
},
69+
},
70+
expect: func(t *testing.T, res *AWSLoadBalancerSpec) {
71+
if res.Scheme.String() != ClassicELBSchemeInternetFacing.String() {
72+
t.Error("Expected internet-facing defaulting for supported incorrect scheme: Internet-facing")
73+
}
74+
},
75+
wantErr: false,
76+
},
77+
{
78+
name: "Supported schemes are 'internet-facing, Internet-facing, internal, or nil', rest will be rejected",
79+
cluster: &AWSCluster{
80+
Spec: AWSClusterSpec{
81+
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{Scheme: &unsupportedIncorrectScheme},
82+
},
83+
},
84+
wantErr: true,
85+
},
4586
}
4687
for _, tt := range tests {
4788
t.Run(tt.name, func(t *testing.T) {
@@ -54,6 +95,24 @@ func TestAWSCluster_ValidateCreate(t *testing.T) {
5495
if err := testEnv.Create(ctx, cluster); (err != nil) != tt.wantErr {
5596
t.Errorf("ValidateCreate() error = %v, wantErr %v", err, tt.wantErr)
5697
}
98+
99+
if tt.wantErr {
100+
return
101+
}
102+
103+
c := &AWSCluster{}
104+
key := client.ObjectKey{
105+
Name: cluster.Name,
106+
Namespace: "default",
107+
}
108+
109+
g := NewWithT(t)
110+
g.Eventually(func() bool {
111+
err := testEnv.Get(ctx, key, c)
112+
return err == nil
113+
}, 10*time.Second).Should(Equal(true))
114+
115+
tt.expect(t, c.Spec.ControlPlaneLoadBalancer)
57116
})
58117
}
59118
}

‎api/v1alpha4/types.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,14 @@ type ClassicELBScheme string
103103
var (
104104
// ClassicELBSchemeInternetFacing defines an internet-facing, publicly
105105
// accessible AWS Classic ELB scheme.
106-
ClassicELBSchemeInternetFacing = ClassicELBScheme("Internet-facing")
106+
ClassicELBSchemeInternetFacing = ClassicELBScheme("internet-facing")
107107

108108
// ClassicELBSchemeInternal defines an internal-only facing
109109
// load balancer internal to an ELB.
110110
ClassicELBSchemeInternal = ClassicELBScheme("internal")
111+
112+
// ClassicELBSchemeIncorrectInternetFacing was inaccurately used to define an internet-facing LB in v0.6 releases > v0.6.6 and v0.7.0 release.
113+
ClassicELBSchemeIncorrectInternetFacing = ClassicELBScheme("Internet-facing")
111114
)
112115

113116
func (e ClassicELBScheme) String() string {

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,11 @@ spec:
138138
to false."
139139
type: boolean
140140
scheme:
141-
default: Internet-facing
141+
default: internet-facing
142142
description: Scheme sets the scheme of the load balancer (defaults
143-
to Internet-facing)
143+
to internet-facing)
144144
enum:
145+
- internet-facing
145146
- Internet-facing
146147
- internal
147148
type: string
@@ -886,10 +887,11 @@ spec:
886887
to false."
887888
type: boolean
888889
scheme:
889-
default: Internet-facing
890+
default: internet-facing
890891
description: Scheme sets the scheme of the load balancer (defaults
891-
to Internet-facing)
892+
to internet-facing)
892893
enum:
894+
- internet-facing
893895
- Internet-facing
894896
- internal
895897
type: string

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,11 @@ spec:
126126
\n Defaults to false."
127127
type: boolean
128128
scheme:
129-
default: Internet-facing
129+
default: internet-facing
130130
description: Scheme sets the scheme of the load balancer
131-
(defaults to Internet-facing)
131+
(defaults to internet-facing)
132132
enum:
133+
- internet-facing
133134
- Internet-facing
134135
- internal
135136
type: string

‎pkg/cloud/services/elb/loadbalancer.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,14 @@ func (s *Service) getAPIServerClassicELBSpec() (*infrav1.ClassicELB, error) {
316316
}
317317
securityGroupIDs = append(securityGroupIDs, s.scope.SecurityGroups()[infrav1.SecurityGroupAPIServerLB].ID)
318318

319+
// 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.
320+
if s.scope.ControlPlaneLoadBalancerScheme().String() == infrav1.ClassicELBSchemeIncorrectInternetFacing.String() {
321+
s.scope.ControlPlaneLoadBalancer().Scheme = &infrav1.ClassicELBSchemeInternetFacing
322+
if err := s.scope.PatchObject(); err != nil {
323+
return nil, err
324+
}
325+
}
326+
319327
res := &infrav1.ClassicELB{
320328
Name: elbName,
321329
Scheme: s.scope.ControlPlaneLoadBalancerScheme(),

0 commit comments

Comments
 (0)