Merge pull request #91928 from liggitt/bug/clarify-apiext-error-message

Give a reason when rejecting defaulting in CRDs
This commit is contained in:
Kubernetes Prow Robot 2020-06-10 04:40:31 -07:00 committed by GitHub
commit 7f61ab8bc4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 21 additions and 9 deletions

View File

@ -55,8 +55,10 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
return ret
}
allowDefaults, rejectDefaultsReason := allowDefaults(requestGV, nil)
opts := validationOptions{
allowDefaults: allowDefaults(requestGV, nil),
allowDefaults: allowDefaults,
disallowDefaultsReason: rejectDefaultsReason,
requireRecognizedConversionReviewVersion: true,
requireImmutableNames: false,
requireOpenAPISchema: requireOpenAPISchema(requestGV, nil),
@ -80,6 +82,8 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
type validationOptions struct {
// allowDefaults permits the validation schema to contain default attributes
allowDefaults bool
// disallowDefaultsReason gives a reason as to why allowDefaults is false (for better user feedback)
disallowDefaultsReason string
// requireRecognizedConversionReviewVersion requires accepted webhook conversion versions to contain a recognized version
requireRecognizedConversionReviewVersion bool
// requireImmutableNames disables changing spec.names
@ -102,8 +106,10 @@ type validationOptions struct {
// ValidateCustomResourceDefinitionUpdate statically validates
func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomResourceDefinition, requestGV schema.GroupVersion) field.ErrorList {
allowDefaults, rejectDefaultsReason := allowDefaults(requestGV, &oldObj.Spec)
opts := validationOptions{
allowDefaults: allowDefaults(requestGV, &oldObj.Spec),
allowDefaults: allowDefaults,
disallowDefaultsReason: rejectDefaultsReason,
requireRecognizedConversionReviewVersion: oldObj.Spec.Conversion == nil || hasValidConversionReviewVersionOrEmpty(oldObj.Spec.Conversion.ConversionReviewVersions),
requireImmutableNames: apiextensions.IsCRDConditionTrue(oldObj, apiextensions.Established),
requireOpenAPISchema: requireOpenAPISchema(requestGV, &oldObj.Spec),
@ -661,6 +667,7 @@ func validateCustomResourceDefinitionValidation(customResourceValidation *apiext
openAPIV3Schema := &specStandardValidatorV3{
allowDefaults: opts.allowDefaults,
disallowDefaultsReason: opts.disallowDefaultsReason,
requireValidPropertyType: opts.requireValidPropertyType,
}
@ -1110,16 +1117,17 @@ func allVersionsSpecifyOpenAPISchema(spec *apiextensions.CustomResourceDefinitio
return true
}
// allowDefaults returns true if the defaulting feature is enabled and the request group version allows adding defaults
func allowDefaults(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
// 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
return true, ""
}
if requestGV == apiextensionsv1beta1.SchemeGroupVersion {
return false
return false, "(cannot set default values in apiextensions.k8s.io/v1beta1 CRDs, must use apiextensions.k8s.io/v1)"
}
return true
return true, ""
}
func specHasDefaults(spec *apiextensions.CustomResourceDefinitionSpec) bool {

View File

@ -38,6 +38,7 @@ import (
type validationMatch struct {
path *field.Path
errorType field.ErrorType
contains string
}
func required(path ...string) validationMatch {
@ -58,9 +59,12 @@ func immutable(path ...string) validationMatch {
func forbidden(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeForbidden}
}
func forbiddenContains(contains string, path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeForbidden, contains: contains}
}
func (v validationMatch) matches(err *field.Error) bool {
return err.Type == v.errorType && err.Field == v.path.String()
return err.Type == v.errorType && err.Field == v.path.String() && strings.Contains(err.Error(), v.contains)
}
func strPtr(s string) *string { return &s }
@ -1641,7 +1645,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
},
requestGV: apiextensionsv1beta1.SchemeGroupVersion,
errors: []validationMatch{
forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "default"), // disallowed via v1beta1
forbiddenContains("cannot set default values in apiextensions.k8s.io/v1beta1", "spec", "validation", "openAPIV3Schema", "properties[a]", "default"), // disallowed via v1beta1
},
},
{