Merge pull request #70901 from neolit123/kubeadm-strict-config

kubeadm: enable strict config unmarhaling

cmd/kubeadm/app/cmd/config_test.go

@@ -247,7 +247,6 @@ func TestMigrate(t *testing.T) {
 		# This is intentionally testing an old API version and the old kind naming and making sure the output is correct
 		apiVersion: kubeadm.k8s.io/v1alpha3
 		kind: InitConfiguration
-		kubernetesVersion: v1.12.0
 	`))
 	configFile, cleanup := tempConfig(t, cfg)
 	defer cleanup()

cmd/kubeadm/app/phases/upgrade/staticpods_test.go

@@ -56,9 +56,12 @@ kind: InitConfiguration
 nodeRegistration:
   name: foo
   criSocket: ""
-apiEndpoint:
-  advertiseAddress: 1.2.3.4
+localAPIEndpoint:
+  advertiseAddress: 192.168.2.2
   bindPort: 6443
+bootstrapTokens:
+- token: ce3aa5.5ec8455bb76b379f
+  ttl: 24h
 ---
 apiVersion: kubeadm.k8s.io/v1beta1
 kind: ClusterConfiguration
@@ -67,21 +70,16 @@ apiServer:
   certSANs: null
   extraArgs: null
 certificatesDir: %s
-controllerManagerExtraArgs: null
 etcd:
   local:
     dataDir: %s
     image: ""
-featureFlags: null
 imageRepository: k8s.gcr.io
 kubernetesVersion: %s
 networking:
   dnsDomain: cluster.local
   podSubnet: ""
   serviceSubnet: 10.96.0.0/12
-schedulerExtraArgs: null
-token: ce3aa5.5ec8455bb76b379f
-tokenTTL: 24h
 useHyperKubeImage: false
 `
 )

cmd/kubeadm/app/util/config/BUILD

