From f9dd6fab1ab6d10ae73ddd1602c9928ae91cd20a Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Thu, 17 Oct 2024 13:15:50 -0400 Subject: [PATCH] UPSTREAM: : admission: validate minimumKubeletVersion Signed-off-by: Peter Hunt --- .../node/validate_node_config.go | 102 +++++++- .../node/validate_node_config_test.go | 219 ++++++++++++++++++ 2 files changed, 312 insertions(+), 9 deletions(-) diff --git a/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config.go b/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config.go index b4b63914f8d71..0c261472f28b9 100644 --- a/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config.go +++ b/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config.go @@ -2,16 +2,25 @@ package node import ( "context" + "errors" "fmt" "io" + configv1 "github.com/openshift/api/config/v1" + nodelib "github.com/openshift/library-go/pkg/apiserver/node" + + openshiftfeatures "github.com/openshift/api/features" "k8s.io/apimachinery/pkg/api/validation" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/admission" - - configv1 "github.com/openshift/api/config/v1" + "k8s.io/apiserver/pkg/admission/initializer" + "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/informers" + corev1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/component-base/featuregate" "k8s.io/kubernetes/openshift-kube-apiserver/admission/customresourcevalidation" ) @@ -25,18 +34,28 @@ var rejectionScenarios = []struct { {fromProfile: configv1.LowUpdateSlowReaction, toProfile: configv1.DefaultUpdateDefaultReaction}, } -const PluginName = "config.openshift.io/RestrictExtremeWorkerLatencyProfile" +const PluginName = "config.openshift.io/ValidateConfigNodeV1" // Register registers a plugin func Register(plugins *admission.Plugins) { plugins.Register(PluginName, func(config io.Reader) (admission.Interface, error) { - return customresourcevalidation.NewValidator( + ret := &validateCustomResourceWithNodeLister{} + delegate, err := customresourcevalidation.NewValidator( map[schema.GroupResource]bool{ configv1.Resource("nodes"): true, }, map[schema.GroupVersionKind]customresourcevalidation.ObjectValidator{ - configv1.GroupVersion.WithKind("Node"): configNodeV1{}, + configv1.GroupVersion.WithKind("Node"): &configNodeV1{ + nodeLister: ret.getNodeLister, + waitForReady: ret.getWaitForReady, + minimumKubeletVersionEnabled: feature.DefaultMutableFeatureGate.Enabled(featuregate.Feature(openshiftfeatures.FeatureGateMinimumKubeletVersion)), + }, }) + if err != nil { + return nil, err + } + ret.ValidationInterface = delegate + return ret, nil }) } @@ -57,7 +76,13 @@ func toConfigNodeV1(uncastObj runtime.Object) (*configv1.Node, field.ErrorList) return obj, nil } -type configNodeV1 struct{} +type configNodeV1 struct { + admission.ValidationInterface + + nodeLister func() corev1listers.NodeLister + waitForReady func() func() bool + minimumKubeletVersionEnabled bool +} func validateConfigNodeForExtremeLatencyProfile(obj, oldObj *configv1.Node) *field.Error { fromProfile := oldObj.Spec.WorkerLatencyProfile @@ -78,18 +103,21 @@ func validateConfigNodeForExtremeLatencyProfile(obj, oldObj *configv1.Node) *fie return nil } -func (configNodeV1) ValidateCreate(_ context.Context, uncastObj runtime.Object) field.ErrorList { +func (c *configNodeV1) ValidateCreate(_ context.Context, uncastObj runtime.Object) field.ErrorList { obj, allErrs := toConfigNodeV1(uncastObj) if len(allErrs) > 0 { return allErrs } allErrs = append(allErrs, validation.ValidateObjectMeta(&obj.ObjectMeta, false, customresourcevalidation.RequireNameCluster, field.NewPath("metadata"))...) + if err := c.validateMinimumKubeletVersion(obj); err != nil { + allErrs = append(allErrs, err) + } return allErrs } -func (configNodeV1) ValidateUpdate(_ context.Context, uncastObj runtime.Object, uncastOldObj runtime.Object) field.ErrorList { +func (c *configNodeV1) ValidateUpdate(_ context.Context, uncastObj runtime.Object, uncastOldObj runtime.Object) field.ErrorList { obj, allErrs := toConfigNodeV1(uncastObj) if len(allErrs) > 0 { return allErrs @@ -103,11 +131,36 @@ func (configNodeV1) ValidateUpdate(_ context.Context, uncastObj runtime.Object, if err := validateConfigNodeForExtremeLatencyProfile(obj, oldObj); err != nil { allErrs = append(allErrs, err) } + if err := c.validateMinimumKubeletVersion(obj); err != nil { + allErrs = append(allErrs, err) + } return allErrs } +func (c *configNodeV1) validateMinimumKubeletVersion(obj *configv1.Node) *field.Error { + if !c.minimumKubeletVersionEnabled { + return nil + } + fieldPath := field.NewPath("spec", "minimumKubeletVersion") + if !c.waitForReady()() { + return field.InternalError(fieldPath, fmt.Errorf("caches not synchronized, cannot validate minimumKubeletVersion")) + } -func (configNodeV1) ValidateStatusUpdate(_ context.Context, uncastObj runtime.Object, uncastOldObj runtime.Object) field.ErrorList { + nodes, err := c.nodeLister().List(labels.Everything()) + if err != nil { + return field.Forbidden(fieldPath, fmt.Sprintf("Getting nodes to compare minimum version %v", err.Error())) + } + + if err := nodelib.ValidateMinimumKubeletVersion(nodes, obj.Spec.MinimumKubeletVersion); err != nil { + if errors.Is(err, nodelib.ErrKubeletOutdated) { + return field.Forbidden(fieldPath, err.Error()) + } + return field.Invalid(fieldPath, obj.Spec.MinimumKubeletVersion, err.Error()) + } + return nil +} + +func (*configNodeV1) ValidateStatusUpdate(_ context.Context, uncastObj runtime.Object, uncastOldObj runtime.Object) field.ErrorList { obj, errs := toConfigNodeV1(uncastObj) if len(errs) > 0 { return errs @@ -122,3 +175,34 @@ func (configNodeV1) ValidateStatusUpdate(_ context.Context, uncastObj runtime.Ob return errs } + +type validateCustomResourceWithNodeLister struct { + nodeLister corev1listers.NodeLister + + handler admission.Handler + admission.ValidationInterface +} + +var _ = initializer.WantsExternalKubeInformerFactory(&validateCustomResourceWithNodeLister{}) + +func (c *validateCustomResourceWithNodeLister) SetExternalKubeInformerFactory(kubeInformers informers.SharedInformerFactory) { + nodeInformer := kubeInformers.Core().V1().Nodes() + c.nodeLister = nodeInformer.Lister() + c.handler.SetReadyFunc(nodeInformer.Informer().HasSynced) +} + +func (c *validateCustomResourceWithNodeLister) ValidateInitialization() error { + if c.nodeLister == nil { + return fmt.Errorf("%s needs a nodes", PluginName) + } + + return nil +} + +func (c *validateCustomResourceWithNodeLister) getNodeLister() corev1listers.NodeLister { + return c.nodeLister +} + +func (c *validateCustomResourceWithNodeLister) getWaitForReady() func() bool { + return c.handler.WaitForReady +} diff --git a/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config_test.go b/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config_test.go index b22c6a2da90a1..6af84972eb862 100644 --- a/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config_test.go +++ b/openshift-kube-apiserver/admission/customresourcevalidation/node/validate_node_config_test.go @@ -7,6 +7,11 @@ import ( "github.com/stretchr/testify/assert" configv1 "github.com/openshift/api/config/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + corev1listers "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" ) func TestValidateConfigNodeForExtremeLatencyProfile(t *testing.T) { @@ -66,3 +71,217 @@ func TestValidateConfigNodeForExtremeLatencyProfile(t *testing.T) { }) } } + +func TestValidateConfigNodeForMinimumKubeletVersion(t *testing.T) { + testCases := []struct { + name string + version string + shouldReject bool + nodes []*v1.Node + nodeListErr error + errType field.ErrorType + errMsg string + }{ + // no rejections + { + name: "should not reject when minimum kubelet version is empty", + version: "", + shouldReject: false, + }, + { + name: "should reject when min kubelet version bogus", + version: "bogus", + shouldReject: true, + nodes: []*v1.Node{ + { + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.30.0", + }, + }, + }, + }, + errType: field.ErrorTypeInvalid, + errMsg: "failed to parse submitted version bogus No Major.Minor.Patch elements found", + }, + { + name: "should reject when kubelet version is bogus", + version: "1.30.0", + shouldReject: true, + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "bogus", + }, + }, + }, + }, + errType: field.ErrorTypeInvalid, + errMsg: "failed to parse node version bogus: No Major.Minor.Patch elements found", + }, + { + name: "should reject when kubelet version is too old", + version: "1.30.0", + shouldReject: true, + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.29.0", + }, + }, + }, + }, + errType: field.ErrorTypeForbidden, + errMsg: "kubelet version is 1.29.0, which is lower than minimumKubeletVersion of 1.30.0", + }, + { + name: "should reject when one kubelet version is too old", + version: "1.30.0", + shouldReject: true, + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.30.0", + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node2", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.29.0", + }, + }, + }, + }, + errType: field.ErrorTypeForbidden, + errMsg: "kubelet version is 1.29.0, which is lower than minimumKubeletVersion of 1.30.0", + }, + { + name: "should not reject when kubelet version is equal", + version: "1.30.0", + shouldReject: false, + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.30.0", + }, + }, + }, + }, + }, + { + name: "should reject when min version incomplete", + version: "1.30", + shouldReject: true, + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.30.0", + }, + }, + }, + }, + errType: field.ErrorTypeInvalid, + errMsg: "failed to parse submitted version 1.30 No Major.Minor.Patch elements found", + }, + { + name: "should reject when kubelet version incomplete", + version: "1.30.0", + shouldReject: true, + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.30", + }, + }, + }, + }, + errType: field.ErrorTypeInvalid, + errMsg: "failed to parse node version 1.30: No Major.Minor.Patch elements found", + }, + { + name: "should not reject when kubelet version is new enough", + version: "1.30.0", + shouldReject: false, + nodes: []*v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{ + KubeletVersion: "1.31.0", + }, + }, + }, + }, + }, + } + for _, testCase := range testCases { + shouldStr := "should not be" + if testCase.shouldReject { + shouldStr = "should be" + } + t.Run(testCase.name, func(t *testing.T) { + obj := configv1.Node{ + Spec: configv1.NodeSpec{ + MinimumKubeletVersion: testCase.version, + }, + } + v := &configNodeV1{ + nodeLister: fakeNodeLister(testCase.nodes), + waitForReady: func() func() bool { + return func() bool { + return true + } + }, + minimumKubeletVersionEnabled: true, + } + + fieldErr := v.validateMinimumKubeletVersion(&obj) + assert.Equal(t, testCase.shouldReject, fieldErr != nil, "minimum kubelet version %q %s rejected", testCase.version, shouldStr) + if testCase.shouldReject { + assert.Equal(t, "spec.minimumKubeletVersion", fieldErr.Field, "field name during for mininumKubeletVersion should be spec.mininumKubeletVersion") + assert.Equal(t, fieldErr.Type, testCase.errType, "error type should be %q", testCase.errType) + assert.Contains(t, fieldErr.Detail, testCase.errMsg, "error message should contain %q", testCase.errMsg) + } + }) + } +} + +func fakeNodeLister(nodes []*v1.Node) func() corev1listers.NodeLister { + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) + for _, node := range nodes { + _ = indexer.Add(node) + } + return func() corev1listers.NodeLister { + return corev1listers.NewNodeLister(indexer) + } +}