Merge pull request #4088 from natasha41575/noMoreGenArgs

replace genargs with two separate annotations

api/internal/plugins/utils/utils.go

@@ -231,10 +231,10 @@ func UpdateResourceOptions(rm resmap.ResMap) (resmap.ResMap, error) {
 		if err := r.SetAnnotations(annotations); err != nil {
 			return nil, err
 		}
-		r.SetOptions(types.NewGenArgs(
+		if needsHash {
-			&types.GeneratorArgs{
+			r.EnableHashSuffix()
-				Behavior: behavior,
+		}
-				Options:  &types.GeneratorOptions{DisableNameSuffixHash: !needsHash}}))
+		r.SetBehavior(types.NewGenerationBehavior(behavior))
 	}
 	return rm, nil
 }

api/internal/utils/makeResIds.go

@@ -16,13 +16,14 @@ const (
 	BuildAnnotationPrefixes           = konfig.ConfigAnnoDomain + "/prefixes"
 	BuildAnnotationSuffixes           = konfig.ConfigAnnoDomain + "/suffixes"
 	BuildAnnotationsRefBy             = konfig.ConfigAnnoDomain + "/refBy"
-	BuildAnnotationsGenOptions        = konfig.ConfigAnnoDomain + "/generatorOptions"
+	BuildAnnotationsGenBehavior       = konfig.ConfigAnnoDomain + "/generatorBehavior"
+	BuildAnnotationsGenAddHashSuffix  = konfig.ConfigAnnoDomain + "/needsHashSuffix"
 
 	// the following are only for patches, to specify whether they can change names
 	// and kinds of their targets
 	BuildAnnotationAllowNameChange = konfig.ConfigAnnoDomain + "/allowNameChange"
 	BuildAnnotationAllowKindChange = konfig.ConfigAnnoDomain + "/allowKindChange"
-	Allowed                        = "allowed"
+	Enabled                        = "enabled"
 )
 
 // MakeResIds returns all of an RNode's current and previous Ids

api/resource/factory.go

@@ -64,19 +64,22 @@ func (rf *Factory) FromMapAndOption(
 		// TODO: return err instead of log.
 		log.Fatal(err)
 	}
-	return rf.makeOne(n, types.NewGenArgs(args))
+	return rf.makeOne(n, args)
 }
 
 // makeOne returns a new instance of Resource.
-func (rf *Factory) makeOne(rn *yaml.RNode, o *types.GenArgs) *Resource {
+func (rf *Factory) makeOne(rn *yaml.RNode, o *types.GeneratorArgs) *Resource {
 	if rn == nil {
 		log.Fatal("RNode must not be null")
 	}
-	if o == nil {
-		o = types.NewGenArgs(nil)
-	}
 	resource := &Resource{RNode: *rn}
-	resource.SetOptions(o)
+	if o != nil {
+		if o.Options == nil || !o.Options.DisableNameSuffixHash {
+			resource.EnableHashSuffix()
+		}
+		resource.SetBehavior(types.NewGenerationBehavior(o.Behavior))
+	}
+
 	return resource
 }
 
@@ -267,7 +270,7 @@ func (rf *Factory) MakeConfigMap(kvLdr ifc.KvLoader, args *types.ConfigMapArgs)
 	if err != nil {
 		return nil, err
 	}
-	return rf.makeOne(rn, types.NewGenArgs(&args.GeneratorArgs)), nil
+	return rf.makeOne(rn, &args.GeneratorArgs), nil
 }
 
 // MakeSecret makes an instance of Resource for Secret
@@ -276,5 +279,5 @@ func (rf *Factory) MakeSecret(kvLdr ifc.KvLoader, args *types.SecretArgs) (*Reso
 	if err != nil {
 		return nil, err
 	}
-	return rf.makeOne(rn, types.NewGenArgs(&args.GeneratorArgs)), nil
+	return rf.makeOne(rn, &args.GeneratorArgs), nil
 }

api/resource/resource.go

