replace Resource.options with annotations (#4061)

api/resource/factory.go

@@ -75,7 +75,9 @@ func (rf *Factory) makeOne(rn *yaml.RNode, o *types.GenArgs) *Resource {
 	if o == nil {
 		o = types.NewGenArgs(nil)
 	}
-	return &Resource{RNode: *rn, options: o}
+	resource := &Resource{RNode: *rn}
+	resource.SetOptions(o)
+	return resource
 }
 
 // SliceFromPatches returns a slice of resources given a patch path

api/resource/resource.go

@@ -22,7 +22,6 @@ import (
 // paired with metadata used by kustomize.
 type Resource struct {
 	kyaml.RNode
-	options     *types.GenArgs
 	refVarNames []string
 }
 
@@ -35,6 +34,7 @@ var BuildAnnotations = []string{
 	utils.BuildAnnotationAllowNameChange,
 	utils.BuildAnnotationAllowKindChange,
 	utils.BuildAnnotationsRefBy,
+	utils.BuildAnnotationsGenOptions,
 }
 
 func (r *Resource) ResetRNode(incoming *Resource) {
@@ -80,6 +80,8 @@ func (r *Resource) DeepCopy() *Resource {
 // CopyMergeMetaDataFieldsFrom copies everything but the non-metadata in
 // the resource.
 // TODO: move to RNode, use GetMeta to improve performance.
+// TODO: make a version of mergeStringMaps that is build-annotation aware
+//   to avoid repeatedly setting refby and genargs annotations
 // Must remove the kustomize bit at the end.
 func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) error {
 	if err := r.SetLabels(
@@ -87,7 +89,7 @@ func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) error {
 		return fmt.Errorf("copyMerge cannot set labels - %w", err)
 	}
 	if err := r.SetAnnotations(
-		mergeStringMaps(other.GetAnnotations(), r.GetAnnotations())); err != nil {
+		mergeStringMapsWithBuildAnnotations(other.GetAnnotations(), r.GetAnnotations())); err != nil {
 		return fmt.Errorf("copyMerge cannot set annotations - %w", err)
 	}
 	if err := r.SetName(other.GetName()); err != nil {
@@ -101,7 +103,6 @@ func (r *Resource) CopyMergeMetaDataFieldsFrom(other *Resource) error {
 }
 
 func (r *Resource) copyKustomizeSpecificFields(other *Resource) {
-	r.options = other.options
 	r.refVarNames = copyStringSlice(other.refVarNames)
 }
 
@@ -274,7 +275,7 @@ func (r *Resource) String() string {
 	if err != nil {
 		return "<" + err.Error() + ">"
 	}
-	return strings.TrimSpace(string(bs)) + r.options.String()
+	return strings.TrimSpace(string(bs))
 }
 
 // AsYAML returns the resource in Yaml form.
@@ -296,20 +297,43 @@ func (r *Resource) MustYaml() string {
 	return string(yml)
 }
 
+func (r *Resource) getGenArgs() *types.GenArgs {
+	annotations := r.GetAnnotations()
+	if genOptsAnno, ok := annotations[utils.BuildAnnotationsGenOptions]; ok {
+		var genOpts types.GeneratorArgs
+		yaml.Unmarshal([]byte(genOptsAnno), &genOpts)
+		return types.NewGenArgs(&genOpts)
+	}
+	return nil
+}
+
 // SetOptions updates the generator options for the resource.
 func (r *Resource) SetOptions(o *types.GenArgs) {
-	r.options = o
+	annotations := r.GetAnnotations()
+	if o.IsNilOrEmpty() {
+		if len(annotations) == 0 {
+			return
+		}
+		if o == nil {
+			delete(annotations, utils.BuildAnnotationsGenOptions)
+		}
+	} else {
+		b, _ := o.AsYaml()
+		annotations[utils.BuildAnnotationsGenOptions] = string(b)
+	}
+	r.SetAnnotations(annotations)
 }
 
 // Behavior returns the behavior for the resource.
 func (r *Resource) Behavior() types.GenerationBehavior {
-	return r.options.Behavior()
+	return r.getGenArgs().Behavior()
 }
 
 // NeedHashSuffix returns true if a resource content
 // hash should be appended to the name of the resource.
 func (r *Resource) NeedHashSuffix() bool {
-	return r.options != nil && r.options.ShouldAddHashSuffixToName()
+	options := r.getGenArgs()
+	return options != nil && options.ShouldAddHashSuffixToName()
 }
 
 // OrgId returns the original, immutable ResId for the resource.
@@ -420,3 +444,17 @@ func mergeStringMaps(maps ...map[string]string) map[string]string {
 	}
 	return result
 }
+
+func mergeStringMapsWithBuildAnnotations(maps ...map[string]string) map[string]string {
+	result := mergeStringMaps(maps...)
+	for i := range BuildAnnotations {
+		if len(maps) > 0 {
+			if v, ok := maps[0][BuildAnnotations[i]]; ok {
+				result[BuildAnnotations[i]] = v
+				continue
+			}
+		}
+		delete(result, BuildAnnotations[i])
+	}
+	return result
+}

api/resource/resource_test.go

@@ -28,10 +28,10 @@ var testConfigMap = factory.FromMap(
 		},
 	})
 
-const genArgOptions = "{nsfx:false,beh:unspecified}"
-
 //nolint:gosec
 const configMapAsString = `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"winnie","namespace":"hundred-acre-wood"}}`
+const configMapAsStringWithOptions = `{"apiVersion":"v1","kind":"ConfigMap","metadata":{"annotations":` +
+	`{"internal.config.kubernetes.io/generatorOptions":"{}\n"},"name":"winnie","namespace":"hundred-acre-wood"}}`
 
 var testDeployment = factory.FromMap(
 	map[string]interface{}{
@@ -43,6 +43,8 @@ var testDeployment = factory.FromMap(
 	})
 
 const deploymentAsString = `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"name":"pooh"}}`
+const deploymentAsStringWithOptions = `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"annotations":` +
+	`{"internal.config.kubernetes.io/generatorOptions":"{}\n"},"name":"pooh"}}`
 
 func TestAsYAML(t *testing.T) {
 	expected := `apiVersion: apps/v1
@@ -66,17 +68,37 @@ func TestResourceString(t *testing.T) {
 	}{
 		{
 			in: testConfigMap,
-			s:  configMapAsString + genArgOptions,
+			s:  configMapAsString,
 		},
 		{
 			in: testDeployment,
-			s:  deploymentAsString + genArgOptions,
+			s:  deploymentAsString,
 		},
 	}
 	for _, test := range tests {
-		if test.in.String() != test.s {
-			t.Fatalf("Expected %s == %s", test.in.String(), test.s)
-		}
+		assert.Equal(t, test.in.String(), test.s)
+	}
+}
+
+func TestResourceStringWithOptionsAnnotations(t *testing.T) {
+	tests := []struct {
+		in *Resource
+		s  string
+	}{
+		{
+			in: testConfigMap,
+			s:  configMapAsStringWithOptions,
+		},
+		{
+			in: testDeployment,
+			s:  deploymentAsStringWithOptions,
+		},
+	}
+	for _, test := range tests {
+		args := &types.GeneratorArgs{}
+		options := types.NewGenArgs(args)
+		test.in.SetOptions(options)
+		assert.Equal(t, test.in.String(), test.s)
 	}
 }
 
@@ -717,9 +739,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    config.kubernetes.io/previousKinds: Secret
-    config.kubernetes.io/previousNames: oldName
-    config.kubernetes.io/previousNamespaces: default
+    internal.config.kubernetes.io/previousKinds: Secret
+    internal.config.kubernetes.io/previousNames: oldName
+    internal.config.kubernetes.io/previousNamespaces: default
   name: newName
 `,
 		},
@@ -729,9 +751,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    config.kubernetes.io/previousKinds: Secret
-    config.kubernetes.io/previousNames: oldName
-    config.kubernetes.io/previousNamespaces: default
+    internal.config.kubernetes.io/previousKinds: Secret
+    internal.config.kubernetes.io/previousNames: oldName
+    internal.config.kubernetes.io/previousNamespaces: default
   name: oldName2
 `,
 			newName: "newName",
@@ -740,9 +762,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    config.kubernetes.io/previousKinds: Secret,Secret
-    config.kubernetes.io/previousNames: oldName,oldName2
-    config.kubernetes.io/previousNamespaces: default,default
+    internal.config.kubernetes.io/previousKinds: Secret,Secret
+    internal.config.kubernetes.io/previousNames: oldName,oldName2
+    internal.config.kubernetes.io/previousNamespaces: default,default
   name: newName
 `,
 		},
@@ -752,9 +774,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    config.kubernetes.io/previousKinds: Secret
-    config.kubernetes.io/previousNames: oldName
-    config.kubernetes.io/previousNamespaces: default
+    internal.config.kubernetes.io/previousKinds: Secret
+    internal.config.kubernetes.io/previousNames: oldName
+    internal.config.kubernetes.io/previousNamespaces: default
   name: oldName2
   namespace: oldNamespace
 `,
@@ -764,9 +786,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    config.kubernetes.io/previousKinds: Secret,Secret
-    config.kubernetes.io/previousNames: oldName,oldName2
-    config.kubernetes.io/previousNamespaces: default,oldNamespace
+    internal.config.kubernetes.io/previousKinds: Secret,Secret
+    internal.config.kubernetes.io/previousNames: oldName,oldName2
+    internal.config.kubernetes.io/previousNamespaces: default,oldNamespace
   name: newName
   namespace: newNamespace
 `,
@@ -814,9 +836,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    config.kubernetes.io/previousKinds: Secret
-    config.kubernetes.io/previousNames: oldName
-    config.kubernetes.io/previousNamespaces: default
+    internal.config.kubernetes.io/previousKinds: Secret
+    internal.config.kubernetes.io/previousNames: oldName
+    internal.config.kubernetes.io/previousNamespaces: default
   name: newName
 `,
 			expected: []resid.ResId{
@@ -833,9 +855,9 @@ metadata:
 kind: Secret
 metadata:
   annotations:
-    config.kubernetes.io/previousKinds: Secret,Secret
-    config.kubernetes.io/previousNames: oldName,oldName2
-    config.kubernetes.io/previousNamespaces: default,oldNamespace
+    internal.config.kubernetes.io/previousKinds: Secret,Secret
+    internal.config.kubernetes.io/previousNames: oldName,oldName2
+    internal.config.kubernetes.io/previousNamespaces: default,oldNamespace
   name: newName
   namespace: newNamespace
 `,
@@ -1150,7 +1172,7 @@ kind: Deployment
 metadata:
   name: clown
   annotations:
-    config.kubernetes.io/refBy: gr1_ver1_knd1|ns1|name1
+    internal.config.kubernetes.io/refBy: gr1_ver1_knd1|ns1|name1
 spec:
   numReplicas: 1
 `)
@@ -1162,7 +1184,7 @@ kind: Deployment
 metadata:
   name: clown
   annotations:
-    config.kubernetes.io/refBy: gr1_ver1_knd1|ns1|name1,gr2_ver2_knd2|ns2|name2
+    internal.config.kubernetes.io/refBy: gr1_ver1_knd1|ns1|name1,gr2_ver2_knd2|ns2|name2
 spec:
   numReplicas: 1
 `)
@@ -1171,3 +1193,35 @@ spec:
 		resid.FromString("gr2_ver2_knd2|ns2|name2"),
 	})
 }
+
+func TestOptions(t *testing.T) {
+	r, err := factory.FromBytes([]byte(`
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: example-configmap-test
+`))
+	assert.NoError(t, err)
+
+	args := &types.GeneratorArgs{
+		Behavior: "merge",
+		Options: &types.GeneratorOptions{
+			DisableNameSuffixHash: true,
+		},
+	}
+
+	options := types.NewGenArgs(args)
+	r.SetOptions(options)
+	assert.Equal(t, r.RNode.MustString(), `apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: example-configmap-test
+  annotations:
+    internal.config.kubernetes.io/generatorOptions: |
+      behavior: merge
+      options:
+        disableNameSuffixHash: true
+`)
+	assert.Equal(t, r.Behavior(), types.BehaviorMerge)
+	assert.Equal(t, r.NeedHashSuffix(), !args.Options.DisableNameSuffixHash)
+}