From 5fed0f76c8eea42b4d62a06f2ea31f14d506eb07 Mon Sep 17 00:00:00 2001 From: yugo kobayashi Date: Tue, 13 Dec 2022 20:36:26 +0000 Subject: [PATCH] refactor Unmarshal Kustomization struct code --- api/internal/localizer/localizer_test.go | 3 +- api/internal/target/kusttarget_test.go | 20 ++------- api/krusty/basic_io_test.go | 5 +-- api/krusty/configmaps_test.go | 43 +++++++------------ api/krusty/crd_test.go | 6 --- api/krusty/disablenamesuffix_test.go | 4 -- api/krusty/generatormergeandreplace_test.go | 2 - api/krusty/generatoroptions_test.go | 4 -- api/krusty/gvknpatch_test.go | 6 +-- api/krusty/inlinepatch_test.go | 2 - api/krusty/kustomizationmetadata_test.go | 4 +- api/krusty/localconfig_test.go | 4 +- api/krusty/managedbylabel_test.go | 5 +-- api/krusty/namespacedgenerators_test.go | 2 - api/krusty/nameupdateinroleref_test.go | 2 - api/krusty/originannotation_test.go | 16 +++---- api/krusty/poddisruptionbudget_test.go | 2 - api/krusty/transformerannotation_test.go | 4 -- api/types/kustomization.go | 15 +------ api/types/kustomization_test.go | 2 +- .../kustfile/kustomizationfile_test.go | 2 +- 21 files changed, 37 insertions(+), 116 deletions(-) diff --git a/api/internal/localizer/localizer_test.go b/api/internal/localizer/localizer_test.go index f07d0a540..198260b73 100644 --- a/api/internal/localizer/localizer_test.go +++ b/api/internal/localizer/localizer_test.go @@ -227,7 +227,8 @@ suffix: invalid`, }) err := Run("/a", "", "", fSysTest) - require.EqualError(t, err, `unable to localize target "/a": invalid kustomization: json: unknown field "suffix"`) + require.EqualError(t, err, + `unable to localize target "/a": invalid kustomization: kustomization unmarshal error: error unmarshaling JSON: while decoding JSON: json: unknown field "suffix"`) checkFSys(t, fSysExpected, fSysTest) } diff --git a/api/internal/target/kusttarget_test.go b/api/internal/target/kusttarget_test.go index 36966105c..7bc9eb891 100644 --- a/api/internal/target/kusttarget_test.go +++ b/api/internal/target/kusttarget_test.go @@ -139,10 +139,7 @@ commonLabels: func TestMakeCustomizedResMap(t *testing.T) { th := kusttest_test.MakeHarness(t) - th.WriteK("/whatever", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -namePrefix: foo- + th.WriteK("/whatever", `namePrefix: foo- nameSuffix: -bar namespace: ns1 commonLabels: @@ -301,10 +298,7 @@ metadata: func TestConfigurationsOverrideDefault(t *testing.T) { th := kusttest_test.MakeHarness(t) - th.WriteK("/merge-config", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -namePrefix: foo- + th.WriteK("/merge-config", `namePrefix: foo- nameSuffix: -bar namespace: ns1 resources: @@ -401,10 +395,7 @@ metadata: func TestDuplicateExternalGeneratorsForbidden(t *testing.T) { th := kusttest_test.MakeHarness(t) - th.WriteK("/generator", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -generators: + th.WriteK("/generator", `generators: - |- apiVersion: generators.example/v1 kind: ManifestGenerator @@ -437,10 +428,7 @@ generators: func TestDuplicateExternalTransformersForbidden(t *testing.T) { th := kusttest_test.MakeHarness(t) - th.WriteK("/transformer", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -transformers: + th.WriteK("/transformer", `transformers: - |- apiVersion: transformers.example.co/v1 kind: ValueAnnotator diff --git a/api/krusty/basic_io_test.go b/api/krusty/basic_io_test.go index 354c4d2d2..1f2ba46c0 100644 --- a/api/krusty/basic_io_test.go +++ b/api/krusty/basic_io_test.go @@ -85,10 +85,7 @@ spec: // test for https://github.com/kubernetes-sigs/kustomize/issues/3812#issuecomment-862339267 func TestBasicIO3812(t *testing.T) { th := kusttest_test.MakeHarness(t) - th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -resources: + th.WriteK(".", `resources: - service.yaml `) diff --git a/api/krusty/configmaps_test.go b/api/krusty/configmaps_test.go index a2b954444..643015c91 100644 --- a/api/krusty/configmaps_test.go +++ b/api/krusty/configmaps_test.go @@ -6,6 +6,7 @@ package krusty_test import ( "testing" + "github.com/stretchr/testify/assert" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) @@ -138,10 +139,10 @@ configMapGenerator: - name: json literals: - 'v2=[{"path": "var/druid/segment-cache"}]' - - >- - druid_segmentCache_locations=[{"path": - "var/druid/segment-cache", - "maxSize": 32000000000, + - >- + druid_segmentCache_locations=[{"path": + "var/druid/segment-cache", + "maxSize": 32000000000, "freeSpacePercent": 1.0}] secretGenerator: - name: bob @@ -201,12 +202,12 @@ metadata: --- apiVersion: v1 data: - druid_segmentCache_locations: '[{"path": "var/druid/segment-cache", "maxSize": - 32000000000, "freeSpacePercent": 1.0}]' + druid_segmentCache_locations: '[{"path": "var/druid/segment-cache", "maxSize": 32000000000, + "freeSpacePercent": 1.0}]' v2: '[{"path": "var/druid/segment-cache"}]' kind: ConfigMap metadata: - name: blah-json-5298bc8g99 + name: blah-json-m8529t979f --- apiVersion: v1 data: @@ -228,7 +229,6 @@ type: Opaque `) } -// TODO: These should be errors instead. func TestGeneratorRepeatsInKustomization(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK(".", ` @@ -261,24 +261,13 @@ krypton xenon radon `) - m := th.Run(".", th.MakeDefaultOptions()) - th.AssertActualEqualsExpected(m, ` -apiVersion: v1 -data: - fruit: apple - nobles: |2 - - helium - neon - argon - krypton - xenon - radon - vegetable: broccoli -kind: ConfigMap -metadata: - name: blah-bob-db529cg5bk -`) + err := th.RunWithErr(".", th.MakeDefaultOptions()) + if err == nil { + t.Fatalf("expected an error") + } + assert.Contains(t, err.Error(), + "kustomization unmarshal error: error converting YAML to JSON: yaml: unmarshal errors:\n"+ + " line 13: key \"literals\" already set in map\n line 18: key \"files\" already set in map") } func TestIssue3393(t *testing.T) { @@ -553,8 +542,6 @@ metadata: func TestDataEndsWithQuotes(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization configMapGenerator: - name: test literals: diff --git a/api/krusty/crd_test.go b/api/krusty/crd_test.go index a448b2ce8..46e7932ee 100644 --- a/api/krusty/crd_test.go +++ b/api/krusty/crd_test.go @@ -11,8 +11,6 @@ import ( func writeBaseWithCrd(th kusttest_test.Harness) { th.WriteK("base", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization crds: - mycrd.json @@ -259,8 +257,6 @@ func TestCrdWithOverlay(t *testing.T) { th := kusttest_test.MakeHarness(t) writeBaseWithCrd(th) th.WriteK("overlay", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization namePrefix: prod- resources: - ../base @@ -307,8 +303,6 @@ spec: func TestCrdWithContainers(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK("crd/containers", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - crd.yaml images: diff --git a/api/krusty/disablenamesuffix_test.go b/api/krusty/disablenamesuffix_test.go index d50a65ed8..da47b62b7 100644 --- a/api/krusty/disablenamesuffix_test.go +++ b/api/krusty/disablenamesuffix_test.go @@ -26,8 +26,6 @@ func findSecret(m resmap.ResMap, prefix string) *resource.Resource { func TestDisableNameSuffixHash(t *testing.T) { th := kusttest_test.MakeHarness(t) const kustomizationContent = ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization namePrefix: foo- nameSuffix: -bar namespace: ns1 @@ -101,8 +99,6 @@ metadata: func TestDisableNameSuffixHashPerObject(t *testing.T) { th := kusttest_test.MakeHarness(t) const kustomizationContent = ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization generatorOptions: disableNameSuffixHash: false secretGenerator: diff --git a/api/krusty/generatormergeandreplace_test.go b/api/krusty/generatormergeandreplace_test.go index fae8e745f..821dcc918 100644 --- a/api/krusty/generatormergeandreplace_test.go +++ b/api/krusty/generatormergeandreplace_test.go @@ -13,8 +13,6 @@ import ( func TestSimpleBase(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK("app/base", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization namePrefix: team-foo- commonLabels: app: mynginx diff --git a/api/krusty/generatoroptions_test.go b/api/krusty/generatoroptions_test.go index 8297f5560..3a940b20e 100644 --- a/api/krusty/generatoroptions_test.go +++ b/api/krusty/generatoroptions_test.go @@ -48,8 +48,6 @@ type: Opaque func TestGeneratorOptionsWithBases(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK("base", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization generatorOptions: disableNameSuffixHash: true labels: @@ -60,8 +58,6 @@ configMapGenerator: - foo=bar `) th.WriteK("overlay", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - ../base generatorOptions: diff --git a/api/krusty/gvknpatch_test.go b/api/krusty/gvknpatch_test.go index 6f782d8df..97e9c4133 100644 --- a/api/krusty/gvknpatch_test.go +++ b/api/krusty/gvknpatch_test.go @@ -86,7 +86,7 @@ patches: - path: patch.yaml target: kind: Deployment - options: + options: allowNameChange: true `) options := th.MakeDefaultOptions() @@ -721,8 +721,6 @@ spec: name: myvol `) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization namePrefix: foo- resources: - resources.yaml @@ -798,8 +796,6 @@ spec: name: myvol `) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - statefulset.yaml patches: diff --git a/api/krusty/inlinepatch_test.go b/api/krusty/inlinepatch_test.go index 7c0d6d439..06a39104a 100644 --- a/api/krusty/inlinepatch_test.go +++ b/api/krusty/inlinepatch_test.go @@ -234,8 +234,6 @@ spec: func TestPathWithCronJobV1(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - resources.yaml patches: diff --git a/api/krusty/kustomizationmetadata_test.go b/api/krusty/kustomizationmetadata_test.go index 8546f78c1..673919bc5 100644 --- a/api/krusty/kustomizationmetadata_test.go +++ b/api/krusty/kustomizationmetadata_test.go @@ -28,8 +28,6 @@ spec: imagePullSecrets: []`) th.WriteK("/app", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization metadata: annotations: config.kubernetes.io/local-config: "true" @@ -37,7 +35,7 @@ metadata: foo: bar name: test_kustomization resources: -- resources.yaml +- resources.yaml `) m := th.Run("/app", th.MakeDefaultOptions()) diff --git a/api/krusty/localconfig_test.go b/api/krusty/localconfig_test.go index 2b84d48d1..d2869b64a 100644 --- a/api/krusty/localconfig_test.go +++ b/api/krusty/localconfig_test.go @@ -14,9 +14,7 @@ import ( // See https://github.com/kubernetes-sigs/kustomize/issues/4124 for details. func TestSKipLocalConfigAfterTransform(t *testing.T) { th := kusttest_test.MakeHarness(t) - th.WriteK(".", `apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -resources: + th.WriteK(".", `resources: - pod.yaml - deployment.yaml transformers: diff --git a/api/krusty/managedbylabel_test.go b/api/krusty/managedbylabel_test.go index 8d12a60bd..8a566c0ee 100644 --- a/api/krusty/managedbylabel_test.go +++ b/api/krusty/managedbylabel_test.go @@ -20,6 +20,7 @@ spec: - port: 7002 ` +// This test may failed when running on package tests using the go command because `v444.333.222` is set on makefile. func TestAddManagedbyLabel(t *testing.T) { tests := []struct { kustFile string @@ -28,8 +29,6 @@ func TestAddManagedbyLabel(t *testing.T) { }{ { kustFile: ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - service.yaml `, @@ -38,8 +37,6 @@ resources: }, { kustFile: ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - service.yaml buildMetadata: [managedByLabel] diff --git a/api/krusty/namespacedgenerators_test.go b/api/krusty/namespacedgenerators_test.go index 96a217baf..e7da6fd03 100644 --- a/api/krusty/namespacedgenerators_test.go +++ b/api/krusty/namespacedgenerators_test.go @@ -12,8 +12,6 @@ import ( func TestNamespacedGenerator(t *testing.T) { th := kusttest_test.MakeHarness(t) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization configMapGenerator: - name: the-non-default-namespace-map namespace: non-default diff --git a/api/krusty/nameupdateinroleref_test.go b/api/krusty/nameupdateinroleref_test.go index 2f3c71d1d..3db9d95fa 100644 --- a/api/krusty/nameupdateinroleref_test.go +++ b/api/krusty/nameupdateinroleref_test.go @@ -217,8 +217,6 @@ fieldSpecs: `) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - workloads.yaml transformers: diff --git a/api/krusty/originannotation_test.go b/api/krusty/originannotation_test.go index 39c32edff..43b52e265 100644 --- a/api/krusty/originannotation_test.go +++ b/api/krusty/originannotation_test.go @@ -27,8 +27,6 @@ spec: - port: 7002 `) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - service.yaml buildMetadata: [originAnnotations] @@ -175,10 +173,10 @@ configMapGenerator: - name: json literals: - 'v2=[{"path": "var/druid/segment-cache"}]' - - >- - druid_segmentCache_locations=[{"path": - "var/druid/segment-cache", - "maxSize": 32000000000, + - >- + druid_segmentCache_locations=[{"path": + "var/druid/segment-cache", + "maxSize": 32000000000, "freeSpacePercent": 1.0}] secretGenerator: - name: bob @@ -245,8 +243,8 @@ metadata: --- apiVersion: v1 data: - druid_segmentCache_locations: '[{"path": "var/druid/segment-cache", "maxSize": - 32000000000, "freeSpacePercent": 1.0}]' + druid_segmentCache_locations: '[{"path": "var/druid/segment-cache", "maxSize": 32000000000, + "freeSpacePercent": 1.0}]' v2: '[{"path": "var/druid/segment-cache"}]' kind: ConfigMap metadata: @@ -256,7 +254,7 @@ metadata: configuredBy: apiVersion: builtin kind: ConfigMapGenerator - name: blah-json-5298bc8g99 + name: blah-json-m8529t979f --- apiVersion: v1 data: diff --git a/api/krusty/poddisruptionbudget_test.go b/api/krusty/poddisruptionbudget_test.go index 4f423fb96..5be8da1d3 100644 --- a/api/krusty/poddisruptionbudget_test.go +++ b/api/krusty/poddisruptionbudget_test.go @@ -84,8 +84,6 @@ spec: maxUnavailable: 100% `) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization patches: - path: pdb-patch.yaml target: diff --git a/api/krusty/transformerannotation_test.go b/api/krusty/transformerannotation_test.go index 77f808b9d..9adf356e2 100644 --- a/api/krusty/transformerannotation_test.go +++ b/api/krusty/transformerannotation_test.go @@ -55,8 +55,6 @@ spec: - port: 7002 `) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - service.yaml buildMetadata: [transformerAnnotations] @@ -93,8 +91,6 @@ spec: - port: 7002 `) th.WriteK(".", ` -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization resources: - service.yaml buildMetadata: [originAnnotations, transformerAnnotations] diff --git a/api/types/kustomization.go b/api/types/kustomization.go index f9512d2a8..87d2e6dc3 100644 --- a/api/types/kustomization.go +++ b/api/types/kustomization.go @@ -4,8 +4,6 @@ package types import ( - "bytes" - "encoding/json" "fmt" "sigs.k8s.io/kustomize/kyaml/filesys" @@ -318,17 +316,8 @@ func (k *Kustomization) EnforceFields() []string { // Unmarshal replace k with the content in YAML input y func (k *Kustomization) Unmarshal(y []byte) error { - j, err := yaml.YAMLToJSON(y) - if err != nil { - return err + if err := yaml.UnmarshalStrict(y, &k); err != nil { + return fmt.Errorf("kustomization unmarshal error: %w", err) } - dec := json.NewDecoder(bytes.NewReader(j)) - dec.DisallowUnknownFields() - var nk Kustomization - err = dec.Decode(&nk) - if err != nil { - return err - } - *k = nk return nil } diff --git a/api/types/kustomization_test.go b/api/types/kustomization_test.go index ee6d01136..24b1fa3aa 100644 --- a/api/types/kustomization_test.go +++ b/api/types/kustomization_test.go @@ -278,7 +278,7 @@ unknown: foo`) if err == nil { t.Fatalf("expect an error") } - expect := "json: unknown field \"unknown\"" + expect := "kustomization unmarshal error: error unmarshaling JSON: while decoding JSON: json: unknown field \"unknown\"" if err.Error() != expect { t.Fatalf("expect %v but got: %v", expect, err.Error()) } diff --git a/kustomize/commands/internal/kustfile/kustomizationfile_test.go b/kustomize/commands/internal/kustfile/kustomizationfile_test.go index 16013e85a..1cc404a32 100644 --- a/kustomize/commands/internal/kustfile/kustomizationfile_test.go +++ b/kustomize/commands/internal/kustfile/kustomizationfile_test.go @@ -383,7 +383,7 @@ foo: } _, err = mf.Read() - if err == nil || err.Error() != "json: unknown field \"foo\"" { + if err == nil || err.Error() != "kustomization unmarshal error: error unmarshaling JSON: while decoding JSON: json: unknown field \"foo\"" { t.Fatalf("Expect an unknown field error but got: %v", err) } }