Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Making ControlPlane LoadBalancer immutable for disabled type #4828

Merged
merged 1 commit into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions api/v1beta2/awscluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,16 @@ func (r *AWSCluster) validateControlPlaneLoadBalancerUpdate(oldlb, newlb *AWSLoa
)
}
} else {
// A disabled Load Balancer has many implications that must be treated as immutable/
// this is mostly used by externally managed Control Plane, and there's no need to support type changes.
// More info: https://kubernetes.slack.com/archives/CD6U2V71N/p1708983246100859?thread_ts=1708973478.410979&cid=CD6U2V71N
if (oldlb.LoadBalancerType == LoadBalancerTypeDisabled && newlb.LoadBalancerType != LoadBalancerTypeDisabled) ||
(newlb.LoadBalancerType == LoadBalancerTypeDisabled && oldlb.LoadBalancerType != LoadBalancerTypeDisabled) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "type"),
newlb.Scheme, "field is immutable when created of disabled type"),
)
}
// If old scheme was not nil, the new scheme should be the same.
if !cmp.Equal(oldlb.Scheme, newlb.Scheme) {
allErrs = append(allErrs,
Expand Down
38 changes: 37 additions & 1 deletion api/v1beta2/awscluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,12 +607,48 @@ func TestAWSClusterValidateCreate(t *testing.T) {
}

func TestAWSClusterValidateUpdate(t *testing.T) {
tests := []struct {
var tests = []struct {
name string
oldCluster *AWSCluster
newCluster *AWSCluster
wantErr bool
}{
{
name: "Control Plane LB type is immutable when switching from disabled to any",
oldCluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
LoadBalancerType: LoadBalancerTypeDisabled,
},
},
},
newCluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
LoadBalancerType: LoadBalancerTypeClassic,
},
},
},
wantErr: true,
},
{
name: "Control Plane LB type is immutable when switching from any to disabled",
oldCluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
LoadBalancerType: LoadBalancerTypeClassic,
},
},
},
newCluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
LoadBalancerType: LoadBalancerTypeDisabled,
},
},
},
wantErr: true,
},
{
name: "region is immutable",
oldCluster: &AWSCluster{
Expand Down