Give a reason when rejecting defaulting in CRDs

The current message for rejecting defaulting is fairly unclear -- I wasn't
sure why it was happening till I read the kubernetes source code.
That's probably not something we want our users to have to do, so this
adds more reasons for rejecting the defaulting to the message given to the
user.  We previously added reasons in certain cases, but did not in the
common cases of using apiextensions/v1beta1 or having defaulting
disabled in a feature gate.  Now we have reasons for all cases.

Author: DirectXMan12

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go

@@ -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 {

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go

@@ -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
 			},
 		},
 		{