mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-31 15:25:57 +00:00
preserve errors for additionalProperties and xvalidations so the API surface stays the same
Once this has had some soak time we can allow CRDs to use these abilities
This commit is contained in:
Alexander Zielenski
parent
74f8e4dd51
commit
5bac4c16b6
@ -24,15 +24,15 @@ import (
|
||||
|
||||
// validateStructuralCompleteness checks that every specified field or array in s is also specified
|
||||
// outside of value validation.
|
||||
func validateStructuralCompleteness(s *Structural, fldPath *field.Path) field.ErrorList {
|
||||
func validateStructuralCompleteness(s *Structural, fldPath *field.Path, opts ValidationOptions) field.ErrorList {
|
||||
if s == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
return validateValueValidationCompleteness(s.ValueValidation, s, fldPath, fldPath)
|
||||
return validateValueValidationCompleteness(s.ValueValidation, s, fldPath, fldPath, opts)
|
||||
}
|
||||
|
||||
func validateValueValidationCompleteness(v *ValueValidation, s *Structural, sPath, vPath *field.Path) field.ErrorList {
|
||||
func validateValueValidationCompleteness(v *ValueValidation, s *Structural, sPath, vPath *field.Path, opts ValidationOptions) field.ErrorList {
|
||||
if v == nil {
|
||||
return nil
|
||||
}
|
||||
@ -42,21 +42,21 @@ func validateValueValidationCompleteness(v *ValueValidation, s *Structural, sPat
|
||||
|
||||
allErrs := field.ErrorList{}
|
||||
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(v.Not, s, sPath, vPath.Child("not"))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(v.Not, s, sPath, vPath.Child("not"), opts)...)
|
||||
for i := range v.AllOf {
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&v.AllOf[i], s, sPath, vPath.Child("allOf").Index(i))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&v.AllOf[i], s, sPath, vPath.Child("allOf").Index(i), opts)...)
|
||||
}
|
||||
for i := range v.AnyOf {
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&v.AnyOf[i], s, sPath, vPath.Child("anyOf").Index(i))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&v.AnyOf[i], s, sPath, vPath.Child("anyOf").Index(i), opts)...)
|
||||
}
|
||||
for i := range v.OneOf {
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&v.OneOf[i], s, sPath, vPath.Child("oneOf").Index(i))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&v.OneOf[i], s, sPath, vPath.Child("oneOf").Index(i), opts)...)
|
||||
}
|
||||
|
||||
return allErrs
|
||||
}
|
||||
|
||||
func validateNestedValueValidationCompleteness(v *NestedValueValidation, s *Structural, sPath, vPath *field.Path) field.ErrorList {
|
||||
func validateNestedValueValidationCompleteness(v *NestedValueValidation, s *Structural, sPath, vPath *field.Path, opts ValidationOptions) field.ErrorList {
|
||||
if v == nil {
|
||||
return nil
|
||||
}
|
||||
@ -66,17 +66,29 @@ func validateNestedValueValidationCompleteness(v *NestedValueValidation, s *Stru
|
||||
|
||||
allErrs := field.ErrorList{}
|
||||
|
||||
allErrs = append(allErrs, validateValueValidationCompleteness(&v.ValueValidation, s, sPath, vPath)...)
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(v.Items, s.Items, sPath.Child("items"), vPath.Child("items"))...)
|
||||
allErrs = append(allErrs, validateValueValidationCompleteness(&v.ValueValidation, s, sPath, vPath, opts)...)
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(v.Items, s.Items, sPath.Child("items"), vPath.Child("items"), opts)...)
|
||||
|
||||
var additionalPropertiesSchema *Structural
|
||||
if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil {
|
||||
additionalPropertiesSchema = s.AdditionalProperties.Structural
|
||||
}
|
||||
|
||||
for k, vFld := range v.Properties {
|
||||
if sFld, ok := s.Properties[k]; !ok {
|
||||
allErrs = append(allErrs, field.Required(sPath.Child("properties").Key(k), fmt.Sprintf("because it is defined in %s", vPath.Child("properties").Key(k))))
|
||||
if additionalPropertiesSchema == nil || !opts.AllowValidationPropertiesWithAdditionalProperties {
|
||||
allErrs = append(allErrs, field.Required(sPath.Child("properties").Key(k), fmt.Sprintf("because it is defined in %s", vPath.Child("properties").Key(k))))
|
||||
} else {
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&vFld, additionalPropertiesSchema, sPath.Child("additionalProperties"), vPath.Child("properties").Key(k), opts)...)
|
||||
}
|
||||
} else {
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&vFld, &sFld, sPath.Child("properties").Key(k), vPath.Child("properties").Key(k))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(&vFld, &sFld, sPath.Child("properties").Key(k), vPath.Child("properties").Key(k), opts)...)
|
||||
}
|
||||
}
|
||||
|
||||
// don't check additionalProperties as this is not allowed (and checked during validation)
|
||||
if v.AdditionalProperties != nil && opts.AllowNestedAdditionalProperties {
|
||||
allErrs = append(allErrs, validateNestedValueValidationCompleteness(v.AdditionalProperties, s.AdditionalProperties.Structural, sPath.Child("additionalProperties"), vPath.Child("additionalProperties"), opts)...)
|
||||
}
|
||||
|
||||
return allErrs
|
||||
}
|
||||
|
||||
@ -42,6 +42,22 @@ const (
|
||||
fieldLevel
|
||||
)
|
||||
|
||||
type ValidationOptions struct {
|
||||
// AllowNestedAdditionalProperties allows additionalProperties to be specified in
|
||||
// nested contexts (allOf, anyOf, oneOf, not).
|
||||
AllowNestedAdditionalProperties bool
|
||||
|
||||
// AllowNestedXValidations allows x-kubernetes-validations to be specified in
|
||||
// nested contexts (allOf, anyOf, oneOf, not).
|
||||
AllowNestedXValidations bool
|
||||
|
||||
// AllowValidationPropertiesWithAdditionalProperties allows
|
||||
// value validations on specific properties that are structually
|
||||
// defined by additionalProperties. i.e. additionalProperties in main structural
|
||||
// schema, but properties within an allOf.
|
||||
AllowValidationPropertiesWithAdditionalProperties bool
|
||||
}
|
||||
|
||||
// ValidateStructural checks that s is a structural schema with the invariants:
|
||||
//
|
||||
// * structurality: both `ForbiddenGenerics` and `ForbiddenExtensions` only have zero values, with the two exceptions for IntOrString.
|
||||
@ -61,10 +77,21 @@ const (
|
||||
// * metadata at the root can only restrict the name and generateName, and not be specified at all in nested contexts.
|
||||
// * additionalProperties at the root is not allowed.
|
||||
func ValidateStructural(fldPath *field.Path, s *Structural) field.ErrorList {
|
||||
return ValidateStructuralWithOptions(fldPath, s, ValidationOptions{
|
||||
// This widens the schema for CRDs, so first few releases will still
|
||||
// not admit any. But it can still be used by libraries and
|
||||
// declarative validation for native types
|
||||
AllowNestedAdditionalProperties: false,
|
||||
AllowNestedXValidations: false,
|
||||
AllowValidationPropertiesWithAdditionalProperties: false,
|
||||
})
|
||||
}
|
||||
|
||||
func ValidateStructuralWithOptions(fldPath *field.Path, s *Structural, opts ValidationOptions) field.ErrorList {
|
||||
allErrs := field.ErrorList{}
|
||||
|
||||
allErrs = append(allErrs, validateStructuralInvariants(s, rootLevel, fldPath)...)
|
||||
allErrs = append(allErrs, validateStructuralCompleteness(s, fldPath)...)
|
||||
allErrs = append(allErrs, validateStructuralInvariants(s, rootLevel, fldPath, opts)...)
|
||||
allErrs = append(allErrs, validateStructuralCompleteness(s, fldPath, opts)...)
|
||||
|
||||
// sort error messages. Otherwise, the errors slice will change every time due to
|
||||
// maps in the types and randomized iteration.
|
||||
@ -76,7 +103,7 @@ func ValidateStructural(fldPath *field.Path, s *Structural) field.ErrorList {
|
||||
}
|
||||
|
||||
// validateStructuralInvariants checks the invariants of a structural schema.
|
||||
func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path) field.ErrorList {
|
||||
func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path, opts ValidationOptions) field.ErrorList {
|
||||
if s == nil {
|
||||
return nil
|
||||
}
|
||||
@ -86,10 +113,10 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path)
|
||||
if s.Type == "array" && s.Items == nil {
|
||||
allErrs = append(allErrs, field.Required(fldPath.Child("items"), "must be specified"))
|
||||
}
|
||||
allErrs = append(allErrs, validateStructuralInvariants(s.Items, itemLevel, fldPath.Child("items"))...)
|
||||
allErrs = append(allErrs, validateStructuralInvariants(s.Items, itemLevel, fldPath.Child("items"), opts)...)
|
||||
|
||||
for k, v := range s.Properties {
|
||||
allErrs = append(allErrs, validateStructuralInvariants(&v, fieldLevel, fldPath.Child("properties").Key(k))...)
|
||||
allErrs = append(allErrs, validateStructuralInvariants(&v, fieldLevel, fldPath.Child("properties").Key(k), opts)...)
|
||||
}
|
||||
|
||||
if s.AdditionalProperties != nil {
|
||||
@ -97,7 +124,7 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path)
|
||||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("additionalProperties"), "must not be used at the root"))
|
||||
}
|
||||
if s.AdditionalProperties.Structural != nil {
|
||||
allErrs = append(allErrs, validateStructuralInvariants(s.AdditionalProperties.Structural, fieldLevel, fldPath.Child("additionalProperties"))...)
|
||||
allErrs = append(allErrs, validateStructuralInvariants(s.AdditionalProperties.Structural, fieldLevel, fldPath.Child("additionalProperties"), opts)...)
|
||||
}
|
||||
}
|
||||
|
||||
@ -116,7 +143,7 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path)
|
||||
skipAnyOf := isIntOrStringAnyOfPattern(s)
|
||||
skipFirstAllOfAnyOf := isIntOrStringAllOfPattern(s)
|
||||
|
||||
allErrs = append(allErrs, validateValueValidation(s.ValueValidation, skipAnyOf, skipFirstAllOfAnyOf, lvl, fldPath)...)
|
||||
allErrs = append(allErrs, validateValueValidation(s.ValueValidation, skipAnyOf, skipFirstAllOfAnyOf, lvl, fldPath, opts)...)
|
||||
|
||||
checkMetadata := (lvl == rootLevel) || s.XEmbeddedResource
|
||||
|
||||
@ -235,16 +262,23 @@ func validateExtensions(x *Extensions, fldPath *field.Path) field.ErrorList {
|
||||
}
|
||||
|
||||
// validateValueValidation checks the value validation in a structural schema.
|
||||
func validateValueValidation(v *ValueValidation, skipAnyOf, skipFirstAllOfAnyOf bool, lvl level, fldPath *field.Path) field.ErrorList {
|
||||
func validateValueValidation(v *ValueValidation, skipAnyOf, skipFirstAllOfAnyOf bool, lvl level, fldPath *field.Path, opts ValidationOptions) field.ErrorList {
|
||||
if v == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
allErrs := field.ErrorList{}
|
||||
|
||||
// We still unconditionally forbid XValidations in quantifiers, the only
|
||||
// quantifier that is allowed to have right now is AllOf. This is due to
|
||||
// implementation constraints - the SchemaValidator would need to become
|
||||
// aware of CEL to properly implement the other quantifiers.
|
||||
optsWithCELDisabled := opts
|
||||
optsWithCELDisabled.AllowNestedXValidations = false
|
||||
|
||||
if !skipAnyOf {
|
||||
for i := range v.AnyOf {
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&v.AnyOf[i], false, false, lvl, fldPath.Child("anyOf").Index(i))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&v.AnyOf[i], false, false, lvl, fldPath.Child("anyOf").Index(i), optsWithCELDisabled)...)
|
||||
}
|
||||
}
|
||||
|
||||
@ -253,14 +287,14 @@ func validateValueValidation(v *ValueValidation, skipAnyOf, skipFirstAllOfAnyOf
|
||||
if skipFirstAllOfAnyOf && i == 0 {
|
||||
skipAnyOf = true
|
||||
}
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&v.AllOf[i], skipAnyOf, false, lvl, fldPath.Child("allOf").Index(i))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&v.AllOf[i], skipAnyOf, false, lvl, fldPath.Child("allOf").Index(i), opts)...)
|
||||
}
|
||||
|
||||
for i := range v.OneOf {
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&v.OneOf[i], false, false, lvl, fldPath.Child("oneOf").Index(i))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&v.OneOf[i], false, false, lvl, fldPath.Child("oneOf").Index(i), optsWithCELDisabled)...)
|
||||
}
|
||||
|
||||
allErrs = append(allErrs, validateNestedValueValidation(v.Not, false, false, lvl, fldPath.Child("not"))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(v.Not, false, false, lvl, fldPath.Child("not"), optsWithCELDisabled)...)
|
||||
|
||||
if len(v.Pattern) > 0 {
|
||||
if _, err := regexp.Compile(v.Pattern); err != nil {
|
||||
@ -272,24 +306,28 @@ func validateValueValidation(v *ValueValidation, skipAnyOf, skipFirstAllOfAnyOf
|
||||
}
|
||||
|
||||
// validateNestedValueValidation checks the nested value validation under a logic junctor in a structural schema.
|
||||
func validateNestedValueValidation(v *NestedValueValidation, skipAnyOf, skipAllOfAnyOf bool, lvl level, fldPath *field.Path) field.ErrorList {
|
||||
func validateNestedValueValidation(v *NestedValueValidation, skipAnyOf, skipAllOfAnyOf bool, lvl level, fldPath *field.Path, opts ValidationOptions) field.ErrorList {
|
||||
if v == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
allErrs := field.ErrorList{}
|
||||
|
||||
allErrs = append(allErrs, validateValueValidation(&v.ValueValidation, skipAnyOf, skipAllOfAnyOf, lvl, fldPath)...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(v.Items, false, false, lvl, fldPath.Child("items"))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(v.AdditionalProperties, false, false, lvl, fldPath.Child("additionalProperties"))...)
|
||||
allErrs = append(allErrs, validateValueValidation(&v.ValueValidation, skipAnyOf, skipAllOfAnyOf, lvl, fldPath, opts)...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(v.Items, false, false, lvl, fldPath.Child("items"), opts)...)
|
||||
|
||||
for k, fld := range v.Properties {
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&fld, false, false, fieldLevel, fldPath.Child("properties").Key(k))...)
|
||||
allErrs = append(allErrs, validateNestedValueValidation(&fld, false, false, fieldLevel, fldPath.Child("properties").Key(k), opts)...)
|
||||
}
|
||||
|
||||
if len(v.ForbiddenGenerics.Type) > 0 {
|
||||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("type"), "must be empty to be structural"))
|
||||
}
|
||||
if v.AdditionalProperties != nil && !opts.AllowNestedAdditionalProperties {
|
||||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("additionalProperties"), "must be undefined to be structural"))
|
||||
} else {
|
||||
allErrs = append(allErrs, validateNestedValueValidation(v.AdditionalProperties, false, false, lvl, fldPath.Child("additionalProperties"), opts)...)
|
||||
}
|
||||
if v.ForbiddenGenerics.Default.Object != nil {
|
||||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("default"), "must be undefined to be structural"))
|
||||
}
|
||||
@ -321,6 +359,9 @@ func validateNestedValueValidation(v *NestedValueValidation, skipAnyOf, skipAllO
|
||||
if v.ForbiddenExtensions.XMapType != nil {
|
||||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-map-type"), "must be undefined to be structural"))
|
||||
}
|
||||
if len(v.ValidationExtensions.XValidations) > 0 && !opts.AllowNestedXValidations {
|
||||
allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-validations"), "must be empty to be structural"))
|
||||
}
|
||||
|
||||
// forbid reasoning about metadata because it can lead to metadata restriction we don't want
|
||||
if _, found := v.Properties["metadata"]; found {
|
||||
|
||||
@ -21,6 +21,7 @@ import (
|
||||
"testing"
|
||||
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/utils/ptr"
|
||||
|
||||
fuzz "github.com/google/gofuzz"
|
||||
)
|
||||
@ -146,6 +147,52 @@ func TestValidateStructuralMetadataInvariants(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateStructuralCompleteness(t *testing.T) {
|
||||
sch := Structural{
|
||||
AdditionalProperties: &StructuralOrBool{
|
||||
Structural: &Structural{
|
||||
Generic: Generic{
|
||||
Type: "object",
|
||||
},
|
||||
Properties: map[string]Structural{
|
||||
"bar": {
|
||||
Generic: Generic{
|
||||
Type: "string",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
ValueValidation: &ValueValidation{
|
||||
AllOf: []NestedValueValidation{
|
||||
{
|
||||
Properties: map[string]NestedValueValidation{
|
||||
"foo": {
|
||||
ValueValidation: ValueValidation{
|
||||
MinLength: ptr.To[int64](2),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, allowedPropertiesForAdditionalProperties := range []bool{false, true} {
|
||||
errs := validateStructuralCompleteness(&sch, nil, ValidationOptions{
|
||||
AllowValidationPropertiesWithAdditionalProperties: allowedPropertiesForAdditionalProperties,
|
||||
})
|
||||
if allowedPropertiesForAdditionalProperties {
|
||||
if len(errs) != 0 {
|
||||
t.Errorf("unexpected validation errors for: %#v", sch)
|
||||
}
|
||||
} else if len(errs) == 0 {
|
||||
t.Errorf("expected validation errors for: %#v", sch)
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
func TestValidateNestedValueValidationComplete(t *testing.T) {
|
||||
fuzzer := fuzz.New()
|
||||
fuzzer.Funcs(
|
||||
@ -154,9 +201,9 @@ func TestValidateNestedValueValidationComplete(t *testing.T) {
|
||||
s.Object = float64(42.0)
|
||||
}
|
||||
},
|
||||
func(s **StructuralOrBool, c fuzz.Continue) {
|
||||
func(s **NestedValueValidation, c fuzz.Continue) {
|
||||
if c.RandBool() {
|
||||
*s = &StructuralOrBool{}
|
||||
*s = &NestedValueValidation{}
|
||||
}
|
||||
},
|
||||
)
|
||||
@ -169,7 +216,7 @@ func TestValidateNestedValueValidationComplete(t *testing.T) {
|
||||
x := reflect.ValueOf(&vv.ForbiddenGenerics).Elem()
|
||||
fuzzer.Fuzz(x.Field(i).Addr().Interface())
|
||||
|
||||
errs := validateNestedValueValidation(vv, false, false, fieldLevel, nil)
|
||||
errs := validateNestedValueValidation(vv, false, false, fieldLevel, nil, ValidationOptions{})
|
||||
if len(errs) == 0 && !reflect.DeepEqual(vv.ForbiddenGenerics, Generic{}) {
|
||||
t.Errorf("expected ForbiddenGenerics validation errors for: %#v", vv)
|
||||
}
|
||||
@ -182,9 +229,40 @@ func TestValidateNestedValueValidationComplete(t *testing.T) {
|
||||
x := reflect.ValueOf(&vv.ForbiddenExtensions).Elem()
|
||||
fuzzer.Fuzz(x.Field(i).Addr().Interface())
|
||||
|
||||
errs := validateNestedValueValidation(vv, false, false, fieldLevel, nil)
|
||||
errs := validateNestedValueValidation(vv, false, false, fieldLevel, nil, ValidationOptions{})
|
||||
if len(errs) == 0 && !reflect.DeepEqual(vv.ForbiddenExtensions, Extensions{}) {
|
||||
t.Errorf("expected ForbiddenExtensions validation errors for: %#v", vv)
|
||||
}
|
||||
}
|
||||
|
||||
for _, allowedNestedXValidations := range []bool{false, true} {
|
||||
for _, allowedNestedAdditionalProperties := range []bool{false, true} {
|
||||
opts := ValidationOptions{
|
||||
AllowNestedXValidations: allowedNestedXValidations,
|
||||
AllowNestedAdditionalProperties: allowedNestedAdditionalProperties,
|
||||
}
|
||||
|
||||
vv := NestedValueValidation{}
|
||||
fuzzer.Fuzz(&vv.ValidationExtensions.XValidations)
|
||||
errs := validateNestedValueValidation(&vv, false, false, fieldLevel, nil, opts)
|
||||
if allowedNestedXValidations {
|
||||
if len(errs) != 0 {
|
||||
t.Errorf("unexpected XValidations validation errors for: %#v", vv)
|
||||
}
|
||||
} else if len(errs) == 0 && len(vv.ValidationExtensions.XValidations) != 0 {
|
||||
t.Errorf("expected XValidations validation errors for: %#v", vv)
|
||||
}
|
||||
|
||||
vv = NestedValueValidation{}
|
||||
fuzzer.Fuzz(&vv.AdditionalProperties)
|
||||
errs = validateNestedValueValidation(&vv, false, false, fieldLevel, nil, opts)
|
||||
if allowedNestedAdditionalProperties {
|
||||
if len(errs) != 0 {
|
||||
t.Errorf("unexpected AdditionalProperties validation errors for: %#v", vv)
|
||||
}
|
||||
} else if len(errs) == 0 && vv.AdditionalProperties != nil {
|
||||
t.Errorf("expected AdditionalProperties validation errors for: %#v", vv)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user