diff --git a/cmd/kubeadm/app/componentconfigs/configset.go b/cmd/kubeadm/app/componentconfigs/configset.go index 906d99fd8cd..55cc2a3aa79 100644 --- a/cmd/kubeadm/app/componentconfigs/configset.go +++ b/cmd/kubeadm/app/componentconfigs/configset.go @@ -31,6 +31,7 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/apis/output" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" + "k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict" ) // handler is a package internal type that handles component config factory and common functionality. @@ -171,6 +172,11 @@ func (cb *configBase) Unmarshal(from kubeadmapi.DocumentMap, into runtime.Object } } + // Print warnings for strict errors + if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{Scheme}, gvk, yaml); err != nil { + klog.Warning(err.Error()) + } + // As long as we support only component configs with a single kind, this is allowed return runtime.DecodeInto(Codecs.UniversalDecoder(), yaml, into) } diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go index eeeb92837ec..3728c293261 100644 --- a/cmd/kubeadm/app/util/config/cluster.go +++ b/cmd/kubeadm/app/util/config/cluster.go @@ -33,6 +33,7 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/clientcmd" certutil "k8s.io/client-go/util/cert" + "k8s.io/klog/v2" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" @@ -40,6 +41,7 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" + "k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict" ) // FetchInitConfigurationFromCluster fetches configuration from a ConfigMap in the cluster @@ -82,6 +84,12 @@ func getInitConfigurationFromCluster(kubeconfigDir string, client clientset.Inte if !ok { return nil, errors.Errorf("unexpected error when reading kubeadm-config ConfigMap: %s key value pair missing", constants.ClusterConfigurationConfigMapKey) } + // If ClusterConfiguration was patched by something other than kubeadm, it may have errors. Warn about them. + if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{kubeadmscheme.Scheme}, + kubeadmapiv1.SchemeGroupVersion.WithKind(constants.ClusterConfigurationKind), + []byte(clusterConfigurationData)); err != nil { + klog.Warning(err.Error()) + } if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), []byte(clusterConfigurationData), &initcfg.ClusterConfiguration); err != nil { return nil, errors.Wrap(err, "failed to decode cluster configuration data") } diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go index 23fdfd426e2..7d1bb67fc02 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration.go +++ b/cmd/kubeadm/app/util/config/initconfiguration.go @@ -302,7 +302,9 @@ func documentMapToInitConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat } // verify the validity of the YAML - strict.VerifyUnmarshalStrict(fileContent, gvk) + if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{kubeadmscheme.Scheme, componentconfigs.Scheme}, gvk, fileContent); err != nil { + klog.Warning(err.Error()) + } if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvk) { // Set initcfg to an empty struct value the deserializer will populate diff --git a/cmd/kubeadm/app/util/config/joinconfiguration.go b/cmd/kubeadm/app/util/config/joinconfiguration.go index e204cdb3479..7cd9eb419df 100644 --- a/cmd/kubeadm/app/util/config/joinconfiguration.go +++ b/cmd/kubeadm/app/util/config/joinconfiguration.go @@ -104,7 +104,9 @@ func documentMapToJoinConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat } // verify the validity of the YAML - strict.VerifyUnmarshalStrict(bytes, gvk) + if err := strict.VerifyUnmarshalStrict([]*runtime.Scheme{kubeadmscheme.Scheme}, gvk, bytes); err != nil { + klog.Warning(err.Error()) + } joinBytes = bytes } diff --git a/cmd/kubeadm/app/util/config/strict/strict.go b/cmd/kubeadm/app/util/config/strict/strict.go index c8630c5e230..b82e7a98ff2 100644 --- a/cmd/kubeadm/app/util/config/strict/strict.go +++ b/cmd/kubeadm/app/util/config/strict/strict.go @@ -19,41 +19,31 @@ package strict import ( "github.com/pkg/errors" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/klog/v2" - "sigs.k8s.io/yaml" - - "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" - "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" + "k8s.io/apimachinery/pkg/runtime/serializer/json" ) -// 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 +// VerifyUnmarshalStrict takes a slice of schems, a JSON/YAML byte slice and a GroupVersionKind +// and verifies if the schema is known and if the byte slice unmarshals with strict mode. +func VerifyUnmarshalStrict(schemes []*runtime.Scheme, gvk schema.GroupVersionKind, bytes []byte) error { + var scheme *runtime.Scheme + for _, s := range schemes { + if _, err := s.New(gvk); err == nil { + scheme = s + break } } - - if err := yaml.UnmarshalStrict(bytes, iface); err != nil { - err := errors.Wrapf(err, "error unmarshaling configuration %#v", gvk) - klog.Warning(err.Error()) - return err + if scheme == nil { + return errors.Errorf("unknown configuration %#v", gvk) } + + opt := json.SerializerOptions{Yaml: true, Pretty: false, Strict: true} + serializer := json.NewSerializerWithOptions(json.DefaultMetaFactory, scheme, scheme, opt) + _, _, err := serializer.Decode(bytes, &gvk, nil) + if err != nil { + return errors.Wrapf(err, "error unmarshaling configuration %#v", gvk) + } + return nil } diff --git a/cmd/kubeadm/app/util/config/strict/strict_test.go b/cmd/kubeadm/app/util/config/strict/strict_test.go index 67da13dfd35..6ace7303ecd 100644 --- a/cmd/kubeadm/app/util/config/strict/strict_test.go +++ b/cmd/kubeadm/app/util/config/strict/strict_test.go @@ -14,19 +14,23 @@ See the License for the specific language governing permissions and limitations under the License. */ -package strict +package strict_test import ( "os" "path/filepath" "testing" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" kubeproxyconfigv1alpha1 "k8s.io/kube-proxy/config/v1alpha1" kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" + kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" + "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" "k8s.io/kubernetes/cmd/kubeadm/app/constants" + "k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict" ) func TestVerifyUnmarshalStrict(t *testing.T) { @@ -34,6 +38,7 @@ func TestVerifyUnmarshalStrict(t *testing.T) { pathTestData = "testdata/" ) + var schemes = []*runtime.Scheme{kubeadmscheme.Scheme, componentconfigs.Scheme} var testFiles = []struct { fileName string kind string @@ -41,6 +46,12 @@ func TestVerifyUnmarshalStrict(t *testing.T) { expectedError bool }{ // tests with file errors + { + fileName: "invalid_casesensitive_field.yaml", + kind: constants.ClusterConfigurationKind, + groupVersion: kubeadmapiv1.SchemeGroupVersion, + expectedError: true, + }, { fileName: "invalid_duplicate_field_clustercfg.yaml", kind: constants.InitConfigurationKind, @@ -152,7 +163,7 @@ func TestVerifyUnmarshalStrict(t *testing.T) { Version: test.groupVersion.Version, Kind: test.kind, } - err = VerifyUnmarshalStrict(bytes, gvk) + err = strict.VerifyUnmarshalStrict(schemes, gvk, bytes) if (err != nil) != test.expectedError { t.Errorf("expected error %v, got %v, error: %v", err != nil, test.expectedError, err) } diff --git a/cmd/kubeadm/app/util/config/strict/testdata/invalid_casesensitive_field.yaml b/cmd/kubeadm/app/util/config/strict/testdata/invalid_casesensitive_field.yaml new file mode 100644 index 00000000000..a4d622fe78a --- /dev/null +++ b/cmd/kubeadm/app/util/config/strict/testdata/invalid_casesensitive_field.yaml @@ -0,0 +1,3 @@ +apiVersion: kubeadm.k8s.io/v1beta2 +kind: ClusterConfiguration +ControlPlaneEndpoint: test1