diff --git a/pkg/cluster/internal/create/actions/config/config.go b/pkg/cluster/internal/create/actions/config/config.go index 1726198070..363878f8b8 100644 --- a/pkg/cluster/internal/create/actions/config/config.go +++ b/pkg/cluster/internal/create/actions/config/config.go @@ -26,6 +26,7 @@ import ( "sigs.k8s.io/kind/pkg/cluster/constants" "sigs.k8s.io/kind/pkg/cluster/nodes" "sigs.k8s.io/kind/pkg/errors" + "sigs.k8s.io/kind/pkg/log" "sigs.k8s.io/kind/pkg/cluster/internal/create/actions" "sigs.k8s.io/kind/pkg/cluster/internal/kubeadm" @@ -87,7 +88,7 @@ func (a *Action) Execute(ctx *actions.ActionContext) error { kubeadmConfigPlusPatches := func(node nodes.Node, data kubeadm.ConfigData) func() error { return func() error { data.NodeName = node.String() - kubeadmConfig, err := getKubeadmConfig(ctx.Config, data, node, provider) + kubeadmConfig, err := getKubeadmConfig(ctx.Config, data, node, provider, ctx.Logger) if err != nil { // TODO(bentheelder): logging here return errors.Wrap(err, "failed to generate kubeadm config content") @@ -154,7 +155,7 @@ func (a *Action) Execute(ctx *actions.ActionContext) error { // getKubeadmConfig generates the kubeadm config contents for the cluster // by running data through the template and applying patches as needed. -func getKubeadmConfig(cfg *config.Cluster, data kubeadm.ConfigData, node nodes.Node, provider string) (path string, err error) { +func getKubeadmConfig(cfg *config.Cluster, data kubeadm.ConfigData, node nodes.Node, provider string, logger log.Logger) (path string, err error) { kubeVersion, err := nodeutils.KubeVersion(node) if err != nil { // TODO(bentheelder): logging here @@ -224,14 +225,14 @@ func getKubeadmConfig(cfg *config.Cluster, data kubeadm.ConfigData, node nodes.N clusterPatches, clusterJSONPatches := allPatchesFromConfig(cfg) // apply cluster-level patches first - patchedConfig, err := patch.KubeYAML(cf, clusterPatches, clusterJSONPatches) + patchedConfig, err := patch.KubeYAML(cf, clusterPatches, clusterJSONPatches, logger) if err != nil { return "", err } // if needed, apply current node's patches if len(configNode.KubeadmConfigPatches) > 0 || len(configNode.KubeadmConfigPatchesJSON6902) > 0 { - patchedConfig, err = patch.KubeYAML(patchedConfig, configNode.KubeadmConfigPatches, configNode.KubeadmConfigPatchesJSON6902) + patchedConfig, err = patch.KubeYAML(patchedConfig, configNode.KubeadmConfigPatches, configNode.KubeadmConfigPatchesJSON6902, logger) if err != nil { return "", err } diff --git a/pkg/cluster/internal/create/actions/installcni/cni.go b/pkg/cluster/internal/create/actions/installcni/cni.go index eeee6a2878..11e49d29fe 100644 --- a/pkg/cluster/internal/create/actions/installcni/cni.go +++ b/pkg/cluster/internal/create/actions/installcni/cni.go @@ -110,7 +110,7 @@ func (a *action) Execute(ctx *actions.ActionContext) error { Patch: patchValue, } - patchedConfig, err := patch.KubeYAML(manifest, nil, []config.PatchJSON6902{controlPlanePatch6902}) + patchedConfig, err := patch.KubeYAML(manifest, nil, []config.PatchJSON6902{controlPlanePatch6902}, ctx.Logger) if err != nil { return err } diff --git a/pkg/internal/patch/kubeyaml.go b/pkg/internal/patch/kubeyaml.go index 9bdd1997fe..19cff68d12 100644 --- a/pkg/internal/patch/kubeyaml.go +++ b/pkg/internal/patch/kubeyaml.go @@ -20,8 +20,8 @@ import ( "strings" "sigs.k8s.io/kind/pkg/errors" - "sigs.k8s.io/kind/pkg/internal/apis/config" + "sigs.k8s.io/kind/pkg/log" ) // KubeYAML takes a Kubernetes object YAML document stream to patch, @@ -34,13 +34,13 @@ import ( // // Patches match if their kind and apiVersion match a document, with the exception // that if the patch does not set apiVersion it will be ignored. -func KubeYAML(toPatch string, patches []string, patches6902 []config.PatchJSON6902) (string, error) { +func KubeYAML(toPatch string, patches []string, patches6902 []config.PatchJSON6902, logger log.Logger) (string, error) { // pre-process, including splitting up documents etc. resources, err := parseResources(toPatch) if err != nil { return "", errors.Wrap(err, "failed to parse yaml to patch") } - mergePatches, err := parseMergePatches(patches) + mergePatches, err := parseMergePatches(patches, logger) if err != nil { return "", errors.Wrap(err, "failed to parse patches") } diff --git a/pkg/internal/patch/kubeyaml_test.go b/pkg/internal/patch/kubeyaml_test.go index 9330728ab5..3b70906a15 100644 --- a/pkg/internal/patch/kubeyaml_test.go +++ b/pkg/internal/patch/kubeyaml_test.go @@ -21,6 +21,7 @@ import ( "sigs.k8s.io/kind/pkg/internal/apis/config" "sigs.k8s.io/kind/pkg/internal/assert" + "sigs.k8s.io/kind/pkg/log" ) func TestKubeYAML(t *testing.T) { @@ -61,12 +62,19 @@ func TestKubeYAML(t *testing.T) { ExpectError: false, ExpectOutput: normalKubeadmConfigTrivialPatchedAnd6902Patched, }, + { + Name: "kubeadm config one merge-patch with malformed yaml (duplicate key)", + ToPatch: normalKubeadmConfig, + Patches: []string{malformedPatchWithDuplicateKey}, + ExpectOutput: malformedPatchWithDuplicateKeyKustomized, + ExpectError: false, + }, } for _, tc := range cases { tc := tc // capture test case t.Run(tc.Name, func(t *testing.T) { t.Parallel() - out, err := KubeYAML(tc.ToPatch, tc.Patches, tc.PatchesJSON6902) + out, err := KubeYAML(tc.ToPatch, tc.Patches, tc.PatchesJSON6902, log.NoopLogger{}) assert.ExpectError(t, tc.ExpectError, err) if err == nil { assert.StringEqual(t, tc.ExpectOutput, out) @@ -402,3 +410,83 @@ kind: KubeProxyConfiguration metadata: name: config ` +const malformedPatchWithDuplicateKey = ` +kind: ClusterConfiguration +apiVersion: kubeadm.k8s.io/v1beta2 + +scheduler: + extraArgs: + some-duplicate-key: value1 + some-duplicate-key: value2 +` + +const malformedPatchWithDuplicateKeyKustomized = `apiServer: + certSANs: + - localhost + - 127.0.0.1 +apiVersion: kubeadm.k8s.io/v1beta2 +clusterName: kind +controlPlaneEndpoint: 192.168.9.3:6443 +controllerManager: + extraArgs: + enable-hostpath-provisioner: "true" +kind: ClusterConfiguration +kubernetesVersion: v1.15.3 +metadata: + name: config +networking: + podSubnet: 10.244.0.0/16 + serviceSubnet: 10.96.0.0/12 +scheduler: + extraArgs: + some-duplicate-key: value2 +--- +apiVersion: kubeadm.k8s.io/v1beta2 +bootstrapTokens: +- token: abcdef.0123456789abcdef +kind: InitConfiguration +localAPIEndpoint: + advertiseAddress: 192.168.9.6 + bindPort: 6443 +metadata: + name: config +nodeRegistration: + criSocket: /run/containerd/containerd.sock + kubeletExtraArgs: + fail-swap-on: "false" + node-ip: 192.168.9.6 +--- +apiVersion: kubeadm.k8s.io/v1beta2 +controlPlane: + localAPIEndpoint: + advertiseAddress: 192.168.9.6 + bindPort: 6443 +discovery: + bootstrapToken: + apiServerEndpoint: 192.168.9.3:6443 + token: abcdef.0123456789abcdef + unsafeSkipCAVerification: true +kind: JoinConfiguration +metadata: + name: config +nodeRegistration: + criSocket: /run/containerd/containerd.sock + kubeletExtraArgs: + fail-swap-on: "false" + node-ip: 192.168.9.6 +--- +apiVersion: kubelet.config.k8s.io/v1beta1 +evictionHard: + imagefs.available: 0% + nodefs.available: 0% + nodefs.inodesFree: 0% +imageGCHighThresholdPercent: 100 +kind: KubeletConfiguration +metadata: + name: config +--- +apiVersion: kubeproxy.config.k8s.io/v1alpha1 +kind: KubeProxyConfiguration +metadata: + name: config +` diff --git a/pkg/internal/patch/mergepatch.go b/pkg/internal/patch/mergepatch.go index c52b9f55e8..c70b3185bb 100644 --- a/pkg/internal/patch/mergepatch.go +++ b/pkg/internal/patch/mergepatch.go @@ -20,6 +20,7 @@ import ( "sigs.k8s.io/yaml" "sigs.k8s.io/kind/pkg/errors" + "sigs.k8s.io/kind/pkg/log" ) type mergePatch struct { @@ -28,7 +29,7 @@ type mergePatch struct { matchInfo matchInfo // for matching resources } -func parseMergePatches(rawPatches []string) ([]mergePatch, error) { +func parseMergePatches(rawPatches []string, logger log.Logger) ([]mergePatch, error) { patches := []mergePatch{} // split document streams before trying to parse them splitRawPatches := make([]string, 0, len(rawPatches)) @@ -44,9 +45,17 @@ func parseMergePatches(rawPatches []string) ([]mergePatch, error) { if err != nil { return nil, errors.WithStack(err) } - json, err := yaml.YAMLToJSON([]byte(raw)) + json, err := yaml.YAMLToJSONStrict([]byte(raw)) if err != nil { - return nil, errors.WithStack(err) + logger.Warnf("Failed to strictly convert patch of kind %s to json: \"%v\". "+ + "Trying more permissive conversion.", + matchInfo.Kind, + err, + ) + json, err = yaml.YAMLToJSON([]byte(raw)) + if err != nil { + return nil, errors.WithStack(err) + } } patches = append(patches, mergePatch{ raw: raw,