Enforce Restricted Fields usage when no default value (#345)

* Enforce Restricted Fields usage when no default value Signed-off-by: Eytan Avisror <eytan_avisror@intuit.com> * Update zz_generated.deepcopy.go Signed-off-by: Eytan Avisror <eytan_avisror@intuit.com> * Add return to errors.Wrap() Signed-off-by: Eytan Avisror <eytan_avisror@intuit.com>
keikoproj · Feb 7, 2022 · bda2adc · bda2adc
1 parent c321140
commit bda2adc
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 5 deletions.
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/controllers/provisioners/config.go b/controllers/provisioners/config.go
@@ -17,9 +17,10 @@ package provisioners
 
 import (
 	"fmt"
-	"k8s.io/apimachinery/pkg/labels"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/labels"
+
 	"github.com/ghodss/yaml"
 	"github.com/keikoproj/instance-manager/api/v1alpha1"
 	"github.com/keikoproj/instance-manager/controllers/common"
@@ -176,7 +177,7 @@ func (c *ProvisionerConfiguration) setRestrictedFields(unstructuredInstanceGroup
 				setFieldInAnyConditional = true
 				err := unstructured.SetNestedField(unstructuredInstanceGroup, field, path...)
 				if err != nil {
-					errors.Wrap(err, "failed to set nested field")
+					return errors.Wrap(err, "failed to set nested field")
 				}
 			}
 		}
@@ -187,7 +188,13 @@ func (c *ProvisionerConfiguration) setRestrictedFields(unstructuredInstanceGroup
 			// default value exists for restricted path
 			err := unstructured.SetNestedField(unstructuredInstanceGroup, field, path...)
 			if err != nil {
-				errors.Wrap(err, "failed to set nested field")
+				return errors.Wrap(err, "failed to set nested field")
+			}
+		} else {
+			// if no default value exist, make sure to ignore CR value for restricted fields
+			err := unstructured.SetNestedField(unstructuredInstanceGroup, nil, path...)
+			if err != nil {
+				return errors.Wrap(err, "failed to set nested field to nil")
 			}
 		}
 	}

diff --git a/controllers/provisioners/config_test.go b/controllers/provisioners/config_test.go
@@ -102,7 +102,8 @@ func TestSetDefaultsRestricted(t *testing.T) {
     - spec.eks.configuration.labels
     - spec.eks.configuration.securityGroups
     - spec.eks.configuration.instanceType
-    - spec.strategy`
+    - spec.strategy
+    - spec.eks.configuration.suspendProcesses`
 
 	mockConditionals := `
 - annotationSelector: "instancemgr.keikoproj.io/os-family=windows"
@@ -135,6 +136,7 @@ spec:
 
 	cm := MockConfigMap(MockConfigData("boundaries", mockBoundaries, "defaults", mockDefaults, "conditionals", mockConditionals))
 	cr := MockResource()
+	cr.Spec.EKSSpec.EKSConfiguration.SuspendedProcesses = []string{"AZRebalance"}
 	cr.Spec.EKSSpec.EKSConfiguration.EksClusterName = "someCluster"
 	cr.Spec.EKSSpec.EKSConfiguration.NodeSecurityGroups = []string{"sg-000000000000"}
 	cr.Spec.EKSSpec.EKSConfiguration.InstanceType = "m5.xlarge"
@@ -155,6 +157,7 @@ spec:
 		},
 	}))
 	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.NodeSecurityGroups).To(gomega.Equal([]string{"sg-123456789012"}))
+	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.SuspendedProcesses).To(gomega.BeNil())
 	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.InstanceType).To(gomega.Equal("m5.large"))
 	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.Labels).To(gomega.Equal(MockLabels("label-key", "label-value")))
 	g.Expect(c.InstanceGroup.Spec.EKSSpec.EKSConfiguration.Taints).To(gomega.Equal([]corev1.Taint{MockTaint("taint-key", "taint-value", "NoSchedule")}))