Skip to content

Commit 1ed970a

Browse files
authored
Fix nil pointer when fetching snowmachineconfig that does not exist (#3795)
1 parent bcab213 commit 1ed970a

4 files changed

Lines changed: 71 additions & 0 deletions

File tree

pkg/cluster/snow.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ func snowEntry() *ConfigManagerEntry {
7070
}
7171
return nil
7272
},
73+
func(c *Config) error {
74+
return ValidateSnowMachineRefExists(c)
75+
},
7376
},
7477
}
7578
}
@@ -192,3 +195,14 @@ func SetSnowDatacenterIndentityRefDefault(s *anywherev1.SnowDatacenterConfig) {
192195
}
193196
}
194197
}
198+
199+
// ValidateSnowMachineRefExists checks the cluster spec machine refs and makes sure
200+
// the snowmachineconfig object exists for each ref with kind == snowmachineconfig.
201+
func ValidateSnowMachineRefExists(c *Config) error {
202+
for _, machineRef := range c.Cluster.MachineConfigRefs() {
203+
if machineRef.Kind == anywherev1.SnowMachineConfigKind && c.SnowMachineConfig(machineRef.Name) == nil {
204+
return fmt.Errorf("unable to find SnowMachineConfig %s", machineRef.Name)
205+
}
206+
}
207+
return nil
208+
}

pkg/cluster/snow_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,3 +311,39 @@ func TestSetSnowDatacenterIndentityRefDefault(t *testing.T) {
311311
})
312312
}
313313
}
314+
315+
func TestValidateSnowMachineRefExistsError(t *testing.T) {
316+
g := NewWithT(t)
317+
c := &cluster.Config{
318+
Cluster: &anywherev1.Cluster{
319+
TypeMeta: metav1.TypeMeta{
320+
Kind: anywherev1.ClusterKind,
321+
APIVersion: anywherev1.SchemeBuilder.GroupVersion.String(),
322+
},
323+
ObjectMeta: metav1.ObjectMeta{
324+
Name: "eksa-unit-test",
325+
Namespace: "ns-1",
326+
},
327+
Spec: anywherev1.ClusterSpec{
328+
WorkerNodeGroupConfigurations: []anywherev1.WorkerNodeGroupConfiguration{
329+
{
330+
MachineGroupRef: &anywherev1.Ref{
331+
Name: "worker-not-exists",
332+
Kind: "SnowMachineConfig",
333+
},
334+
},
335+
},
336+
},
337+
},
338+
SnowMachineConfigs: map[string]*anywherev1.SnowMachineConfig{
339+
"worker-1": {
340+
ObjectMeta: metav1.ObjectMeta{
341+
Name: "worker-1",
342+
},
343+
},
344+
},
345+
}
346+
g.Expect(cluster.ValidateConfig(c)).To(
347+
MatchError(ContainSubstring("unable to find SnowMachineConfig worker-not-exists")),
348+
)
349+
}

pkg/providers/snow/reconciler/reconciler.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@ func (r *Reconciler) ValidateMachineConfigs(ctx context.Context, log logr.Logger
7272
}
7373
}
7474

75+
if err := cluster.ValidateSnowMachineRefExists(clusterSpec.Config); err != nil {
76+
log.Error(err, "Invalid SnowMachineConfig")
77+
failureMessage := err.Error()
78+
clusterSpec.Cluster.Status.FailureMessage = &failureMessage
79+
return controller.Result{}, err
80+
}
81+
7582
return controller.Result{}, nil
7683
}
7784

pkg/providers/snow/reconciler/reconciler_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,20 @@ func TestReconcilerValidateMachineConfigsInvalidControlPlaneMachineConfig(t *tes
8787
tt.Expect(*tt.cluster.Status.FailureMessage).To(ContainSubstring("Something wrong"))
8888
}
8989

90+
func TestReconcilerValidateMachineConfigsSnowMachineRefNotExists(t *testing.T) {
91+
tt := newReconcilerTest(t)
92+
tt.withFakeClient()
93+
94+
spec := tt.buildSpec()
95+
spec.Cluster.Spec.ControlPlaneConfiguration.MachineGroupRef.Name = "cp-machine-not-exists"
96+
97+
_, err := tt.reconciler().ValidateMachineConfigs(tt.ctx, test.NewNullLogger(), spec)
98+
99+
tt.Expect(err).To(MatchError(ContainSubstring("unable to find SnowMachineConfig cp-machine-not-exists")))
100+
tt.Expect(tt.cluster.Status.FailureMessage).ToNot(BeZero())
101+
tt.Expect(*tt.cluster.Status.FailureMessage).To(ContainSubstring("unable to find SnowMachineConfig cp-machine-not-exists"))
102+
}
103+
90104
func TestReconcilerReconcileWorkers(t *testing.T) {
91105
tt := newReconcilerTest(t)
92106
tt.createAllObjs()

0 commit comments

Comments
 (0)