Merge pull request #143646 from yongruilin/dv-static-manifest

Run declarative validation on kubelet static pods

pkg/admission/plugin/policy/manifest/loader/mutating_loader.go

@@ -21,11 +21,13 @@ limitations under the License.
 package loader
 
 import (
+	"context"
 	"fmt"
 
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	policyloader "k8s.io/apiserver/pkg/admission/plugin/policy/manifest/loader"
+	"k8s.io/apiserver/pkg/registry/rest"
 	admissionregistration "k8s.io/kubernetes/pkg/apis/admissionregistration"
 	"k8s.io/kubernetes/pkg/apis/admissionregistration/validation"
 )
@@ -116,7 +118,8 @@ func defaultAndValidateMAP(policy *admissionregistrationv1.MutatingAdmissionPoli
 		return fmt.Errorf("MutatingAdmissionPolicy %q: conversion error: %w", policy.Name, err)
 	}
 	errs := validation.ValidateMutatingAdmissionPolicy(internalObj)
-	if errs = validateDeclaratively(internalObj, errs); len(errs) > 0 {
+	errs = rest.ValidateDeclarativelyForManifest(context.Background(), scheme, admissionregistrationv1.SchemeGroupVersion, internalObj, errs, rest.DeclarativeValidationConfig{})
+	if len(errs) > 0 {
 		return fmt.Errorf("MutatingAdmissionPolicy %q: %w", policy.Name, errs.ToAggregate())
 	}
 	resultObj := &admissionregistrationv1.MutatingAdmissionPolicy{}
@@ -136,7 +139,7 @@ func defaultAndValidateMAPB(binding *admissionregistrationv1.MutatingAdmissionPo
 		return fmt.Errorf("MutatingAdmissionPolicyBinding %q: conversion error: %w", binding.Name, err)
 	}
 	errs := validation.ValidateMutatingAdmissionPolicyBinding(internalObj)
-	errs = validateDeclaratively(internalObj, errs)
+	errs = rest.ValidateDeclarativelyForManifest(context.Background(), scheme, admissionregistrationv1.SchemeGroupVersion, internalObj, errs, rest.DeclarativeValidationConfig{})
 	if len(errs) > 0 {
 		return fmt.Errorf("MutatingAdmissionPolicyBinding %q: %w", binding.Name, errs.ToAggregate())
 	}

pkg/admission/plugin/policy/manifest/loader/validating_loader.go

@@ -23,13 +23,10 @@ import (
 	"fmt"
 
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
-	"k8s.io/apimachinery/pkg/api/operation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 	policyloader "k8s.io/apiserver/pkg/admission/plugin/policy/manifest/loader"
-	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
 	admissionregistration "k8s.io/kubernetes/pkg/apis/admissionregistration"
 	admissionregistrationinstall "k8s.io/kubernetes/pkg/apis/admissionregistration/install"
@@ -132,7 +129,8 @@ func defaultAndValidateVAP(policy *admissionregistrationv1.ValidatingAdmissionPo
 		return fmt.Errorf("ValidatingAdmissionPolicy %q: conversion error: %w", policy.Name, err)
 	}
 	errs := validation.ValidateValidatingAdmissionPolicy(internalObj)
-	if errs = validateDeclaratively(internalObj, errs); len(errs) > 0 {
+	errs = rest.ValidateDeclarativelyForManifest(context.Background(), scheme, admissionregistrationv1.SchemeGroupVersion, internalObj, errs, rest.DeclarativeValidationConfig{})
+	if len(errs) > 0 {
 		return fmt.Errorf("ValidatingAdmissionPolicy %q: %w", policy.Name, errs.ToAggregate())
 	}
 	resultObj := &admissionregistrationv1.ValidatingAdmissionPolicy{}
@@ -152,7 +150,7 @@ func defaultAndValidateVAPB(binding *admissionregistrationv1.ValidatingAdmission
 		return fmt.Errorf("ValidatingAdmissionPolicyBinding %q: conversion error: %w", binding.Name, err)
 	}
 	errs := validation.ValidateValidatingAdmissionPolicyBinding(internalObj)
-	errs = validateDeclaratively(internalObj, errs)
+	errs = rest.ValidateDeclarativelyForManifest(context.Background(), scheme, admissionregistrationv1.SchemeGroupVersion, internalObj, errs, rest.DeclarativeValidationConfig{})
 	if len(errs) > 0 {
 		return fmt.Errorf("ValidatingAdmissionPolicyBinding %q: %w", binding.Name, errs.ToAggregate())
 	}
@@ -163,11 +161,3 @@ func defaultAndValidateVAPB(binding *admissionregistrationv1.ValidatingAdmission
 	*binding = *resultObj
 	return nil
 }
-
-func validateDeclaratively(obj runtime.Object, errs field.ErrorList) field.ErrorList {
-	ctx := genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{
-		APIGroup:   admissionregistrationv1.GroupName,
-		APIVersion: admissionregistrationv1.SchemeGroupVersion.Version,
-	})
-	return rest.ValidateDeclarativelyWithMigrationChecks(ctx, scheme, obj, nil, errs, operation.Create, rest.DeclarativeValidationConfig{})
-}

pkg/admission/plugin/webhook/manifest/loader/loader.go

@@ -25,13 +25,10 @@ import (
 	"fmt"
 
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
-	"k8s.io/apimachinery/pkg/api/operation"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 	stagingloader "k8s.io/apiserver/pkg/admission/plugin/webhook/manifest/loader"
-	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
 	admissionregistration "k8s.io/kubernetes/pkg/apis/admissionregistration"
 	admissionregistrationinstall "k8s.io/kubernetes/pkg/apis/admissionregistration/install"
@@ -140,7 +137,7 @@ func defaultAndValidateVWC(config *admissionregistrationv1.ValidatingWebhookConf
 		return fmt.Errorf("ValidatingWebhookConfiguration %q: conversion error: %w", config.Name, err)
 	}
 	errs := validation.ValidateValidatingWebhookConfiguration(internalObj)
-	errs = validateDeclaratively(internalObj, errs)
+	errs = rest.ValidateDeclarativelyForManifest(context.Background(), scheme, admissionregistrationv1.SchemeGroupVersion, internalObj, errs, rest.DeclarativeValidationConfig{})
 	if len(errs) > 0 {
 		return fmt.Errorf("ValidatingWebhookConfiguration %q: %w", config.Name, errs.ToAggregate())
 	}
@@ -162,7 +159,7 @@ func defaultAndValidateMWC(config *admissionregistrationv1.MutatingWebhookConfig
 		return fmt.Errorf("MutatingWebhookConfiguration %q: conversion error: %w", config.Name, err)
 	}
 	errs := validation.ValidateMutatingWebhookConfiguration(internalObj)
-	errs = validateDeclaratively(internalObj, errs)
+	errs = rest.ValidateDeclarativelyForManifest(context.Background(), scheme, admissionregistrationv1.SchemeGroupVersion, internalObj, errs, rest.DeclarativeValidationConfig{})
 	if len(errs) > 0 {
 		return fmt.Errorf("MutatingWebhookConfiguration %q: %w", config.Name, errs.ToAggregate())
 	}
@@ -173,11 +170,3 @@ func defaultAndValidateMWC(config *admissionregistrationv1.MutatingWebhookConfig
 	*config = *resultConfig
 	return nil
 }
-
-func validateDeclaratively(obj runtime.Object, errs field.ErrorList) field.ErrorList {
-	ctx := genericapirequest.WithRequestInfo(context.Background(), &genericapirequest.RequestInfo{
-		APIGroup:   admissionregistrationv1.GroupName,
-		APIVersion: admissionregistrationv1.SchemeGroupVersion.Version,
-	})
-	return rest.ValidateDeclarativelyWithMigrationChecks(ctx, scheme, obj, nil, errs, operation.Create, rest.DeclarativeValidationConfig{})
-}

pkg/kubelet/config/common.go

@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"context"
 	"encoding/hex"
 	"errors"
 	"fmt"
@@ -27,7 +28,9 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	utilyaml "k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/apiserver/pkg/registry/rest"
 	podutil "k8s.io/kubernetes/pkg/api/pod"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/apis/core/helper"
@@ -118,6 +121,18 @@ var ErrStaticPodTriedToUseClusterTrustBundle = errors.New("static pods may not u
 // A static pod tried to use a resource claim.
 var ErrStaticPodTriedToUseResourceClaims = errors.New("static pods may not use ResourceClaims")
 
+// validateStaticPod applies the API server's create validation, handwritten and declarative,
+// because a rejected mirror pod does not stop the static pod from running. Unlike the API server,
+// it validates fields of disabled features instead of dropping them, since static pods must match
+// the kubelet's feature gates.
+func validateStaticPod(logger klog.Logger, pod *api.Pod) field.ErrorList {
+	opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&pod.Spec, nil, &pod.ObjectMeta, nil)
+	errs := validation.ValidatePodCreate(pod, opts)
+	ctx := klog.NewContext(context.Background(), logger)
+	// The pod strategy uses the default config; if it gains options, pass the same ones here.
+	return rest.ValidateDeclarativelyForManifest(ctx, legacyscheme.Scheme, v1.SchemeGroupVersion, pod, errs, rest.DeclarativeValidationConfig{})
+}
+
 // tryDecodeSinglePod takes data and tries to extract valid Pod config information from it.
 func tryDecodeSinglePod(logger klog.Logger, data []byte, defaultFn defaultFunc) (parsed bool, pod *v1.Pod, err error) {
 	// JSON is valid YAML, so this should work for everything.
@@ -144,8 +159,7 @@ func tryDecodeSinglePod(logger klog.Logger, data []byte, defaultFn defaultFunc)
 	if err = defaultFn(logger, newPod); err != nil {
 		return true, pod, err
 	}
-	opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&newPod.Spec, nil, &newPod.ObjectMeta, nil)
-	if errs := validation.ValidatePodCreate(newPod, opts); len(errs) > 0 {
+	if errs := validateStaticPod(logger, newPod); len(errs) > 0 {
 		return true, pod, fmt.Errorf("invalid pod: %v", errs)
 	}
 	v1Pod := &v1.Pod{}
@@ -193,8 +207,7 @@ func tryDecodePodList(logger klog.Logger, data []byte, defaultFn defaultFunc) (p
 		if err = defaultFn(logger, newPod); err != nil {
 			return true, pods, err
 		}
-		opts := podutil.GetValidationOptionsFromPodSpecAndMeta(&newPod.Spec, nil, &newPod.ObjectMeta, nil)
-		if errs := validation.ValidatePodCreate(newPod, opts); len(errs) > 0 {
+		if errs := validateStaticPod(logger, newPod); len(errs) > 0 {
 			err = fmt.Errorf("invalid pod: %v", errs)
 			return true, pods, err
 		}

pkg/kubelet/config/common_test.go

@@ -531,3 +531,33 @@ func TestGetStaticPodPriorityWarning(t *testing.T) {
 		}
 	}
 }
+
+func TestDecodeStaticPodRunsDeclarativeValidation(t *testing.T) {
+	featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EvictionRequestAPI, true)
+	logger, _ := ktesting.NewTestContext(t)
+	pod := &v1.Pod{
+		ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "mynamespace"},
+		Spec: v1.PodSpec{
+			Containers: []v1.Container{{Name: "image", Image: "test/image"}},
+			// Only declarative validation enforces the priority bounds.
+			EvictionResponders: []v1.EvictionResponder{{Name: "acme.io/foo", Priority: new(int32(100001))}},
+		},
+	}
+	const wantErr = "spec.evictionResponders[0].priority"
+
+	single, err := runtime.Encode(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), pod)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if _, _, err := tryDecodeSinglePod(logger, single, noDefault); err == nil || !strings.Contains(err.Error(), wantErr) {
+		t.Errorf("tryDecodeSinglePod: got error %v, want one containing %q", err, wantErr)
+	}
+
+	list, err := runtime.Encode(clientscheme.Codecs.LegacyCodec(v1.SchemeGroupVersion), &v1.PodList{Items: []v1.Pod{*pod}})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if _, _, err := tryDecodePodList(logger, list, noDefault); err == nil || !strings.Contains(err.Error(), wantErr) {
+		t.Errorf("tryDecodePodList: got error %v, want one containing %q", err, wantErr)
+	}
+}

plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/validation/validation_test.go

@@ -56,6 +56,13 @@ func TestValidateConfiguration(t *testing.T) {
 		},
 		testName:   "Invalid case",
 		testStatus: false,
+	}, {
+		// Add declarative validation to this config before removing the handwritten key check.
+		config: internalapi.Configuration{
+			Default: []api.Toleration{{Key: "foo bar", Operator: "Exists", Effect: "NoSchedule"}},
+		},
+		testName:   "Invalid key",
+		testStatus: false,
 	}}
 
 	for i := range tests {

staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go

@@ -429,6 +429,14 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti
 	return errs
 }
 
+// ValidateDeclarativelyForManifest runs ValidateDeclarativelyWithMigrationChecks as a create, for
+// objects validated outside a REST request, such as static manifests. gv is the API version whose
+// declarative validation applies, and config should match the one the kind's REST strategy uses.
+func ValidateDeclarativelyForManifest(ctx context.Context, scheme *runtime.Scheme, gv schema.GroupVersion, obj runtime.Object, errs field.ErrorList, config DeclarativeValidationConfig) field.ErrorList {
+	ctx = genericapirequest.WithRequestInfo(ctx, &genericapirequest.RequestInfo{APIGroup: gv.Group, APIVersion: gv.Version})
+	return ValidateDeclarativelyWithMigrationChecks(ctx, scheme, obj, nil, errs, operation.Create, config)
+}
+
 // RecordDuplicateValidationErrors increments a metric and log the error when duplicate validation errors are found.
 func RecordDuplicateValidationErrors(ctx context.Context, qualifiedKind schema.GroupKind, errs field.ErrorList) {
 	logger := klog.FromContext(ctx)

staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go

@@ -848,3 +848,24 @@ func TestValidateDeclarativelyWithMigrationChecks(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateDeclarativelyForManifest(t *testing.T) {
+	gv := schema.GroupVersion{Version: "v1"}
+	scheme := runtime.NewScheme()
+	scheme.AddKnownTypes(gv, &v1.Pod{})
+	declarativeErr := field.Invalid(field.NewPath("spec"), nil, "declarative")
+	scheme.AddValidationFunc(&v1.Pod{}, func(ctx context.Context, op operation.Operation, object, oldObject any) field.ErrorList {
+		if !op.Options["TestOption"] {
+			return nil
+		}
+		return field.ErrorList{declarativeErr}
+	})
+	handwrittenErr := field.Required(field.NewPath("metadata", "name"), "")
+	config := DeclarativeValidationConfig{Options: map[string]bool{"TestOption": true}}
+
+	got := ValidateDeclarativelyForManifest(context.Background(), scheme, gv, &v1.Pod{}, field.ErrorList{handwrittenErr}, config)
+	want := field.ErrorList{handwrittenErr, declarativeErr}
+	if !equalErrorLists(got, want) {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}