diff --git a/cmd/config/internal/commands/cat_test.go b/cmd/config/internal/commands/cat_test.go index c02b8f200..d9441713d 100644 --- a/cmd/config/internal/commands/cat_test.go +++ b/cmd/config/internal/commands/cat_test.go @@ -114,8 +114,8 @@ metadata: app: nginx annotations: app: nginx - config.kubernetes.io/package: . - config.kubernetes.io/path: f2.yaml + config.kubernetes.io/package: '.' + config.kubernetes.io/path: 'f2.yaml' spec: replicas: 3 `, b.String()) { @@ -218,8 +218,8 @@ metadata: name: foo annotations: config.kubernetes.io/local-config: "true" - config.kubernetes.io/package: . - config.kubernetes.io/path: f2.yaml + config.kubernetes.io/package: '.' + config.kubernetes.io/path: 'f2.yaml' configFn: container: image: gcr.io/example/image:version @@ -314,8 +314,8 @@ metadata: name: foo annotations: config.kubernetes.io/local-config: "true" - config.kubernetes.io/package: . - config.kubernetes.io/path: f2.yaml + config.kubernetes.io/package: '.' + config.kubernetes.io/path: 'f2.yaml' configFn: container: image: gcr.io/example/reconciler:v1 @@ -438,8 +438,8 @@ metadata: app: nginx annotations: app: nginx - config.kubernetes.io/package: . - config.kubernetes.io/path: f2.yaml + config.kubernetes.io/package: '.' + config.kubernetes.io/path: 'f2.yaml' spec: replicas: 3 `, string(actual)) { @@ -560,8 +560,8 @@ metadata: app: nginx annotations: app: nginx - config.kubernetes.io/package: . - config.kubernetes.io/path: f2.yaml + config.kubernetes.io/package: '.' + config.kubernetes.io/path: 'f2.yaml' spec: replicas: 3 `, string(actual)) { diff --git a/cmd/config/internal/commands/cmdwrap_test.go b/cmd/config/internal/commands/cmdwrap_test.go index affa88bf1..a55a951e5 100644 --- a/cmd/config/internal/commands/cmdwrap_test.go +++ b/cmd/config/internal/commands/cmdwrap_test.go @@ -78,8 +78,8 @@ items: name: test app: nginx annotations: - config.kubernetes.io/index: "0" - config.kubernetes.io/path: config/test_deployment.yaml + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'config/test_deployment.yaml' spec: replicas: 11 selector: @@ -109,8 +109,8 @@ items: name: test app: nginx annotations: - config.kubernetes.io/index: "0" - config.kubernetes.io/path: config/test_service.yaml + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'config/test_service.yaml' spec: selector: name: test @@ -133,8 +133,8 @@ items: name: test app: nginx annotations: - config.kubernetes.io/index: "0" - config.kubernetes.io/path: config/test_deployment.yaml + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'config/test_deployment.yaml' spec: replicas: 11 selector: @@ -161,8 +161,8 @@ items: name: test app: nginx annotations: - config.kubernetes.io/index: "0" - config.kubernetes.io/path: config/test_service.yaml + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'config/test_service.yaml' spec: selector: name: test @@ -185,8 +185,8 @@ items: name: test app: nginx annotations: - config.kubernetes.io/index: "0" - config.kubernetes.io/path: config/test_deployment.yaml + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'config/test_deployment.yaml' spec: replicas: 11 selector: @@ -216,8 +216,8 @@ items: name: test app: nginx annotations: - config.kubernetes.io/index: "0" - config.kubernetes.io/path: config/test_service.yaml + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'config/test_service.yaml' spec: selector: name: test diff --git a/kyaml/kio/filters/fmtr.go b/kyaml/kio/filters/fmtr.go index 201c74f0d..04796ed9b 100644 --- a/kyaml/kio/filters/fmtr.go +++ b/kyaml/kio/filters/fmtr.go @@ -90,7 +90,6 @@ type formatter struct { // fmtNode recursively formats the Document Contents. func (f *formatter) fmtNode(n *yaml.Node, path string) error { - n.Style = 0 // sort the order of mapping fields if n.Kind == yaml.MappingNode { sort.Sort(sortedMapContents(*n)) diff --git a/kyaml/kio/filters/fmtr_test.go b/kyaml/kio/filters/fmtr_test.go index a60aa23a4..e40c403ea 100644 --- a/kyaml/kio/filters/fmtr_test.go +++ b/kyaml/kio/filters/fmtr_test.go @@ -17,6 +17,35 @@ import ( "sigs.k8s.io/kustomize/kyaml/kio/filters/testyaml" ) +func TestFormatInput_Style(t *testing.T) { + y := ` +apiVersion: v1 +kind: Foo +metadata: + name: foo +spec: + notBoolean: "true" + notBoolean2: "on" + isBoolean: on + isBoolean2: true +` + + expected := `apiVersion: v1 +kind: Foo +metadata: + name: foo +spec: + isBoolean: on + isBoolean2: true + notBoolean: "true" + notBoolean2: "on" +` + + s, err := FormatInput(strings.NewReader(y)) + assert.NoError(t, err) + assert.Equal(t, expected, s.String()) +} + // TestFormatInput_configMap verifies a ConfigMap yaml is formatted correctly func TestFormatInput_configMap(t *testing.T) { y := ` @@ -229,7 +258,7 @@ webhooks: - CONNECT - CREATE - UPDATE # this list is indented by 2 - scope: Namespaced + scope: "Namespaced" timeoutSeconds: 1 ` s, err := FormatInput(strings.NewReader(y)) @@ -467,7 +496,7 @@ func TestFormatFileOrDirectory_YamlExtFileWithJson(t *testing.T) { // check the result is formatted as yaml b, err := ioutil.ReadFile(f.Name()) assert.NoError(t, err) - assert.Equal(t, string(testyaml.FormattedYaml1), string(b)) + assert.Equal(t, string(testyaml.FormattedJSON1), string(b)) } // TestFormatFileOrDirectory_partialKubernetesYamlFile verifies that if a yaml file contains both diff --git a/kyaml/kio/filters/testyaml/testyaml.go b/kyaml/kio/filters/testyaml/testyaml.go index 67593d026..555d95da0 100644 --- a/kyaml/kio/filters/testyaml/testyaml.go +++ b/kyaml/kio/filters/testyaml/testyaml.go @@ -55,3 +55,7 @@ status2: - 1 - 2 `) + +var FormattedJSON1 = []byte(`{"apiVersion": "example.com/v1beta1", "kind": "MyType", "spec": "a", "status": {"conditions": [ + 3, 1, 2]}} +`) diff --git a/kyaml/kio/pkgio_writer_test.go b/kyaml/kio/pkgio_writer_test.go index b6bf6ce30..61562ab8a 100644 --- a/kyaml/kio/pkgio_writer_test.go +++ b/kyaml/kio/pkgio_writer_test.go @@ -68,13 +68,13 @@ func TestLocalPackageWriter_Write_keepReaderAnnotations(t *testing.T) { metadata: annotations: config.kubernetes.io/index: 0 - config.kubernetes.io/path: a/b/a_test.yaml + config.kubernetes.io/path: "a/b/a_test.yaml" --- c: d # second metadata: annotations: config.kubernetes.io/index: 1 - config.kubernetes.io/path: a/b/a_test.yaml + config.kubernetes.io/path: "a/b/a_test.yaml" `, string(b)) b, err = ioutil.ReadFile(filepath.Join(d, "a", "b", "b_test.yaml")) @@ -89,7 +89,7 @@ g: metadata: annotations: config.kubernetes.io/index: 0 - config.kubernetes.io/path: a/b/b_test.yaml + config.kubernetes.io/path: "a/b/b_test.yaml" `, string(b)) } diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 74b2c027d..9de3e26eb 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -446,14 +446,18 @@ type FieldSetter struct { // value on the ScalarNode. Name string `yaml:"name,omitempty"` - // YNode is the value to set. + // Value is the value to set. // Optional if Kind is set. Value *RNode `yaml:"value,omitempty"` StringValue string `yaml:"stringValue,omitempty"` + + // OverrideStyle can be set to override the style of the existing node + // when setting it. Otherwise, if an existing node is found, the style is + // retained. + OverrideStyle bool `yaml:"overrideStyle,omitempty"` } -// FieldSetter returns an GrepFilter that sets the named field to the given value. func (s FieldSetter) Filter(rn *RNode) (*RNode, error) { if s.StringValue != "" && s.Value == nil { s.Value = NewScalarRNode(s.StringValue) @@ -463,6 +467,12 @@ func (s FieldSetter) Filter(rn *RNode) (*RNode, error) { if err := ErrorIfInvalid(rn, yaml.ScalarNode); err != nil { return rn, err } + // only apply the style if there is not an existing style + // or we want to override it + if !s.OverrideStyle || s.Value.YNode().Style == 0 { + // keep the original style if it exists + s.Value.YNode().Style = rn.YNode().Style + } rn.SetYNode(s.Value.YNode()) return rn, nil } @@ -477,6 +487,12 @@ func (s FieldSetter) Filter(rn *RNode) (*RNode, error) { return nil, err } if field != nil { + // only apply the style if there is not an existing style + // or we want to override it + if !s.OverrideStyle || field.YNode().Style == 0 { + // keep the original style if it exists + s.Value.YNode().Style = field.YNode().Style + } // need to def ref the Node since field is ephemeral field.SetYNode(s.Value.YNode()) return field, nil diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 1bb05ce4d..e9590aa3f 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -646,6 +646,25 @@ metadata: `, assertNoErrorString(t)(r0.String())) } +func TestUpdateAnnotation_Fn(t *testing.T) { + // create metadata.annotations field + r0 := assertNoError(t)(Parse(`apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + a.b.c: "h.i.j" +`)) + + rn := assertNoError(t)(r0.Pipe(SetAnnotation("a.b.c", "d.e.f"))) + assert.Equal(t, "\"d.e.f\"\n", assertNoErrorString(t)(rn.String())) + assert.Equal(t, `apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + a.b.c: "d.e.f" +`, assertNoErrorString(t)(r0.String())) +} + func TestRNode_GetMeta(t *testing.T) { s := `apiVersion: v1/apps kind: Deployment diff --git a/kyaml/yaml/merge2/element_test.go b/kyaml/yaml/merge2/element_test.go index bdb1100d2..2a9cd5d6e 100644 --- a/kyaml/yaml/merge2/element_test.go +++ b/kyaml/yaml/merge2/element_test.go @@ -23,8 +23,7 @@ kind: Deployment items: - name: foo image: foo:v1 - command: - - run.sh + command: ['run.sh'] `, }, @@ -47,8 +46,7 @@ kind: Deployment items: - name: foo image: foo:v1 - command: - - run.sh + command: ['run.sh'] `, }, @@ -62,15 +60,14 @@ items: `, ` kind: Deployment -items: {} +items: [] `, ` kind: Deployment items: - name: foo image: foo:v1 - command: - - run.sh + command: ['run.sh'] `, }, @@ -90,8 +87,7 @@ kind: Deployment items: - name: foo image: foo:v1 - command: - - run.sh + command: ['run.sh'] `, }, @@ -118,8 +114,7 @@ items: image: foo:v1 - name: bar image: bar:v1 - command: - - run2.sh + command: ['run2.sh'] `, }, @@ -146,8 +141,7 @@ items: image: foo:v1 - name: bar image: bar:v1 - command: - - run2.sh + command: ['run2.sh'] `, }, @@ -174,8 +168,7 @@ items: image: foo:v1 - name: bar image: bar:v1 - command: - - run2.sh + command: ['run2.sh'] `, }, @@ -205,8 +198,7 @@ items: image: foo:v1 - name: bar image: bar:v1 - command: - - run2.sh + command: ['run2.sh'] `, }, @@ -225,8 +217,7 @@ items: image: foo:v1 - name: bar image: bar:v1 - command: - - run2.sh + command: ['run2.sh'] `, ` kind: Deployment @@ -235,8 +226,7 @@ items: image: foo:v1 - name: bar image: bar:v1 - command: - - run2.sh + command: ['run2.sh'] `, }, @@ -255,8 +245,7 @@ items: image: foo:v1 - name: bar image: bar:v1 - command: - - run2.sh + command: ['run2.sh'] `, ` kind: Deployment diff --git a/kyaml/yaml/merge2/merge2.go b/kyaml/yaml/merge2/merge2.go index f4620f748..86f3b5ece 100644 --- a/kyaml/yaml/merge2/merge2.go +++ b/kyaml/yaml/merge2/merge2.go @@ -43,6 +43,9 @@ func (m Merger) VisitMap(nodes walk.Sources) (*yaml.RNode, error) { if err := m.SetComments(nodes); err != nil { return nil, err } + if err := m.SetStyle(nodes); err != nil { + return nil, err + } if yaml.IsEmpty(nodes.Dest()) { // Add return nodes.Origin(), nil @@ -59,6 +62,9 @@ func (m Merger) VisitScalar(nodes walk.Sources) (*yaml.RNode, error) { if err := m.SetComments(nodes); err != nil { return nil, err } + if err := m.SetStyle(nodes); err != nil { + return nil, err + } // Override value if nodes.Origin() != nil { return nodes.Origin(), nil @@ -71,6 +77,9 @@ func (m Merger) VisitList(nodes walk.Sources, kind walk.ListKind) (*yaml.RNode, if err := m.SetComments(nodes); err != nil { return nil, err } + if err := m.SetStyle(nodes); err != nil { + return nil, err + } if kind == walk.NonAssociateList { // Override value if nodes.Origin() != nil { @@ -92,6 +101,25 @@ func (m Merger) VisitList(nodes walk.Sources, kind walk.ListKind) (*yaml.RNode, return nodes.Dest(), nil } +func (m Merger) SetStyle(sources walk.Sources) error { + source := sources.Origin() + dest := sources.Dest() + if dest == nil || dest.YNode() == nil || source == nil || source.YNode() == nil { + // avoid panic + return nil + } + + // copy the style from the source. + // special case: if the dest was an empty map or seq, then it probably had + // folded style applied, but we actually want to keep the style of the origin + // in this case (even if it was the default). otherwise the merged elements + // will get folded even though this probably isn't what is desired. + if dest.YNode().Kind != yaml.ScalarNode && len(dest.YNode().Content) == 0 { + dest.YNode().Style = source.YNode().Style + } + return nil +} + // SetComments copies the dest comments to the source comments if they are present // on the source. func (m Merger) SetComments(sources walk.Sources) error { diff --git a/kyaml/yaml/merge2/merge2_old_test.go b/kyaml/yaml/merge2/merge2_old_test.go index 95fce2b41..e4855b72c 100644 --- a/kyaml/yaml/merge2/merge2_old_test.go +++ b/kyaml/yaml/merge2/merge2_old_test.go @@ -83,10 +83,7 @@ spec: containers: - name: nginx image: nginx:1.7.9 - args: - - c - - a - - b + args: ['c', 'a', 'b'] env: - name: DEMO_GREETING value: "Hello from the environment" @@ -139,10 +136,7 @@ spec: containers: - name: nginx image: nginx:1.7.9 - args: - - c - - a - - b + args: ['c', 'a', 'b'] env: - name: DEMO_GREETING value: "Hello from the environment" @@ -208,10 +202,7 @@ spec: containers: - name: nginx image: nginx:1.7.9 - args: - - c - - a - - b + args: ['c', 'a', 'b'] env: - name: DEMO_GREETING value: "Hello from the environment" @@ -276,10 +267,7 @@ spec: containers: - name: nginx image: nginx:1.7.9 - args: - - c - - a - - b + args: ['c', 'a', 'b'] env: - name: DEMO_GREETING value: "New Demo Greeting" @@ -343,10 +331,7 @@ spec: containers: - name: nginx image: nginx:1.7.9 - args: - - e - - d - - f + args: ['e', 'd', 'f'] env: - name: DEMO_GREETING value: "Hello from the environment" diff --git a/kyaml/yaml/merge3/element_test.go b/kyaml/yaml/merge3/element_test.go index d3a28a549..7438d5ce8 100644 --- a/kyaml/yaml/merge3/element_test.go +++ b/kyaml/yaml/merge3/element_test.go @@ -174,8 +174,7 @@ kind: Deployment containers: - name: foo image: foo:bar - command: - - run2.sh + command: ['run2.sh'] `, nil}, // @@ -206,8 +205,7 @@ kind: Deployment containers: - name: foo image: foo:bar - command: - - run2.sh + command: ['run2.sh'] `, nil}, // @@ -239,8 +237,7 @@ kind: Deployment containers: - name: foo image: foo:bar - command: - - run2.sh + command: ['run2.sh'] `, nil}, // @@ -367,8 +364,7 @@ kind: Deployment containers: - name: foo image: foo:bar - command: - - run.sh + command: ['run.sh'] `, nil}, // diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index 46fa745fe..99d3eb668 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -146,9 +146,6 @@ func NewListRNode(values ...string) *RNode { // NewRNode returns a new RNode pointer containing the provided Node. func NewRNode(value *yaml.Node) *RNode { - if value != nil { - value.Style = 0 - } return &RNode{value: value} } @@ -389,7 +386,7 @@ const ( Flow = "Flow" ) -// String returns a string valuer for a Node, applying the supplied formatting options +// String returns a string value for a Node, applying the supplied formatting options func String(node *yaml.Node, opts ...string) (string, error) { if node == nil { return "", nil