@@ -35,7 +35,8 @@ var BuildAnnotations = []string{
 	utils.BuildAnnotationAllowNameChange,
 	utils.BuildAnnotationAllowKindChange,
 	utils.BuildAnnotationsRefBy,
-	utils.BuildAnnotationsGenOptions,
+	utils.BuildAnnotationsGenBehavior,
+	utils.BuildAnnotationsGenAddHashSuffix,
 
 	kioutil.PathAnnotation,
 	kioutil.IndexAnnotation,
@@ -246,34 +247,38 @@ func (r *Resource) setPreviousId(ns string, n string, k string) *Resource {
 
 // AllowNameChange allows name changes to the resource.
 func (r *Resource) AllowNameChange() {
-	annotations := r.GetAnnotations()
+	r.enable(utils.BuildAnnotationAllowNameChange)
-	annotations[utils.BuildAnnotationAllowNameChange] = utils.Allowed
-	if err := r.SetAnnotations(annotations); err != nil {
-		panic(err)
-	}
 }
 
+// NameChangeAllowed checks if a patch resource is allowed to change another resource's name.
 func (r *Resource) NameChangeAllowed() bool {
-	annotations := r.GetAnnotations()
+	return r.isEnabled(utils.BuildAnnotationAllowNameChange)
-	v, ok := annotations[utils.BuildAnnotationAllowNameChange]
-	return ok && v == utils.Allowed
 }
 
 // AllowKindChange allows kind changes to the resource.
 func (r *Resource) AllowKindChange() {
+	r.enable(utils.BuildAnnotationAllowKindChange)
+}
+
+// KindChangeAllowed checks if a patch resource is allowed to change another resource's kind.
+func (r *Resource) KindChangeAllowed() bool {
+	return r.isEnabled(utils.BuildAnnotationAllowKindChange)
+}
+
+func (r *Resource) isEnabled(annoKey string) bool {
 	annotations := r.GetAnnotations()
-	annotations[utils.BuildAnnotationAllowKindChange] = utils.Allowed
+	v, ok := annotations[annoKey]
+	return ok && v == utils.Enabled
+}
+
+func (r *Resource) enable(annoKey string) {
+	annotations := r.GetAnnotations()
+	annotations[annoKey] = utils.Enabled
 	if err := r.SetAnnotations(annotations); err != nil {
 		panic(err)
 	}
 }
 
-func (r *Resource) KindChangeAllowed() bool {
-	annotations := r.GetAnnotations()
-	v, ok := annotations[utils.BuildAnnotationAllowKindChange]
-	return ok && v == utils.Allowed
-}
-
 // String returns resource as JSON.
 func (r *Resource) String() string {
 	bs, err := r.MarshalJSON()
@@ -302,43 +307,34 @@ 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) {
-	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.getGenArgs().Behavior()
+	annotations := r.GetAnnotations()
+	if v, ok := annotations[utils.BuildAnnotationsGenBehavior]; ok {
+		return types.NewGenerationBehavior(v)
+	}
+	return types.NewGenerationBehavior("")
+}
+
+// SetBehavior sets the behavior for the resource.
+func (r *Resource) SetBehavior(behavior types.GenerationBehavior) {
+	annotations := r.GetAnnotations()
+	annotations[utils.BuildAnnotationsGenBehavior] = behavior.String()
+	if err := r.SetAnnotations(annotations); err != nil {
+		panic(err)
+	}
 }
 
 // NeedHashSuffix returns true if a resource content
 // hash should be appended to the name of the resource.
 func (r *Resource) NeedHashSuffix() bool {
-	options := r.getGenArgs()
+	return r.isEnabled(utils.BuildAnnotationsGenAddHashSuffix)
-	return options != nil && options.ShouldAddHashSuffixToName()
+}
+
+// EnableHashSuffix marks the resource as needing a content
+// hash to be appended to the name of the resource.
+func (r *Resource) EnableHashSuffix() {
+	r.enable(utils.BuildAnnotationsGenAddHashSuffix)
 }
 
 // OrgId returns the original, immutable ResId for the resource.

api/resource/resource_test.go

@@ -30,8 +30,6 @@ var testConfigMap = factory.FromMap(
 
 //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,8 +41,6 @@ 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
@@ -80,28 +76,6 @@ func TestResourceString(t *testing.T) {
 	}
 }
 
-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)
-	}
-}
-
 func TestResourceId(t *testing.T) {
 	tests := []struct {
 		in *Resource
@@ -1193,35 +1167,3 @@ 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)
-}

