Skip to content

Commit ee91bfe

Browse files
Shilpa-GokulShilpa-Gokul
committed
Add UT and validation in webhooks for additional listener
1 parent e929b88 commit ee91bfe

8 files changed

+211
-32
lines changed

‎api/v1beta2/ibmvpccluster_types.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ type AdditionalListenerSpec struct {
130130

131131
// Selector is used to select the machines with same label to assign the listener
132132
// +kubebuilder:validation:Optional
133-
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Selector is immutable"
134133
Selector metav1.LabelSelector `json:"selector,omitempty"`
135134
}
136135

‎cloud/scope/powervs_machine_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,7 +1585,7 @@ func TestCreateVPCLoadBalancerPoolMemberPowerVSMachine(t *testing.T) {
15851585
mockClient.EXPECT().GetLoadBalancer(gomock.AssignableToTypeOf(&vpcv1.GetLoadBalancerOptions{})).Return(loadBalancers, nil, nil).AnyTimes()
15861586
mockClient.EXPECT().GetLoadBalancerListener(gomock.AssignableToTypeOf(&vpcv1.GetLoadBalancerListenerOptions{})).Return(loadBalancerListener, nil, nil).AnyTimes()
15871587
mockClient.EXPECT().ListLoadBalancerPoolMembers(gomock.AssignableToTypeOf(&vpcv1.ListLoadBalancerPoolMembersOptions{})).Return(&vpcv1.LoadBalancerPoolMemberCollection{}, nil, nil).AnyTimes()
1588-
result, err := scope.CreateVPCLoadBalancerPoolMember()
1588+
result, err := scope.CreateVPCLoadBalancerPoolMember(ctx)
15891589

15901590
g.Expect(err).To(BeNil())
15911591
g.Expect(result).To(BeNil())
@@ -1672,7 +1672,7 @@ func TestCreateVPCLoadBalancerPoolMemberPowerVSMachine(t *testing.T) {
16721672
expectedLoadBalancerPoolMemberID := "pool-member-3"
16731673
expectedLoadBalancerPoolMember := &vpcv1.LoadBalancerPoolMember{ID: ptr.To(expectedLoadBalancerPoolMemberID)}
16741674
mockClient.EXPECT().CreateLoadBalancerPoolMember(gomock.AssignableToTypeOf(&vpcv1.CreateLoadBalancerPoolMemberOptions{})).Return(expectedLoadBalancerPoolMember, nil, nil).AnyTimes()
1675-
result, err := scope.CreateVPCLoadBalancerPoolMember()
1675+
result, err := scope.CreateVPCLoadBalancerPoolMember(ctx)
16761676

16771677
g.Expect(err).To(BeNil())
16781678
g.Expect(*result.ID).To(Equal(expectedLoadBalancerPoolMemberID))
@@ -1751,7 +1751,7 @@ func TestCreateVPCLoadBalancerPoolMemberPowerVSMachine(t *testing.T) {
17511751
mockClient.EXPECT().GetLoadBalancer(gomock.AssignableToTypeOf(&vpcv1.GetLoadBalancerOptions{})).Return(loadBalancers, nil, nil).AnyTimes()
17521752
mockClient.EXPECT().GetLoadBalancerListener(gomock.AssignableToTypeOf(&vpcv1.GetLoadBalancerListenerOptions{})).Return(loadBalancerListener, nil, nil).AnyTimes()
17531753
mockClient.EXPECT().ListLoadBalancerPoolMembers(gomock.AssignableToTypeOf(&vpcv1.ListLoadBalancerPoolMembersOptions{})).Return(&vpcv1.LoadBalancerPoolMemberCollection{}, nil, nil).AnyTimes()
1754-
result, err := scope.CreateVPCLoadBalancerPoolMember()
1754+
result, err := scope.CreateVPCLoadBalancerPoolMember(ctx)
17551755

17561756
g.Expect(err).To(BeNil())
17571757
g.Expect(result).To(BeNil())
@@ -1846,15 +1846,15 @@ func TestCreateVPCLoadBalancerPoolMemberPowerVSMachine(t *testing.T) {
18461846
expectedLoadBalancerPoolMemberID6443 := "pool-member-6443"
18471847
expectedLoadBalancerPoolMember6443 := &vpcv1.LoadBalancerPoolMember{ID: ptr.To(expectedLoadBalancerPoolMemberID6443)}
18481848
mockClient.EXPECT().CreateLoadBalancerPoolMember(gomock.AssignableToTypeOf(&vpcv1.CreateLoadBalancerPoolMemberOptions{})).Return(expectedLoadBalancerPoolMember6443, nil, nil).Times(1)
1849-
result, err := scope.CreateVPCLoadBalancerPoolMember()
1849+
result, err := scope.CreateVPCLoadBalancerPoolMember(ctx)
18501850

18511851
g.Expect(err).To(BeNil())
18521852
g.Expect(*result.ID).To(Equal(expectedLoadBalancerPoolMemberID6443))
18531853

18541854
expectedLoadBalancerPoolMemberID24 := "pool-member-24"
18551855
expectedLoadBalancerPoolMember24 := &vpcv1.LoadBalancerPoolMember{ID: ptr.To(expectedLoadBalancerPoolMemberID24)}
18561856
mockClient.EXPECT().CreateLoadBalancerPoolMember(gomock.AssignableToTypeOf(&vpcv1.CreateLoadBalancerPoolMemberOptions{})).Return(expectedLoadBalancerPoolMember24, nil, nil).Times(1)
1857-
result1, err1 := scope.CreateVPCLoadBalancerPoolMember()
1857+
result1, err1 := scope.CreateVPCLoadBalancerPoolMember(ctx)
18581858

18591859
g.Expect(err1).To(BeNil())
18601860
g.Expect(*result1.ID).To(Equal(expectedLoadBalancerPoolMemberID24))

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,9 +348,6 @@ spec:
348348
type: object
349349
type: object
350350
x-kubernetes-map-type: atomic
351-
x-kubernetes-validations:
352-
- message: Selector is immutable
353-
rule: self == oldSelf
354351
required:
355352
- port
356353
type: object

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,9 +384,6 @@ spec:
384384
type: object
385385
type: object
386386
x-kubernetes-map-type: atomic
387-
x-kubernetes-validations:
388-
- message: Selector is immutable
389-
rule: self == oldSelf
390387
required:
391388
- port
392389
type: object

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -340,9 +340,6 @@ spec:
340340
type: object
341341
type: object
342342
x-kubernetes-map-type: atomic
343-
x-kubernetes-validations:
344-
- message: Selector is immutable
345-
rule: self == oldSelf
346343
required:
347344
- port
348345
type: object
@@ -664,9 +661,6 @@ spec:
664661
type: object
665662
type: object
666663
x-kubernetes-map-type: atomic
667-
x-kubernetes-validations:
668-
- message: Selector is immutable
669-
rule: self == oldSelf
670664
required:
671665
- port
672666
type: object

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,6 @@ spec:
184184
type: object
185185
type: object
186186
x-kubernetes-map-type: atomic
187-
x-kubernetes-validations:
188-
- message: Selector is immutable
189-
rule: self == oldSelf
190187
required:
191188
- port
192189
type: object
@@ -516,9 +513,6 @@ spec:
516513
type: object
517514
type: object
518515
x-kubernetes-map-type: atomic
519-
x-kubernetes-validations:
520-
- message: Selector is immutable
521-
rule: self == oldSelf
522516
required:
523517
- port
524518
type: object

‎internal/webhooks/ibmpowervscluster.go

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ package webhooks
1919
import (
2020
"context"
2121
"fmt"
22+
"reflect"
2223
"strconv"
2324

2425
regionUtil "github.com/ppc64le-cloud/powervs-utils"
2526

2627
apierrors "k8s.io/apimachinery/pkg/api/errors"
28+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2729
"k8s.io/apimachinery/pkg/runtime"
2830
"k8s.io/apimachinery/pkg/runtime/schema"
2931
"k8s.io/apimachinery/pkg/util/validation/field"
@@ -64,40 +66,50 @@ func (r *IBMPowerVSCluster) ValidateCreate(_ context.Context, obj runtime.Object
6466
if !ok {
6567
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a IBMPowerVSCluster but got a %T", obj))
6668
}
67-
return validateIBMPowerVSCluster(objValue)
69+
return validateIBMPowerVSCluster(nil, objValue)
6870
}
6971

7072
// ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type.
71-
func (r *IBMPowerVSCluster) ValidateUpdate(_ context.Context, _, newObj runtime.Object) (warnings admission.Warnings, err error) {
72-
objValue, ok := newObj.(*infrav1beta2.IBMPowerVSCluster)
73+
func (r *IBMPowerVSCluster) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (warnings admission.Warnings, err error) {
74+
oldObjValue, ok := oldObj.(*infrav1beta2.IBMPowerVSCluster)
75+
if !ok {
76+
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a IBMPowerVSCluster but got a %T", oldObj))
77+
}
78+
newObjValue, ok := newObj.(*infrav1beta2.IBMPowerVSCluster)
7379
if !ok {
7480
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a IBMPowerVSCluster but got a %T", newObj))
7581
}
76-
return validateIBMPowerVSCluster(objValue)
82+
return validateIBMPowerVSCluster(oldObjValue, newObjValue)
7783
}
7884

7985
// ValidateDelete implements webhook.CustomValidator so a webhook will be registered for the type.
8086
func (r *IBMPowerVSCluster) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
8187
return nil, nil
8288
}
8389

84-
func validateIBMPowerVSCluster(cluster *infrav1beta2.IBMPowerVSCluster) (admission.Warnings, error) {
90+
func validateIBMPowerVSCluster(oldCluster, newCluster *infrav1beta2.IBMPowerVSCluster) (admission.Warnings, error) {
8591
var allErrs field.ErrorList
86-
if err := validateIBMPowerVSClusterNetwork(cluster); err != nil {
92+
if err := validateIBMPowerVSClusterNetwork(newCluster); err != nil {
8793
allErrs = append(allErrs, err)
8894
}
8995

90-
if err := validateIBMPowerVSClusterCreateInfraPrereq(cluster); err != nil {
96+
if err := validateIBMPowerVSClusterCreateInfraPrereq(newCluster); err != nil {
9197
allErrs = append(allErrs, err...)
9298
}
99+
// Need not validate for create operation
100+
if oldCluster != nil {
101+
if err := validateAdditionalListenerSelector(newCluster, oldCluster); err != nil {
102+
allErrs = append(allErrs, err...)
103+
}
104+
}
93105

94106
if len(allErrs) == 0 {
95107
return nil, nil
96108
}
97109

98110
return nil, apierrors.NewInvalid(
99111
schema.GroupKind{Group: "infrastructure.cluster.x-k8s.io", Kind: "IBMPowerVSCluster"},
100-
cluster.Name, allErrs)
112+
newCluster.Name, allErrs)
101113
}
102114

103115
func validateIBMPowerVSClusterNetwork(cluster *infrav1beta2.IBMPowerVSCluster) *field.Error {
@@ -228,3 +240,24 @@ func validateIBMPowerVSClusterCreateInfraPrereq(cluster *infrav1beta2.IBMPowerVS
228240

229241
return allErrs
230242
}
243+
244+
func validateAdditionalListenerSelector(newCluster, oldCluster *infrav1beta2.IBMPowerVSCluster) (allErrs field.ErrorList) {
245+
newLoadBalancerListeners := map[string]metav1.LabelSelector{}
246+
for _, loadbalancer := range newCluster.Spec.LoadBalancers {
247+
for _, additionalListener := range loadbalancer.AdditionalListeners {
248+
// if additionalListener.Protocol == nil {
249+
// additionalListener.Protocol = &infrav1beta2.VPCLoadBalancerListenerProtocolTCP
250+
// }
251+
newLoadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)] = additionalListener.Selector
252+
}
253+
}
254+
fmt.Println(oldCluster.Spec.LoadBalancers)
255+
for _, loadbalancer := range oldCluster.Spec.LoadBalancers {
256+
for _, additionalListener := range loadbalancer.AdditionalListeners {
257+
if _, ok := newLoadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)]; ok && !reflect.DeepEqual(newLoadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)], additionalListener.Selector) {
258+
allErrs = append(allErrs, field.Forbidden(field.NewPath("selector"), "Selector is immutable"))
259+
}
260+
}
261+
}
262+
return allErrs
263+
}

‎internal/webhooks/ibmpowervscluster_test.go

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,171 @@ func TestIBMPowerVSCluster_update(t *testing.T) {
177177
},
178178
wantErr: true,
179179
},
180+
{
181+
name: "Should error if the additionalListener selector is changed for same port and protocol",
182+
oldPowervsCluster: &infrav1beta2.IBMPowerVSCluster{
183+
Spec: infrav1beta2.IBMPowerVSClusterSpec{
184+
ServiceInstanceID: "capi-si-id",
185+
Network: infrav1beta2.IBMPowerVSResourceReference{
186+
ID: ptr.To("capi-net-id"),
187+
},
188+
LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{
189+
{
190+
Name: "load-balancer-1",
191+
AdditionalListeners: []infrav1beta2.AdditionalListenerSpec{
192+
{
193+
Port: 23,
194+
Protocol: &infrav1beta2.VPCLoadBalancerListenerProtocolTCP,
195+
Selector: metav1.LabelSelector{
196+
MatchLabels: map[string]string{
197+
"listener-selector": "port-23",
198+
},
199+
},
200+
},
201+
},
202+
},
203+
},
204+
},
205+
},
206+
newPowervsCluster: &infrav1beta2.IBMPowerVSCluster{
207+
Spec: infrav1beta2.IBMPowerVSClusterSpec{
208+
ServiceInstanceID: "capi-si-id",
209+
Network: infrav1beta2.IBMPowerVSResourceReference{
210+
ID: ptr.To("capi-net-id"),
211+
},
212+
LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{
213+
{
214+
Name: "load-balancer-1",
215+
AdditionalListeners: []infrav1beta2.AdditionalListenerSpec{
216+
{
217+
Port: 23,
218+
Protocol: &infrav1beta2.VPCLoadBalancerListenerProtocolTCP,
219+
Selector: metav1.LabelSelector{
220+
MatchLabels: map[string]string{
221+
"listener-selector": "port-23-1",
222+
},
223+
},
224+
},
225+
},
226+
},
227+
},
228+
},
229+
},
230+
wantErr: true,
231+
},
232+
{
233+
name: "Should work if there is an additional listener added",
234+
oldPowervsCluster: &infrav1beta2.IBMPowerVSCluster{
235+
Spec: infrav1beta2.IBMPowerVSClusterSpec{
236+
ServiceInstanceID: "capi-si-id",
237+
Network: infrav1beta2.IBMPowerVSResourceReference{
238+
ID: ptr.To("capi-net-id"),
239+
},
240+
LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{
241+
{
242+
Name: "load-balancer-1",
243+
AdditionalListeners: []infrav1beta2.AdditionalListenerSpec{
244+
{
245+
Port: 23,
246+
Protocol: &infrav1beta2.VPCLoadBalancerListenerProtocolTCP,
247+
Selector: metav1.LabelSelector{
248+
MatchLabels: map[string]string{
249+
"listener-selector": "port-23",
250+
},
251+
},
252+
},
253+
},
254+
},
255+
},
256+
},
257+
},
258+
newPowervsCluster: &infrav1beta2.IBMPowerVSCluster{
259+
Spec: infrav1beta2.IBMPowerVSClusterSpec{
260+
ServiceInstanceID: "capi-si-id",
261+
Network: infrav1beta2.IBMPowerVSResourceReference{
262+
ID: ptr.To("capi-net-id"),
263+
},
264+
LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{
265+
{
266+
Name: "load-balancer-1",
267+
AdditionalListeners: []infrav1beta2.AdditionalListenerSpec{
268+
{
269+
Port: 23,
270+
Protocol: &infrav1beta2.VPCLoadBalancerListenerProtocolTCP,
271+
Selector: metav1.LabelSelector{
272+
MatchLabels: map[string]string{
273+
"listener-selector": "port-23",
274+
},
275+
},
276+
},
277+
{
278+
Port: 25,
279+
Protocol: &infrav1beta2.VPCLoadBalancerListenerProtocolTCP,
280+
Selector: metav1.LabelSelector{
281+
MatchLabels: map[string]string{
282+
"listener-selector": "port-25",
283+
},
284+
},
285+
},
286+
},
287+
},
288+
},
289+
},
290+
},
291+
wantErr: false,
292+
},
293+
{
294+
name: "Should work if the additionalListener selector is updated with new port and protocol",
295+
oldPowervsCluster: &infrav1beta2.IBMPowerVSCluster{
296+
Spec: infrav1beta2.IBMPowerVSClusterSpec{
297+
ServiceInstanceID: "capi-si-id",
298+
Network: infrav1beta2.IBMPowerVSResourceReference{
299+
ID: ptr.To("capi-net-id"),
300+
},
301+
LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{
302+
{
303+
Name: "load-balancer-1",
304+
AdditionalListeners: []infrav1beta2.AdditionalListenerSpec{
305+
{
306+
Port: 23,
307+
Protocol: &infrav1beta2.VPCLoadBalancerListenerProtocolTCP,
308+
Selector: metav1.LabelSelector{
309+
MatchLabels: map[string]string{
310+
"listener-selector": "port-23",
311+
},
312+
},
313+
},
314+
},
315+
},
316+
},
317+
},
318+
},
319+
newPowervsCluster: &infrav1beta2.IBMPowerVSCluster{
320+
Spec: infrav1beta2.IBMPowerVSClusterSpec{
321+
ServiceInstanceID: "capi-si-id",
322+
Network: infrav1beta2.IBMPowerVSResourceReference{
323+
ID: ptr.To("capi-net-id"),
324+
},
325+
LoadBalancers: []infrav1beta2.VPCLoadBalancerSpec{
326+
{
327+
Name: "load-balancer-1",
328+
AdditionalListeners: []infrav1beta2.AdditionalListenerSpec{
329+
{
330+
Port: 25,
331+
Protocol: &infrav1beta2.VPCLoadBalancerListenerProtocolTCP,
332+
Selector: metav1.LabelSelector{
333+
MatchLabels: map[string]string{
334+
"listener-selector": "port-25",
335+
},
336+
},
337+
},
338+
},
339+
},
340+
},
341+
},
342+
},
343+
wantErr: false,
344+
},
180345
}
181346

182347
for _, tc := range tests {

0 commit comments

Comments
 (0)