fix: correct merge logic for conditional rules. Add userData merge logic

Signed-off-by: Jonah Back <[email protected]>
keikoproj · May 2, 2021 · d7367d5 · d7367d5
1 parent 06da69c
commit d7367d5
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 7 deletions.
diff --git a/controllers/provisioners/config.go b/controllers/provisioners/config.go
@@ -34,12 +34,14 @@ var (
 	EKSTagsPath           = fmt.Sprintf("%v.tags", EKSConfigurationPath)
 	EKSVolumesPath        = fmt.Sprintf("%v.volumes", EKSConfigurationPath)
 	EKSLifecycleHooksPath = fmt.Sprintf("%v.lifecycleHooks", EKSConfigurationPath)
+	EKSUserDataPath       = fmt.Sprintf("%v.userData", EKSConfigurationPath)
 
 	// MergeSchema defines the key to merge by
 	MergeSchema = map[string]string{
 		EKSTagsPath:           "key",
 		EKSVolumesPath:        "name",
 		EKSLifecycleHooksPath: "name",
+		EKSUserDataPath:       "name",
 	}
 )
 
@@ -255,13 +257,15 @@ func (c *ProvisionerConfiguration) setSharedFields(obj map[string]interface{}) e
 		for _, conditional := range applicableConditionals {
 			conditionalValue := common.FieldValue(pathStr, conditional.Defaults)
 			if conditionalValue != nil {
-				if isConflict(conditionalValue, resourceVal) {
-					merge := Merge(conditionalValue, resourceVal, pathStr, false)
+				if isConflict(conditionalValue, defaultVal) {
+					merge := Merge(conditionalValue, defaultVal, pathStr, false)
 					if err := common.SetFieldValue(pathStr, obj, merge); err != nil {
 						return errors.Wrap(err, "failed to merge field")
 					}
-					resourceVal = merge
+					defaultVal = merge
 					continue
+				} else {
+					defaultVal = conditionalValue
 				}
 			}
 		}
@@ -291,7 +295,17 @@ func (c *ProvisionerConfiguration) setSharedFields(obj map[string]interface{}) e
 		for _, conditional := range applicableConditionals {
 			conditionalValue := common.FieldValue(pathStr, conditional.Defaults)
 			if conditionalValue != nil {
-				defaultVal = conditionalValue
+				if isConflict(conditionalValue, defaultVal) {
+					//Merge conditional into default, with conditional overriding any conflicting values.
+					merge := Merge(conditionalValue,defaultVal, pathStr, false)
+					if err := common.SetFieldValue(pathStr, obj, merge); err != nil {
+						return errors.Wrap(err, "failed to merge field")
+					}
+					defaultVal = merge
+					continue
+				} else {
+					defaultVal = conditionalValue
+				}
 			}
 		}
 

diff --git a/controllers/provisioners/config_test.go b/controllers/provisioners/config_test.go
@@ -344,19 +344,35 @@ func TestSetDefaultsWithSharedConditionalMergeOverride(t *testing.T) {
 	mockBoundaries := `
     shared:
       mergeOverride: 
+      - spec.eks.configuration.userData
       - spec.eks.configuration.tags`
-
 	mockConditionals := `
 - annotationSelector: "instancemgr.keikoproj.io/arch in (arm64),instancemgr.keikoproj.io/os-family in (windows)"
   defaults:
     spec:
       eks:
         configuration:
+          userData: 
+          - data: |
+              [settings.bootstrap-containers.warm]
+              source = "docker:image"
+              mode = "always"
+              essential = "true"
+            name: warm
+            stage: PostBootstrap
           tags:
           - key: tag-A
             value: value-A
           - key: tag-B
-            value: value-B`
+            value: value-B
+- annotationSelector: "instancemgr.keikoproj.io/arch in (arm64),instancemgr.keikoproj.io/os-family in (windows)"
+  defaults:
+    spec:
+      eks:
+        configuration:
+          tags:
+          - key: tag-X
+            value: value-X`
 	mockDefaults := `
 spec:
   strategy:
@@ -396,10 +412,12 @@ spec:
 	//Should not replace
 	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.NodeSecurityGroups).To(gomega.Equal([]string{"sg-000000000000"}))
 	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.Tags).To(gomega.Equal([]map[string]string{
+		MockTag("tag-X", "value-X"),
 		MockTag("tag-A", "value-D"),
 		MockTag("tag-B", "value-B"),
 		MockTag("tag-C", "value-C"),
 	}))
+	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.UserData).To(gomega.HaveLen(1))
 }
 
 func TestSetDefaultsWithInvalidConditionalYAML(t *testing.T) {
@@ -545,8 +563,8 @@ spec:
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 	//Should not replace
 	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.NodeSecurityGroups).To(gomega.Equal([]string{
-		"sg-1234",
 		"sg-923456789012",
+		"sg-1234",
 		"sg-000000000000",
 	}))
 }