kubeadm: improve strict validation for configuration

- Modify VerifyUnmarshalStrict to use serializer/json instead
of sigs.k8s.io/yaml. In strict mode, the serializers
in serializer/json use the new sigs.k8s.io/json library
that also catches case sensitive errors for field names -
e.g. foo vs Foo. Include test case for that in strict/testdata.
- Move the hardcoded schemes to check to the side of the
caller - i.e. accept a slice of runtime.Scheme.
- Move the klog warnings outside of VerifyUnmarshalStrict
and make them the responsibility of the caller.
- Call VerifyUnmarshalStrict when downloading the configuration
from kubeadm-config or the kube-proxy or kubelet-config CMs.
This validation is useful if the user has manually patched the CMs.
This commit is contained in:
Lubomir I. Ivanov 2022-01-24 16:16:47 +02:00
parent 8f80ae88f2
commit ee5c927f06
7 changed files with 56 additions and 34 deletions

View File

@ -31,6 +31,7 @@ import (
"k8s.io/kubernetes/cmd/kubeadm/app/apis/output" "k8s.io/kubernetes/cmd/kubeadm/app/apis/output"
kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
"k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" "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. // 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 // As long as we support only component configs with a single kind, this is allowed
return runtime.DecodeInto(Codecs.UniversalDecoder(), yaml, into) return runtime.DecodeInto(Codecs.UniversalDecoder(), yaml, into)
} }

View File

@ -33,6 +33,7 @@ import (
clientset "k8s.io/client-go/kubernetes" clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/clientcmd"
certutil "k8s.io/client-go/util/cert" certutil "k8s.io/client-go/util/cert"
"k8s.io/klog/v2"
kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" 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/componentconfigs"
"k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" "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 // FetchInitConfigurationFromCluster fetches configuration from a ConfigMap in the cluster
@ -82,6 +84,12 @@ func getInitConfigurationFromCluster(kubeconfigDir string, client clientset.Inte
if !ok { if !ok {
return nil, errors.Errorf("unexpected error when reading kubeadm-config ConfigMap: %s key value pair missing", constants.ClusterConfigurationConfigMapKey) 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 { if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), []byte(clusterConfigurationData), &initcfg.ClusterConfiguration); err != nil {
return nil, errors.Wrap(err, "failed to decode cluster configuration data") return nil, errors.Wrap(err, "failed to decode cluster configuration data")
} }

View File

@ -302,7 +302,9 @@ func documentMapToInitConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat
} }
// verify the validity of the YAML // 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) { if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvk) {
// Set initcfg to an empty struct value the deserializer will populate // Set initcfg to an empty struct value the deserializer will populate

View File

@ -104,7 +104,9 @@ func documentMapToJoinConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecat
} }
// verify the validity of the YAML // 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 joinBytes = bytes
} }

View File

@ -19,41 +19,31 @@ package strict
import ( import (
"github.com/pkg/errors" "github.com/pkg/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/klog/v2" "k8s.io/apimachinery/pkg/runtime/serializer/json"
"sigs.k8s.io/yaml"
"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme"
"k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs"
) )
// VerifyUnmarshalStrict takes a YAML byte slice and a GroupVersionKind and verifies if the YAML // VerifyUnmarshalStrict takes a slice of schems, a JSON/YAML byte slice and a GroupVersionKind
// schema is known and if it unmarshals with strict mode. // 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 {
// TODO(neolit123): The returned error here is currently ignored everywhere and a klog warning is thrown instead. var scheme *runtime.Scheme
// We don't want to turn this into an actual error yet. Eventually this can be controlled with an optional CLI flag. for _, s := range schemes {
func VerifyUnmarshalStrict(bytes []byte, gvk schema.GroupVersionKind) error { if _, err := s.New(gvk); err == nil {
scheme = s
var ( break
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 scheme == nil {
if err := yaml.UnmarshalStrict(bytes, iface); err != nil { return errors.Errorf("unknown configuration %#v", gvk)
err := errors.Wrapf(err, "error unmarshaling configuration %#v", gvk)
klog.Warning(err.Error())
return err
} }
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 return nil
} }

View File

@ -14,19 +14,23 @@ See the License for the specific language governing permissions and
limitations under the License. limitations under the License.
*/ */
package strict package strict_test
import ( import (
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/schema"
kubeproxyconfigv1alpha1 "k8s.io/kube-proxy/config/v1alpha1" kubeproxyconfigv1alpha1 "k8s.io/kube-proxy/config/v1alpha1"
kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" 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" 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/constants"
"k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict"
) )
func TestVerifyUnmarshalStrict(t *testing.T) { func TestVerifyUnmarshalStrict(t *testing.T) {
@ -34,6 +38,7 @@ func TestVerifyUnmarshalStrict(t *testing.T) {
pathTestData = "testdata/" pathTestData = "testdata/"
) )
var schemes = []*runtime.Scheme{kubeadmscheme.Scheme, componentconfigs.Scheme}
var testFiles = []struct { var testFiles = []struct {
fileName string fileName string
kind string kind string
@ -41,6 +46,12 @@ func TestVerifyUnmarshalStrict(t *testing.T) {
expectedError bool expectedError bool
}{ }{
// tests with file errors // tests with file errors
{
fileName: "invalid_casesensitive_field.yaml",
kind: constants.ClusterConfigurationKind,
groupVersion: kubeadmapiv1.SchemeGroupVersion,
expectedError: true,
},
{ {
fileName: "invalid_duplicate_field_clustercfg.yaml", fileName: "invalid_duplicate_field_clustercfg.yaml",
kind: constants.InitConfigurationKind, kind: constants.InitConfigurationKind,
@ -152,7 +163,7 @@ func TestVerifyUnmarshalStrict(t *testing.T) {
Version: test.groupVersion.Version, Version: test.groupVersion.Version,
Kind: test.kind, Kind: test.kind,
} }
err = VerifyUnmarshalStrict(bytes, gvk) err = strict.VerifyUnmarshalStrict(schemes, gvk, bytes)
if (err != nil) != test.expectedError { if (err != nil) != test.expectedError {
t.Errorf("expected error %v, got %v, error: %v", err != nil, test.expectedError, err) t.Errorf("expected error %v, got %v, error: %v", err != nil, test.expectedError, err)
} }

View File

@ -0,0 +1,3 @@
apiVersion: kubeadm.k8s.io/v1beta2
kind: ClusterConfiguration
ControlPlaneEndpoint: test1