@@ -23,6 +23,7 @@ go_library(
         "//cmd/kubeadm/app/componentconfigs:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/util:go_default_library",
+        "//cmd/kubeadm/app/util/config/strict:go_default_library",
         "//pkg/util/node:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
@@ -76,6 +77,9 @@ filegroup(
 
 filegroup(
     name = "all-srcs",
-    srcs = [":package-srcs"],
+    srcs = [
+        ":package-srcs",
+        "//cmd/kubeadm/app/util/config/strict:all-srcs",
+    ],
     tags = ["automanaged"],
 )

cmd/kubeadm/app/util/config/initconfiguration.go

@@ -39,6 +39,7 @@ import (
 	"k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
+	"k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict"
 	nodeutil "k8s.io/kubernetes/pkg/util/node"
 )
 
@@ -211,6 +212,9 @@ func BytesToInternalConfig(b []byte) (*kubeadmapi.InitConfiguration, error) {
 	}
 
 	for gvk, fileContent := range gvkmap {
+		// verify the validity of the YAML
+		strict.VerifyUnmarshalStrict(fileContent, gvk)
+
 		// Try to get the registration for the ComponentConfig based on the kind
 		regKind := componentconfigs.RegistrationKind(gvk.Kind)
 		registration, found := componentconfigs.Known[regKind]

cmd/kubeadm/app/util/config/joinconfiguration.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
+	"k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict"
 )
 
 // SetJoinDynamicDefaults checks and sets configuration values for the JoinConfiguration object
@@ -88,6 +89,8 @@ func JoinConfigFileAndDefaultsToInternalConfig(cfgPath string, defaultversionedc
 		for gvk, bytes := range gvkmap {
 			if gvk.Kind == constants.JoinConfigurationKind {
 				joinBytes = bytes
+				// verify the validity of the YAML
+				strict.VerifyUnmarshalStrict(bytes, gvk)
 			}
 		}
 

cmd/kubeadm/app/util/config/strict/BUILD

@@ -0,0 +1,46 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["strict.go"],
+    importpath = "k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//cmd/kubeadm/app/apis/kubeadm/scheme:go_default_library",
+        "//cmd/kubeadm/app/componentconfigs:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//vendor/github.com/pkg/errors:go_default_library",
+        "//vendor/k8s.io/klog:go_default_library",
+        "//vendor/sigs.k8s.io/yaml:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["strict_test.go"],
+    data = glob(["testdata/**"]),
+    embed = [":go_default_library"],
+    deps = [
+        "//cmd/kubeadm/app/apis/kubeadm/v1alpha3:go_default_library",
+        "//cmd/kubeadm/app/apis/kubeadm/v1beta1:go_default_library",
+        "//cmd/kubeadm/app/componentconfigs:go_default_library",
+        "//cmd/kubeadm/app/constants:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
+        "//staging/src/k8s.io/kube-proxy/config/v1alpha1:go_default_library",
+        "//staging/src/k8s.io/kubelet/config/v1beta1:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

cmd/kubeadm/app/util/config/strict/strict.go

@@ -0,0 +1,58 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package strict
+
+import (
+	"github.com/pkg/errors"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/klog"
+	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme"
+	"k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs"
+	"sigs.k8s.io/yaml"
+)
+
+// VerifyUnmarshalStrict takes a YAML byte slice and a GroupVersionKind and verifies if the YAML
+// schema is known and if it unmarshals with strict mode.
+//
+// TODO(neolit123): The returned error here is currently ignored everywhere and a klog warning is thrown instead.
+// We don't want to turn this into an actual error yet. Eventually this can be controlled with an optional CLI flag.
+func VerifyUnmarshalStrict(bytes []byte, gvk schema.GroupVersionKind) error {
+
+	var (
+		iface interface{}
+		err   error
+	)
+
+	iface, err = scheme.Scheme.New(gvk)
+	if err != nil {
+		iface, err = componentconfigs.Scheme.New(gvk)
+		if err != nil {
+			err := errors.Errorf("unknown configuration %#v for scheme definitions in %q and %q",
+				gvk, scheme.Scheme.Name(), componentconfigs.Scheme.Name())
+			klog.Warning(err.Error())
+			return err
+		}
+	}
+
+	if err := yaml.UnmarshalStrict(bytes, iface); err != nil {
+		err := errors.Wrapf(err, "error unmarshaling configuration %#v", gvk)
+		klog.Warning(err.Error())
+		return err
+	}
+	return nil
+}

cmd/kubeadm/app/util/config/strict/strict_test.go

@@ -0,0 +1,168 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package strict
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	kubeproxyconfigv1alpha1 "k8s.io/kube-proxy/config/v1alpha1"
+	kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1"
+	kubeadmapiv1alpha3 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha3"
+	kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1"
+	"k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs"
+	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
+)
+
+func TestVerifyUnmarshalStrict(t *testing.T) {
+	const (
+		pathTestData = "testdata/"
+	)
+
+	var testFiles = []struct {
+		fileName      string
+		kind          string
+		groupVersion  schema.GroupVersion
+		expectedError bool
+	}{
+		// tests with file errors
+		{
+			fileName:      "invalid_duplicate_field_clustercfg.yaml",
+			kind:          constants.InitConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_duplicate_field_initcfg.yaml",
+			kind:          constants.InitConfigurationKind,
+			groupVersion:  kubeadmapiv1alpha3.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_duplicate_field_joincfg.yaml",
+			kind:          constants.JoinConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_duplicate_field_kubeletcfg.yaml",
+			kind:          string(componentconfigs.KubeletConfigurationKind),
+			groupVersion:  kubeletconfigv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_duplicate_field_kubeproxycfg.yaml",
+			kind:          string(componentconfigs.KubeProxyConfigurationKind),
+			groupVersion:  kubeproxyconfigv1alpha1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_unknown_field_clustercfg.yaml",
+			kind:          constants.ClusterConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_unknown_field_initcfg.yaml",
+			kind:          constants.InitConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_unknown_field_joincfg.yaml",
+			kind:          constants.JoinConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_unknown_field_kubeletcfg.yaml",
+			kind:          string(componentconfigs.KubeletConfigurationKind),
+			groupVersion:  kubeletconfigv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		{
+			fileName:      "invalid_unknown_field_kubeproxycfg.yaml",
+			kind:          string(componentconfigs.KubeProxyConfigurationKind),
+			groupVersion:  kubeproxyconfigv1alpha1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		// test unknown groupVersion and kind
+		{
+			fileName:      "valid_clustercfg.yaml",
+			kind:          constants.ClusterConfigurationKind,
+			groupVersion:  schema.GroupVersion{Group: "someGroup", Version: "v1"},
+			expectedError: true,
+		},
+		{
+			fileName:      "valid_clustercfg.yaml",
+			kind:          "SomeUnknownKind",
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: true,
+		},
+		// valid tests
+		{
+			fileName:      "valid_clustercfg.yaml",
+			kind:          constants.ClusterConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: false,
+		},
+		{
+			fileName:      "valid_initcfg.yaml",
+			kind:          constants.InitConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: false,
+		},
+		{
+			fileName:      "valid_joincfg.yaml",
+			kind:          constants.JoinConfigurationKind,
+			groupVersion:  kubeadmapiv1beta1.SchemeGroupVersion,
+			expectedError: false,
+		},
+		{
+			fileName:      "valid_kubeletcfg.yaml",
+			kind:          string(componentconfigs.KubeletConfigurationKind),
+			groupVersion:  kubeletconfigv1beta1.SchemeGroupVersion,
+			expectedError: false,
+		},
+		{
+			fileName:      "valid_kubeproxycfg.yaml",
+			kind:          string(componentconfigs.KubeProxyConfigurationKind),
+			groupVersion:  kubeproxyconfigv1alpha1.SchemeGroupVersion,
+			expectedError: false,
+		},
+	}
+
+	for _, test := range testFiles {
+		t.Run(test.fileName, func(t *testing.T) {
+			bytes, err := ioutil.ReadFile(filepath.Join(pathTestData, test.fileName))
+			if err != nil {
+				t.Fatalf("couldn't read test data: %v", err)
+			}
+			gvk := schema.GroupVersionKind{
+				Group:   test.groupVersion.Group,
+				Version: test.groupVersion.Version,
+				Kind:    test.kind,
+			}
+			err = VerifyUnmarshalStrict(bytes, gvk)
+			if (err != nil) != test.expectedError {
+				t.Errorf("expected error %v, got %v, error: %v", err != nil, test.expectedError, err)
+			}
+		})
+	}
+}

cmd/kubeadm/app/util/config/strict/testdata/invalid_duplicate_field_clustercfg.yaml

@@ -0,0 +1,4 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: ClusterConfiguration
+controlPlaneEndpoint: test1
+controlPlaneEndpoint: test2

cmd/kubeadm/app/util/config/strict/testdata/invalid_duplicate_field_initcfg.yaml

@@ -0,0 +1,6 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: InitConfiguration
+bootstrapTokens:
+- token: "9a08jv.c0izixklcxtmnze7"
+bootstrapTokens:
+- token: "9a08jv.c0izixklcxtmnze7"

cmd/kubeadm/app/util/config/strict/testdata/invalid_duplicate_field_joincfg.yaml

@@ -0,0 +1,4 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: JoinConfiguration
+caCertPath: relativepath
+caCertPath: relativepath

cmd/kubeadm/app/util/config/strict/testdata/invalid_duplicate_field_kubeletcfg.yaml

@@ -0,0 +1,4 @@
+apiVersion: kubelet.config.k8s.io/v1beta1
+kind: KubeletConfiguration
+address: 1.2.3.4
+address: 1.2.3.4

cmd/kubeadm/app/util/config/strict/testdata/invalid_duplicate_field_kubeproxycfg.yaml

@@ -0,0 +1,4 @@
+apiVersion: kubeproxy.config.k8s.io/v1alpha1
+kind: KubeProxyConfiguration
+portRange: ""
+portRange: ""

cmd/kubeadm/app/util/config/strict/testdata/invalid_unknown_field_clustercfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: ClusterConfiguration
+unknownField: test

cmd/kubeadm/app/util/config/strict/testdata/invalid_unknown_field_initcfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: InitConfiguration
+unknownField: test

cmd/kubeadm/app/util/config/strict/testdata/invalid_unknown_field_joincfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: JoinConfiguration
+unknownField: test

cmd/kubeadm/app/util/config/strict/testdata/invalid_unknown_field_kubeletcfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubelet.config.k8s.io/v1beta1
+kind: KubeletConfiguration
+unknownField: unknown

cmd/kubeadm/app/util/config/strict/testdata/invalid_unknown_field_kubeproxycfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeproxy.config.k8s.io/v1alpha1
+kind: KubeProxyConfiguration
+unknownField: unknown

cmd/kubeadm/app/util/config/strict/testdata/valid_clustercfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: ClusterConfiguration
+controlPlaneEndpoint: 202.0.100.1

cmd/kubeadm/app/util/config/strict/testdata/valid_initcfg.yaml

@@ -0,0 +1,4 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: InitConfiguration
+bootstrapTokens:
+- token: "9a08jv.c0izixklcxtmnze7"

cmd/kubeadm/app/util/config/strict/testdata/valid_joincfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeadm.k8s.io/v1beta1
+kind: JoinConfiguration
+caCertPath: relativepath

cmd/kubeadm/app/util/config/strict/testdata/valid_kubeletcfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubelet.config.k8s.io/v1beta1
+kind: KubeletConfiguration
+address: 1.2.3.4

cmd/kubeadm/app/util/config/strict/testdata/valid_kubeproxycfg.yaml

@@ -0,0 +1,3 @@
+apiVersion: kubeproxy.config.k8s.io/v1alpha1
+kind: KubeProxyConfiguration
+portRange: ""