github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 11:50:44 +00:00
Code Issues Projects Releases Wiki Activity

Give a reason when rejecting defaulting in CRDs

Browse Source 
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.
This commit is contained in:
Solly Ross 2019-11-05 17:07:03 -08:00 committed by Jordan Liggitt
parent 2c7d431a20
commit 7c448e37b3
2 changed files with 21 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go
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 {

8
staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
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
},
},
{