From 68ab3b87d9456db3bc586d70bb60d6180bf1a47d Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Mon, 22 Jun 2020 20:17:17 -0700 Subject: [PATCH] Required setters for apply --- .../internal/commands/cmdcreatesetter.go | 2 + .../internal/commands/cmdlistsetters.go | 10 ++++- .../internal/commands/cmdlistsetters_test.go | 43 +++++++++++-------- cmd/config/internal/commands/cmdset_test.go | 6 +++ .../commands/e2e/list_setters_test.go | 4 +- cmd/config/internal/commands/e2e/set_test.go | 1 + kyaml/setters2/add.go | 5 +++ kyaml/setters2/delete.go | 4 +- kyaml/setters2/list.go | 2 + kyaml/setters2/set.go | 4 ++ kyaml/setters2/set_test.go | 12 ++++++ kyaml/setters2/settersutil/settercreator.go | 7 ++- 12 files changed, 75 insertions(+), 25 deletions(-) diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index 04ed314f3..809e222ea 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -50,6 +50,8 @@ func NewCreateSetterRunner(parent string) *CreateSetterRunner { set.Flags().MarkHidden("partial") set.Flags().StringVar(&setterVersion, "version", "", "use this version of the setter format") + set.Flags().BoolVar(&r.CreateSetter.Required, "required", false, + "indicates that this setter must be set by package consumer before live apply/preview") set.Flags().StringVar(&r.CreateSetter.SchemaPath, "schema-path", "", `openAPI schema file path for setter constraints -- file content `+ `e.g. {"type": "string", "maxLength": 15, "enum": ["allowedValue1", "allowedValue2"]}`) diff --git a/cmd/config/internal/commands/cmdlistsetters.go b/cmd/config/internal/commands/cmdlistsetters.go index cb84a21a3..ec74c043d 100644 --- a/cmd/config/internal/commands/cmdlistsetters.go +++ b/cmd/config/internal/commands/cmdlistsetters.go @@ -79,7 +79,7 @@ func (r *ListSettersRunner) ListSetters(c *cobra.Command, args []string) error { return err } table := newTable(c.OutOrStdout(), r.Markdown) - table.SetHeader([]string{"NAME", "VALUE", "SET BY", "DESCRIPTION", "COUNT"}) + table.SetHeader([]string{"NAME", "VALUE", "SET BY", "DESCRIPTION", "COUNT", "REQUIRED"}) for i := range r.List.Setters { s := r.List.Setters[i] v := s.Value @@ -89,8 +89,14 @@ func (r *ListSettersRunner) ListSetters(c *cobra.Command, args []string) error { v = strings.Join(s.ListValues, ",") v = fmt.Sprintf("[%s]", v) } + var required string + if s.Required { + required = "Yes" + } else { + required = "No" + } table.Append([]string{ - s.Name, v, s.SetBy, s.Description, fmt.Sprintf("%d", s.Count)}) + s.Name, v, s.SetBy, s.Description, fmt.Sprintf("%d", s.Count), required}) } table.Render() diff --git a/cmd/config/internal/commands/cmdlistsetters_test.go b/cmd/config/internal/commands/cmdlistsetters_test.go index 4a1fc20d6..b397854a9 100644 --- a/cmd/config/internal/commands/cmdlistsetters_test.go +++ b/cmd/config/internal/commands/cmdlistsetters_test.go @@ -34,6 +34,7 @@ openAPI: name: replicas value: "3" setBy: me + required: true description: "hello world" `, input: ` @@ -44,8 +45,8 @@ metadata: spec: replicas: 3 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} `, - expected: ` NAME VALUE SET BY DESCRIPTION COUNT - replicas 3 me hello world 1 + expected: ` NAME VALUE SET BY DESCRIPTION COUNT REQUIRED + replicas 3 me hello world 1 Yes `, }, { @@ -74,6 +75,8 @@ openAPI: name: tag value: "1.7.9" setBy: me3 + required: true + isSet: false io.k8s.cli.substitutions.image: x-k8s-cli: substitution: @@ -100,10 +103,10 @@ spec: - name: nginx2 image: nginx # {"$ref": "#/definitions/io.k8s.cli.setters.image"} `, - expected: ` NAME VALUE SET BY DESCRIPTION COUNT - image nginx me2 hello world 2 2 - replicas 3 me1 hello world 1 1 - tag 1.7.9 me3 hello world 3 1 + expected: ` NAME VALUE SET BY DESCRIPTION COUNT REQUIRED + image nginx me2 hello world 2 2 No + replicas 3 me1 hello world 1 1 No + tag 1.7.9 me3 hello world 3 1 Yes SUBSTITUTION PATTERN REFERENCES image IMAGE:TAG [image,tag] `, @@ -174,10 +177,10 @@ spec: - name: nginx2 image: nginx `, - expected: ` NAME VALUE SET BY DESCRIPTION COUNT - image nginx me2 hello world 2 3 - replicas 3 me1 hello world 1 2 - tag 1.7.9 me3 hello world 3 2 + expected: ` NAME VALUE SET BY DESCRIPTION COUNT REQUIRED + image nginx me2 hello world 2 3 No + replicas 3 me1 hello world 1 2 No + tag 1.7.9 me3 hello world 3 2 No SUBSTITUTION PATTERN REFERENCES image IMAGE:TAG [image,tag] `, @@ -202,6 +205,7 @@ openAPI: name: image value: "nginx" setBy: me2 + required: true io.k8s.cli.setters.tag: description: "hello world 3" x-k8s-cli: @@ -249,8 +253,8 @@ spec: - name: nginx2 image: nginx `, - expected: ` NAME VALUE SET BY DESCRIPTION COUNT - image nginx me2 hello world 2 3 + expected: ` NAME VALUE SET BY DESCRIPTION COUNT REQUIRED + image nginx me2 hello world 2 3 Yes SUBSTITUTION PATTERN REFERENCES image IMAGE:TAG [image,tag] `, @@ -277,6 +281,7 @@ openAPI: - b - c setBy: me + required: true `, input: ` apiVersion: example.com/v1beta1 @@ -290,8 +295,8 @@ spec: - "b" - "c" `, - expected: ` NAME VALUE SET BY DESCRIPTION COUNT - list [a,b,c] me hello world 1 + expected: ` NAME VALUE SET BY DESCRIPTION COUNT REQUIRED + list [a,b,c] me hello world 1 Yes `, }, @@ -327,6 +332,8 @@ openAPI: setter: name: my-tag-setter value: 1.7.9 + required: true + isSet: true io.k8s.cli.substitutions.my-image-subst: x-k8s-cli: substitution: @@ -353,10 +360,10 @@ openAPI: name: my-other-setter value: nginxotherthing `, - expected: ` NAME VALUE SET BY DESCRIPTION COUNT - my-image-setter nginx 2 - my-other-setter nginxotherthing 1 - my-tag-setter 1.7.9 2 + expected: ` NAME VALUE SET BY DESCRIPTION COUNT REQUIRED + my-image-setter nginx 2 No + my-other-setter nginxotherthing 1 No + my-tag-setter 1.7.9 2 Yes SUBSTITUTION PATTERN REFERENCES my-image-subst ${my-image-setter}::${my-tag-setter} [my-image-setter,my-tag-setter] my-nested-subst something/${my-image-subst}/${my-other-setter} [my-image-subst,my-other-setter] diff --git a/cmd/config/internal/commands/cmdset_test.go b/cmd/config/internal/commands/cmdset_test.go index 1191cd392..78db9a8f0 100644 --- a/cmd/config/internal/commands/cmdset_test.go +++ b/cmd/config/internal/commands/cmdset_test.go @@ -64,6 +64,7 @@ openAPI: name: replicas value: "4" setBy: pw + isSet: true `, expectedResources: ` apiVersion: apps/v1 @@ -158,6 +159,7 @@ openAPI: setter: name: replicas value: "4" + isSet: true `, expectedResources: ` apiVersion: apps/v1 @@ -229,6 +231,7 @@ openAPI: setter: name: tag value: "1.8.1" + isSet: true io.k8s.cli.substitutions.image: x-k8s-cli: substitution: @@ -539,6 +542,7 @@ openAPI: setter: name: replicas value: "4" + isSet: true `, expectedResources: ` apiVersion: apps/v1 @@ -639,6 +643,7 @@ openAPI: listValues: - "10" - "11" + isSet: true `, expectedResources: ` apiVersion: example.com/v1beta1 @@ -771,6 +776,7 @@ openAPI: setter: name: my-image-setter value: ubuntu + isSet: true io.k8s.cli.setters.my-tag-setter: x-k8s-cli: setter: diff --git a/cmd/config/internal/commands/e2e/list_setters_test.go b/cmd/config/internal/commands/e2e/list_setters_test.go index 47ce68bbe..0b8cd541f 100644 --- a/cmd/config/internal/commands/e2e/list_setters_test.go +++ b/cmd/config/internal/commands/e2e/list_setters_test.go @@ -34,8 +34,8 @@ openAPI: `, }, expectedStdOut: ` -NAME VALUE SET BY DESCRIPTION COUNT - replicas 3 1 +NAME VALUE SET BY DESCRIPTION COUNT REQUIRED + replicas 3 1 No `, }, } diff --git a/cmd/config/internal/commands/e2e/set_test.go b/cmd/config/internal/commands/e2e/set_test.go index 320227a3a..ca9a21bd5 100644 --- a/cmd/config/internal/commands/e2e/set_test.go +++ b/cmd/config/internal/commands/e2e/set_test.go @@ -54,6 +54,7 @@ openAPI: setter: name: replicas value: "4" + isSet: true `, }, }, diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index 8517c66a4..bd8055a60 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -162,6 +162,11 @@ type SetterDefinition struct { // Example -- may be used for t-shirt sizing values by allowing cpu to be // set to small, medium or large, and then mapping these values to cpu values -- 0.5, 2, 8 EnumValues map[string]string `yaml:"enumValues,omitempty"` + + // Required indicates that the setter must be set by package consumer before + // live apply/preview. This field is added to the setter definition to record + // the package publisher's intent to make the setter required to be set. + Required bool `yaml:"required,omitempty"` } func (sd SetterDefinition) AddToFile(path string) error { diff --git a/kyaml/setters2/delete.go b/kyaml/setters2/delete.go index a01852e09..448a3467d 100644 --- a/kyaml/setters2/delete.go +++ b/kyaml/setters2/delete.go @@ -132,12 +132,12 @@ func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { return nil, errors.Errorf("setter is used in substitution %s, please delete the substitution first", subst) } - _, err = definitions.Pipe(yaml.FieldClearer{Name:key}) + _, err = definitions.Pipe(yaml.FieldClearer{Name: key}) if err != nil { return nil, err } // remove definitions if it's empty - _, err = object.Pipe(yaml.Lookup(openapi.SupplementaryOpenAPIFieldName), yaml.FieldClearer{Name:"definitions", IfEmpty: true}) + _, err = object.Pipe(yaml.Lookup(openapi.SupplementaryOpenAPIFieldName), yaml.FieldClearer{Name: "definitions", IfEmpty: true}) if err != nil { return nil, err } diff --git a/kyaml/setters2/list.go b/kyaml/setters2/list.go index 0a15ded99..33e2906a2 100644 --- a/kyaml/setters2/list.go +++ b/kyaml/setters2/list.go @@ -181,6 +181,8 @@ func (l *List) listSubst(object *yaml.RNode) error { } // count returns the number of fields set by the setter with name +// set filter is leveraged for this but the resources are not written +// back to files as only LocalPackageReader is invoked and not writer func (l *List) count(path, name string) (int, error) { s := &Set{Name: name} err := kio.Pipeline{ diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index c95b0f0bc..cb5594e55 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -419,6 +419,10 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { return nil, err } + if err := def.PipeE(&yaml.FieldSetter{Name: "isSet", StringValue: "true"}); err != nil { + return nil, err + } + if s.Description != "" { d, err := object.Pipe(yaml.LookupCreate( yaml.MappingNode, "openAPI", "definitions", key)) diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go index 189b76750..0352393e3 100644 --- a/kyaml/setters2/set_test.go +++ b/kyaml/setters2/set_test.go @@ -927,6 +927,8 @@ openAPI: setter: name: replicas value: "4" + required: true + isSet: true io.k8s.cli.setters.no-match-2': x-k8s-cli: setter: @@ -946,6 +948,8 @@ openAPI: setter: name: replicas value: "3" + required: true + isSet: true io.k8s.cli.setters.no-match-2': x-k8s-cli: setter: @@ -974,6 +978,7 @@ openAPI: setter: name: replicas value: "3" + isSet: true `, }, { @@ -1013,6 +1018,7 @@ openAPI: setter: name: replicas value: "3" + isSet: true description: hello world io.k8s.cli.setters.no-match-2': x-k8s-cli: @@ -1059,6 +1065,7 @@ openAPI: name: replicas value: "3" setBy: carl + isSet: true io.k8s.cli.setters.no-match-2': x-k8s-cli: setter: @@ -1106,6 +1113,7 @@ openAPI: setter: name: replicas value: "3" + isSet: true io.k8s.cli.setters.no-match-2': x-k8s-cli: setter: @@ -1159,6 +1167,7 @@ openAPI: enumValues: foo: bar baz: biz + isSet: true io.k8s.cli.setters.no-match-2': x-k8s-cli: setter: @@ -1245,6 +1254,7 @@ openAPI: setter: name: args listValues: ["1"] + required: true `, expected: ` openAPI: @@ -1255,6 +1265,8 @@ openAPI: setter: name: args listValues: ["2", "3", "4"] + required: true + isSet: true `, }, } diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index e2468922b..f49cf57ef 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -37,6 +37,11 @@ type SetterCreator struct { // FieldValue if set will add the OpenAPI reference to fields if they have this value. // Optional. If unspecified match all field values. FieldValue string + + // Required indicates that the setter must be set by package consumer before + // live apply/preview. This field is added to the setter definition to record + // the package publisher's intent to make the setter required to be set. + Required bool } func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { @@ -47,7 +52,7 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { // Update the OpenAPI definitions to hace the setter sd := setters2.SetterDefinition{ Name: c.Name, Value: c.FieldValue, Description: c.Description, SetBy: c.SetBy, - Type: c.Type, Schema: schema, + Type: c.Type, Schema: schema, Required: c.Required, } if err := sd.AddToFile(openAPIPath); err != nil { return err