From 1426137883699aa05d8f2ba6444ce5d9f37c6371 Mon Sep 17 00:00:00 2001 From: jregan Date: Wed, 23 Dec 2020 16:23:45 -0800 Subject: [PATCH] Isolate scalar quoting oddities to one test set. The apimachinery code path, in its final marshalling for output, calls Marshall https://github.com/go-yaml/yaml/blob/v2/yaml.go#L199 This code path (via apimachinery Unstructured types) has no JSON schema tags https://yaml.org/spec/1.2/spec.html#id2803311 so it adds quotes to values that smell like booleans and ints (e.g. `false` becomes `"false"`). The kyaml code path, OTOH, uses such tags, so generally does not quote ints and booleans. This PR isolates this difference in behavior to one set of tests (using data fields in configmaps in api/krusty/configmaps_test.go) so that they don't confuse other tests that cover completely different behaviors. --- api/krusty/component_test.go | 84 +++++++++--------- api/krusty/configmaps_test.go | 104 +++++++++++++++++++++-- api/krusty/fnplugin_test.go | 23 +++-- api/krusty/nameprefixsuffixpatch_test.go | 10 +-- api/krusty/namespacedgenerators_test.go | 10 +-- api/krusty/options.go | 7 ++ api/krusty/transformerplugin_test.go | 4 +- api/testutils/kusttest/harness.go | 6 -- 8 files changed, 172 insertions(+), 76 deletions(-) diff --git a/api/krusty/component_test.go b/api/krusty/component_test.go index 09bdbf746..25ae4f858 100644 --- a/api/krusty/component_test.go +++ b/api/krusty/component_test.go @@ -39,8 +39,8 @@ resources: configMapGenerator: - name: my-configmap literals: - - testValue=1 - - otherValue=10 + - testValue=purple + - otherValue=green `) th.WriteF("/app/base/deploy.yaml", ` apiVersion: v1 @@ -64,8 +64,8 @@ configMapGenerator: - name: my-configmap behavior: merge literals: - - testValue=2 - - compValue=5 + - testValue=blue + - compValue=red `) th.WriteF("/app/comp/stub.yaml", ` apiVersion: v1 @@ -125,12 +125,12 @@ spec: --- apiVersion: v1 data: - compValue: "5" - otherValue: "10" - testValue: "2" + compValue: red + otherValue: green + testValue: blue kind: ConfigMap metadata: - name: comp-my-configmap-kc6k2kmkh9 + name: comp-my-configmap-97647ckcmg --- apiVersion: v1 kind: Deployment @@ -154,7 +154,7 @@ configMapGenerator: - name: my-configmap behavior: merge literals: - - otherValue=9 + - otherValue=orange `), writeK("/app/prod", ` resources: @@ -177,12 +177,12 @@ spec: --- apiVersion: v1 data: - compValue: "5" - otherValue: "9" - testValue: "2" + compValue: red + otherValue: orange + testValue: blue kind: ConfigMap metadata: - name: comp-my-configmap-55249mf5kb + name: comp-my-configmap-g486mb229k --- apiVersion: v1 kind: Deployment @@ -208,7 +208,7 @@ configMapGenerator: - name: my-configmap behavior: merge literals: - - otherValue=9 + - otherValue=orange `), writeK("/app/prod", ` resources: @@ -230,12 +230,12 @@ spec: --- apiVersion: v1 data: - compValue: "5" - otherValue: "9" - testValue: "2" + compValue: red + otherValue: orange + testValue: blue kind: ConfigMap metadata: - name: comp-my-configmap-55249mf5kb + name: comp-my-configmap-g486mb229k --- apiVersion: v1 kind: Deployment @@ -273,11 +273,11 @@ spec: --- apiVersion: v1 data: - otherValue: "10" - testValue: "1" + otherValue: green + testValue: purple kind: ConfigMap metadata: - name: my-configmap-2g9c94mhb8 + name: my-configmap-9cd648hm8f --- apiVersion: v1 kind: Deployment @@ -288,12 +288,12 @@ spec: --- apiVersion: v1 data: - compValue: "5" - otherValue: "10" - testValue: "2" + compValue: red + otherValue: green + testValue: blue kind: ConfigMap metadata: - name: comp-my-configmap-kc6k2kmkh9 + name: comp-my-configmap-97647ckcmg --- apiVersion: v1 kind: Deployment @@ -319,8 +319,8 @@ configMapGenerator: - name: my-configmap behavior: merge literals: - - compValue=5 - - testValue=2 + - compValue=red + - testValue=blue `), }, runPath: "/app/direct-component", @@ -334,12 +334,12 @@ spec: --- apiVersion: v1 data: - compValue: "5" - otherValue: "10" - testValue: "2" + compValue: red + otherValue: green + testValue: blue kind: ConfigMap metadata: - name: my-configmap-kc6k2kmkh9 + name: my-configmap-97647ckcmg `, }, "missing-optional-component-api-version": { @@ -350,7 +350,7 @@ configMapGenerator: - name: my-configmap behavior: merge literals: - - otherValue=9 + - otherValue=orange `), }, runPath: "/app/prod", @@ -364,11 +364,11 @@ spec: --- apiVersion: v1 data: - otherValue: "9" - testValue: "1" + otherValue: orange + testValue: purple kind: ConfigMap metadata: - name: my-configmap-5g7gh5mgt5 + name: my-configmap-6hhdg8gkdg --- apiVersion: v1 kind: Deployment @@ -411,11 +411,11 @@ spec: --- apiVersion: v1 data: - otherValue: "10" - testValue: "1" + otherValue: green + testValue: purple kind: ConfigMap metadata: - name: my-configmap-a-b-2g9c94mhb8 + name: my-configmap-a-b-9cd648hm8f --- apiVersion: v1 kind: Deployment @@ -426,11 +426,11 @@ spec: --- apiVersion: v1 data: - otherValue: "10" - testValue: "1" + otherValue: green + testValue: purple kind: ConfigMap metadata: - name: my-configmap-b-2g9c94mhb8 + name: my-configmap-b-9cd648hm8f `, }, @@ -562,7 +562,7 @@ configMapGenerator: - name: my-configmap behavior: merge literals: - - otherValue=9 + - otherValue=orange `), }, runPath: "/app/prod", diff --git a/api/krusty/configmaps_test.go b/api/krusty/configmaps_test.go index 8cbc1f632..5736206c8 100644 --- a/api/krusty/configmaps_test.go +++ b/api/krusty/configmaps_test.go @@ -7,10 +7,97 @@ import ( "fmt" "testing" - "sigs.k8s.io/kustomize/api/konfig" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) +// Numbers and booleans are quoted +func TestGeneratorIntVsStringNoMerge(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteK(".", ` +resources: +- service.yaml +configMapGenerator: +- name: bob + literals: + - fruit=Indian Gooseberry + - year=2020 + - crisis=true +`) + th.WriteF("service.yaml", ` +apiVersion: v1 +kind: Service +metadata: + annotations: + port: 8080 + happy: true + color: green + name: demo +spec: + clusterIP: None +`) + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected( + m, ` +apiVersion: v1 +kind: Service +metadata: + annotations: + color: green + happy: true + port: 8080 + name: demo +spec: + clusterIP: None +--- +apiVersion: v1 +data: + crisis: "true" + fruit: Indian Gooseberry + year: "2020" +kind: ConfigMap +metadata: + name: bob-79t79mt227 +`) +} + +// Observation: Numbers no longer quoted +func TestGeneratorIntVsStringWithMerge(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteK("base", ` +configMapGenerator: +- name: bob + literals: + - fruit=Indian Gooseberry + - year=2020 + - crisis=true +`) + th.WriteK("overlay", ` +resources: +- ../base +configMapGenerator: +- name: bob + behavior: merge + literals: + - month=12 +`) + opts := th.MakeDefaultOptions() + m := th.Run("overlay", opts) + expFmt := `apiVersion: v1 +data: + crisis: %s + fruit: Indian Gooseberry + month: %s + year: %s +kind: ConfigMap +metadata: + name: bob-%s +` + th.AssertActualEqualsExpected( + m, opts.IfApiMachineryElseKyaml( + fmt.Sprintf(expFmt, `"true"`, `"12"`, `"2020"`, `bk46gm59c6`), + fmt.Sprintf(expFmt, `true`, `12`, `2020`, `bkmtk2t2fb`))) +} + // Generate a Secret and a ConfigMap from the same data // to compare the result. func TestGeneratorBasics(t *testing.T) { @@ -61,8 +148,8 @@ electromagnetic strong nuclear weak nuclear `) - - m := th.Run("/app", th.MakeDefaultOptionsWithProperEnableKyaml()) + opts := th.MakeDefaultOptions() + m := th.Run("/app", opts) expFmt := `apiVersion: v1 data: MOUNTAIN: everest @@ -102,19 +189,20 @@ data: vegetable: YnJvY2NvbGk= kind: Secret metadata: - name: blah-bob-9t25t44gg4 + name: blah-bob-%s type: Opaque ` th.AssertActualEqualsExpected( - m, konfig.IfApiMachineryElseKyaml( + m, opts.IfApiMachineryElseKyaml( fmt.Sprintf(expFmt, - `CmdyYXZpdGF0aW9uYWwKZWxlY3Ryb21hZ25ldGljCnN0cm9uZyBudWNsZWFyCndlYWsgbnVjbGVhcgo`, - `CkxpZmUgaXMgc2hvcnQuCkJ1dCB0aGUgeWVhcnMgYXJlIGxvbmcuCk5vdCB3aGlsZSB0aGUgZXZpbCBkYXlzIGNvbWUgbm90Lgo`), + `CmdyYXZpdGF0aW9uYWwKZWxlY3Ryb21hZ25ldGljCnN0cm9uZyBudWNsZWFyCndlYWsgbnVjbGVhcgo=`, + `CkxpZmUgaXMgc2hvcnQuCkJ1dCB0aGUgeWVhcnMgYXJlIGxvbmcuCk5vdCB3aGlsZSB0aGUgZXZpbCBkYXlzIGNvbWUgbm90Lgo=`, + `ftht6hfgmb`), fmt.Sprintf(expFmt, `| CmdyYXZpdGF0aW9uYWwKZWxlY3Ryb21hZ25ldGljCnN0cm9uZyBudWNsZWFyCndlYWsgbn VjbGVhcgo=`, `| CkxpZmUgaXMgc2hvcnQuCkJ1dCB0aGUgeWVhcnMgYXJlIGxvbmcuCk5vdCB3aGlsZSB0aG - UgZXZpbCBkYXlzIGNvbWUgbm90Lgo=`))) + UgZXZpbCBkYXlzIGNvbWUgbm90Lgo=`, `9t25t44gg4`))) } // TODO: These should be errors instead. diff --git a/api/krusty/fnplugin_test.go b/api/krusty/fnplugin_test.go index b45316a23..97aa4b153 100644 --- a/api/krusty/fnplugin_test.go +++ b/api/krusty/fnplugin_test.go @@ -1,6 +1,7 @@ package krusty_test import ( + "fmt" "os/exec" "testing" @@ -128,8 +129,9 @@ stringData: bootcmd: - mkdir /mnt/vda `) - m := th.Run("/app", th.MakeOptionsPluginsEnabled()) - th.AssertActualEqualsExpected(m, ` + opts := th.MakeOptionsPluginsEnabled() + m := th.Run("/app", opts) + expFmt := ` apiVersion: v1 kind: Secret metadata: @@ -152,7 +154,7 @@ metadata: name: demo name: demo-budget spec: - minAvailable: 67% + minAvailable: 67%% selector: matchLabels: app: cockroachdb @@ -185,9 +187,7 @@ metadata: annotations: config.kubernetes.io/path: config/demo_service.yaml prometheus.io/path: _status/vars - prometheus.io/port: "8080" - prometheus.io/scrape: "true" - service.alpha.kubernetes.io/tolerate-unready-endpoints: "true" +%s 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,7 +302,14 @@ 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/nameprefixsuffixpatch_test.go b/api/krusty/nameprefixsuffixpatch_test.go index ed511d185..da83b92dc 100644 --- a/api/krusty/nameprefixsuffixpatch_test.go +++ b/api/krusty/nameprefixsuffixpatch_test.go @@ -38,7 +38,7 @@ configMapGenerator: literals: - MYSQL_USER=default - MYSQL_DATABASE=default - - PORT=3306 + - HOST=everest `) th.WriteK(".", ` @@ -82,13 +82,13 @@ patches: th.AssertActualEqualsExpected(m, ` apiVersion: v1 data: + HOST: everest MYSQL_DATABASE: db MYSQL_PASSWORD: correct horse battery staple MYSQL_USER: my-user - PORT: "3306" kind: ConfigMap metadata: - name: mysql-9792mdchtg + name: mysql-t7tt4cdbmf --- apiVersion: apps/v1 kind: Deployment @@ -102,10 +102,10 @@ spec: - valueFrom: configMapKeyRef: key: MYSQL_DATABASE - name: mysql-9792mdchtg + name: mysql-t7tt4cdbmf envFrom: - configMapRef: - name: mysql-9792mdchtg + name: mysql-t7tt4cdbmf name: handler `) } diff --git a/api/krusty/namespacedgenerators_test.go b/api/krusty/namespacedgenerators_test.go index 30cbdac00..e589b9246 100644 --- a/api/krusty/namespacedgenerators_test.go +++ b/api/krusty/namespacedgenerators_test.go @@ -80,7 +80,7 @@ namespace: base configMapGenerator: - name: testCase literals: - - base=true + - base=apple `) th.WriteK("/app/overlay", ` resources: @@ -92,17 +92,17 @@ configMapGenerator: - name: testCase behavior: merge literals: - - overlay=true + - overlay=peach `) m := th.Run("/app/overlay", th.MakeDefaultOptions()) th.AssertActualEqualsExpected(m, ` apiVersion: v1 data: - base: "true" - overlay: "true" + base: apple + overlay: peach kind: ConfigMap metadata: - name: testCase-bcbmmg48hd + name: testCase-gmfch8gkbt namespace: overlay `) } diff --git a/api/krusty/options.go b/api/krusty/options.go index 85e72dac2..9e5d4f094 100644 --- a/api/krusty/options.go +++ b/api/krusty/options.go @@ -55,3 +55,10 @@ func MakeDefaultOptions() *Options { AllowResourceIdChanges: false, } } + +func (o Options) IfApiMachineryElseKyaml(s1, s2 string) string { + if !o.UseKyaml { + return s1 + } + return s2 +} diff --git a/api/krusty/transformerplugin_test.go b/api/krusty/transformerplugin_test.go index 110baaabd..049e53915 100644 --- a/api/krusty/transformerplugin_test.go +++ b/api/krusty/transformerplugin_test.go @@ -74,7 +74,7 @@ kind: ConfigMap metadata: name: configmap-a annotations: - kustomize.k8s.io/Generated: "false" + fruit: peach data: foo: $FOO `) @@ -87,7 +87,7 @@ data: kind: ConfigMap metadata: annotations: - kustomize.k8s.io/Generated: "false" + fruit: peach name: configmap-a --- apiVersion: v1 diff --git a/api/testutils/kusttest/harness.go b/api/testutils/kusttest/harness.go index ebc50eb25..f1d8706ac 100644 --- a/api/testutils/kusttest/harness.go +++ b/api/testutils/kusttest/harness.go @@ -70,12 +70,6 @@ func (th Harness) MakeDefaultOptions() krusty.Options { return th.MakeOptionsPluginsDisabled() } -func (th Harness) MakeDefaultOptionsWithProperEnableKyaml() krusty.Options { - o := th.MakeOptionsPluginsDisabled() - o.UseKyaml = konfig.FlagEnableKyamlDefaultValue - return o -} - // This has no impact on Builtin plugins, as they are always enabled. func (th Harness) MakeOptionsPluginsDisabled() krusty.Options { return *krusty.MakeDefaultOptions()