api/types/genargs.go

@@ -1,61 +0,0 @@
-// Copyright 2019 The Kubernetes Authors.
-// SPDX-License-Identifier: Apache-2.0
-
-package types
-
-import (
-	"strconv"
-	"strings"
-
-	"sigs.k8s.io/kustomize/kyaml/yaml"
-)
-
-// GenArgs is a facade over GeneratorArgs, exposing a few readonly properties.
-type GenArgs struct {
-	args *GeneratorArgs
-}
-
-// NewGenArgs returns a new instance of GenArgs.
-func NewGenArgs(args *GeneratorArgs) *GenArgs {
-	return &GenArgs{args: args}
-}
-
-func (g *GenArgs) String() string {
-	if g == nil {
-		return "{nilGenArgs}"
-	}
-	return "{" +
-		strings.Join([]string{
-			"nsfx:" + strconv.FormatBool(g.ShouldAddHashSuffixToName()),
-			"beh:" + g.Behavior().String()},
-			",") +
-		"}"
-}
-
-// ShouldAddHashSuffixToName returns true if a resource
-// content hash should be appended to the name of the resource.
-func (g *GenArgs) ShouldAddHashSuffixToName() bool {
-	return g.args != nil &&
-		(g.args.Options == nil || !g.args.Options.DisableNameSuffixHash)
-}
-
-// Behavior returns Behavior field of GeneratorArgs
-func (g *GenArgs) Behavior() GenerationBehavior {
-	if g == nil || g.args == nil {
-		return BehaviorUnspecified
-	}
-	return NewGenerationBehavior(g.args.Behavior)
-}
-
-// IsNilOrEmpty returns true if g is nil or if the args are empty
-func (g *GenArgs) IsNilOrEmpty() bool {
-	return g == nil || g.args == nil
-}
-
-// AsYaml returns a yaml marshalling of the underlying Genargs
-func (g *GenArgs) AsYaml() ([]byte, error) {
-	if g == nil {
-		return yaml.Marshal(nil)
-	}
-	return yaml.Marshal(g.args)
-}

api/types/genargs_test.go

@@ -1,39 +0,0 @@
-// Copyright 2019 The Kubernetes Authors.
-// SPDX-License-Identifier: Apache-2.0
-
-package types_test
-
-import (
-	"testing"
-
-	. "sigs.k8s.io/kustomize/api/types"
-)
-
-func TestGenArgs_String(t *testing.T) {
-	tests := []struct {
-		ga       *GenArgs
-		expected string
-	}{
-		{
-			ga:       nil,
-			expected: "{nilGenArgs}",
-		},
-		{
-			ga:       &GenArgs{},
-			expected: "{nsfx:false,beh:unspecified}",
-		},
-		{
-			ga: NewGenArgs(
-				&GeneratorArgs{
-					Behavior: "merge",
-					Options:  &GeneratorOptions{DisableNameSuffixHash: false},
-				}),
-			expected: "{nsfx:true,beh:merge}",
-		},
-	}
-	for _, test := range tests {
-		if test.ga.String() != test.expected {
-			t.Fatalf("Expected '%s', got '%s'", test.expected, test.ga.String())
-		}
-	}
-}

plugin/someteam.example.com/v1/bashedconfigmap/BashedConfigMap_test.go

@@ -29,8 +29,8 @@ data:
 kind: ConfigMap
 metadata:
   annotations:
-    internal.config.kubernetes.io/generatorOptions: |
+    internal.config.kubernetes.io/generatorBehavior: unspecified
-      options: {}
+    internal.config.kubernetes.io/needsHashSuffix: enabled
   name: example-configmap-test
 `)
 	if m.Resources()[0].NeedHashSuffix() != true {