Merge pull request #146 from opendevstack/fix/omit-legacy-fields

Remove legacy fields /userNames and /groupNames

Author: Michael Sauter

internal/test/fixtures/item-omitted-fields/rolebinding-platform.yml

@@ -0,0 +1,16 @@
+apiVersion: authorization.openshift.io/v1
+groupNames:
+- dedicated-admins
+- system:serviceaccounts:dedicated-admin
+kind: RoleBinding
+metadata:
+  creationTimestamp: null
+  name: admin-0
+roleRef:
+  name: admin
+subjects:
+- kind: Group
+  name: dedicated-admins
+- kind: SystemGroup
+  name: system:serviceaccounts:dedicated-admin
+userNames: null

internal/test/fixtures/item-omitted-fields/rolebinding-template.yml

@@ -0,0 +1,13 @@
+apiVersion: authorization.openshift.io/v1
+groupNames: null
+kind: RoleBinding
+metadata:
+  name: admin-0
+roleRef:
+  name: admin
+subjects:
+- kind: ServiceAccount
+  name: jenkins
+  namespace: foo-cd
+userNames:
+- system:serviceaccount:foo-cd:jenkins

internal/test/golden/export/rolebinding-generate-name.yml

@@ -2,7 +2,6 @@ apiVersion: template.openshift.io/v1
 kind: Template
 objects:
 - apiVersion: authorization.openshift.io/v1
-  groupNames: null
   kind: RoleBinding
   metadata:
     generateName: system:image-pusher-
@@ -12,5 +11,3 @@ objects:
   - kind: ServiceAccount
     name: default
     namespace: foo-dev
-  userNames:
-  - system:serviceaccount:foo-dev:default

internal/test/golden/item-omitted-fields/rolebinding-changed.txt

@@ -0,0 +1,13 @@
+--- Current State (OpenShift cluster)
++++ Desired State (Processed template)
+@@ -5,8 +5,7 @@
+ roleRef:
+   name: admin
+ subjects:
+-- kind: Group
+-  name: dedicated-admins
+-- kind: SystemGroup
+-  name: system:serviceaccounts:dedicated-admin
++- kind: ServiceAccount
++  name: jenkins
++  namespace: foo-cd

pkg/openshift/changeset_test.go

@@ -333,6 +333,79 @@ func TestCalculateChangesAppliedConfiguration(t *testing.T) {
 	}
 }
 
+func TestCalculateChangesOmittedFields(t *testing.T) {
+
+	tests := map[string]struct {
+		platformFixture        string
+		templateFixture        string
+		expectedAction         string
+		expectedPatches        jsonPatches
+		expectedDiffGoldenFile string
+	}{
+		"Rolebinding with legacy fields": {
+			platformFixture: "rolebinding-platform",
+			templateFixture: "rolebinding-template",
+			expectedAction:  "Update",
+			expectedPatches: jsonPatches{
+				&jsonPatch{
+					Op:    "replace",
+					Path:  "/subjects/0/kind",
+					Value: "ServiceAccount",
+				},
+				&jsonPatch{
+					Op:    "replace",
+					Path:  "/subjects/0/name",
+					Value: "jenkins",
+				},
+				&jsonPatch{
+					Op:    "add",
+					Path:  "/subjects/0/namespace",
+					Value: "foo-cd",
+				},
+				&jsonPatch{
+					Op:   "remove",
+					Path: "/subjects/1",
+				},
+			},
+			expectedDiffGoldenFile: "rolebinding-changed",
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			platformItem := getPlatformItem(t, "item-omitted-fields/"+tc.platformFixture+".yml")
+			templateItem := getTemplateItem(t, "item-omitted-fields/"+tc.templateFixture+".yml")
+			changes, err := calculateChanges(templateItem, platformItem, []string{}, true)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(changes) != 1 {
+				t.Fatalf("Expected 1 change, got: %d", len(changes))
+			}
+			actualChange := changes[0]
+			if actualChange.Action != tc.expectedAction {
+				t.Fatalf("Expected change action to be: %s, got: %s. Patches: \n%s", tc.expectedAction, actualChange.Action, actualChange.PrettyJSONPatches())
+			}
+			if len(actualChange.Patches) != len(tc.expectedPatches) {
+				t.Fatalf("Expected patches:\n%s\n--- got: ---\n%s", pretty(tc.expectedPatches), actualChange.PrettyJSONPatches())
+			}
+			for i, got := range actualChange.Patches {
+				want := tc.expectedPatches[i]
+				if diff := cmp.Diff(want, got); diff != "" {
+					t.Errorf("Change diff mismatch (-want +got):\n%s", diff)
+				}
+			}
+			if len(tc.expectedDiffGoldenFile) > 0 {
+				want := strings.TrimSpace(getGoldenDiff(t, "item-omitted-fields", tc.expectedDiffGoldenFile+".txt"))
+				got := strings.TrimSpace(actualChange.Diff(true))
+				if diff := cmp.Diff(want, got); diff != "" {
+					t.Errorf("Change diff mismatch (-want +got):\n%s", diff)
+				}
+			}
+		})
+	}
+}
+
 func TestEmptyValuesDoNotCauseDrift(t *testing.T) {
 	platformItem := getPlatformItem(t, "empty-values/bc-platform.yml")
 	templateItem := getTemplateItem(t, "empty-values/bc-template.yml")

pkg/openshift/item.go

@@ -30,6 +30,8 @@ var (
 		"/status",
 		"/spec/volumeName",
 		"/spec/template/metadata/creationTimestamp",
+		"/groupNames",
+		"/userNames",
 	}
 	platformManagedRegexFields = []string{
 		"^/spec/triggers/[0-9]*/imageChangeParams/lastTriggeredImage",
@@ -246,9 +248,13 @@ func (i *ResourceItem) parseConfig(m map[string]interface{}) error {
 	}
 
 	// Remove platform-managed simple fields
+	legacyFields := []string{"/userNames", "/groupNames"}
 	for _, p := range platformManagedSimpleFields {
 		deletePointer, _ := gojsonpointer.NewJsonPointer(p)
 		_, _ = deletePointer.Delete(m)
+		if utils.Includes(legacyFields, p) {
+			cli.VerboseMsg("Removed", p, "which is used for legacy clients, but not supported by Tailor")
+		}
 	}
 
 	i.Config = m