Remove branching on kyaml enablement

This commit is contained in:
monopole
2021-03-09 06:38:15 -08:00
parent 26e9b8b3b8
commit 839fd2b971
16 changed files with 92 additions and 312 deletions

View File

@@ -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)
}
}

View File

@@ -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)

View File

@@ -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{

View File

@@ -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

View File

@@ -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(

View File

@@ -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.

View File

@@ -36,7 +36,7 @@ type Kustomizer struct {
func MakeKustomizer(o *Options) *Kustomizer {
return &Kustomizer{
options: o,
depProvider: provider.NewDepProvider(o.UseKyaml),
depProvider: provider.NewDepProvider(),
}
}

View File

@@ -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

View File

@@ -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) {

View File

@@ -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

View File

@@ -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
`)
}

View File

@@ -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 {

View File

@@ -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))
}

View File

@@ -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",