From 7b0ec99d90393051b8a0b872787d6976f63f4ba5 Mon Sep 17 00:00:00 2001 From: Shoshana Malfatto Date: Wed, 23 Mar 2022 14:25:19 -0700 Subject: [PATCH] retain quotes in namespace transformer filter (#4421) * check tag values for double quoting * reuse setentry * don't override single quotes in merge and fix cm generator immutable val * get rid of comment * starlark annotation tests * don't commit test image changes * set network to bool * isSet bool * updating e2e config tool * leave createtag * fn command failing unmarshal test * fn command test * don't set style in run-fs * use setentry to set tag * remove e2e test changes and make IsStringValue an RNode method --- api/filters/filtersutil/setters.go | 3 -- api/filters/namespace/namespace.go | 6 ++-- api/filters/namespace/namespace_test.go | 34 +++++++++++++++++++ api/filters/replicacount/replicacount.go | 2 +- api/filters/replicacount/replicacount_test.go | 1 + api/internal/generators/configmap_test.go | 4 +-- api/internal/generators/utils.go | 8 ++++- cmd/config/internal/commands/run-fns.go | 11 ++++-- kyaml/fn/framework/command/command_test.go | 17 ++++++++-- .../command/testdata/standalone/config.yaml | 3 +- .../command/testdata/standalone/expected.yaml | 6 ++-- kyaml/setters2/set.go | 7 +++- kyaml/yaml/fns.go | 6 ++++ kyaml/yaml/rnode.go | 5 +++ 14 files changed, 95 insertions(+), 18 deletions(-) diff --git a/api/filters/filtersutil/setters.go b/api/filters/filtersutil/setters.go index fd44f80c59..0714c4128c 100644 --- a/api/filters/filtersutil/setters.go +++ b/api/filters/filtersutil/setters.go @@ -21,9 +21,6 @@ func SetEntry(key, value, tag string) SetFn { Value: value, Tag: tag, } - if tag == yaml.NodeTagString && yaml.IsYaml1_1NonString(n) { - n.Style = yaml.DoubleQuotedStyle - } return func(node *yaml.RNode) error { return node.PipeE(yaml.FieldSetter{ Name: key, diff --git a/api/filters/namespace/namespace.go b/api/filters/namespace/namespace.go index 85484bf702..016a14a134 100644 --- a/api/filters/namespace/namespace.go +++ b/api/filters/namespace/namespace.go @@ -52,7 +52,7 @@ func (ns Filter) run(node *yaml.RNode) (*yaml.RNode, error) { // transformations based on data -- :) err := node.PipeE(fsslice.Filter{ FsSlice: ns.FsSlice, - SetValue: ns.trackableSetter.SetScalar(ns.Namespace), + SetValue: ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString), CreateKind: yaml.ScalarNode, // Namespace is a ScalarNode CreateTag: yaml.NodeTagString, }) @@ -85,7 +85,7 @@ func (ns Filter) metaNamespaceHack(obj *yaml.RNode, gvk resid.Gvk) error { FsSlice: []types.FieldSpec{ {Path: types.MetadataNamespacePath, CreateIfNotPresent: true}, }, - SetValue: ns.trackableSetter.SetScalar(ns.Namespace), + SetValue: ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString), CreateKind: yaml.ScalarNode, // Namespace is a ScalarNode } _, err := f.Filter(obj) @@ -138,7 +138,7 @@ func (ns Filter) roleBindingHack(obj *yaml.RNode, gvk resid.Gvk) error { return err } - return ns.trackableSetter.SetScalar(ns.Namespace)(node) + return ns.trackableSetter.SetEntry("", ns.Namespace, yaml.NodeTagString)(node) }) diff --git a/api/filters/namespace/namespace_test.go b/api/filters/namespace/namespace_test.go index 302abe0351..32a24d29fc 100644 --- a/api/filters/namespace/namespace_test.go +++ b/api/filters/namespace/namespace_test.go @@ -332,26 +332,60 @@ a: expectedSetValueArgs: []filtertest_test.SetValueArg{ { Value: "bar", + Tag: "!!str", NodePath: []string{"metadata", "namespace"}, }, { Value: "bar", + Tag: "!!str", NodePath: []string{"a", "b", "c"}, }, { Value: "bar", + Tag: "!!str", NodePath: []string{"metadata", "namespace"}, }, { Value: "bar", + Tag: "!!str", NodePath: []string{"namespace"}, }, { Value: "bar", + Tag: "!!str", NodePath: []string{"a", "b", "c"}, }, }, }, + + { + name: "numeric", + input: ` +apiVersion: example.com/v1 +kind: Foo +metadata: + name: instance +--- +apiVersion: example.com/v1 +kind: Bar +metadata: + name: instance +`, + expected: ` +apiVersion: example.com/v1 +kind: Foo +metadata: + name: instance + namespace: "01234" +--- +apiVersion: example.com/v1 +kind: Bar +metadata: + name: instance + namespace: "01234" +`, + filter: namespace.Filter{Namespace: "01234"}, + }, } type TestCase struct { diff --git a/api/filters/replicacount/replicacount.go b/api/filters/replicacount/replicacount.go index 84005f26af..f2d98d7dde 100644 --- a/api/filters/replicacount/replicacount.go +++ b/api/filters/replicacount/replicacount.go @@ -41,5 +41,5 @@ func (rc Filter) run(node *yaml.RNode) (*yaml.RNode, error) { } func (rc Filter) set(node *yaml.RNode) error { - return rc.trackableSetter.SetScalar(strconv.FormatInt(rc.Replica.Count, 10))(node) + return rc.trackableSetter.SetEntry("", strconv.FormatInt(rc.Replica.Count, 10), yaml.NodeTagInt)(node) } diff --git a/api/filters/replicacount/replicacount_test.go b/api/filters/replicacount/replicacount_test.go index e690192fc1..b33b49474e 100644 --- a/api/filters/replicacount/replicacount_test.go +++ b/api/filters/replicacount/replicacount_test.go @@ -192,6 +192,7 @@ spec: expectedSetValueArgs: []filtertest_test.SetValueArg{ { Value: "42", + Tag: "!!int", NodePath: []string{"spec", "replicas"}, }, }, diff --git a/api/internal/generators/configmap_test.go b/api/internal/generators/configmap_test.go index f42ba8f70c..8551963a4e 100644 --- a/api/internal/generators/configmap_test.go +++ b/api/internal/generators/configmap_test.go @@ -140,7 +140,7 @@ metadata: foo: 'bar' data: a: x - b: y + b: "y" c: Hello World d: "true" `, @@ -181,7 +181,7 @@ metadata: river: 'Missouri' data: a: x - b: y + b: "y" c: Hello World d: "true" immutable: true diff --git a/api/internal/generators/utils.go b/api/internal/generators/utils.go index 82040d4207..d6ea5fbf00 100644 --- a/api/internal/generators/utils.go +++ b/api/internal/generators/utils.go @@ -83,9 +83,15 @@ func setImmutable( return nil } if opts.Immutable { - if _, err := rn.Pipe(yaml.SetField("immutable", yaml.NewScalarRNode("true"))); err != nil { + n := &yaml.Node{ + Kind: yaml.ScalarNode, + Value: "true", + Tag: yaml.NodeTagBool, + } + if _, err := rn.Pipe(yaml.FieldSetter{Name: "immutable", Value: yaml.NewRNode(n)}); err != nil { return err } } + return nil } diff --git a/cmd/config/internal/commands/run-fns.go b/cmd/config/internal/commands/run-fns.go index 684d940c20..481969fb7a 100644 --- a/cmd/config/internal/commands/run-fns.go +++ b/cmd/config/internal/commands/run-fns.go @@ -135,9 +135,14 @@ func (r *RunFnRunner) getContainerFunctions(c *cobra.Command, dataItems []string return nil, err } if r.Network { + n := &yaml.Node{ + Kind: yaml.ScalarNode, + Value: "true", + Tag: yaml.NodeTagBool, + } err = fn.PipeE( yaml.Lookup("container"), - yaml.SetField("network", yaml.NewScalarRNode("true"))) + yaml.SetField("network", yaml.NewRNode(n))) if err != nil { return nil, err } @@ -231,7 +236,9 @@ data: {} if len(kv) != 2 { return nil, fmt.Errorf("args must have keys and values separated by =") } - err := dataField.PipeE(yaml.SetField(kv[0], yaml.NewScalarRNode(kv[1]))) + // do not set style since function should determine tag and style + err := dataField.PipeE( + yaml.FieldSetter{Name: kv[0], Value: yaml.NewScalarRNode(kv[1]), OverrideStyle: true}) if err != nil { return nil, err } diff --git a/kyaml/fn/framework/command/command_test.go b/kyaml/fn/framework/command/command_test.go index 2c8444ef57..ca9342a944 100644 --- a/kyaml/fn/framework/command/command_test.go +++ b/kyaml/fn/framework/command/command_test.go @@ -5,6 +5,7 @@ package command_test import ( "bytes" + "fmt" "io/ioutil" "os" "path/filepath" @@ -66,6 +67,7 @@ ENTRYPOINT ["function"] func TestCommand_standalone(t *testing.T) { var config struct { A string `json:"a" yaml:"a"` + B int `json:"b" yaml:"b"` } fn := func(items []*yaml.RNode) ([]*yaml.RNode, error) { @@ -83,6 +85,10 @@ metadata: if err != nil { return nil, err } + err = items[i].PipeE(yaml.SetAnnotation("b", fmt.Sprintf("%v", config.B))) + if err != nil { + return nil, err + } } return items, nil @@ -99,6 +105,7 @@ metadata: func TestCommand_standalone_stdin(t *testing.T) { var config struct { A string `json:"a" yaml:"a"` + B int `json:"b" yaml:"b"` } p := &framework.SimpleProcessor{ @@ -119,6 +126,10 @@ metadata: if err != nil { return nil, err } + err = items[i].PipeE(yaml.SetAnnotation("b", fmt.Sprintf("%v", config.B))) + if err != nil { + return nil, err + } } return items, nil @@ -150,7 +161,8 @@ metadata: namespace: default annotations: foo: bar1 - a: 'b' + a: 'c' + b: '1' spec: replicas: 1 --- @@ -161,6 +173,7 @@ metadata: namespace: default annotations: foo: bar2 - a: 'b' + a: 'c' + b: '1' `), strings.TrimSpace(out.String())) } diff --git a/kyaml/fn/framework/command/testdata/standalone/config.yaml b/kyaml/fn/framework/command/testdata/standalone/config.yaml index 81f93e0665..30e387816e 100644 --- a/kyaml/fn/framework/command/testdata/standalone/config.yaml +++ b/kyaml/fn/framework/command/testdata/standalone/config.yaml @@ -3,4 +3,5 @@ apiVersion: example.com/v1alpha1 kind: Foo -a: b \ No newline at end of file +a: c +b: 1 diff --git a/kyaml/fn/framework/command/testdata/standalone/expected.yaml b/kyaml/fn/framework/command/testdata/standalone/expected.yaml index 381ccf4c94..54bc1e48cc 100644 --- a/kyaml/fn/framework/command/testdata/standalone/expected.yaml +++ b/kyaml/fn/framework/command/testdata/standalone/expected.yaml @@ -8,7 +8,8 @@ metadata: namespace: default annotations: foo: bar2 - a: 'b' + a: 'c' + b: '1' --- apiVersion: apps/v1 kind: Deployment @@ -17,4 +18,5 @@ metadata: namespace: default annotations: foo: bar1 - a: 'b' + a: 'c' + b: '1' diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index a6095ca63f..4b078d1c7f 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -479,7 +479,12 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { } if s.IsSet { - if err := def.PipeE(&yaml.FieldSetter{Name: "isSet", StringValue: "true"}); err != nil { + n := &yaml.Node{ + Kind: yaml.ScalarNode, + Value: "true", + Tag: yaml.NodeTagBool, + } + if err := def.PipeE(&yaml.FieldSetter{Name: "isSet", Value: yaml.NewRNode(n)}); err != nil { return nil, err } } diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 1c8ca386fb..c4ab1fb53d 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -685,6 +685,12 @@ func (s FieldSetter) Filter(rn *RNode) (*RNode, error) { s.Value = NewScalarRNode(s.StringValue) } + // need to set style for strings not recognized by yaml 1.1 to quoted if not previously set + // TODO: fix in upstream yaml library so this can be handled with yaml SetString + if s.Value.IsStringValue() && !s.OverrideStyle && s.Value.YNode().Style == 0 && IsYaml1_1NonString(s.Value.YNode()) { + s.Value.YNode().Style = yaml.DoubleQuotedStyle + } + if s.Name == "" { if err := ErrorIfInvalid(rn, yaml.ScalarNode); err != nil { return rn, err diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index 84b16c71df..ed69fe992d 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -248,6 +248,11 @@ func (rn *RNode) IsNilOrEmpty() bool { IsYNodeZero(rn.YNode()) } +// IsStringValue is true if the RNode is not nil and is scalar string node +func (rn *RNode) IsStringValue() bool { + return !rn.IsNil() && IsYNodeString(rn.YNode()) +} + // GetMeta returns the ResourceMeta for an RNode func (rn *RNode) GetMeta() (ResourceMeta, error) { if IsMissingOrNull(rn) {