Skip to content

Commit 69ac5fa

Browse files
supershalsupershal
committed
fix: nil check for all references
1 parent 1199ae4 commit 69ac5fa

File tree

2 files changed

+200
-13
lines changed

2 files changed

+200
-13
lines changed

‎pkg/controllers/namespacesync/references.go‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,28 +57,36 @@ func walkReferences(
5757
ref *corev1.ObjectReference,
5858
) error,
5959
) error {
60-
for _, ref := range []*corev1.ObjectReference{
61-
cc.Spec.Infrastructure.Ref,
62-
cc.Spec.ControlPlane.Ref,
63-
} {
64-
if err := fn(ctx, ref); err != nil {
60+
if cc == nil {
61+
return nil
62+
}
63+
if cc.Spec.Infrastructure.Ref != nil {
64+
if err := fn(ctx, cc.Spec.Infrastructure.Ref); err != nil {
65+
return err
66+
}
67+
}
68+
69+
if cc.Spec.ControlPlane.Ref != nil {
70+
if err := fn(ctx, cc.Spec.ControlPlane.Ref); err != nil {
6571
return err
6672
}
6773
}
68-
// Managed kubernetes providers like EKS and AKS will not have a MachineInfrastructure reference for control plane.
69-
if cpInfra := cc.Spec.ControlPlane.MachineInfrastructure; cpInfra != nil {
74+
75+
if cpInfra := cc.Spec.ControlPlane.MachineInfrastructure; cpInfra != nil && cpInfra.Ref != nil {
7076
if err := fn(ctx, cpInfra.Ref); err != nil {
7177
return err
7278
}
7379
}
80+
7481
for mdIdx := range cc.Spec.Workers.MachineDeployments {
7582
md := &cc.Spec.Workers.MachineDeployments[mdIdx]
76-
77-
for _, ref := range []*corev1.ObjectReference{
78-
md.Template.Infrastructure.Ref,
79-
md.Template.Bootstrap.Ref,
80-
} {
81-
if err := fn(ctx, ref); err != nil {
83+
if md.Template.Infrastructure.Ref != nil {
84+
if err := fn(ctx, md.Template.Infrastructure.Ref); err != nil {
85+
return err
86+
}
87+
}
88+
if md.Template.Bootstrap.Ref != nil {
89+
if err := fn(ctx, md.Template.Bootstrap.Ref); err != nil {
8290
return err
8391
}
8492
}

‎pkg/controllers/namespacesync/references_test.go‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// Copyright 2024 Nutanix. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package namespacesync
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
. "github.com/onsi/gomega"
11+
corev1 "k8s.io/api/core/v1"
12+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
13+
14+
"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/internal/test/builder"
15+
)
16+
17+
func TestWalkReferences(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
clusterClass *clusterv1.ClusterClass
21+
}{
22+
{
23+
name: "nil ClusterClass should return nil without calling callback",
24+
clusterClass: nil,
25+
},
26+
{
27+
name: "empty ClusterClass with no template references",
28+
clusterClass: builder.ClusterClass("default", "test-cc").Build(),
29+
},
30+
{
31+
name: "ClusterClass with Infrastructure cluster template reference only",
32+
clusterClass: builder.ClusterClass("default", "test-cc").
33+
WithInfrastructureClusterTemplate(
34+
builder.InfrastructureClusterTemplate("default", "infra-template").Build(),
35+
).Build(),
36+
},
37+
{
38+
name: "ClusterClass with ControlPlane template reference only",
39+
clusterClass: builder.ClusterClass("default", "test-cc").
40+
WithControlPlaneTemplate(
41+
builder.ControlPlaneTemplate("default", "cp-template").Build(),
42+
).Build(),
43+
},
44+
{
45+
name: "ClusterClass with MachineInfrastructure template reference",
46+
clusterClass: builder.ClusterClass("default", "test-cc").
47+
WithControlPlaneInfrastructureMachineTemplate(
48+
builder.InfrastructureMachineTemplate("default", "cp-machine-template").Build(),
49+
).Build(),
50+
},
51+
{
52+
name: "ClusterClass with MachineDeployments template references",
53+
clusterClass: builder.ClusterClass("default", "test-cc").
54+
WithWorkerMachineDeploymentClasses(
55+
*builder.MachineDeploymentClass("worker-1").
56+
WithInfrastructureTemplate(
57+
builder.InfrastructureMachineTemplate("default", "worker-infra-template").Build(),
58+
).
59+
WithBootstrapTemplate(
60+
builder.BootstrapTemplate("default", "worker-bootstrap-template").Build(),
61+
).Build(),
62+
).Build(),
63+
},
64+
{
65+
name: "ClusterClass with MachineDeployments having nil Infrastructure template reference",
66+
clusterClass: builder.ClusterClass("default", "test-cc").
67+
WithWorkerMachineDeploymentClasses(
68+
*builder.MachineDeploymentClass("worker-1").
69+
WithBootstrapTemplate(
70+
builder.BootstrapTemplate("default", "worker-bootstrap-template").Build(),
71+
).Build(),
72+
).Build(),
73+
},
74+
{
75+
name: "ClusterClass with MachineDeployments having nil Bootstrap template reference",
76+
clusterClass: builder.ClusterClass("default", "test-cc").
77+
WithWorkerMachineDeploymentClasses(
78+
*builder.MachineDeploymentClass("worker-1").
79+
WithInfrastructureTemplate(
80+
builder.InfrastructureMachineTemplate("default", "worker-infra-template").Build(),
81+
).Build(),
82+
).Build(),
83+
},
84+
{
85+
name: "callback function returns error",
86+
clusterClass: builder.ClusterClass("default", "test-cc").
87+
WithInfrastructureClusterTemplate(
88+
builder.InfrastructureClusterTemplate("default", "infra-template").Build(),
89+
).Build(),
90+
},
91+
{
92+
name: "complete ClusterClass with all template references",
93+
clusterClass: builder.ClusterClass("default", "test-cc").
94+
WithInfrastructureClusterTemplate(
95+
builder.InfrastructureClusterTemplate("default", "infra-template").Build(),
96+
).
97+
WithControlPlaneTemplate(
98+
builder.ControlPlaneTemplate("default", "cp-template").Build(),
99+
).
100+
WithControlPlaneInfrastructureMachineTemplate(
101+
builder.InfrastructureMachineTemplate("default", "cp-machine-template").Build(),
102+
).
103+
WithWorkerMachineDeploymentClasses(
104+
*builder.MachineDeploymentClass("worker-1").
105+
WithInfrastructureTemplate(
106+
builder.InfrastructureMachineTemplate("default", "worker-infra-template").Build(),
107+
).
108+
WithBootstrapTemplate(
109+
builder.BootstrapTemplate("default", "worker-bootstrap-template").Build(),
110+
).Build(),
111+
*builder.MachineDeploymentClass("worker-2").
112+
WithInfrastructureTemplate(
113+
builder.InfrastructureMachineTemplate("default", "worker2-infra-template").Build(),
114+
).
115+
WithBootstrapTemplate(
116+
builder.BootstrapTemplate("default", "worker2-bootstrap-template").Build(),
117+
).Build(),
118+
).Build(),
119+
},
120+
}
121+
122+
for _, tt := range tests {
123+
t.Run(tt.name, func(t *testing.T) {
124+
t.Parallel()
125+
g := NewWithT(t)
126+
ctx := context.Background()
127+
var capturedRefs []*corev1.ObjectReference
128+
129+
callback := func(ctx context.Context, ref *corev1.ObjectReference) error {
130+
capturedRefs = append(capturedRefs, ref)
131+
return nil
132+
}
133+
134+
err := walkReferences(ctx, tt.clusterClass, callback)
135+
136+
g.Expect(err).ToNot(HaveOccurred())
137+
138+
// Verify that captured references match expected ones
139+
if tt.clusterClass != nil {
140+
expectedRefs := collectExpectedRefs(tt.clusterClass)
141+
g.Expect(capturedRefs).
142+
To(HaveLen(len(expectedRefs)), "Expected %d references, got %d", len(expectedRefs), len(capturedRefs))
143+
144+
for i, expectedRef := range expectedRefs {
145+
if expectedRef != nil {
146+
g.Expect(capturedRefs[i]).To(Equal(expectedRef), "Reference doesn't match")
147+
}
148+
}
149+
}
150+
})
151+
}
152+
}
153+
154+
func collectExpectedRefs(cc *clusterv1.ClusterClass) []*corev1.ObjectReference {
155+
var refs []*corev1.ObjectReference
156+
157+
if cc.Spec.Infrastructure.Ref != nil {
158+
refs = append(refs, cc.Spec.Infrastructure.Ref)
159+
}
160+
161+
if cc.Spec.ControlPlane.Ref != nil {
162+
refs = append(refs, cc.Spec.ControlPlane.Ref)
163+
}
164+
165+
if cpInfra := cc.Spec.ControlPlane.MachineInfrastructure; cpInfra != nil && cpInfra.Ref != nil {
166+
refs = append(refs, cpInfra.Ref)
167+
}
168+
169+
for _, md := range cc.Spec.Workers.MachineDeployments {
170+
if md.Template.Infrastructure.Ref != nil {
171+
refs = append(refs, md.Template.Infrastructure.Ref)
172+
}
173+
if md.Template.Bootstrap.Ref != nil {
174+
refs = append(refs, md.Template.Bootstrap.Ref)
175+
}
176+
}
177+
178+
return refs
179+
}

0 commit comments

Comments
 (0)