From cf61a360e05d3fa570e4cb311cc57ee1d5ad102f Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Wed, 26 Feb 2020 20:13:06 -0800 Subject: [PATCH 1/2] Support for enum mappings in setters --- kyaml/setters2/add.go | 4 + kyaml/setters2/set.go | 37 +++++- kyaml/setters2/set_test.go | 224 +++++++++++++++++++++++++++++++++++++ kyaml/setters2/types.go | 5 +- 4 files changed, 266 insertions(+), 4 deletions(-) diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index 8bf7cc49d..c5f3fb8e9 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -104,6 +104,10 @@ type SetterDefinition struct { // Count is the number of fields set by this setter. Count int `yaml:"count,omitempty"` + + // EnumValues is a map of possible setter values to actual field values. + // May be used for t-shirt sizing values -- e.g. set cpu to small, medium, large + EnumValues map[string]string `yaml:"enumValues,omitempty"` } func (sd SetterDefinition) AddToFile(path string) error { diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index e610c035e..a3bc305b1 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -86,8 +86,14 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension) (bool, error) { if err != nil { return false, errors.Wrap(err) } - // substitute the setters current value into the substitution pattern - p = strings.ReplaceAll(p, v.Marker, subSetter.Setter.Value) + + if val, found := subSetter.Setter.EnumValues[subSetter.Setter.Value]; found { + // resolve enum for substitution + p = strings.ReplaceAll(p, v.Marker, val) + } else { + // substitute the setters current value into the substitution pattern + p = strings.ReplaceAll(p, v.Marker, subSetter.Setter.Value) + } if subSetter.Setter.Name == s.Name { // the substitution depends on the specified setter @@ -114,6 +120,11 @@ func (s *Set) set(field *yaml.RNode, ext *cliExtension) bool { // TODO(pwittrock): validate the field value + if val, found := ext.Setter.EnumValues[ext.Setter.Value]; found { + field.YNode().Value = val + return true + } + // this has a full setter, set its value field.YNode().Value = ext.Setter.Value return true @@ -146,6 +157,28 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { if def == nil { return nil, errors.Errorf("no setter %s found", s.Name) } + + // if the setter is an enumeration, then make sure the value matches + if values, err := def.Pipe(yaml.Lookup("enumValues")); err != nil { + return nil, err + } else if values != nil { + fields, err := values.Fields() + if err != nil { + return nil, err + } + var match bool + for i := range fields { + if fields[i] == s.Value { + match = true + break + } + } + if !match { + return nil, errors.Errorf("%s does not match the possible values for %s: [%s]", + s.Value, s.Name, strings.Join(fields, ",")) + } + } + if err := def.PipeE(&yaml.FieldSetter{Name: "value", StringValue: s.Value}); err != nil { return nil, err } diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go index 28bf43ac1..705df8c01 100644 --- a/kyaml/setters2/set_test.go +++ b/kyaml/setters2/set_test.go @@ -58,6 +58,92 @@ metadata: name: nginx-deployment spec: replicas: 4 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, + }, + { + name: "set-replicas-enum", + setter: "replicas", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.no-match-1': + x-k8s-cli: + setter: + name: no-match-1 + value: "1" + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "medium" + enumValues: + small: "1" + medium: "5" + large: "50" + io.k8s.cli.setters.no-match-2': + x-k8s-cli: + setter: + name: no-match-2 + value: "2" + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 1 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 5 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, + }, + { + name: "set-replicas-enum-large", + setter: "replicas", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.no-match-1': + x-k8s-cli: + setter: + name: no-match-1 + value: "1" + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "large" + enumValues: + small: "1" + medium: "5" + large: "50" + io.k8s.cli.setters.no-match-2': + x-k8s-cli: + setter: + name: no-match-2 + value: "2" + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 1 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 50 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} `, }, { @@ -155,6 +241,61 @@ spec: containers: - name: nginx image: nginx:1.8.1 # {"$ref": "#/definitions/io.k8s.cli.substitutions.image"} + `, + }, + { + name: "substitute-image-name-enum", + setter: "image-tag", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.image-name: + x-k8s-cli: + setter: + name: image-name + value: "helloworld" + enumValues: + nginx: gcr.io/nginx + helloworld: us.gcr.io/helloworld + io.k8s.cli.setters.image-tag: + x-k8s-cli: + setter: + name: image-tag + value: "1.8.1" + io.k8s.cli.substitutions.image: + x-k8s-cli: + substitution: + name: image + pattern: IMAGE_NAME:IMAGE_TAG + values: + - marker: "IMAGE_NAME" + ref: "#/definitions/io.k8s.cli.setters.image-name" + - marker: "IMAGE_TAG" + ref: "#/definitions/io.k8s.cli.setters.image-tag" + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: nginx + image: nginx:1.7.9 # {"$ref": "#/definitions/io.k8s.cli.substitutions.image"} + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + template: + spec: + containers: + - name: nginx + image: us.gcr.io/helloworld:1.8.1 # {"$ref": "#/definitions/io.k8s.cli.substitutions.image"} `, }, { @@ -631,6 +772,89 @@ openAPI: setBy: "package-default" `, }, + { + name: "set-replicas-with-enum", + setter: "replicas", + value: "baz", + input: ` +openAPI: + definitions: + io.k8s.cli.setters.no-match-1': + x-k8s-cli: + setter: + name: no-match-1 + value: "1" + setBy: "package-default" + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "foo" + enumValues: + foo: bar + baz: biz + io.k8s.cli.setters.no-match-2': + x-k8s-cli: + setter: + name: no-match-2 + value: "2" + setBy: "package-default" + `, + expected: ` +openAPI: + definitions: + io.k8s.cli.setters.no-match-1': + x-k8s-cli: + setter: + name: no-match-1 + value: "1" + setBy: "package-default" + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "baz" + enumValues: + foo: bar + baz: biz + io.k8s.cli.setters.no-match-2': + x-k8s-cli: + setter: + name: no-match-2 + value: "2" + setBy: "package-default" +`, + }, + { + name: "set-replicas-fail", + setter: "replicas", + value: "hello", + input: ` +openAPI: + definitions: + io.k8s.cli.setters.no-match-1': + x-k8s-cli: + setter: + name: no-match-1 + value: "1" + setBy: "package-default" + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "foo" + enumValues: + foo: bar + baz: biz + io.k8s.cli.setters.no-match-2': + x-k8s-cli: + setter: + name: no-match-2 + value: "2" + setBy: "package-default" + `, + err: "hello does not match the possible values for replicas: [foo,baz]", + }, { name: "error", setter: "replicas", diff --git a/kyaml/setters2/types.go b/kyaml/setters2/types.go index bd8f62a53..933c5f58e 100644 --- a/kyaml/setters2/types.go +++ b/kyaml/setters2/types.go @@ -19,8 +19,9 @@ type cliExtension struct { } type setter struct { - Name string `yaml:"name,omitempty" json:"name,omitempty"` - Value string `yaml:"value,omitempty" json:"value,omitempty"` + Name string `yaml:"name,omitempty" json:"name,omitempty"` + Value string `yaml:"value,omitempty" json:"value,omitempty"` + EnumValues map[string]string `yaml:"enumValues,omitempty" json:"enumValues,omitempty"` } type substitution struct { From 3c776b34351630acba5cd804253107f13f32d826 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Wed, 26 Feb 2020 20:17:54 -0800 Subject: [PATCH 2/2] fix setter clear set-by test --- cmd/config/internal/commands/cmdset_test.go | 1 - kyaml/setters2/add.go | 6 +++++- kyaml/setters2/set.go | 16 ++++++++++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/cmd/config/internal/commands/cmdset_test.go b/cmd/config/internal/commands/cmdset_test.go index f617660b7..289bcfa95 100644 --- a/cmd/config/internal/commands/cmdset_test.go +++ b/cmd/config/internal/commands/cmdset_test.go @@ -109,7 +109,6 @@ openAPI: setter: name: replicas value: "4" - setBy: me `, expectedResources: ` apiVersion: apps/v1 diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index c5f3fb8e9..d082b5d58 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -106,7 +106,11 @@ type SetterDefinition struct { Count int `yaml:"count,omitempty"` // EnumValues is a map of possible setter values to actual field values. - // May be used for t-shirt sizing values -- e.g. set cpu to small, medium, large + // If EnumValues is specified, then the value set the by user 1) MUST + // be present in the enumValues map as a key, and 2) the map entry value + // MUST be used as the value to set in the configuration (rather than the key) + // 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"` } diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index a3bc305b1..fdff66dfe 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -88,7 +88,8 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension) (bool, error) { } if val, found := subSetter.Setter.EnumValues[subSetter.Setter.Value]; found { - // resolve enum for substitution + // the setter has an enum-map. we should replace the marker with the + // enum value looked up from the map rather than the enum key p = strings.ReplaceAll(p, v.Marker, val) } else { // substitute the setters current value into the substitution pattern @@ -121,6 +122,8 @@ func (s *Set) set(field *yaml.RNode, ext *cliExtension) bool { // TODO(pwittrock): validate the field value if val, found := ext.Setter.EnumValues[ext.Setter.Value]; found { + // the setter has an enum-map. we should replace the marker with the + // enum value looked up from the map rather than the enum key field.YNode().Value = val return true } @@ -158,22 +161,31 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { return nil, errors.Errorf("no setter %s found", s.Name) } - // if the setter is an enumeration, then make sure the value matches + // if the setter contains an enumValues map, then ensure the set value appears + // as a key in the map if values, err := def.Pipe(yaml.Lookup("enumValues")); err != nil { + // error looking up the enumValues return nil, err } else if values != nil { + // contains enumValues map -- validate the set value against the map entries + + // get the enumValues keys fields, err := values.Fields() if err != nil { return nil, err } + + // search for the user provided value in the set of allowed values var match bool for i := range fields { if fields[i] == s.Value { + // found a match, we are good match = true break } } if !match { + // no match found -- provide an informative error to the user return nil, errors.Errorf("%s does not match the possible values for %s: [%s]", s.Value, s.Name, strings.Join(fields, ",")) }