Skip to content

Commit 6e99421

Browse files
ddavid-numspotjfbus
authored andcommitted
🐛 fix(OscMachine/validation/webhook): loadbalancer disabling fixes
- validate assertion against empty loadbalancer when disabled - prevent validation of osc loadbalancer attributes when disabled - prevent setdefault values in osc loadbalancer while reconciling oscmachine vms - prevent osc loadbalancer webhook validation in update webhook when disabled Co-Authored-By: Jean-François Bustarret <jean-francois.bustarret@outscale.com>
1 parent d67876e commit 6e99421

File tree

4 files changed

+57
-16
lines changed

4 files changed

+57
-16
lines changed

api/v1beta1/osccluster_validation.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"net/netip"
1313
"regexp"
14+
"slices"
1415

1516
"k8s.io/apimachinery/pkg/util/validation/field"
1617
)
@@ -29,11 +30,14 @@ const (
2930
// ValidateOscClusterSpec validates a OscClusterSpec.
3031
func ValidateOscClusterSpec(spec OscClusterSpec) field.ErrorList {
3132
var allErrs field.ErrorList
33+
34+
lbDisabled := slices.Contains(spec.Network.Disable, DisableLB)
35+
36+
allErrs = append(allErrs, ValidateLoadbalancer(spec.Network.LoadBalancer, lbDisabled)...)
3237
allErrs = append(allErrs, ValidateNet(spec.Network.Net, spec.Network.UseExisting)...)
3338
allErrs = append(allErrs, ValidateSubnets(spec.Network.Subnets, spec.Network.Net, spec.Network.UseExisting)...)
3439
allErrs = append(allErrs, ValidateNatServices(spec.Network.NatServices, spec.Network.Subnets, spec.Network.Net, spec.Network.UseExisting)...)
3540
allErrs = append(allErrs, ValidateSecurityGroups(spec.Network.SecurityGroups, spec.Network.Net, spec.Network.UseExisting)...)
36-
allErrs = append(allErrs, ValidateLoadbalancer(spec.Network.LoadBalancer)...)
3741
allErrs = append(allErrs, ValidateAllowFromIPs(spec.Network.AllowFromIPRanges)...)
3842
return allErrs
3943
}
@@ -143,8 +147,11 @@ func ValidateSecurityGroupRules(specs []OscSecurityGroupRule) field.ErrorList {
143147
return erl
144148
}
145149

146-
func ValidateLoadbalancer(spec OscLoadBalancer) field.ErrorList {
150+
func ValidateLoadbalancer(spec OscLoadBalancer, lbDisabled bool) field.ErrorList {
147151
var erl field.ErrorList
152+
if lbDisabled {
153+
return AppendValidation(erl, ValidateEmptyLoadBalancer(field.NewPath("network", "loadBalancer"), spec))
154+
}
148155
erl = AppendValidation(erl,
149156
ValidateLoadBalancerName(field.NewPath("network", "loadBalancer", "loadbalancername"), spec.LoadBalancerName),
150157
Optional(ValidateLoadBalancerType(field.NewPath("network", "loadBalancer", "loadbalancertype"), spec.LoadBalancerType)),
@@ -274,6 +281,14 @@ func ValidatePortRange(p *field.Path, from, to int32, msg string) *field.Error {
274281
}
275282
}
276283

284+
// ValidateEmptyLoadBalancer checks that the loadBalancerName is a valid name of load balancer
285+
func ValidateEmptyLoadBalancer(p *field.Path, spec OscLoadBalancer) *field.Error {
286+
if spec != (OscLoadBalancer{}) {
287+
return field.Forbidden(p, "loadBalancer must be empty when disabled")
288+
}
289+
return nil
290+
}
291+
277292
var isValidateLoadBalancerName = regexp.MustCompile(`^[0-9A-Za-z\s\-]{0,32}$`).MatchString
278293

279294
// ValidateLoadBalancerName checks that the loadBalancerName is a valid name of load balancer

api/v1beta1/osccluster_webhook.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ package v1beta1
99
import (
1010
"context"
1111
"fmt"
12+
"slices"
1213

1314
apierrors "k8s.io/apimachinery/pkg/api/errors"
1415
"k8s.io/apimachinery/pkg/runtime"
@@ -61,18 +62,19 @@ func (OscClusterWebhook) ValidateUpdate(_ context.Context, obj runtime.Object, o
6162
}
6263
var allErrs field.ErrorList
6364
old := oldRaw.(*OscCluster)
64-
65-
if r.Spec.Network.LoadBalancer.LoadBalancerName != old.Spec.Network.LoadBalancer.LoadBalancerName {
66-
allErrs = append(allErrs,
67-
field.Invalid(field.NewPath("network", "loadBalancer", "loadbalancername"),
68-
r.Spec.Network.LoadBalancer.LoadBalancerName, "field is immutable"),
69-
)
70-
}
71-
if r.Spec.Network.LoadBalancer.LoadBalancerType != old.Spec.Network.LoadBalancer.LoadBalancerType {
72-
allErrs = append(allErrs,
73-
field.Invalid(field.NewPath("network", "loadBalancer", "loadbalancertype"),
74-
r.Spec.Network.LoadBalancer.LoadBalancerType, "field is immutable"),
75-
)
65+
if !slices.Contains(r.Spec.Network.Disable, DisableLB) {
66+
if r.Spec.Network.LoadBalancer.LoadBalancerName != old.Spec.Network.LoadBalancer.LoadBalancerName {
67+
allErrs = append(allErrs,
68+
field.Invalid(field.NewPath("network", "loadBalancer", "loadbalancername"),
69+
r.Spec.Network.LoadBalancer.LoadBalancerName, "field is immutable"),
70+
)
71+
}
72+
if r.Spec.Network.LoadBalancer.LoadBalancerType != old.Spec.Network.LoadBalancer.LoadBalancerType {
73+
allErrs = append(allErrs,
74+
field.Invalid(field.NewPath("network", "loadBalancer", "loadbalancertype"),
75+
r.Spec.Network.LoadBalancer.LoadBalancerType, "field is immutable"),
76+
)
77+
}
7678
}
7779
if len(allErrs) == 0 {
7880
return nil, nil

api/v1beta1/osccluster_webhook_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,30 @@ func TestOscCluster_ValidateCreate(t *testing.T) {
2222
clusterSpec infrastructurev1beta1.OscClusterSpec
2323
expValidateCreateErr error
2424
}{
25+
{
26+
name: "disabled and empty loadBalancer",
27+
clusterSpec: infrastructurev1beta1.OscClusterSpec{
28+
Network: infrastructurev1beta1.OscNetwork{
29+
Disable: []infrastructurev1beta1.OscDisable{
30+
infrastructurev1beta1.DisableLB,
31+
},
32+
},
33+
},
34+
},
35+
{
36+
name: "disabled and non empty loadBalancer",
37+
clusterSpec: infrastructurev1beta1.OscClusterSpec{
38+
Network: infrastructurev1beta1.OscNetwork{
39+
Disable: []infrastructurev1beta1.OscDisable{
40+
infrastructurev1beta1.DisableLB,
41+
},
42+
LoadBalancer: infrastructurev1beta1.OscLoadBalancer{
43+
LoadBalancerName: "test-webhook@test",
44+
},
45+
},
46+
},
47+
expValidateCreateErr: errors.New("OscCluster.infrastructure.cluster.x-k8s.io \"webhook-test\" is invalid: network.loadBalancer: Forbidden: loadBalancer must be empty when disabled"),
48+
},
2549
{
2650
name: "bad loadBalancerName",
2751
clusterSpec: infrastructurev1beta1.OscClusterSpec{

controllers/oscmachine_vm.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func (r *OscMachineReconciler) reconcileVm(ctx context.Context, clusterScope *sc
145145

146146
machineScope.SetReady()
147147

148-
if vmSpec.GetRole() == infrastructurev1beta1.RoleControlPlane {
148+
if vmSpec.GetRole() == infrastructurev1beta1.RoleControlPlane && !clusterScope.IsLBDisabled() {
149149
svc := r.Cloud.LoadBalancer(clusterScope.Tenant)
150150
loadBalancerName := clusterScope.GetLoadBalancer().LoadBalancerName
151151
loadbalancer, err := svc.GetLoadBalancer(ctx, loadBalancerName)
@@ -220,7 +220,7 @@ func (r *OscMachineReconciler) reconcileDeleteVm(ctx context.Context, clusterSco
220220
}
221221

222222
vmSpec := machineScope.GetVm()
223-
if vmSpec.GetRole() == infrastructurev1beta1.RoleControlPlane {
223+
if vmSpec.GetRole() == infrastructurev1beta1.RoleControlPlane && !clusterScope.IsLBDisabled() {
224224
svc := r.Cloud.LoadBalancer(clusterScope.Tenant)
225225
loadBalancerName := clusterScope.GetLoadBalancer().LoadBalancerName
226226
loadbalancer, err := svc.GetLoadBalancer(ctx, loadBalancerName)

0 commit comments

Comments
 (0)