Drop legacy validation logic for CRD API

This commit is contained in:
Jordan Liggitt 2021-08-09 13:13:20 -04:00
parent 1ceb118e3c
commit 97c5b8de9a
4 changed files with 153 additions and 690 deletions

View File

@ -27,7 +27,6 @@ import (
structuraldefaulting "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting"
apiequality "k8s.io/apimachinery/pkg/api/equality"
genericvalidation "k8s.io/apimachinery/pkg/api/validation"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/sets"
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
@ -47,7 +46,7 @@ var (
)
// ValidateCustomResourceDefinition statically validates
func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinition) field.ErrorList {
nameValidationFn := func(name string, prefix bool) []string {
ret := genericvalidation.NameIsDNSSubdomain(name, prefix)
requiredName := obj.Spec.Names.Plural + "." + obj.Spec.Group
@ -57,15 +56,13 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
return ret
}
allowDefaults, rejectDefaultsReason := allowDefaults(requestGV, nil)
opts := validationOptions{
allowDefaults: allowDefaults,
disallowDefaultsReason: rejectDefaultsReason,
allowDefaults: true,
requireRecognizedConversionReviewVersion: true,
requireImmutableNames: false,
requireOpenAPISchema: requireOpenAPISchema(requestGV, nil),
requireValidPropertyType: requireValidPropertyType(requestGV, nil),
requireStructuralSchema: requireStructuralSchema(requestGV, nil),
requireOpenAPISchema: true,
requireValidPropertyType: true,
requireStructuralSchema: true,
requirePrunedDefaults: true,
requireAtomicSetType: true,
requireMapListKeysMapSetValidation: true,
@ -75,8 +72,8 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
allErrs = append(allErrs, validateCustomResourceDefinitionSpec(&obj.Spec, opts, field.NewPath("spec"))...)
allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...)
allErrs = append(allErrs, ValidateCustomResourceDefinitionStoredVersions(obj.Status.StoredVersions, obj.Spec.Versions, field.NewPath("status").Child("storedVersions"))...)
allErrs = append(allErrs, validateAPIApproval(obj, nil, requestGV)...)
allErrs = append(allErrs, validatePreserveUnknownFields(obj, nil, requestGV)...)
allErrs = append(allErrs, validateAPIApproval(obj, nil)...)
allErrs = append(allErrs, validatePreserveUnknownFields(obj, nil)...)
return allErrs
}
@ -107,16 +104,14 @@ type validationOptions struct {
}
// ValidateCustomResourceDefinitionUpdate statically validates
func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
allowDefaults, rejectDefaultsReason := allowDefaults(requestGV, &oldObj.Spec)
func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition) field.ErrorList {
opts := validationOptions{
allowDefaults: allowDefaults,
disallowDefaultsReason: rejectDefaultsReason,
allowDefaults: true,
requireRecognizedConversionReviewVersion: oldObj.Spec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldObj.Spec.Conversion.ConversionReviewVersions),
requireImmutableNames: apiextensions.IsCRDConditionTrue(oldObj, apiextensions.Established),
requireOpenAPISchema: requireOpenAPISchema(requestGV, &oldObj.Spec),
requireValidPropertyType: requireValidPropertyType(requestGV, &oldObj.Spec),
requireStructuralSchema: requireStructuralSchema(requestGV, &oldObj.Spec),
requireOpenAPISchema: requireOpenAPISchema(&oldObj.Spec),
requireValidPropertyType: requireValidPropertyType(&oldObj.Spec),
requireStructuralSchema: requireStructuralSchema(&oldObj.Spec),
requirePrunedDefaults: requirePrunedDefaults(&oldObj.Spec),
requireAtomicSetType: requireAtomicSetType(&oldObj.Spec),
requireMapListKeysMapSetValidation: requireMapListKeysMapSetValidation(&oldObj.Spec),
@ -126,8 +121,8 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes
allErrs = append(allErrs, validateCustomResourceDefinitionSpecUpdate(&obj.Spec, &oldObj.Spec, opts, field.NewPath("spec"))...)
allErrs = append(allErrs, ValidateCustomResourceDefinitionStatus(&obj.Status, field.NewPath("status"))...)
allErrs = append(allErrs, ValidateCustomResourceDefinitionStoredVersions(obj.Status.StoredVersions, obj.Spec.Versions, field.NewPath("status").Child("storedVersions"))...)
allErrs = append(allErrs, validateAPIApproval(obj, oldObj, requestGV)...)
allErrs = append(allErrs, validatePreserveUnknownFields(obj, oldObj, requestGV)...)
allErrs = append(allErrs, validateAPIApproval(obj, oldObj)...)
allErrs = append(allErrs, validatePreserveUnknownFields(obj, oldObj)...)
return allErrs
}
@ -1129,11 +1124,7 @@ func allowedAtRootSchema(field string) bool {
}
// requireOpenAPISchema returns true if the request group version requires a schema
func requireOpenAPISchema(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
// for backwards compatibility
return false
}
func requireOpenAPISchema(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
if oldCRDSpec != nil && !allVersionsSpecifyOpenAPISchema(oldCRDSpec) {
// don't tighten validation on existing persisted data
return false
@ -1152,19 +1143,6 @@ func allVersionsSpecifyOpenAPISchema(spec *apiextensions.CustomResourceDefinitio
return true
}
// allowDefaults returns true if the defaulting feature is enabled and the request group version allows adding defaults,
// or false and a reason for the user if defaults are not allowed.
func allowDefaults(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) (bool, string) {
if oldCRDSpec != nil && specHasDefaults(oldCRDSpec) {
// don't tighten validation on existing persisted data
return true, ""
}
if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
return false, "(cannot set default values in apiextensions.k8s.io/v1beta1 CRDs, must use apiextensions.k8s.io/v1)"
}
return true, ""
}
func specHasDefaults(spec *apiextensions.CustomResourceDefinitionSpec) bool {
return hasSchemaWith(spec, schemaHasDefaults)
}
@ -1277,11 +1255,7 @@ func schemaHasKubernetesExtensions(s *apiextensions.JSONSchemaProps) bool {
}
// requireStructuralSchema returns true if schemas specified must be structural
func requireStructuralSchema(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
// for compatibility
return false
}
func requireStructuralSchema(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
if oldCRDSpec != nil && specHasNonStructuralSchema(oldCRDSpec) {
// don't tighten validation on existing persisted data
return false
@ -1380,11 +1354,7 @@ func hasInvalidMapListKeysMapSet(schema *apiextensions.JSONSchemaProps) bool {
}
// requireValidPropertyType returns true if valid openapi v3 types should be required for the given API version
func requireValidPropertyType(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
// for compatibility
return false
}
func requireValidPropertyType(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
if oldCRDSpec != nil && specHasInvalidTypes(oldCRDSpec) {
// don't tighten validation on existing persisted data
return false
@ -1393,13 +1363,8 @@ func requireValidPropertyType(requestGV schema.GroupVersion, oldCRDSpec *apiexte
}
// validateAPIApproval returns a list of errors if the API approval annotation isn't valid
func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition) field.ErrorList {
// check to see if we need confirm API approval for kube group.
if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
// no-op for compatibility with v1beta1
return nil
}
if !apihelpers.IsProtectedCommunityGroup(newCRD.Spec.Group) {
// no-op for non-protected groups
return nil
@ -1433,12 +1398,7 @@ func validateAPIApproval(newCRD, oldCRD *apiextensions.CustomResourceDefinition,
}
}
func validatePreserveUnknownFields(crd, oldCRD *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
// no-op for compatibility with v1beta1
return nil
}
func validatePreserveUnknownFields(crd, oldCRD *apiextensions.CustomResourceDefinition) field.ErrorList {
if oldCRD != nil && oldCRD.Spec.PreserveUnknownFields != nil && *oldCRD.Spec.PreserveUnknownFields {
// no-op for compatibility with existing data
return nil

View File

@ -27,9 +27,7 @@ import (
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
@ -111,12 +109,7 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) {
// Validate validates a new CustomResourceDefinition.
func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
var groupVersion schema.GroupVersion
if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found {
groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
}
return validation.ValidateCustomResourceDefinition(obj.(*apiextensions.CustomResourceDefinition), groupVersion)
return validation.ValidateCustomResourceDefinition(obj.(*apiextensions.CustomResourceDefinition))
}
// WarningsOnCreate returns warnings for the creation of the given object.
@ -139,12 +132,7 @@ func (strategy) Canonicalize(obj runtime.Object) {
// ValidateUpdate is the default update validation for an end user updating status.
func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
var groupVersion schema.GroupVersion
if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found {
groupVersion = schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion}
}
return validation.ValidateCustomResourceDefinitionUpdate(obj.(*apiextensions.CustomResourceDefinition), old.(*apiextensions.CustomResourceDefinition), groupVersion)
return validation.ValidateCustomResourceDefinitionUpdate(obj.(*apiextensions.CustomResourceDefinition), old.(*apiextensions.CustomResourceDefinition))
}
// WarningsOnUpdate returns warnings for the given update.

View File

@ -23,7 +23,6 @@ import (
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/pointer"
)
@ -43,29 +42,19 @@ func TestValidateAPIApproval(t *testing.T) {
tests := []struct {
name string
version string
group string
annotationValue string
oldAnnotationValue *string
validateError func(t *testing.T, errors field.ErrorList)
}{
{
name: "ignore v1beta1",
version: "v1beta1",
group: "sigs.k8s.io",
annotationValue: "invalid",
validateError: okFn,
},
{
name: "ignore non-k8s group",
version: "v1",
group: "other.io",
annotationValue: "invalid",
validateError: okFn,
},
{
name: "invalid annotation create",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "invalid",
validateError: func(t *testing.T, errors field.ErrorList) {
@ -80,7 +69,6 @@ func TestValidateAPIApproval(t *testing.T) {
},
{
name: "invalid annotation update",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "invalid",
oldAnnotationValue: strPtr("invalid"),
@ -88,7 +76,6 @@ func TestValidateAPIApproval(t *testing.T) {
},
{
name: "invalid annotation to missing",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "",
oldAnnotationValue: strPtr("invalid"),
@ -104,7 +91,6 @@ func TestValidateAPIApproval(t *testing.T) {
},
{
name: "missing to invalid annotation",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "invalid",
oldAnnotationValue: strPtr(""),
@ -120,7 +106,6 @@ func TestValidateAPIApproval(t *testing.T) {
},
{
name: "missing annotation",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "",
validateError: func(t *testing.T, errors field.ErrorList) {
@ -135,7 +120,6 @@ func TestValidateAPIApproval(t *testing.T) {
},
{
name: "missing annotation update",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "",
oldAnnotationValue: strPtr(""),
@ -143,33 +127,16 @@ func TestValidateAPIApproval(t *testing.T) {
},
{
name: "url",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "https://github.com/kubernetes/kubernetes/pull/79724",
validateError: okFn,
},
{
name: "unapproved",
version: "v1",
group: "sigs.k8s.io",
annotationValue: "unapproved, other reason",
validateError: okFn,
},
{
name: "next version validates",
version: "v2",
group: "sigs.k8s.io",
annotationValue: "invalid",
validateError: func(t *testing.T, errors field.ErrorList) {
t.Helper()
if len(errors) == 0 {
t.Fatal("expected errors, got none")
}
if e, a := `metadata.annotations[api-approved.kubernetes.io]: Invalid value: "invalid": protected groups must have approval annotation "api-approved.kubernetes.io" with either a URL or a reason starting with "unapproved", see https://github.com/kubernetes/enhancements/pull/1111`, errors.ToAggregate().Error(); e != a {
t.Fatal(errors)
}
},
},
}
for _, test := range tests {
@ -212,9 +179,9 @@ func TestValidateAPIApproval(t *testing.T) {
var actual field.ErrorList
if oldCRD == nil {
actual = validation.ValidateCustomResourceDefinition(crd, schema.GroupVersion{Group: "apiextensions.k8s.io", Version: test.version})
actual = validation.ValidateCustomResourceDefinition(crd)
} else {
actual = validation.ValidateCustomResourceDefinitionUpdate(crd, oldCRD, schema.GroupVersion{Group: "apiextensions.k8s.io", Version: test.version})
actual = validation.ValidateCustomResourceDefinitionUpdate(crd, oldCRD)
}
test.validateError(t, actual)
})