diff --git a/api/filters/annotations/example_test.go b/api/filters/annotations/example_test.go index 270193496..75eb51314 100644 --- a/api/filters/annotations/example_test.go +++ b/api/filters/annotations/example_test.go @@ -28,7 +28,9 @@ metadata: `)}}, Filters: []kio.Filter{Filter{ Annotations: map[string]string{ - "foo": "bar", + "foo": "bar", + "booleanValue": "true", + "numberValue": "42", }, FsSlice: fss, }}, @@ -44,12 +46,16 @@ metadata: // metadata: // name: instance // annotations: + // booleanValue: "true" // foo: bar + // numberValue: "42" // --- // apiVersion: example.com/v1 // kind: Bar // metadata: // name: instance // annotations: + // booleanValue: "true" // foo: bar + // numberValue: "42" } diff --git a/api/krusty/basic_io_test.go b/api/krusty/basic_io_test.go index 9700ec41a..1ed567605 100644 --- a/api/krusty/basic_io_test.go +++ b/api/krusty/basic_io_test.go @@ -4,7 +4,6 @@ package krusty_test import ( - "fmt" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -28,7 +27,8 @@ func TestBasicIO_1(t *testing.T) { // return an error (that is then ignored by GetAnnotations) // } // The error happens when any annotation value can be interpreted as - // a boolean or number. + // a boolean or number. Such annotations cannot be successfully applied + // to an object in a cluster unless they are quoted. t.SkipNow() } th.WriteK(".", ` @@ -49,6 +49,7 @@ spec: `) m := th.Run(".", opts) // The annotations are sorted by key, hence the order change. + // Quotes are added intentionally. th.AssertActualEqualsExpected( m, ` apiVersion: v1 @@ -56,8 +57,8 @@ kind: Service metadata: annotations: color: green - happy: true - port: 8080 + happy: "true" + port: "8080" name: demo spec: clusterIP: None @@ -71,9 +72,7 @@ func TestBasicIO_2(t *testing.T) { resources: - service.yaml `) - // All the annotation values are quoted. - // The apimachinery code path retains the quotes, the kyaml - // path drops them at the moment. + // All the annotation values are quoted in the input. th.WriteF("service.yaml", ` apiVersion: v1 kind: Service @@ -88,20 +87,16 @@ spec: `) m := th.Run(".", opts) // The annotations are sorted by key, hence the order change. - expFmt := ` + th.AssertActualEqualsExpected(m, ` apiVersion: v1 kind: Service metadata: annotations: color: green - happy: %s - port: %s + happy: "true" + port: "8080" name: demo spec: clusterIP: None -` - th.AssertActualEqualsExpected( - m, opts.IfApiMachineryElseKyaml( - fmt.Sprintf(expFmt, `"true"`, `"8080"`), - fmt.Sprintf(expFmt, `true`, `8080`))) +`) } diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index 97aa4b153..b45316a23 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -1,7 +1,6 @@ package krusty_test import ( - "fmt" "os/exec" "testing" @@ -129,9 +128,8 @@ stringData: bootcmd: - mkdir /mnt/vda `) - opts := th.MakeOptionsPluginsEnabled() - m := th.Run("/app", opts) - expFmt := ` + m := th.Run("/app", th.MakeOptionsPluginsEnabled()) + th.AssertActualEqualsExpected(m, ` apiVersion: v1 kind: Secret metadata: @@ -154,7 +152,7 @@ metadata: name: demo name: demo-budget spec: - minAvailable: 67%% + minAvailable: 67% selector: matchLabels: app: cockroachdb @@ -187,7 +185,9 @@ metadata: annotations: config.kubernetes.io/path: config/demo_service.yaml prometheus.io/path: _status/vars -%s + prometheus.io/port: "8080" + prometheus.io/scrape: "true" + service.alpha.kubernetes.io/tolerate-unready-endpoints: "true" labels: app: cockroachdb name: demo @@ -244,7 +244,7 @@ spec: - /bin/bash - -ecx - | - # The use of qualified ` + "`hostname -f`" + ` is crucial: + # The use of qualified `+"`hostname -f`"+` is crucial: # Other nodes aren't able to look up the unqualified hostname. CRARGS=("start" "--logtostderr" "--insecure" "--host" "$(hostname -f)" "--http-host" "0.0.0.0") # We only want to initialize a new cluster (by omitting the join flag) @@ -302,14 +302,7 @@ spec: resources: requests: storage: 1Gi -` - th.AssertActualEqualsExpected(m, opts.IfApiMachineryElseKyaml( - fmt.Sprintf(expFmt, ` prometheus.io/port: "8080" - prometheus.io/scrape: "true" - service.alpha.kubernetes.io/tolerate-unready-endpoints: "true"`), - fmt.Sprintf(expFmt, ` prometheus.io/port: 8080 - prometheus.io/scrape: true - service.alpha.kubernetes.io/tolerate-unready-endpoints: true`))) +`) } func TestFnContainerTransformer(t *testing.T) { diff --git a/api/krusty/variableref_test.go b/api/krusty/variableref_test.go index 41b55ffcf..e2c95591d 100644 --- a/api/krusty/variableref_test.go +++ b/api/krusty/variableref_test.go @@ -757,9 +757,9 @@ kind: Service metadata: annotations: prometheus.io/path: _status/vars - prometheus.io/port: %s - prometheus.io/scrape: %s - service.alpha.kubernetes.io/tolerate-unready-endpoints: %s + prometheus.io/port: "8080" + prometheus.io/scrape: "true" + service.alpha.kubernetes.io/tolerate-unready-endpoints: "true" labels: app: cockroachdb name: dev-base-cockroachdb @@ -929,12 +929,8 @@ metadata: ` th.AssertActualEqualsExpected(m, opts.IfApiMachineryElseKyaml( - fmt.Sprintf( - expFmt, - `"8080"`, `"true"`, `"true"`, `8080`, `8080`, `8080`, `8080`), - fmt.Sprintf( - expFmt, - `8080`, `true`, `true`, `"8080"`, `"8080"`, `"8080"`, `"8080"`), + fmt.Sprintf(expFmt, `8080`, `8080`, `8080`, `8080`), + fmt.Sprintf(expFmt, `"8080"`, `"8080"`, `"8080"`, `"8080"`), )) } diff --git a/kyaml/yaml/datamap.go b/kyaml/yaml/datamap.go index b3e7d5683..3181ff30f 100644 --- a/kyaml/yaml/datamap.go +++ b/kyaml/yaml/datamap.go @@ -52,11 +52,13 @@ func makeConfigMapValueRNode(s string) (field string, rN *RNode) { } func (rn *RNode) LoadMapIntoSecretData(m map[string]string) error { + mapNode, err := rn.Pipe(LookupCreate(MappingNode, DataField)) + if err != nil { + return err + } for _, k := range SortedMapKeys(m) { vrN := makeSecretValueRNode(m[k]) - if _, err := rn.Pipe( - LookupCreate(MappingNode, DataField), - SetField(k, vrN)); err != nil { + if _, err := mapNode.Pipe(SetField(k, vrN)); err != nil { return err } } diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index a8131705c..f0e001cfe 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -372,18 +372,7 @@ func (rn *RNode) GetAnnotations() (map[string]string, error) { // SetAnnotations tries to set the metadata annotations field. func (rn *RNode) SetAnnotations(m map[string]string) error { - meta, err := rn.Pipe(Lookup(MetadataField)) - if err != nil { - return err - } - if len(m) == 0 { - if meta == nil { - return nil - } - return meta.PipeE(Clear(AnnotationsField)) - } - return rn.SetMapField( - NewMapRNode(&m), MetadataField, AnnotationsField) + return rn.setMapInMetadata(m, AnnotationsField) } // GetLabels gets the metadata labels field. @@ -397,18 +386,32 @@ func (rn *RNode) GetLabels() (map[string]string, error) { // SetLabels sets the metadata labels field. func (rn *RNode) SetLabels(m map[string]string) error { + return rn.setMapInMetadata(m, LabelsField) +} + +// This established proper quoting on string values, and sorts by key. +func (rn *RNode) setMapInMetadata(m map[string]string, field string) error { meta, err := rn.Pipe(Lookup(MetadataField)) if err != nil { return err } - if len(m) == 0 { - if meta == nil { - return nil - } - return meta.PipeE(Clear(LabelsField)) + if err = meta.PipeE(Clear(field)); err != nil { + return err } - return rn.SetMapField( - NewMapRNode(&m), MetadataField, LabelsField) + if len(m) == 0 { + return nil + } + mapNode, err := meta.Pipe(LookupCreate(MappingNode, field)) + if err != nil { + return err + } + for _, k := range SortedMapKeys(m) { + if _, err := mapNode.Pipe( + SetField(k, NewStringRNode(m[k]))); err != nil { + return err + } + } + return nil } func (rn *RNode) SetMapField(value *RNode, path ...string) error { diff --git a/plugin/builtin/annotationstransformer/AnnotationsTransformer_test.go b/plugin/builtin/annotationstransformer/AnnotationsTransformer_test.go index 8c35f74a7..da11974bc 100644 --- a/plugin/builtin/annotationstransformer/AnnotationsTransformer_test.go +++ b/plugin/builtin/annotationstransformer/AnnotationsTransformer_test.go @@ -9,8 +9,12 @@ import ( kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) -var ( - config = ` +func TestAnnotationsTransformer(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t). + PrepBuiltin("AnnotationsTransformer") + defer th.Reset() + + th.RunTransformerAndCheckResult(` apiVersion: builtin kind: AnnotationsTransformer metadata: @@ -18,11 +22,14 @@ metadata: annotations: app: myApp greeting/morning: a string with blanks + booleanNaked: true + booleanQuoted: "true" + numberNaked: 42 + numberQuoted: "42" fieldSpecs: - path: metadata/annotations create: true -` - input = ` +`, ` apiVersion: v1 kind: Service metadata: @@ -30,25 +37,20 @@ metadata: spec: ports: - port: 7002 -` - expectedOutput = ` +`, ` apiVersion: v1 kind: Service metadata: annotations: app: myApp + booleanNaked: "true" + booleanQuoted: "true" greeting/morning: a string with blanks + numberNaked: "42" + numberQuoted: "42" name: myService spec: ports: - port: 7002 -` -) - -func TestAnnotationsTransformer(t *testing.T) { - th := kusttest_test.MakeEnhancedHarness(t). - PrepBuiltin("AnnotationsTransformer") - defer th.Reset() - - th.RunTransformerAndCheckResult(config, input, expectedOutput) +`) }