diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index 25c072c827c..61ae4e89083 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -47,6 +47,10 @@ const ( DefaultOutboundRuleIdleTimeoutInMinutes = 4 // DefaultAzureCloud is the public cloud that will be used by most users. DefaultAzureCloud = "AzurePublicCloud" + // DefaultLoadBalancingRuleName is the default load balancer rule name. + DefaultLoadBalancingRuleName = "LBRuleHTTPS" + // DefaultHealthProbeName is the default health probe name. + DefaultHealthProbeName = "HTTPSProbe" ) func (c *AzureCluster) setDefaults() { @@ -303,6 +307,13 @@ func (c *AzureCluster) setAPIServerLBDefaults() { } } c.SetAPIServerLBBackendPoolNameDefault() + + if lb.LoadBalancingRule.Name == "" { + lb.LoadBalancingRule.Name = DefaultLoadBalancingRuleName + } + if lb.HealthProbe.Name == "" { + lb.HealthProbe.Name = DefaultHealthProbeName + } } // SetNodeOutboundLBDefaults sets the default values for the NodeOutboundLB. diff --git a/api/v1beta1/azurecluster_default_test.go b/api/v1beta1/azurecluster_default_test.go index a1b92019427..25dcfba0940 100644 --- a/api/v1beta1/azurecluster_default_test.go +++ b/api/v1beta1/azurecluster_default_test.go @@ -138,9 +138,14 @@ func TestVnetDefaults(t *testing.T) { }, }, }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ - SKU: SKUStandard, - + SKU: SKUStandard, Type: Public, }, }, @@ -1321,6 +1326,12 @@ func TestAPIServerLBDefaults(t *testing.T) { BackendPool: BackendPool{ Name: "cluster-test-public-lb-backendPool", }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Public, @@ -1370,6 +1381,12 @@ func TestAPIServerLBDefaults(t *testing.T) { BackendPool: BackendPool{ Name: "cluster-test-public-lb-backendPool", }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Public, @@ -1414,6 +1431,12 @@ func TestAPIServerLBDefaults(t *testing.T) { BackendPool: BackendPool{ Name: "cluster-test-internal-lb-backendPool", }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Internal, @@ -1460,6 +1483,12 @@ func TestAPIServerLBDefaults(t *testing.T) { BackendPool: BackendPool{ Name: "cluster-test-internal-lb-backendPool", }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Internal, @@ -1508,6 +1537,12 @@ func TestAPIServerLBDefaults(t *testing.T) { BackendPool: BackendPool{ Name: "custom-backend-pool", }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Internal, @@ -1557,6 +1592,12 @@ func TestAPIServerLBDefaults(t *testing.T) { BackendPool: BackendPool{ Name: "custom-backend-pool", }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Internal, @@ -1630,6 +1671,132 @@ func TestAPIServerLBDefaults(t *testing.T) { BackendPool: BackendPool{ Name: "cluster-test-public-lb-backendPool", }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, + LoadBalancerClassSpec: LoadBalancerClassSpec{ + SKU: SKUStandard, + Type: Public, + IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes), + }, + }, + }, + }, + }, + }, + { + name: "load balancer with custom rule and probe names", + cluster: &AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + LoadBalancingRule: LoadBalancingRule{ + Name: "CustomLBRule", + }, + HealthProbe: HealthProbe{ + Name: "CustomProbe", + }, + LoadBalancerClassSpec: LoadBalancerClassSpec{ + Type: Public, + }, + }, + }, + }, + }, + output: &AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + Name: "cluster-test-public-lb", + FrontendIPs: []FrontendIP{ + { + Name: "cluster-test-public-lb-frontEnd", + PublicIP: &PublicIPSpec{ + Name: "pip-cluster-test-apiserver", + DNSName: "", + }, + }, + }, + BackendPool: BackendPool{ + Name: "cluster-test-public-lb-backendPool", + }, + LoadBalancingRule: LoadBalancingRule{ + Name: "CustomLBRule", + }, + HealthProbe: HealthProbe{ + Name: "CustomProbe", + }, + LoadBalancerClassSpec: LoadBalancerClassSpec{ + SKU: SKUStandard, + Type: Public, + IdleTimeoutInMinutes: ptr.To[int32](DefaultOutboundRuleIdleTimeoutInMinutes), + }, + }, + }, + }, + }, + }, + { + name: "load balancer with empty rule and probe names should use defaults", + cluster: &AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + LoadBalancingRule: LoadBalancingRule{ + Name: "", + }, + HealthProbe: HealthProbe{ + Name: "", + }, + LoadBalancerClassSpec: LoadBalancerClassSpec{ + Type: Public, + }, + }, + }, + }, + }, + output: &AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: AzureClusterSpec{ + ControlPlaneEnabled: true, + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + Name: "cluster-test-public-lb", + FrontendIPs: []FrontendIP{ + { + Name: "cluster-test-public-lb-frontEnd", + PublicIP: &PublicIPSpec{ + Name: "pip-cluster-test-apiserver", + DNSName: "", + }, + }, + }, + BackendPool: BackendPool{ + Name: "cluster-test-public-lb-backendPool", + }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Public, diff --git a/api/v1beta1/azurecluster_validation.go b/api/v1beta1/azurecluster_validation.go index 479e9cddd4b..7af0b6f8f57 100644 --- a/api/v1beta1/azurecluster_validation.go +++ b/api/v1beta1/azurecluster_validation.go @@ -391,7 +391,11 @@ func validateSecurityRule(rule SecurityRule, fldPath *field.Path) (allErrs field return allErrs } -func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []string, fldPath *field.Path) field.ErrorList { +func immutableAzureClusterFieldError(fldPath *field.Path, fieldName string) *field.Error { + return field.Forbidden(fldPath, fmt.Sprintf("%s should not be modified after AzureCluster creation.", fieldName)) +} + +func validateAPIServerLB(lb, old *LoadBalancerSpec, cidrs []string, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList lbClassSpec := lb.LoadBalancerClassSpec @@ -406,9 +410,19 @@ func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []st if err := validateLoadBalancerName(lb.Name, fldPath.Child("name")); err != nil { allErrs = append(allErrs, err) } - // Name should be immutable. - if old != nil && old.Name != "" && old.Name != lb.Name { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer name should not be modified after AzureCluster creation.")) + if old != nil { + // Name should be immutable. + if old.Name != "" && old.Name != lb.Name { + allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath, "API Server load balancer name")) + } + if (old.LoadBalancingRule.Name != "" && old.LoadBalancingRule.Name != lb.LoadBalancingRule.Name) || + (old.LoadBalancingRule.Name == "" && lb.LoadBalancingRule.Name != DefaultLoadBalancingRuleName) { + allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("loadBalancingRule").Child("name"), "Load balancer rule name")) + } + if (old.HealthProbe.Name != "" && old.HealthProbe.Name != lb.HealthProbe.Name) || + (old.HealthProbe.Name == "" && lb.HealthProbe.Name != DefaultHealthProbeName) { + allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("healthProbe").Child("name"), "Health probe name")) + } } publicIPCount, privateIPCount := 0, 0 @@ -458,7 +472,7 @@ func validateAPIServerLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, cidrs []st } if old != nil && len(old.FrontendIPs) != 0 && old.FrontendIPs[0].PrivateIPAddress != lb.FrontendIPs[0].PrivateIPAddress { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "API Server load balancer private IP should not be modified after AzureCluster creation.")) + allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("name"), "API Server load balancer private IP")) } } } @@ -484,11 +498,11 @@ func validateNodeOutboundLB(lb *LoadBalancerSpec, old *LoadBalancerSpec, apiserv } if old != nil && old.ID != lb.ID { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("id"), "Node outbound load balancer ID should not be modified after AzureCluster creation.")) + allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("id"), "Node outbound load balancer ID")) } if old != nil && old.Name != lb.Name { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("name"), "Node outbound load balancer Name should not be modified after AzureCluster creation.")) + allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("name"), "Node outbound load balancer Name")) } if old != nil && old.FrontendIPsCount == lb.FrontendIPsCount { @@ -598,12 +612,12 @@ func validateClassSpecForAPIServerLB(lb LoadBalancerClassSpec, old *LoadBalancer // SKU should be immutable. if old != nil && old.SKU != "" && old.SKU != lb.SKU { - allErrs = append(allErrs, field.Forbidden(apiServerLBPath.Child("sku"), "API Server load balancer SKU should not be modified after AzureCluster creation.")) + allErrs = append(allErrs, immutableAzureClusterFieldError(apiServerLBPath.Child("sku"), "API Server load balancer SKU")) } // Type should be immutable. if old != nil && old.Type != "" && old.Type != lb.Type { - allErrs = append(allErrs, field.Forbidden(apiServerLBPath.Child("type"), "API Server load balancer type should not be modified after AzureCluster creation.")) + allErrs = append(allErrs, immutableAzureClusterFieldError(apiServerLBPath.Child("type"), "API Server load balancer type")) } // IdletimeoutInMinutes should be immutable. @@ -633,7 +647,7 @@ func validateClassSpecForNodeOutboundLB(lb *LoadBalancerClassSpec, old *LoadBala } if old != nil && old.SKU != lb.SKU { - allErrs = append(allErrs, field.Forbidden(fldPath.Child("sku"), "Node outbound load balancer SKU should not be modified after AzureCluster creation.")) + allErrs = append(allErrs, immutableAzureClusterFieldError(fldPath.Child("sku"), "Node outbound load balancer SKU")) } if old != nil && old.Type != lb.Type { diff --git a/api/v1beta1/azurecluster_validation_test.go b/api/v1beta1/azurecluster_validation_test.go index 5859f0aae01..08c21adc895 100644 --- a/api/v1beta1/azurecluster_validation_test.go +++ b/api/v1beta1/azurecluster_validation_test.go @@ -1285,6 +1285,146 @@ func TestValidateAPIServerLB(t *testing.T) { Detail: "Internal LB IP address needs to be in control plane subnet range ([10.0.0.0/24 10.1.0.0/24])", }, }, + { + name: "no old spec (creation)", + lb: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "CustomLBRule"}, + HealthProbe: HealthProbe{Name: "CustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + old: nil, + wantErr: false, + }, + { + name: "both loadBalancingRule and healthProbe name unchanged", + lb: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "CustomLBRule"}, + HealthProbe: HealthProbe{Name: "CustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + old: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "CustomLBRule"}, + HealthProbe: HealthProbe{Name: "CustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + wantErr: false, + }, + { + name: "loadBalancingRule changed from custom to custom", + lb: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "NewCustomLBRule"}, + HealthProbe: HealthProbe{Name: "CustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + old: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "OldCustomLBRule"}, + HealthProbe: HealthProbe{Name: "CustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueForbidden", + Field: "apiServerLB.loadBalancingRule.name", + Detail: "Load balancer rule name should not be modified after AzureCluster creation.", + }, + }, + { + name: "loadBalancingRule upgrade scenario: old empty, new default", + lb: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: DefaultLoadBalancingRuleName}, + HealthProbe: HealthProbe{Name: DefaultHealthProbeName}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + old: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: ""}, + HealthProbe: HealthProbe{Name: ""}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + wantErr: false, + }, + { + name: "loadBalancingRule upgrade scenario: old empty, new custom", + lb: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "CustomLBRule"}, + HealthProbe: HealthProbe{Name: DefaultHealthProbeName}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + old: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: ""}, + HealthProbe: HealthProbe{Name: ""}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueForbidden", + Field: "apiServerLB.loadBalancingRule.name", + Detail: "Load balancer rule name should not be modified after AzureCluster creation.", + }, + }, + { + name: "healthProbe changed from custom to custom", + lb: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "CustomLBRule"}, + HealthProbe: HealthProbe{Name: "NewCustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + old: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: "CustomLBRule"}, + HealthProbe: HealthProbe{Name: "OldCustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueForbidden", + Field: "apiServerLB.healthProbe.name", + Detail: "Health probe name should not be modified after AzureCluster creation.", + }, + }, + { + name: "healthProbe upgrade scenario: old empty, new custom", + lb: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: DefaultLoadBalancingRuleName}, + HealthProbe: HealthProbe{Name: "CustomProbe"}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + old: &LoadBalancerSpec{ + Name: "test-lb", + LoadBalancingRule: LoadBalancingRule{Name: ""}, + HealthProbe: HealthProbe{Name: ""}, + LoadBalancerClassSpec: LoadBalancerClassSpec{SKU: SKUStandard, Type: Public}, + FrontendIPs: []FrontendIP{{Name: "ip-config", PublicIP: &PublicIPSpec{Name: "test-ip"}}}, + }, + wantErr: true, + expectedErr: field.Error{ + Type: "FieldValueForbidden", + Field: "apiServerLB.healthProbe.name", + Detail: "Health probe name should not be modified after AzureCluster creation.", + }, + }, } for _, test := range testcases { @@ -1905,6 +2045,12 @@ func createValidAPIServerLB() *LoadBalancerSpec { }, }, }, + LoadBalancingRule: LoadBalancingRule{ + Name: DefaultLoadBalancingRuleName, + }, + HealthProbe: HealthProbe{ + Name: DefaultHealthProbeName, + }, LoadBalancerClassSpec: LoadBalancerClassSpec{ SKU: SKUStandard, Type: Public, diff --git a/api/v1beta1/azurecluster_webhook.go b/api/v1beta1/azurecluster_webhook.go index def1503c053..dfe1292b7bb 100644 --- a/api/v1beta1/azurecluster_webhook.go +++ b/api/v1beta1/azurecluster_webhook.go @@ -120,7 +120,7 @@ func (*AzureClusterWebhook) ValidateUpdate(_ context.Context, oldRaw, newObj run // The equality failure could be because of default mismatch between v1alpha3 and v1beta1. This happens because // the new object `r` will have run through the default webhooks but the old object `old` would not have so. // This means if the old object was in v1alpha3, it would not get the new defaults set in v1beta1 resulting - // in object inequality. To workaround this, we set the v1beta1 defaults here so that the old object also gets + // in object inequality. To work around this, we set the v1beta1 defaults here so that the old object also gets // the new defaults. old.setAzureEnvironmentDefault() diff --git a/api/v1beta1/azurecluster_webhook_test.go b/api/v1beta1/azurecluster_webhook_test.go index a7848e7db00..52375bbfeaf 100644 --- a/api/v1beta1/azurecluster_webhook_test.go +++ b/api/v1beta1/azurecluster_webhook_test.go @@ -339,6 +339,90 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) { }(), wantErr: false, }, + { + name: "loadBalancingRule name is immutable", + oldCluster: &AzureCluster{ + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + LoadBalancingRule: LoadBalancingRule{ + Name: "old-rule-name", + }, + }, + }, + }, + }, + cluster: &AzureCluster{ + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + LoadBalancingRule: LoadBalancingRule{ + Name: "new-rule-name", + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "healthProbe name is immutable", + oldCluster: &AzureCluster{ + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + HealthProbe: HealthProbe{ + Name: "old-probe-name", + }, + }, + }, + }, + }, + cluster: &AzureCluster{ + Spec: AzureClusterSpec{ + NetworkSpec: NetworkSpec{ + APIServerLB: &LoadBalancerSpec{ + HealthProbe: HealthProbe{ + Name: "new-probe-name", + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "upgrade scenario - block setting custom names when old has empty fields", + oldCluster: func() *AzureCluster { + cluster := createValidCluster() + cluster.Spec.NetworkSpec.APIServerLB.LoadBalancingRule.Name = "" + cluster.Spec.NetworkSpec.APIServerLB.HealthProbe.Name = "" + return cluster + }(), + cluster: func() *AzureCluster { + cluster := createValidCluster() + cluster.Spec.NetworkSpec.APIServerLB.LoadBalancingRule.Name = "CustomLBRule" + cluster.Spec.NetworkSpec.APIServerLB.HealthProbe.Name = "CustomProbe" + return cluster + }(), + wantErr: true, + }, + { + name: "upgrade scenario - allow setting default values when old has empty fields", + oldCluster: func() *AzureCluster { + cluster := createValidCluster() + cluster.Spec.NetworkSpec.APIServerLB.LoadBalancingRule.Name = "" + cluster.Spec.NetworkSpec.APIServerLB.HealthProbe.Name = "" + return cluster + }(), + cluster: func() *AzureCluster { + cluster := createValidCluster() + cluster.Spec.NetworkSpec.APIServerLB.LoadBalancingRule.Name = DefaultLoadBalancingRuleName + cluster.Spec.NetworkSpec.APIServerLB.HealthProbe.Name = DefaultHealthProbeName + return cluster + }(), + wantErr: false, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index ba3a74680b1..5c6e05b1daf 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -360,10 +360,30 @@ type LoadBalancerSpec struct { // BackendPool describes the backend pool of the load balancer. // +optional BackendPool BackendPool `json:"backendPool,omitempty"` + // LoadBalancingRule defines the load balancer rule configuration. + // +optional + LoadBalancingRule LoadBalancingRule `json:"loadBalancingRule,omitempty"` + // HealthProbe defines the health probe configuration. + // +optional + HealthProbe HealthProbe `json:"healthProbe,omitempty"` LoadBalancerClassSpec `json:",inline"` } +// LoadBalancingRule defines the load balancer rule configuration. +type LoadBalancingRule struct { + // Name specifies the name of the load balancer rule. + // +optional + Name string `json:"name,omitempty"` +} + +// HealthProbe defines the health probe configuration. +type HealthProbe struct { + // Name specifies the name of the health probe. + // +optional + Name string `json:"name,omitempty"` +} + // SKU defines an Azure load balancer SKU. type SKU string diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 036180f4bed..7ffd79065c1 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -3165,6 +3165,21 @@ func (in *HTTPProxyConfig) DeepCopy() *HTTPProxyConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HealthProbe) DeepCopyInto(out *HealthProbe) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HealthProbe. +func (in *HealthProbe) DeepCopy() *HealthProbe { + if in == nil { + return nil + } + out := new(HealthProbe) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IPTag) DeepCopyInto(out *IPTag) { *out = *in @@ -3441,6 +3456,8 @@ func (in *LoadBalancerSpec) DeepCopyInto(out *LoadBalancerSpec) { **out = **in } out.BackendPool = in.BackendPool + out.LoadBalancingRule = in.LoadBalancingRule + out.HealthProbe = in.HealthProbe in.LoadBalancerClassSpec.DeepCopyInto(&out.LoadBalancerClassSpec) } @@ -3454,6 +3471,21 @@ func (in *LoadBalancerSpec) DeepCopy() *LoadBalancerSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LoadBalancingRule) DeepCopyInto(out *LoadBalancingRule) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancingRule. +func (in *LoadBalancingRule) DeepCopy() *LoadBalancingRule { + if in == nil { + return nil + } + out := new(LoadBalancingRule) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ManagedClusterAutoUpgradeProfile) DeepCopyInto(out *ManagedClusterAutoUpgradeProfile) { *out = *in diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index b30f1b501a7..5a237b71367 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -250,23 +250,25 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter { if s.ControlPlaneEnabled() { frontendLB := &loadbalancers.LBSpec{ // API Server LB - Name: s.APIServerLB().Name, - ResourceGroup: s.ResourceGroup(), - SubscriptionID: s.SubscriptionID(), - ClusterName: s.ClusterName(), - Location: s.Location(), - ExtendedLocation: s.ExtendedLocation(), - VNetName: s.Vnet().Name, - VNetResourceGroup: s.Vnet().ResourceGroup, - SubnetName: s.ControlPlaneSubnet().Name, - APIServerPort: s.APIServerPort(), - Type: s.APIServerLB().Type, - SKU: s.APIServerLB().SKU, - Role: infrav1.APIServerRole, - BackendPoolName: s.APIServerLB().BackendPool.Name, - IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, - AdditionalTags: s.AdditionalTags(), - AdditionalPorts: s.AdditionalAPIServerLBPorts(), + Name: s.APIServerLB().Name, + ResourceGroup: s.ResourceGroup(), + SubscriptionID: s.SubscriptionID(), + ClusterName: s.ClusterName(), + Location: s.Location(), + ExtendedLocation: s.ExtendedLocation(), + VNetName: s.Vnet().Name, + VNetResourceGroup: s.Vnet().ResourceGroup, + SubnetName: s.ControlPlaneSubnet().Name, + APIServerPort: s.APIServerPort(), + Type: s.APIServerLB().Type, + SKU: s.APIServerLB().SKU, + Role: infrav1.APIServerRole, + BackendPoolName: s.APIServerLB().BackendPool.Name, + IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, + AdditionalTags: s.AdditionalTags(), + AdditionalPorts: s.AdditionalAPIServerLBPorts(), + LoadBalancingRuleName: s.APIServerLB().LoadBalancingRule.Name, + HealthProbeName: s.APIServerLB().HealthProbe.Name, } if s.APIServerLB().FrontendIPs != nil { @@ -284,23 +286,25 @@ func (s *ClusterScope) LBSpecs() []azure.ResourceSpecGetter { if s.APIServerLB().Type != infrav1.Internal && feature.Gates.Enabled(feature.APIServerILB) { internalLB := &loadbalancers.LBSpec{ - Name: s.APIServerLB().Name + "-internal", - ResourceGroup: s.ResourceGroup(), - SubscriptionID: s.SubscriptionID(), - ClusterName: s.ClusterName(), - Location: s.Location(), - ExtendedLocation: s.ExtendedLocation(), - VNetName: s.Vnet().Name, - VNetResourceGroup: s.Vnet().ResourceGroup, - SubnetName: s.ControlPlaneSubnet().Name, - APIServerPort: s.APIServerPort(), - Type: infrav1.Internal, - SKU: s.APIServerLB().SKU, - Role: infrav1.APIServerRoleInternal, - BackendPoolName: s.APIServerLB().BackendPool.Name + "-internal", - IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, - AdditionalTags: s.AdditionalTags(), - AdditionalPorts: s.AdditionalAPIServerLBPorts(), + Name: s.APIServerLB().Name + "-internal", + ResourceGroup: s.ResourceGroup(), + SubscriptionID: s.SubscriptionID(), + ClusterName: s.ClusterName(), + Location: s.Location(), + ExtendedLocation: s.ExtendedLocation(), + VNetName: s.Vnet().Name, + VNetResourceGroup: s.Vnet().ResourceGroup, + SubnetName: s.ControlPlaneSubnet().Name, + APIServerPort: s.APIServerPort(), + Type: infrav1.Internal, + SKU: s.APIServerLB().SKU, + Role: infrav1.APIServerRoleInternal, + BackendPoolName: s.APIServerLB().BackendPool.Name + "-internal", + IdleTimeoutInMinutes: s.APIServerLB().IdleTimeoutInMinutes, + AdditionalTags: s.AdditionalTags(), + AdditionalPorts: s.AdditionalAPIServerLBPorts(), + LoadBalancingRuleName: s.APIServerLB().LoadBalancingRule.Name, + HealthProbeName: s.APIServerLB().HealthProbe.Name, } privateIPFound := false diff --git a/azure/services/loadbalancers/loadbalancers.go b/azure/services/loadbalancers/loadbalancers.go index a5ff22d65a6..1b1ec5e01ae 100644 --- a/azure/services/loadbalancers/loadbalancers.go +++ b/azure/services/loadbalancers/loadbalancers.go @@ -29,9 +29,7 @@ import ( const ( serviceName = "loadbalancers" - httpsProbe = "HTTPSProbe" httpsProbeRequestPath = "/readyz" - lbRuleHTTPS = "LBRuleHTTPS" outboundNAT = "OutboundNATAllProtocols" ) diff --git a/azure/services/loadbalancers/loadbalancers_test.go b/azure/services/loadbalancers/loadbalancers_test.go index 3a61789a507..c8bacfda6c3 100644 --- a/azure/services/loadbalancers/loadbalancers_test.go +++ b/azure/services/loadbalancers/loadbalancers_test.go @@ -57,7 +57,9 @@ var ( }, }, }, - APIServerPort: 6443, + APIServerPort: 6443, + LoadBalancingRuleName: infrav1.DefaultLoadBalancingRuleName, + HealthProbeName: infrav1.DefaultHealthProbeName, } fakePublicAPILBSpecWithAdditionalPorts = LBSpec{ @@ -86,6 +88,8 @@ var ( Name: "rke2-agent", Port: 9345, }}, + LoadBalancingRuleName: infrav1.DefaultLoadBalancingRuleName, + HealthProbeName: infrav1.DefaultHealthProbeName, } fakeInternalAPILBSpec = LBSpec{ @@ -108,7 +112,9 @@ var ( }, }, }, - APIServerPort: 6443, + APIServerPort: 6443, + LoadBalancingRuleName: infrav1.DefaultLoadBalancingRuleName, + HealthProbeName: infrav1.DefaultHealthProbeName, } fakeNodeOutboundLBSpec = LBSpec{ diff --git a/azure/services/loadbalancers/spec.go b/azure/services/loadbalancers/spec.go index 5fd3cedf9f2..c8d4ff38cff 100644 --- a/azure/services/loadbalancers/spec.go +++ b/azure/services/loadbalancers/spec.go @@ -30,24 +30,26 @@ import ( // LBSpec defines the specification for a Load Balancer. type LBSpec struct { - Name string - ResourceGroup string - SubscriptionID string - ClusterName string - Location string - ExtendedLocation *infrav1.ExtendedLocationSpec - Role string - Type infrav1.LBType - SKU infrav1.SKU - VNetName string - VNetResourceGroup string - SubnetName string - BackendPoolName string - FrontendIPConfigs []infrav1.FrontendIP - APIServerPort int32 - IdleTimeoutInMinutes *int32 - AdditionalTags map[string]string - AdditionalPorts []infrav1.LoadBalancerPort + Name string + ResourceGroup string + SubscriptionID string + ClusterName string + Location string + ExtendedLocation *infrav1.ExtendedLocationSpec + Role string + Type infrav1.LBType + SKU infrav1.SKU + VNetName string + VNetResourceGroup string + SubnetName string + BackendPoolName string + FrontendIPConfigs []infrav1.FrontendIP + APIServerPort int32 + IdleTimeoutInMinutes *int32 + AdditionalTags map[string]string + AdditionalPorts []infrav1.LoadBalancerPort + LoadBalancingRuleName string + HealthProbeName string } // ResourceName returns the name of the load balancer. @@ -222,9 +224,17 @@ func getLoadBalancingRules(lbSpec LBSpec, frontendIDs []*armnetwork.SubResource) if len(frontendIDs) != 0 { frontendIPConfig = frontendIDs[0] } + ruleName := infrav1.DefaultLoadBalancingRuleName + if lbSpec.LoadBalancingRuleName != "" { + ruleName = lbSpec.LoadBalancingRuleName + } + probeName := infrav1.DefaultHealthProbeName + if lbSpec.HealthProbeName != "" { + probeName = lbSpec.HealthProbeName + } rules := []*armnetwork.LoadBalancingRule{ { - Name: ptr.To(lbRuleHTTPS), + Name: ptr.To(ruleName), Properties: &armnetwork.LoadBalancingRulePropertiesFormat{ DisableOutboundSnat: ptr.To(true), Protocol: ptr.To(armnetwork.TransportProtocolTCP), @@ -238,7 +248,7 @@ func getLoadBalancingRules(lbSpec LBSpec, frontendIDs []*armnetwork.SubResource) ID: ptr.To(azure.AddressPoolID(lbSpec.SubscriptionID, lbSpec.ResourceGroup, lbSpec.Name, lbSpec.BackendPoolName)), }, Probe: &armnetwork.SubResource{ - ID: ptr.To(azure.ProbeID(lbSpec.SubscriptionID, lbSpec.ResourceGroup, lbSpec.Name, httpsProbe)), + ID: ptr.To(azure.ProbeID(lbSpec.SubscriptionID, lbSpec.ResourceGroup, lbSpec.Name, probeName)), }, }, }, @@ -260,7 +270,7 @@ func getLoadBalancingRules(lbSpec LBSpec, frontendIDs []*armnetwork.SubResource) ID: ptr.To(azure.AddressPoolID(lbSpec.SubscriptionID, lbSpec.ResourceGroup, lbSpec.Name, lbSpec.BackendPoolName)), }, Probe: &armnetwork.SubResource{ - ID: ptr.To(azure.ProbeID(lbSpec.SubscriptionID, lbSpec.ResourceGroup, lbSpec.Name, httpsProbe)), + ID: ptr.To(azure.ProbeID(lbSpec.SubscriptionID, lbSpec.ResourceGroup, lbSpec.Name, infrav1.DefaultHealthProbeName)), }, }, }) @@ -281,9 +291,13 @@ func getBackendAddressPools(lbSpec LBSpec) []*armnetwork.BackendAddressPool { func getProbes(lbSpec LBSpec) []*armnetwork.Probe { if lbSpec.Role == infrav1.APIServerRole || lbSpec.Role == infrav1.APIServerRoleInternal { + probeName := infrav1.DefaultHealthProbeName + if lbSpec.HealthProbeName != "" { + probeName = lbSpec.HealthProbeName + } return []*armnetwork.Probe{ { - Name: ptr.To(httpsProbe), + Name: ptr.To(probeName), Properties: &armnetwork.ProbePropertiesFormat{ Protocol: ptr.To(armnetwork.ProbeProtocolHTTPS), Port: ptr.To[int32](lbSpec.APIServerPort), diff --git a/azure/services/loadbalancers/spec_test.go b/azure/services/loadbalancers/spec_test.go index 9e75779a7c1..caffbe397a1 100644 --- a/azure/services/loadbalancers/spec_test.go +++ b/azure/services/loadbalancers/spec_test.go @@ -168,6 +168,80 @@ func TestParameters(t *testing.T) { }, expectedError: "", }, + { + name: "load balancer with custom rule and probe names", + spec: &LBSpec{ + Name: "my-publiclb", + ResourceGroup: "my-rg", + SubscriptionID: "123", + ClusterName: "my-cluster", + Location: "my-location", + Role: infrav1.APIServerRole, + Type: infrav1.Public, + SKU: infrav1.SKUStandard, + SubnetName: "my-cp-subnet", + BackendPoolName: "my-publiclb-backendPool", + IdleTimeoutInMinutes: ptr.To[int32](4), + FrontendIPConfigs: []infrav1.FrontendIP{ + { + Name: "my-publiclb-frontEnd", + PublicIP: &infrav1.PublicIPSpec{ + Name: "my-publicip", + DNSName: "my-cluster.12345.mydomain.com", + }, + }, + }, + APIServerPort: 6443, + LoadBalancingRuleName: "CustomLBRule", + HealthProbeName: "CustomProbe", + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(armnetwork.LoadBalancer{})) + lb := result.(armnetwork.LoadBalancer) + g.Expect(lb.Properties.LoadBalancingRules).To(HaveLen(1)) + g.Expect(*lb.Properties.LoadBalancingRules[0].Name).To(Equal("CustomLBRule")) + g.Expect(lb.Properties.Probes).To(HaveLen(1)) + g.Expect(*lb.Properties.Probes[0].Name).To(Equal("CustomProbe")) + }, + expectedError: "", + }, + { + name: "load balancer with default rule and probe names", + spec: &LBSpec{ + Name: "my-publiclb", + ResourceGroup: "my-rg", + SubscriptionID: "123", + ClusterName: "my-cluster", + Location: "my-location", + Role: infrav1.APIServerRole, + Type: infrav1.Public, + SKU: infrav1.SKUStandard, + SubnetName: "my-cp-subnet", + BackendPoolName: "my-publiclb-backendPool", + IdleTimeoutInMinutes: ptr.To[int32](4), + FrontendIPConfigs: []infrav1.FrontendIP{ + { + Name: "my-publiclb-frontEnd", + PublicIP: &infrav1.PublicIPSpec{ + Name: "my-publicip", + DNSName: "my-cluster.12345.mydomain.com", + }, + }, + }, + APIServerPort: 6443, + }, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeAssignableToTypeOf(armnetwork.LoadBalancer{})) + lb := result.(armnetwork.LoadBalancer) + g.Expect(lb.Properties.LoadBalancingRules).To(HaveLen(1)) + g.Expect(*lb.Properties.LoadBalancingRules[0].Name).To(Equal("LBRuleHTTPS")) + g.Expect(lb.Properties.Probes).To(HaveLen(1)) + g.Expect(*lb.Properties.Probes[0].Name).To(Equal("HTTPSProbe")) + }, + expectedError: "", + }, { name: "load balancer exists with missing outbound rules", spec: &fakePublicAPILBSpec, @@ -291,7 +365,7 @@ func newSamplePublicAPIServerLB(verifyFrontendIP bool, verifyBackendAddressPools }, LoadBalancingRules: []*armnetwork.LoadBalancingRule{ { - Name: ptr.To(lbRuleHTTPS), + Name: ptr.To(infrav1.DefaultLoadBalancingRuleName), Properties: &armnetwork.LoadBalancingRulePropertiesFormat{ DisableOutboundSnat: ptr.To(true), Protocol: ptr.To(armnetwork.TransportProtocolTCP), @@ -314,7 +388,7 @@ func newSamplePublicAPIServerLB(verifyFrontendIP bool, verifyBackendAddressPools }, Probes: []*armnetwork.Probe{ { - Name: ptr.To(httpsProbe), + Name: ptr.To(infrav1.DefaultHealthProbeName), Properties: &armnetwork.ProbePropertiesFormat{ Protocol: ptr.To(armnetwork.ProbeProtocolHTTPS), Port: ptr.To[int32](6443), @@ -377,7 +451,7 @@ func newDefaultInternalAPIServerLB() armnetwork.LoadBalancer { }, LoadBalancingRules: []*armnetwork.LoadBalancingRule{ { - Name: ptr.To(lbRuleHTTPS), + Name: ptr.To(infrav1.DefaultLoadBalancingRuleName), Properties: &armnetwork.LoadBalancingRulePropertiesFormat{ DisableOutboundSnat: ptr.To(true), Protocol: ptr.To(armnetwork.TransportProtocolTCP), @@ -401,7 +475,7 @@ func newDefaultInternalAPIServerLB() armnetwork.LoadBalancer { OutboundRules: []*armnetwork.OutboundRule{}, Probes: []*armnetwork.Probe{ { - Name: ptr.To(httpsProbe), + Name: ptr.To(infrav1.DefaultHealthProbeName), Properties: &armnetwork.ProbePropertiesFormat{ Protocol: ptr.To(armnetwork.ProbeProtocolHTTPS), Port: ptr.To[int32](6443), diff --git a/config/capz/manager_image_patch.yaml b/config/capz/manager_image_patch.yaml index 47f8612fd1f..01467e26f06 100644 --- a/config/capz/manager_image_patch.yaml +++ b/config/capz/manager_image_patch.yaml @@ -8,5 +8,5 @@ spec: spec: containers: # Change the value of image field below to your controller image URL - - image: gcr.io/k8s-staging-cluster-api-azure/cluster-api-azure-controller:main + - image: localhost:5000/cluster-api-azure-controller-amd64:dev name: manager diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml index 927feee4b73..d28aa4e32f3 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml @@ -742,6 +742,13 @@ spec: IP addresses for the load balancer. format: int32 type: integer + healthProbe: + description: HealthProbe defines the health probe configuration. + properties: + name: + description: Name specifies the name of the health probe. + type: string + type: object id: description: |- ID is the Azure resource ID of the load balancer. @@ -752,6 +759,15 @@ spec: the TCP idle connection. format: int32 type: integer + loadBalancingRule: + description: LoadBalancingRule defines the load balancer rule + configuration. + properties: + name: + description: Name specifies the name of the load balancer + rule. + type: string + type: object name: type: string sku: @@ -825,6 +841,13 @@ spec: IP addresses for the load balancer. format: int32 type: integer + healthProbe: + description: HealthProbe defines the health probe configuration. + properties: + name: + description: Name specifies the name of the health probe. + type: string + type: object id: description: |- ID is the Azure resource ID of the load balancer. @@ -835,6 +858,15 @@ spec: the TCP idle connection. format: int32 type: integer + loadBalancingRule: + description: LoadBalancingRule defines the load balancer rule + configuration. + properties: + name: + description: Name specifies the name of the load balancer + rule. + type: string + type: object name: type: string sku: @@ -907,6 +939,13 @@ spec: IP addresses for the load balancer. format: int32 type: integer + healthProbe: + description: HealthProbe defines the health probe configuration. + properties: + name: + description: Name specifies the name of the health probe. + type: string + type: object id: description: |- ID is the Azure resource ID of the load balancer. @@ -917,6 +956,15 @@ spec: the TCP idle connection. format: int32 type: integer + loadBalancingRule: + description: LoadBalancingRule defines the load balancer rule + configuration. + properties: + name: + description: Name specifies the name of the load balancer + rule. + type: string + type: object name: type: string sku: