From 0fd4a0acc10814077fb15521d554cd3c508c05fb Mon Sep 17 00:00:00 2001 From: yongruilin Date: Thu, 22 May 2025 16:50:49 +0000 Subject: [PATCH] CSR: Add declarative validation test suite Introduce a test suite that ensures declarative test cases are fully tested and that validation errors are compared with handwritten validation to ensure consistency. --- .../declarative_validation_test.go | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 pkg/registry/certificates/certificates/declarative_validation_test.go diff --git a/pkg/registry/certificates/certificates/declarative_validation_test.go b/pkg/registry/certificates/certificates/declarative_validation_test.go new file mode 100644 index 00000000000..39a5e178125 --- /dev/null +++ b/pkg/registry/certificates/certificates/declarative_validation_test.go @@ -0,0 +1,152 @@ +/* +Copyright 2025 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 certificates + +import ( + "testing" + + "k8s.io/apimachinery/pkg/util/validation/field" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + apitesting "k8s.io/kubernetes/pkg/api/testing" + api "k8s.io/kubernetes/pkg/apis/certificates" + "k8s.io/kubernetes/pkg/features" +) + +var apiVersions = []string{"v1", "v1alpha1", "v1beta1"} + +func TestDeclarativeValidateForDeclarative(t *testing.T) { + for _, apiVersion := range apiVersions { + testDeclarativeValidateForDeclarative(t, apiVersion) + } +} + +func testDeclarativeValidateForDeclarative(t *testing.T, apiVersion string) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewDefaultContext(), &genericapirequest.RequestInfo{ + APIGroup: "certificates.k8s.io", + APIVersion: apiVersion, + }) + testCases := map[string]struct { + input api.CertificateSigningRequest + expectedErrs field.ErrorList + }{ + // TODO: Add test cases + } + for k, tc := range testCases { + t.Run(k, func(t *testing.T) { + var declarativeTakeoverErrs field.ErrorList + var imperativeErrs field.ErrorList + for _, gateVal := range []bool{true, false} { + // We only need to test both gate enabled and disabled together, because + // 1) the DeclarativeValidationTakeover won't take effect if DeclarativeValidation is disabled. + // 2) the validation output, when only DeclarativeValidation is enabled, is the same as when both gates are disabled. + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeclarativeValidation, gateVal) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeclarativeValidationTakeover, gateVal) + + errs := Strategy.Validate(ctx, &tc.input) + if gateVal { + declarativeTakeoverErrs = errs + } else { + imperativeErrs = errs + } + // The errOutputMatcher is used to verify the output matches the expected errors in test cases. + errOutputMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin() + if len(tc.expectedErrs) > 0 { + errOutputMatcher.Test(t, tc.expectedErrs, errs) + } else if len(errs) != 0 { + t.Errorf("expected no errors, but got: %v", errs) + } + } + // The equivalenceMatcher is used to verify the output errors from hand-written imperative validation + // are equivalent to the output errors when DeclarativeValidationTakeover is enabled. + equivalenceMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin() + equivalenceMatcher.Test(t, imperativeErrs, declarativeTakeoverErrs) + + apitesting.VerifyVersionedValidationEquivalence(t, &tc.input, nil) + }) + } +} + +func TestValidateUpdateForDeclarative(t *testing.T) { + for _, apiVersion := range apiVersions { + testValidateUpdateForDeclarative(t, apiVersion) + } +} + +func testValidateUpdateForDeclarative(t *testing.T, apiVersion string) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewDefaultContext(), &genericapirequest.RequestInfo{ + APIGroup: "certificates.k8s.io", + APIVersion: apiVersion, + }) + testCases := map[string]struct { + old api.CertificateSigningRequest + update api.CertificateSigningRequest + expectedErrs field.ErrorList + }{ + // TODO: Add test cases + } + for k, tc := range testCases { + t.Run(k, func(t *testing.T) { + tc.old.ResourceVersion = "1" + tc.update.ResourceVersion = "1" + var declarativeTakeoverErrs field.ErrorList + var imperativeErrs field.ErrorList + for _, gateVal := range []bool{true, false} { + // We only need to test both gate enabled and disabled together, because + // 1) the DeclarativeValidationTakeover won't take effect if DeclarativeValidation is disabled. + // 2) the validation output, when only DeclarativeValidation is enabled, is the same as when both gates are disabled. + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeclarativeValidation, gateVal) + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DeclarativeValidationTakeover, gateVal) + errs := Strategy.ValidateUpdate(ctx, &tc.update, &tc.old) + if gateVal { + declarativeTakeoverErrs = errs + } else { + imperativeErrs = errs + } + // The errOutputMatcher is used to verify the output matches the expected errors in test cases. + errOutputMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin() + + if len(tc.expectedErrs) > 0 { + errOutputMatcher.Test(t, tc.expectedErrs, errs) + } else if len(errs) != 0 { + t.Errorf("expected no errors, but got: %v", errs) + } + } + // The equivalenceMatcher is used to verify the output errors from hand-written imperative validation + // are equivalent to the output errors when DeclarativeValidationTakeover is enabled. + equivalenceMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin() + // TODO: remove this once ErrorMatcher has been extended to handle this form of deduplication. + dedupedImperativeErrs := field.ErrorList{} + for _, err := range imperativeErrs { + found := false + for _, existingErr := range dedupedImperativeErrs { + if equivalenceMatcher.Matches(existingErr, err) { + found = true + break + } + } + if !found { + dedupedImperativeErrs = append(dedupedImperativeErrs, err) + } + } + equivalenceMatcher.Test(t, dedupedImperativeErrs, declarativeTakeoverErrs) + + apitesting.VerifyVersionedValidationEquivalence(t, &tc.update, &tc.old) + }) + } +}