Fix machine config panic when ref object not found (#8527)

* Fix machine config panic when ref object not found Signed-off-by: Rahul Ganesh <rahulgab@amazon.com> * combine two validations in single for loop Signed-off-by: Rahul Ganesh <rahulgab@amazon.com> --------- Signed-off-by: Rahul Ganesh <rahulgab@amazon.com> Co-authored-by: Rahul Ganesh <rahulgab@amazon.com>
aws · Jul 23, 2024 · 71a3e4e · 71a3e4e
1 parent ab153a2
commit 71a3e4e
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 10 deletions.
diff --git a/pkg/cluster/cloudstack.go b/pkg/cluster/cloudstack.go
@@ -46,9 +46,15 @@ func cloudstackEntry() *ConfigManagerEntry {
 				return nil
 			},
 			func(c *Config) error {
-				for _, m := range c.CloudStackMachineConfigs {
-					if err := m.Validate(); err != nil {
-						return err
+				if c.CloudStackMachineConfigs != nil { // We need this conditional check as CloudStackMachineConfigs will be nil for other providers
+					for _, mcRef := range c.Cluster.MachineConfigRefs() {
+						m, ok := c.CloudStackMachineConfigs[mcRef.Name]
+						if !ok {
+							return fmt.Errorf("CloudStackMachineConfig %s not found", mcRef.Name)
+						}
+						if err := m.Validate(); err != nil {
+							return err
+						}
 					}
 				}
 				return nil

diff --git a/pkg/cluster/cloudstack_test.go b/pkg/cluster/cloudstack_test.go
@@ -31,6 +31,16 @@ func TestValidateCloudStackDatacenterNotFoundError(t *testing.T) {
 	g.Expect(err).To(MatchError(ContainSubstring("CloudStackDatacenterConfig eksa-unit-test not found")))
 }
 
+func TestValidateCloudStackMachineConfigNotFoundError(t *testing.T) {
+	g := NewWithT(t)
+	got, _ := cluster.ParseConfigFromFile("testdata/cluster_1_20_cloudstack.yaml")
+	got.Cluster.Spec.ControlPlaneConfiguration.MachineGroupRef.Name = "dummy-machine-config"
+
+	cm, _ := cluster.NewDefaultConfigManager()
+	err := cm.Validate(got)
+	g.Expect(err).To(MatchError(ContainSubstring("CloudStackMachineConfig dummy-machine-config not found")))
+}
+
 func TestDefaultConfigClientBuilderBuildCloudStackClusterSuccess(t *testing.T) {
 	g := NewWithT(t)
 	ctx := context.Background()

diff --git a/pkg/cluster/nutanix.go b/pkg/cluster/nutanix.go
@@ -39,6 +39,16 @@ func nutanixEntry() *ConfigManagerEntry {
 				}
 				return nil
 			},
+			func(c *Config) error {
+				if c.NutanixMachineConfigs != nil { // We need this conditional check as NutanixMachineConfigs will be nil for other providers
+					for _, mcRef := range c.Cluster.MachineConfigRefs() {
+						if _, ok := c.NutanixMachineConfigs[mcRef.Name]; !ok {
+							return fmt.Errorf("NutanixMachineConfig %s not found", mcRef.Name)
+						}
+					}
+				}
+				return nil
+			},
 		},
 	}
 }

diff --git a/pkg/cluster/nutanix_test.go b/pkg/cluster/nutanix_test.go
@@ -62,6 +62,12 @@ func TestValidateNutanixEntry(t *testing.T) {
 	err = cm.Validate(config)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "NutanixDatacenterConfig eksa-unit-test not found")
+
+	missingMachineconfig := "dummy-machine-config"
+	config.Cluster.Spec.ControlPlaneConfiguration.MachineGroupRef.Name = missingMachineconfig
+	err = cm.Validate(config)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), fmt.Sprintf("NutanixMachineConfig %s not found", missingMachineconfig))
 }
 
 func TestNutanixConfigClientBuilder(t *testing.T) {

diff --git a/pkg/cluster/testdata/cluster_tinkerbell_1_19.yaml b/pkg/cluster/testdata/cluster_tinkerbell_1_19.yaml
@@ -56,6 +56,22 @@ spec:
       sshAuthorizedKeys:
         - "ssh-rsa AAAAB3"
 
+---
+apiVersion: anywhere.eks.amazonaws.com/v1alpha1
+kind: TinkerbellMachineConfig
+metadata:
+  name: test-md
+  namespace: test-namespace
+spec:
+  osFamily: ubuntu
+  templateRef:
+    kind: TinkerbellTemplateConfig
+    name: tink-test
+  users:
+    - name: tink-user
+      sshAuthorizedKeys:
+        - "ssh-rsa AAAAB3"
+
 ---
 apiVersion: anywhere.eks.amazonaws.com/v1alpha1
 kind: TinkerbellTemplateConfig

diff --git a/pkg/cluster/tinkerbell.go b/pkg/cluster/tinkerbell.go
@@ -40,9 +40,15 @@ func tinkerbellEntry() *ConfigManagerEntry {
 				return nil
 			},
 			func(c *Config) error {
-				for _, t := range c.TinkerbellMachineConfigs {
-					if err := validateSameNamespace(c, t); err != nil {
-						return err
+				if c.TinkerbellMachineConfigs != nil { // We need this conditional check as TinkerbellMachineConfigs will be nil for other providers
+					for _, mcRef := range c.Cluster.MachineConfigRefs() {
+						m, ok := c.TinkerbellMachineConfigs[mcRef.Name]
+						if !ok {
+							return fmt.Errorf("TinkerbellMachineConfig %s not found", mcRef.Name)
+						}
+						if err := validateSameNamespace(c, m); err != nil {
+							return err
+						}
 					}
 				}
 				return nil

diff --git a/pkg/cluster/tinkerbell_test.go b/pkg/cluster/tinkerbell_test.go
@@ -88,7 +88,7 @@ func TestParseConfigFromFileTinkerbellCluster(t *testing.T) {
 		},
 	))
 
-	g.Expect(got.TinkerbellMachineConfigs).To(HaveLen(1))
+	g.Expect(got.TinkerbellMachineConfigs).To(HaveLen(2))
 	g.Expect(got.TinkerbellMachineConfigs["test-cp"]).To(
 		BeComparableTo(
 			&anywherev1.TinkerbellMachineConfig{
@@ -116,6 +116,33 @@ func TestParseConfigFromFileTinkerbellCluster(t *testing.T) {
 			},
 		),
 	)
+	g.Expect(got.TinkerbellMachineConfigs["test-md"]).To(
+		BeComparableTo(
+			&anywherev1.TinkerbellMachineConfig{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "TinkerbellMachineConfig",
+					APIVersion: "anywhere.eks.amazonaws.com/v1alpha1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-md",
+					Namespace: "test-namespace",
+				},
+				Spec: anywherev1.TinkerbellMachineConfigSpec{
+					TemplateRef: anywherev1.Ref{
+						Kind: "TinkerbellTemplateConfig",
+						Name: "tink-test",
+					},
+					OSFamily: "ubuntu",
+					Users: []anywherev1.UserConfiguration{
+						{
+							Name:              "tink-user",
+							SshAuthorizedKeys: []string{"ssh-rsa AAAAB3"},
+						},
+					},
+				},
+			},
+		),
+	)
 
 	g.Expect(got.TinkerbellTemplateConfigs).To(HaveLen(1))
 	g.Expect(got.TinkerbellTemplateConfigs["tink-test"]).To(
@@ -173,6 +200,16 @@ func TestValidateTinkerbellDatacenterNotFoundError(t *testing.T) {
 	g.Expect(err).To(MatchError(ContainSubstring("TinkerbellDatacenterConfig test not found")))
 }
 
+func TestValidateTinkerbellMachineConfigNotFoundError(t *testing.T) {
+	g := NewWithT(t)
+	got, _ := cluster.ParseConfigFromFile("testdata/cluster_tinkerbell_1_19.yaml")
+	got.Cluster.Spec.ControlPlaneConfiguration.MachineGroupRef.Name = "dummy-machine-config"
+
+	cm, _ := cluster.NewDefaultConfigManager()
+	err := cm.Validate(got)
+	g.Expect(err).To(MatchError(ContainSubstring("TinkerbellMachineConfig dummy-machine-config not found")))
+}
+
 func TestDefaultConfigClientBuilderTinkerbellCluster(t *testing.T) {
 	g := NewWithT(t)
 	ctx := context.Background()

diff --git a/pkg/cluster/vsphere.go b/pkg/cluster/vsphere.go
@@ -47,9 +47,15 @@ func vsphereEntry() *ConfigManagerEntry {
 				return nil
 			},
 			func(c *Config) error {
-				for _, m := range c.VSphereMachineConfigs {
-					if err := m.Validate(); err != nil {
-						return err
+				if c.VSphereMachineConfigs != nil { // We need this conditional check as VSphereMachineConfigs will be nil for other providers
+					for _, mcRef := range c.Cluster.MachineConfigRefs() {
+						m, ok := c.VSphereMachineConfigs[mcRef.Name]
+						if !ok {
+							return fmt.Errorf("VSphereMachineConfig %s not found", mcRef.Name)
+						}
+						if err := m.Validate(); err != nil {
+							return err
+						}
 					}
 				}
 				return nil

diff --git a/pkg/cluster/vsphere_test.go b/pkg/cluster/vsphere_test.go
@@ -33,6 +33,16 @@ func TestValidateVSphereDatacenterNotFoundError(t *testing.T) {
 	g.Expect(err).To(MatchError(ContainSubstring("VSphereDatacenterConfig eksa-unit-test2 not found")))
 }
 
+func TestValidateVsphereMachineConfigNotFoundError(t *testing.T) {
+	g := NewWithT(t)
+	got, _ := cluster.ParseConfigFromFile("testdata/cluster_1_19.yaml")
+	got.Cluster.Spec.ControlPlaneConfiguration.MachineGroupRef.Name = "dummy-machine-config"
+
+	cm, _ := cluster.NewDefaultConfigManager()
+	err := cm.Validate(got)
+	g.Expect(err).To(MatchError(ContainSubstring("VSphereMachineConfig dummy-machine-config not found")))
+}
+
 func TestDefaultConfigClientBuilderVSphereCluster(t *testing.T) {
 	g := NewWithT(t)
 	ctx := context.Background()