From 0ad0d02629ec1c152b70d871c31f34689704ade4 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Mon, 12 Jul 2021 10:34:41 -0700 Subject: [PATCH] replace Resource.options with annotations --- api/internal/utils/makeResIds.go | 3 +- api/resource/factory.go | 4 ++- api/resource/resource.go | 28 +++++++++++---- api/resource/resource_test.go | 60 ++++++++++++++++++++++++++++++++ api/types/genargs.go | 12 +++---- 5 files changed, 93 insertions(+), 14 deletions(-) diff --git a/api/internal/utils/makeResIds.go b/api/internal/utils/makeResIds.go index a249fc9b83c..455a149877d 100644 --- a/api/internal/utils/makeResIds.go +++ b/api/internal/utils/makeResIds.go @@ -12,10 +12,11 @@ import ( const ( BuildAnnotationPreviousKinds = konfig.ConfigAnnoDomain + "/previousKinds" BuildAnnotationPreviousNames = konfig.ConfigAnnoDomain + "/previousNames" + BuildAnnotationPreviousNamespaces = konfig.ConfigAnnoDomain + "/previousNamespaces" BuildAnnotationPrefixes = konfig.ConfigAnnoDomain + "/prefixes" BuildAnnotationSuffixes = konfig.ConfigAnnoDomain + "/suffixes" - BuildAnnotationPreviousNamespaces = konfig.ConfigAnnoDomain + "/previousNamespaces" BuildAnnotationsRefBy = konfig.ConfigAnnoDomain + "/refBy" + BuildAnnotationsGenOptions = konfig.ConfigAnnoDomain + "/generatorOptions" // the following are only for patches, to specify whether they can change names // and kinds of their targets diff --git a/api/resource/factory.go b/api/resource/factory.go index 97ced84ba0c..d01e21b6bb9 100644 --- a/api/resource/factory.go +++ b/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 diff --git a/api/resource/resource.go b/api/resource/resource.go index 2ec0209ec89..3676d7ceabb 100644 --- a/api/resource/resource.go +++ b/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) { @@ -101,7 +101,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 +273,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 +295,37 @@ func (r *Resource) MustYaml() string { return string(yml) } +func (r *Resource) getOptions() *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 &types.GenArgs{} +} + // SetOptions updates the generator options for the resource. func (r *Resource) SetOptions(o *types.GenArgs) { - r.options = o + if o == nil || o.Args == nil { + return + } + annotations := r.GetAnnotations() + b, _ := yaml.Marshal(o.Args) + 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.getOptions().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.getOptions() + return options != nil && options.ShouldAddHashSuffixToName() } // OrgId returns the original, immutable ResId for the resource. diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index 373b5a1d54a..1662f3806a6 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -30,6 +30,8 @@ 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\":" + + "{\"config.kubernetes.io/generatorOptions\":\"{}\\n\"},\"name\":\"winnie\",\"namespace\":\"hundred-acre-wood\"}}" var testDeployment = factory.FromMap( map[string]interface{}{ @@ -41,6 +43,9 @@ var testDeployment = factory.FromMap( }) const deploymentAsString = `{"apiVersion":"apps/v1","kind":"Deployment","metadata":{"name":"pooh"}}` +const deploymentAsStringWithOptions = "{\"apiVersion\":\"apps/v1\",\"kind\":\"Deployment" + + "\",\"metadata\":{\"annotations\":{\"config.kubernetes.io/generatorOptions\":\"{}\\n\"}" + + ",\"name\":\"pooh\"}}" func TestAsYAML(t *testing.T) { expected := `apiVersion: apps/v1 @@ -78,6 +83,30 @@ 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 { + test.in.SetOptions(&types.GenArgs{ + Args: &types.GeneratorArgs{}, + }) + if test.in.String() != test.s { + t.Fatalf("Expected %s == %s", test.in.String(), test.s) + } + } +} + func TestResourceId(t *testing.T) { tests := []struct { in *Resource @@ -1169,3 +1198,34 @@ 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) + options := &types.GenArgs{ + Args: &types.GeneratorArgs{ + Behavior: "merge", + Options: &types.GeneratorOptions{ + DisableNameSuffixHash: true, + }, + }, + } + r.SetOptions(options) + assert.Equal(t, r.RNode.MustString(), `apiVersion: v1 +kind: ConfigMap +metadata: + name: example-configmap-test + annotations: + config.kubernetes.io/generatorOptions: | + behavior: merge + options: + disableNameSuffixHash: true +`) + assert.Equal(t, r.Behavior(), types.BehaviorMerge) + assert.Equal(t, r.NeedHashSuffix(), !options.Args.Options.DisableNameSuffixHash) +} diff --git a/api/types/genargs.go b/api/types/genargs.go index 586d7f1af1d..4ef8821eecd 100644 --- a/api/types/genargs.go +++ b/api/types/genargs.go @@ -10,12 +10,12 @@ import ( // GenArgs is a facade over GeneratorArgs, exposing a few readonly properties. type GenArgs struct { - args *GeneratorArgs + Args *GeneratorArgs } // NewGenArgs returns a new instance of GenArgs. func NewGenArgs(args *GeneratorArgs) *GenArgs { - return &GenArgs{args: args} + return &GenArgs{Args: args} } func (g *GenArgs) String() string { @@ -33,14 +33,14 @@ func (g *GenArgs) String() 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) + 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 { + if g == nil || g.Args == nil { return BehaviorUnspecified } - return NewGenerationBehavior(g.args.Behavior) + return NewGenerationBehavior(g.Args.Behavior) }