Repository: keikoproj/instance-manager (go)

Issue:
Changing a boundary does not roll back changes on CRs
When using the admin config map, and changing a specific boundary, the changes does not seem to take effect.

e.g.
If `spec.eks.configuration.suspendProcesses` is a merge field, and a CR is using AZRebalance as suspendProcesses, once the boundary is changed to restricted (and there is no default value for suspendProcesses), the ASG remains with AZRebalance enabled.

Ideally, once the boundary is changed, the change should be reconciled across all Custom Resources.

Pull request:
Enforce Restricted Fields usage when no default value
Signed-off-by: Eytan Avisror <eytan_avisror@intuit.com>

Fixes #341 

This fixes a gap in our admin configuration where we allow CRs to use restricted fields if there are no defaults.
This change sets field to Nil if there is no default value provided and field is a restricted field.

Patch:
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index 83dce64..90f591c 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -1,4 +1,3 @@
-//go:build !ignore_autogenerated
 // +build !ignore_autogenerated
 
 /*
diff --git a/controllers/provisioners/config.go b/controllers/provisioners/config.go
index a173d10..7e46294 100644
--- 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")
 			}
 		}
 	}


Test patch:
diff --git a/controllers/provisioners/config_test.go b/controllers/provisioners/config_test.go
index 41397d0..3cc74e8 100644
--- 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")}))