From 839fd2b9714c76bbe76c7a0715bfafa3540e165f Mon Sep 17 00:00:00 2001 From: monopole Date: Tue, 9 Mar 2021 06:38:15 -0800 Subject: [PATCH] Remove branching on kyaml enablement --- .../patchstrategicmerge.go | 3 +- .../namereferencetransformer_test.go | 17 ++--- .../accumulator/refvartransformer_test.go | 9 +-- api/konfig/general.go | 24 ------- api/krusty/basic_io_test.go | 22 +------ api/krusty/configmaps_test.go | 34 ++++------ api/krusty/kustomizer.go | 2 +- api/krusty/multiplepatch_test.go | 66 +++++-------------- api/krusty/namereference_test.go | 57 ++++++---------- api/krusty/options.go | 13 ---- api/krusty/poddisruptionbudget_test.go | 47 ++++++------- api/provider/depprovider.go | 22 +------ api/resmap/factory_test.go | 18 +---- api/resource/factory_test.go | 5 +- .../PatchStrategicMergeTransformer_test.go | 40 ++--------- .../secretgenerator/SecretGenerator_test.go | 25 ++----- 16 files changed, 92 insertions(+), 312 deletions(-) diff --git a/api/filters/patchstrategicmerge/patchstrategicmerge.go b/api/filters/patchstrategicmerge/patchstrategicmerge.go index 3fa532df0..1a70d19aa 100644 --- a/api/filters/patchstrategicmerge/patchstrategicmerge.go +++ b/api/filters/patchstrategicmerge/patchstrategicmerge.go @@ -4,7 +4,6 @@ package patchstrategicmerge import ( - "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/yaml" "sigs.k8s.io/kustomize/kyaml/yaml/merge2" @@ -29,7 +28,7 @@ func (pf Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) { if err != nil { return nil, err } - if !konfig.FlagEnableKyamlDefaultValue || r != nil { + if r != nil { result = append(result, r) } } diff --git a/api/internal/accumulator/namereferencetransformer_test.go b/api/internal/accumulator/namereferencetransformer_test.go index 88a4315cd..d413f96dc 100644 --- a/api/internal/accumulator/namereferencetransformer_test.go +++ b/api/internal/accumulator/namereferencetransformer_test.go @@ -8,7 +8,6 @@ import ( "testing" "sigs.k8s.io/kustomize/api/internal/plugins/builtinconfig" - "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/provider" "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" @@ -521,17 +520,9 @@ func TestNameReferenceUnhappyRun(t *testing.T) { }, }, }).ResMap(), - // TODO(#3304): DECISION - kyaml better; not a bug. - expectedErr: konfig.IfApiMachineryElseKyaml( - `updating name reference in 'rules/resourceNames' field of `+ - `'rbac.authorization.k8s.io_v1_ClusterRole|~X|cr'`+ - `: considering field 'rules/resourceNames' of object -{"apiVersion": "rbac.authorization.k8s.io/v1", "kind": "ClusterRole", "metadata": { - "name": "cr"}, "rules": [{"resourceNames": {"foo": "bar"}, "resources": ["secrets"]}]} -: visit traversal on path: [resourceNames]: path config error; no 'name' field in node`, - `updating name reference in 'rules/resourceNames' field of `+ - `'rbac.authorization.k8s.io_v1_ClusterRole|~X|cr'`+ - `: considering field 'rules/resourceNames' of object + expectedErr: `updating name reference in 'rules/resourceNames' field of ` + + `'rbac.authorization.k8s.io_v1_ClusterRole|~X|cr'` + + `: considering field 'rules/resourceNames' of object apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: @@ -541,7 +532,7 @@ rules: foo: bar resources: - secrets -: visit traversal on path: [resourceNames]: path config error; no 'name' field in node`)}, +: visit traversal on path: [resourceNames]: path config error; no 'name' field in node`}, } nrt := newNameReferenceTransformer(builtinconfig.MakeDefaultConfig().NameReference) diff --git a/api/internal/accumulator/refvartransformer_test.go b/api/internal/accumulator/refvartransformer_test.go index 8b4d31c16..446d90fd6 100644 --- a/api/internal/accumulator/refvartransformer_test.go +++ b/api/internal/accumulator/refvartransformer_test.go @@ -7,7 +7,6 @@ import ( "reflect" "testing" - "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" resmaptest_test "sigs.k8s.io/kustomize/api/testutils/resmaptest" @@ -121,12 +120,7 @@ func TestRefVarTransformer(t *testing.T) { "slice": []interface{}{5}, // noticeably *not* a []string }}).ResMap(), }, - // TODO(#3304): DECISION - kyaml better; not a bug. - errMessage: konfig.IfApiMachineryElseKyaml( - `considering field 'data/slice' of object -{"apiVersion": "v1", "data": {"slice": [5]}, "kind": "ConfigMap", "metadata": {"name": "cm1"}} -: invalid value type expect a string`, - `considering field 'data/slice' of object + errMessage: `considering field 'data/slice' of object apiVersion: v1 data: slice: @@ -135,7 +129,6 @@ kind: ConfigMap metadata: name: cm1 : invalid value type expect a string`, - ), }, "var replacement in nil": { given: given{ diff --git a/api/konfig/general.go b/api/konfig/general.go index 40167bae0..6c86391df 100644 --- a/api/konfig/general.go +++ b/api/konfig/general.go @@ -19,31 +19,7 @@ func DefaultKustomizationFileName() string { return RecognizedKustomizationFileNames()[0] } -// IfApiMachineryElseKyaml returns true if executing the apimachinery code -// path, else we're executing the kyaml code paths. -func IfApiMachineryElseKyaml(s1, s2 string) string { - if !FlagEnableKyamlDefaultValue { - return s1 - } - return s2 -} - const ( - // FlagEnableKyamlDefaultValue is the default value for the --enable_kyaml - // flag. This value is also used in unit tests. See provider.DepProvider. - // - // TODO(#3588): Delete this constant. - // - // All tests should pass for either true or false values - // of this constant, without having to check its value. - // In the cases where there's a different outcome, either decide - // that the difference is acceptable, or make the difference go away. - // - // Historically, tests passed for enable_kyaml == false, i.e. using - // apimachinery libs. This doesn't mean the code was better, it just - // means regression tests preserved those outcomes. - FlagEnableKyamlDefaultValue = true - // An environment variable to consult for kustomization // configuration data. See: // https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html diff --git a/api/krusty/basic_io_test.go b/api/krusty/basic_io_test.go index 1ed567605..6a2b9c302 100644 --- a/api/krusty/basic_io_test.go +++ b/api/krusty/basic_io_test.go @@ -11,26 +11,6 @@ import ( func TestBasicIO_1(t *testing.T) { th := kusttest_test.MakeHarness(t) - opts := th.MakeDefaultOptions() - if !opts.UseKyaml { - // This test won't pass under apimachinery, because in the bowels of - // that code (see GetAnnotations in v0.17.0 of - // k8s.io/apimachinery/pkg/apis/meta/v1/unstructured/unstructured.go) - // an error returned from NestedStringMap is discarded, and an - // empty annotation map is silently returned, making this test fail - // The swallowed error arises from code like: - // var v interface{} - // v = true - // if str, ok := v.(string); ok { - // save the value in a map (doesn't happen) - // } else { - // return an error (that is then ignored by GetAnnotations) - // } - // The error happens when any annotation value can be interpreted as - // a boolean or number. Such annotations cannot be successfully applied - // to an object in a cluster unless they are quoted. - t.SkipNow() - } th.WriteK(".", ` resources: - service.yaml @@ -47,7 +27,7 @@ metadata: spec: clusterIP: None `) - m := th.Run(".", opts) + m := th.Run(".", th.MakeDefaultOptions()) // The annotations are sorted by key, hence the order change. // Quotes are added intentionally. th.AssertActualEqualsExpected( diff --git a/api/krusty/configmaps_test.go b/api/krusty/configmaps_test.go index 485aa1649..024d8ffb8 100644 --- a/api/krusty/configmaps_test.go +++ b/api/krusty/configmaps_test.go @@ -4,7 +4,6 @@ package krusty_test import ( - "fmt" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -176,7 +175,9 @@ weak nuclear `) opts := th.MakeDefaultOptions() m := th.Run(".", opts) - expFmt := `apiVersion: v1 + th.AssertActualEqualsExpected( + m, ` +apiVersion: v1 data: BIRD: falcon MOUNTAIN: everest @@ -212,32 +213,19 @@ data: BIRD: ZmFsY29u MOUNTAIN: ZXZlcmVzdA== OCEAN: cGFjaWZpYw== - forces.txt: %s + forces.txt: | + CmdyYXZpdGF0aW9uYWwKZWxlY3Ryb21hZ25ldGljCnN0cm9uZyBudWNsZWFyCndlYWsgbn + VjbGVhcgo= fruit: YXBwbGU= - passphrase: %s + passphrase: | + CkxpZmUgaXMgc2hvcnQuCkJ1dCB0aGUgeWVhcnMgYXJlIGxvbmcuCk5vdCB3aGlsZSB0aG + UgZXZpbCBkYXlzIGNvbWUgbm90Lgo= vegetable: YnJvY2NvbGk= kind: Secret metadata: - name: blah-bob-%s + name: blah-bob-58g62h555c type: Opaque -` - th.AssertActualEqualsExpected( - m, - // TODO(#3304): DECISION - kyaml better; not a bug. - opts.IfApiMachineryElseKyaml( - fmt.Sprintf( - expFmt, - `CmdyYXZpdGF0aW9uYWwKZWxlY3Ryb21hZ25ldGljCnN0cm9uZyBudWNsZWFyCndlYWsgbn - VjbGVhcgo=`, - `CkxpZmUgaXMgc2hvcnQuCkJ1dCB0aGUgeWVhcnMgYXJlIGxvbmcuCk5vdCB3aGlsZSB0aG - UgZXZpbCBkYXlzIGNvbWUgbm90Lgo=`, - `ftht6hfgmb`), - fmt.Sprintf( - expFmt, `| - CmdyYXZpdGF0aW9uYWwKZWxlY3Ryb21hZ25ldGljCnN0cm9uZyBudWNsZWFyCndlYWsgbn - VjbGVhcgo=`, `| - CkxpZmUgaXMgc2hvcnQuCkJ1dCB0aGUgeWVhcnMgYXJlIGxvbmcuCk5vdCB3aGlsZSB0aG - UgZXZpbCBkYXlzIGNvbWUgbm90Lgo=`, `58g62h555c`))) +`) } // TODO: These should be errors instead. diff --git a/api/krusty/kustomizer.go b/api/krusty/kustomizer.go index 8120f9351..591db8ed9 100644 --- a/api/krusty/kustomizer.go +++ b/api/krusty/kustomizer.go @@ -36,7 +36,7 @@ type Kustomizer struct { func MakeKustomizer(o *Options) *Kustomizer { return &Kustomizer{ options: o, - depProvider: provider.NewDepProvider(o.UseKyaml), + depProvider: provider.NewDepProvider(), } } diff --git a/api/krusty/multiplepatch_test.go b/api/krusty/multiplepatch_test.go index 723ff16b2..37c9ccd43 100644 --- a/api/krusty/multiplepatch_test.go +++ b/api/krusty/multiplepatch_test.go @@ -4,8 +4,6 @@ package krusty_test import ( - "fmt" - "strings" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -300,7 +298,11 @@ spec: `) opts := th.MakeDefaultOptions() m := th.Run("overlay", opts) - expFmt := `apiVersion: apps/v1 + th.AssertActualEqualsExpected( + // TODO(#3394): Should be possible to delete emptyDir with a patch. + // TODO(#3304): DECISION - still a bug, emptyDir should be deleted. + m, ` +apiVersion: apps/v1 kind: Deployment metadata: name: nginx @@ -315,21 +317,12 @@ spec: name: nginx - image: sidecar:latest name: sidecar - volumes:%s - name: nginx-persistent-storage -` - th.AssertActualEqualsExpected( - // TODO(#3394): Should be possible to delete emptyDir with a patch. - // TODO(#3304): DECISION - still a bug, emptyDir should be deleted. - m, opts.IfApiMachineryElseKyaml( - fmt.Sprintf(expFmt, ` - - gcePersistentDisk: - pdName: nginx-persistent-storage`), - fmt.Sprintf(expFmt, ` + volumes: - emptyDir: {} gcePersistentDisk: - pdName: nginx-persistent-storage`), - )) + pdName: nginx-persistent-storage + name: nginx-persistent-storage +`) } func TestSimpleMultiplePatches(t *testing.T) { @@ -753,12 +746,10 @@ spec: - name: ENABLE_FEATURE_FOO value: FALSE `) - opts := th.MakeDefaultOptions() - if opts.UseKyaml { - // kyaml doesn't try to detect conflicts in patches - // (so ENABLE_FEATURE_FOO FALSE wins). - m := th.Run("overlay/staging", opts) - th.AssertActualEqualsExpected(m, ` + // kyaml doesn't try to detect conflicts in patches + // (so ENABLE_FEATURE_FOO FALSE wins). + m := th.Run("overlay/staging", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` apiVersion: apps/v1 kind: Deployment metadata: @@ -852,16 +843,6 @@ metadata: env: staging name: staging-configmap-in-overlay-dc6fm46dhm `) - } else { - err := th.RunWithErr("overlay/staging", opts) - if err == nil { - t.Fatalf("expected conflict") - } - if !strings.Contains( - err.Error(), "conflict between ") { - t.Fatalf("Unexpected err: %v", err) - } - } } func TestMultiplePatchesWithOnePatchDeleteDirective(t *testing.T) { @@ -1029,12 +1010,10 @@ spec: - $patch: delete name: nginx `) - opt := th.MakeDefaultOptions() - if opt.UseKyaml { - // kyaml doesn't fail on conflicts in patches; both containers - // (nginx and sidecar) are deleted per this patching instruction. - m := th.Run("overlay/staging", opt) - th.AssertActualEqualsExpected(m, ` + // kyaml doesn't fail on conflicts in patches; both containers + // (nginx and sidecar) are deleted per this patching instruction. + m := th.Run("overlay/staging", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` apiVersion: apps/v1 kind: Deployment metadata: @@ -1112,17 +1091,6 @@ metadata: env: staging name: staging-configmap-in-overlay-dc6fm46dhm `) - } else { - // No kyaml means error on a patch conflict. - err := th.RunWithErr("overlay/staging", opt) - if err == nil { - t.Fatalf("Expected error") - } - if !strings.Contains( - err.Error(), "both containing ") { - t.Fatalf("Unexpected err: %v", err) - } - } } // test for #3513 diff --git a/api/krusty/namereference_test.go b/api/krusty/namereference_test.go index a30ec2ecb..ed3bb4e43 100644 --- a/api/krusty/namereference_test.go +++ b/api/krusty/namereference_test.go @@ -1,7 +1,6 @@ package krusty_test import ( - "fmt" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -265,9 +264,9 @@ kind: ServiceAccount metadata: name: external-dns `) - opts := th.MakeDefaultOptions() - m := th.Run(".", opts) - expFmt := ` + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected( + m, ` apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: @@ -353,7 +352,7 @@ spec: volumes: - name: azure-config-file secret: - secretName: azure-config-file-%s + secretName: azure-config-file-66cc4224mm --- apiVersion: v1 kind: ServiceAccount @@ -366,13 +365,18 @@ metadata: --- apiVersion: v1 data: - azure.json: %s + azure.json: | + ewoJInRlbmFudElkIjogIlhYWFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIiwKCS + JzdWJzY3JpcHRpb25JZCI6ICJYWFhYWC1YWFhYWFgtWFhYWFgtWFhYWFhYLVhYWFhYWCIs + CgkicmVzb3VyY2VHcm91cCI6ICJETlMtRVVXLVhYWC1SRyIsCgkidXNlTWFuYWdlZElkZW + 50aXR5RXh0ZW5zaW9uIjogdHJ1ZSwKCSJ1c2VyQXNzaWduZWRJZGVudGl0eUlEIjogIlhY + WFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIgp9Cg== kind: Secret metadata: labels: app: external-dns instance: public - name: azure-config-file-%s + name: azure-config-file-66cc4224mm namespace: kube-system type: Opaque --- @@ -461,7 +465,7 @@ spec: volumes: - name: azure-config-file secret: - secretName: azure-config-file-private-%s + secretName: azure-config-file-private-66cc4224mm --- apiVersion: v1 kind: ServiceAccount @@ -474,42 +478,21 @@ metadata: --- apiVersion: v1 data: - azure.json: %s + azure.json: | + ewoJInRlbmFudElkIjogIlhYWFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIiwKCS + JzdWJzY3JpcHRpb25JZCI6ICJYWFhYWC1YWFhYWFgtWFhYWFgtWFhYWFhYLVhYWFhYWCIs + CgkicmVzb3VyY2VHcm91cCI6ICJETlMtRVVXLVhYWC1SRyIsCgkidXNlTWFuYWdlZElkZW + 50aXR5RXh0ZW5zaW9uIjogdHJ1ZSwKCSJ1c2VyQXNzaWduZWRJZGVudGl0eUlEIjogIlhY + WFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIgp9Cg== kind: Secret metadata: labels: app: external-dns instance: private - name: azure-config-file-private-%s + name: azure-config-file-private-66cc4224mm namespace: kube-system type: Opaque -` - const ( - nameHashKyaml = "66cc4224mm" - contentKyaml = `| - ewoJInRlbmFudElkIjogIlhYWFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIiwKCS - JzdWJzY3JpcHRpb25JZCI6ICJYWFhYWC1YWFhYWFgtWFhYWFgtWFhYWFhYLVhYWFhYWCIs - CgkicmVzb3VyY2VHcm91cCI6ICJETlMtRVVXLVhYWC1SRyIsCgkidXNlTWFuYWdlZElkZW - 50aXR5RXh0ZW5zaW9uIjogdHJ1ZSwKCSJ1c2VyQXNzaWduZWRJZGVudGl0eUlEIjogIlhY - WFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIgp9Cg==` - nameHashApiMach = "g2k4bkgt4d" - // nolint: lll - contentApiMach = `ewoJInRlbmFudElkIjogIlhYWFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIiwKCSJzdWJzY3JpcHRpb25JZCI6ICJYWFhYWC1YWFhYWFgtWFhYWFgtWFhYWFhYLVhYWFhYWCIsCgkicmVzb3VyY2VHcm91cCI6ICJETlMtRVVXLVhYWC1SRyIsCgkidXNlTWFuYWdlZElkZW50aXR5RXh0ZW5zaW9uIjogdHJ1ZSwKCSJ1c2VyQXNzaWduZWRJZGVudGl0eUlEIjogIlhYWFhYLVhYWFhYWC1YWFhYWC1YWFhYWFgtWFhYWFhYIgp9Cg==` - ) - th.AssertActualEqualsExpected( - m, - // TODO(#3304): DECISION - kyaml better; not a bug. - opts.IfApiMachineryElseKyaml( - fmt.Sprintf(expFmt, - nameHashApiMach, - contentApiMach, nameHashApiMach, - nameHashApiMach, - contentApiMach, nameHashApiMach), - fmt.Sprintf(expFmt, - nameHashKyaml, - contentKyaml, nameHashKyaml, - nameHashKyaml, - contentKyaml, nameHashKyaml))) +`) } func TestEmptyFieldSpecValue(t *testing.T) { diff --git a/api/krusty/options.go b/api/krusty/options.go index 91d7fac05..a7229fee2 100644 --- a/api/krusty/options.go +++ b/api/krusty/options.go @@ -34,11 +34,6 @@ type Options struct { // Options related to kustomize plugins. PluginConfig *types.PluginConfig - // TODO(#3588): Delete this field (it's always true). - // When true, use kyaml/ packages to manipulate KRM yaml. - // When false, use k8sdeps/ instead (uses k8s.io/api* packages). - UseKyaml bool - // When true, allow name and kind changing via a patch // When false, patch name/kind don't overwrite target name/kind AllowResourceIdChanges bool @@ -52,18 +47,10 @@ func MakeDefaultOptions() *Options { LoadRestrictions: types.LoadRestrictionsRootOnly, DoPrune: false, PluginConfig: konfig.DisabledPluginConfig(), - UseKyaml: konfig.FlagEnableKyamlDefaultValue, AllowResourceIdChanges: false, } } -func (o Options) IfApiMachineryElseKyaml(s1, s2 string) string { - if !o.UseKyaml { - return s1 - } - return s2 -} - // GetBuiltinPluginNames returns a list of builtin plugin names func GetBuiltinPluginNames() []string { var ret []string diff --git a/api/krusty/poddisruptionbudget_test.go b/api/krusty/poddisruptionbudget_test.go index 55b143cd9..4f423fb96 100644 --- a/api/krusty/poddisruptionbudget_test.go +++ b/api/krusty/poddisruptionbudget_test.go @@ -4,7 +4,6 @@ package krusty_test import ( - "fmt" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -57,7 +56,6 @@ spec: func TestPodDisruptionBudgetMerging(t *testing.T) { th := kusttest_test.MakeHarness(t) - opts := th.MakeDefaultOptions() th.WriteF("pdb-patch.yaml", ` apiVersion: policy/v1beta1 kind: PodDisruptionBudget @@ -97,26 +95,7 @@ patches: resources: - my_file.yaml `) - m := th.Run(".", opts) - expFmt := ` -apiVersion: policy/v1beta1 -kind: PodDisruptionBudget -metadata: - labels: - faceit-pdb: default - name: championships-api -spec: - maxUnavailable: %s ---- -apiVersion: policy/v1beta1 -kind: PodDisruptionBudget -metadata: - labels: - faceit-pdb: default - name: championships-api-2 -spec: - maxUnavailable: %s -` + m := th.Run(".", th.MakeDefaultOptions()) // In a PodDisruptionBudget, the fields maxUnavailable // minAvailable are mutually exclusive, and both can hold // either an integer, i.e. 10, or string that has to be @@ -126,9 +105,23 @@ spec: // the percent sign, quotes can be added and the API server will // accept it, but they don't have to be added. th.AssertActualEqualsExpected( - m, - // TODO(#3304): DECISION - kyaml better; not a bug. - opts.IfApiMachineryElseKyaml( - fmt.Sprintf(expFmt, `"1"`, `"1"`), - fmt.Sprintf(expFmt, `1`, `1`))) + m, ` +apiVersion: policy/v1beta1 +kind: PodDisruptionBudget +metadata: + labels: + faceit-pdb: default + name: championships-api +spec: + maxUnavailable: 1 +--- +apiVersion: policy/v1beta1 +kind: PodDisruptionBudget +metadata: + labels: + faceit-pdb: default + name: championships-api-2 +spec: + maxUnavailable: 1 +`) } diff --git a/api/provider/depprovider.go b/api/provider/depprovider.go index 4bd567f3e..ff950578f 100644 --- a/api/provider/depprovider.go +++ b/api/provider/depprovider.go @@ -4,13 +4,10 @@ package provider import ( - "log" - "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/internal/conflict" "sigs.k8s.io/kustomize/api/internal/validate" "sigs.k8s.io/kustomize/api/internal/wrappy" - "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/resource" ) @@ -154,15 +151,7 @@ type DepProvider struct { fieldValidator ifc.Validator } -// The dependencies this method needs have been deleted - -// see comments above. This method will be deleted -// along with DepProvider in the final step. -func makeK8sdepBasedInstances() *DepProvider { - log.Fatal("This binary cannot use k8s.io code; it must use kyaml.") - return nil -} - -func makeKyamlBasedInstances() *DepProvider { +func NewDepProvider() *DepProvider { kf := &wrappy.WNodeFactory{} rf := resource.NewFactory(kf) return &DepProvider{ @@ -173,15 +162,8 @@ func makeKyamlBasedInstances() *DepProvider { } } -func NewDepProvider(useKyaml bool) *DepProvider { - if useKyaml { - return makeKyamlBasedInstances() - } - return makeK8sdepBasedInstances() -} - func NewDefaultDepProvider() *DepProvider { - return NewDepProvider(konfig.FlagEnableKyamlDefaultValue) + return NewDepProvider() } func (dp *DepProvider) GetKunstructuredFactory() ifc.KunstructuredFactory { diff --git a/api/resmap/factory_test.go b/api/resmap/factory_test.go index a060e0790..040bc6d19 100644 --- a/api/resmap/factory_test.go +++ b/api/resmap/factory_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/ifc" - "sigs.k8s.io/kustomize/api/konfig" "sigs.k8s.io/kustomize/api/kv" "sigs.k8s.io/kustomize/api/loader" . "sigs.k8s.io/kustomize/api/resmap" @@ -391,20 +390,7 @@ spec: assert.NoError(t, err) yml, err = rm.AsYaml() assert.NoError(t, err) - - // TODO(#3304): DECISION - kyaml better; not a bug. - assert.Equal(t, konfig.IfApiMachineryElseKyaml(`apiVersion: example.com/v1 -kind: Foo -metadata: - name: my-foo -spec: - bar: - B: null - C: Z - D: W - baz: - hello: world -`, `apiVersion: example.com/v1 + assert.Equal(t, `apiVersion: example.com/v1 kind: Foo metadata: name: my-foo @@ -414,5 +400,5 @@ spec: D: W baz: hello: world -`), string(yml)) +`, string(yml)) } diff --git a/api/resource/factory_test.go b/api/resource/factory_test.go index 3f5c56215..e205c138c 100644 --- a/api/resource/factory_test.go +++ b/api/resource/factory_test.go @@ -7,8 +7,6 @@ import ( "fmt" "testing" - "sigs.k8s.io/kustomize/api/konfig" - "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/api/filesys" "sigs.k8s.io/kustomize/api/loader" @@ -349,8 +347,7 @@ kind: List // yaml, json, Resource, RNode, Unstructured etc. // These conversions go away after closing #3506 // TODO(#3271) This shouldn't have an error, but does when kyaml is used. - // TODO(#3304): DECISION - still a bug, but not a blocker to #3304 or #2506 - expectedErr: konfig.FlagEnableKyamlDefaultValue, + expectedErr: true, }, { name: "listWithNoEntries", diff --git a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go index fede27ba0..7f9a229dc 100644 --- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go +++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go @@ -9,12 +9,11 @@ import ( "testing" "github.com/stretchr/testify/assert" - "sigs.k8s.io/kustomize/api/konfig" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) // TODO(#3304): DECISION - OK to move to kyaml and not do conflict detection. -const skipConflictDetectionTests = konfig.FlagEnableKyamlDefaultValue +const skipConflictDetectionTests = true func errorContains(err error, possibilities ...string) bool { for _, x := range possibilities { @@ -604,25 +603,11 @@ spec: `) assert.NoError(t, err) th.AssertActualEqualsExpectedNoIdAnnotations( - resMap, // In kyaml/yaml.merge2, the empty "B: " is dropped // when patch1 and patch2 are merged, so the patch // applied is effectively only patch2.yaml. // So it cannot delete the "B: Y" - // TODO(#3304): DECISION - undecided. - konfig.IfApiMachineryElseKyaml(` -apiVersion: example.com/v1 -kind: Foo -metadata: - name: my-foo -spec: - bar: - A: X - C: Z - D: W - baz: - hello: world -`, ` + resMap, ` apiVersion: example.com/v1 kind: Foo metadata: @@ -635,7 +620,7 @@ spec: D: W baz: hello: world -`)) +`) resMap, err = th.RunTransformer(` apiVersion: builtin kind: PatchStrategicMergeTransformer @@ -654,24 +639,9 @@ spec: `) assert.NoError(t, err) th.AssertActualEqualsExpectedNoIdAnnotations( - resMap, // This time only patch2 was applied. Same answer on the kyaml // path, but different answer on apimachinery path (B becomes "true"?) - // TODO(#3304): DECISION - kyaml doing better here, not a bug. - konfig.IfApiMachineryElseKyaml(` -apiVersion: example.com/v1 -kind: Foo -metadata: - name: my-foo -spec: - bar: - A: X - B: true - C: Z - D: W - baz: - hello: world -`, ` + resMap, ` apiVersion: example.com/v1 kind: Foo metadata: @@ -684,7 +654,7 @@ spec: D: W baz: hello: world -`)) +`) } func TestStrategicMergeTransformerNoSchemaMultiPatchesWithConflict(t *testing.T) { diff --git a/plugin/builtin/secretgenerator/SecretGenerator_test.go b/plugin/builtin/secretgenerator/SecretGenerator_test.go index 6f4b04a8e..f3d91cdf9 100644 --- a/plugin/builtin/secretgenerator/SecretGenerator_test.go +++ b/plugin/builtin/secretgenerator/SecretGenerator_test.go @@ -4,10 +4,8 @@ package main_test import ( - "fmt" "testing" - "sigs.k8s.io/kustomize/api/konfig" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) @@ -43,32 +41,21 @@ literals: - FRUIT=apple - VEGETABLE=carrot `) - - expFmt := ` + th.AssertActualEqualsExpected( + rm, ` apiVersion: v1 data: DB_PASSWORD: aWxvdmV5b3U= FRUIT: YXBwbGU= ROUTER_PASSWORD: YWRtaW4= VEGETABLE: Y2Fycm90 - %s + obscure: | + CkxvcmVtIGlwc3VtIGRvbG9yIHNpdCBhbWV0LApjb25zZWN0ZXR1ciBhZGlwaXNjaW5nIG + VsaXQuCg== kind: Secret metadata: name: mySecret namespace: whatever type: Opaque -` - th.AssertActualEqualsExpected( - rm, - // TODO(#3304): DECISION - kyaml doing better here, not a bug. - konfig.IfApiMachineryElseKyaml( - fmt.Sprintf( - expFmt, - `obscure: CkxvcmVtIGlwc3VtIGRvbG9yIHNpdCBhbWV0LApjb25zZWN0ZXR1ciBhZGlwaXNjaW5nIGVsaXQuCg==`), - fmt.Sprintf( - expFmt, - `obscure: | - CkxvcmVtIGlwc3VtIGRvbG9yIHNpdCBhbWV0LApjb25zZWN0ZXR1ciBhZGlwaXNjaW5nIG - VsaXQuCg==`), - )) +`) }