github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-01 15:58:37 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #108859 from cici37/placeholder

Browse Source 
Return a placeholder error for blocking failures and skip CEL validation.
This commit is contained in:
Kubernetes Prow Robot 2022-03-25 18:07:58 -07:00 committed by GitHub
commit 0a63090445
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 484 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -47,6 +47,9 @@ func required(path ...string) validationMatch {
func invalid(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeInvalid}
}
func invalidtypecode(path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...), errorType: field.ErrorTypeTypeInvalid}
}
func invalidIndex(index int, path ...string) validationMatch {
return validationMatch{path: field.NewPath(path[0], path[1:]...).Index(index), errorType: field.ErrorTypeInvalid}
}
@ -2101,7 +2104,7 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
},
errors: []validationMatch{
invalid("spec", "validation", "openAPIV3Schema", "properties[a]", "default"),
invalid("spec", "validation", "openAPIV3Schema", "properties[c]", "default", "foo"),
invalidtypecode("spec", "validation", "openAPIV3Schema", "properties[c]", "default", "foo"),
invalid("spec", "validation", "openAPIV3Schema", "properties[d]", "default", "bad"),
// we also expected unpruned and valid defaults under x-kubernetes-preserve-unknown-fields. We could be more
// strict here, but want to encourage proper specifications by forbidding other defaults.

32
staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go
View File

@ -79,6 +79,38 @@ func ValidateCustomResource(fldPath *field.Path, customResource interface{}, val
}
allErrs = append(allErrs, field.NotSupported(errPath, err.Value, values))
case openapierrors.TooLongFailCode:
value := interface{}("")
if err.Value != nil {
value = err.Value
}
max := int64(-1)
if i, ok := err.Value.(int64); ok {
max = i
}
allErrs = append(allErrs, field.TooLongMaxLength(errPath, value, int(max)))
case openapierrors.MaxItemsFailCode:
max := int64(-1)
if i, ok := err.Value.(int64); ok {
max = i
}
allErrs = append(allErrs, field.TooMany(errPath, int(max), -1))
case openapierrors.TooManyPropertiesCode:
max := int64(-1)
if i, ok := err.Value.(int64); ok {
max = i
}
allErrs = append(allErrs, field.TooMany(errPath, -1, int(max)))
case openapierrors.InvalidTypeCode:
value := interface{}("")
if err.Value != nil {
value = err.Value
}
allErrs = append(allErrs, field.TypeInvalid(errPath, value, err.Error()))
default:
value := interface{}("")
if err.Value != nil {

49
staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go
View File

@ -26,10 +26,9 @@ import (
"github.com/google/go-cmp/cmp"
utilpointer "k8s.io/utils/pointer"
kjson "sigs.k8s.io/json"
kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
@ -41,6 +40,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/util/json"
"k8s.io/apimachinery/pkg/util/sets"
kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec"
)
// TestRoundTrip checks the conversion to go-openapi types.
@ -535,6 +535,51 @@ func TestValidateCustomResource(t *testing.T) {
}},
},
},
{name: "maxProperties",
schema: apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"fieldX": {
Type: "object",
MaxProperties: utilpointer.Int64(2),
},
},
},
failingObjects: []failingObject{
{object: map[string]interface{}{"fieldX": map[string]interface{}{"a": true, "b": true, "c": true}}, expectErrs: []string{
`fieldX: Too many: must have at most 2 items`,
}},
},
},
{name: "maxItems",
schema: apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"fieldX": {
Type: "array",
MaxItems: utilpointer.Int64(2),
},
},
},
failingObjects: []failingObject{
{object: map[string]interface{}{"fieldX": []interface{}{"a", "b", "c"}}, expectErrs: []string{
`fieldX: Too many: 3: has too many items`,
}},
},
},
{name: "maxLength",
schema: apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"fieldX": {
Type: "string",
MaxLength: utilpointer.Int64(2),
},
},
},
failingObjects: []failingObject{
{object: map[string]interface{}{"fieldX": "abc"}, expectErrs: []string{
`fieldX: Too long: value is too long`,
}},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

8
staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/status_strategy.go
View File

@ -99,8 +99,12 @@ func (a statusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Obj
// validate x-kubernetes-validations rules
if celValidator, ok := a.customResourceStrategy.celValidators[v]; ok {
err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchemas[v], uNew.Object, uOld.Object, cel.RuntimeCELCostBudget)
errs = append(errs, err...)
if has, err := hasBlockingErr(errs); has {
errs = append(errs, err)
} else {
err, _ := celValidator.Validate(ctx, nil, a.customResourceStrategy.structuralSchemas[v], uNew.Object, uOld.Object, cel.RuntimeCELCostBudget)
errs = append(errs, err...)
}
}
return errs
}

26
staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go
View File

@ -174,8 +174,12 @@ func (a customResourceStrategy) Validate(ctx context.Context, obj runtime.Object
// validate x-kubernetes-validations rules
if celValidator, ok := a.celValidators[v]; ok {
err, _ := celValidator.Validate(ctx, nil, a.structuralSchemas[v], u.Object, nil, cel.RuntimeCELCostBudget)
errs = append(errs, err...)
if has, err := hasBlockingErr(errs); has {
errs = append(errs, err)
} else {
err, _ := celValidator.Validate(ctx, nil, a.structuralSchemas[v], u.Object, nil, cel.RuntimeCELCostBudget)
errs = append(errs, err...)
}
}
}
@ -227,8 +231,12 @@ func (a customResourceStrategy) ValidateUpdate(ctx context.Context, obj, old run
// validate x-kubernetes-validations rules
if celValidator, ok := a.celValidators[v]; ok {
err, _ := celValidator.Validate(ctx, nil, a.structuralSchemas[v], uNew.Object, uOld.Object, cel.RuntimeCELCostBudget)
errs = append(errs, err...)
if has, err := hasBlockingErr(errs); has {
errs = append(errs, err)
} else {
err, _ := celValidator.Validate(ctx, nil, a.structuralSchemas[v], uNew.Object, uOld.Object, cel.RuntimeCELCostBudget)
errs = append(errs, err...)
}
}
return errs
@ -271,3 +279,13 @@ func (a customResourceStrategy) MatchCustomResourceDefinitionStorage(label label
GetAttrs: a.GetAttrs,
}
}
// OpenAPIv3 type/maxLength/maxItems/MaxProperties/required/wrong type field validation failures are viewed as blocking err for CEL validation
func hasBlockingErr(errs field.ErrorList) (bool, *field.Error) {
for _, err := range errs {
if err.Type == field.ErrorTypeRequired || err.Type == field.ErrorTypeTooLong || err.Type == field.ErrorTypeTooMany || err.Type == field.ErrorTypeTypeInvalid {
return true, field.Invalid(nil, nil, "some validation rules were not checked because the object was invalid; correct the existing errors to complete validation")
}
}
return false, nil
}

56
staging/src/k8s.io/apimachinery/pkg/util/validation/field/errors.go
View File

@ -42,12 +42,24 @@ func (v *Error) Error() string {
return fmt.Sprintf("%s: %s", v.Field, v.ErrorBody())
}
type omitValueType struct{}
var omitValue = omitValueType{}
// ErrorBody returns the error message without the field name. This is useful
// for building nice-looking higher-level error reporting.
func (v *Error) ErrorBody() string {
var s string
switch v.Type {
case ErrorTypeRequired, ErrorTypeForbidden, ErrorTypeTooLong, ErrorTypeInternal:
switch {
case v.Type == ErrorTypeRequired:
s = v.Type.String()
case v.Type == ErrorTypeForbidden:
s = v.Type.String()
case v.Type == ErrorTypeTooLong:
s = v.Type.String()
case v.Type == ErrorTypeInternal:
s = v.Type.String()
case v.BadValue == omitValue:
s = v.Type.String()
default:
value := v.BadValue
@ -123,6 +135,8 @@ const (
// ErrorTypeInternal is used to report other errors that are not related
// to user input. See InternalError().
ErrorTypeInternal ErrorType = "InternalError"
// ErrorTypeTypeInvalid is for the value did not match the schema type for that field
ErrorTypeTypeInvalid ErrorType = "FieldValueTypeInvalid"
)
// String converts a ErrorType into its corresponding canonical error message.
@ -146,11 +160,18 @@ func (t ErrorType) String() string {
return "Too many"
case ErrorTypeInternal:
return "Internal error"
case ErrorTypeTypeInvalid:
return "Invalid value"
default:
panic(fmt.Sprintf("unrecognized validation error: %q", string(t)))
}
}
// TypeInvalid returns a *Error indicating "type is invalid"
func TypeInvalid(field *Path, value interface{}, detail string) *Error {
return &Error{ErrorTypeTypeInvalid, field.String(), value, detail}
}
// NotFound returns a *Error indicating "value not found". This is
// used to report failure to find a requested value (e.g. looking up an ID).
func NotFound(field *Path, value interface{}) *Error {
@ -207,11 +228,40 @@ func TooLong(field *Path, value interface{}, maxLength int) *Error {
return &Error{ErrorTypeTooLong, field.String(), value, fmt.Sprintf("must have at most %d bytes", maxLength)}
}
// TooLongMaxLength returns a *Error indicating "too long". This is used to
// report that the given value is too long. This is similar to
// Invalid, but the returned error will not include the too-long
// value. If maxLength is negative, no max length will be included in the message.
func TooLongMaxLength(field *Path, value interface{}, maxLength int) *Error {
var msg string
if maxLength >= 0 {
msg = fmt.Sprintf("may not be longer than %d", maxLength)
} else {
msg = "value is too long"
}
return &Error{ErrorTypeTooLong, field.String(), value, msg}
}
// TooMany returns a *Error indicating "too many". This is used to
// report that a given list has too many items. This is similar to TooLong,
// but the returned error indicates quantity instead of length.
func TooMany(field *Path, actualQuantity, maxQuantity int) *Error {
return &Error{ErrorTypeTooMany, field.String(), actualQuantity, fmt.Sprintf("must have at most %d items", maxQuantity)}
var msg string
if maxQuantity >= 0 {
msg = fmt.Sprintf("must have at most %d items", maxQuantity)
} else {
msg = "has too many items"
}
var actual interface{}
if actualQuantity >= 0 {
actual = actualQuantity
} else {
actual = omitValue
}
return &Error{ErrorTypeTooMany, field.String(), actual, msg}
}
// InternalError returns a *Error indicating "internal error". This is used

320
test/integration/apiserver/crd_validation_expressions_test.go
View File

@ -420,6 +420,241 @@ func TestCustomResourceValidators(t *testing.T) {
})
}
// TestCustomResourceValidatorsWithBlockingErrors tests x-kubernetes-validations is skipped when
// blocking errors occurred.
func TestCustomResourceValidatorsWithBlockingErrors(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.CustomResourceValidationExpressions, true)()
server, err := apiservertesting.StartTestServer(t, apiservertesting.NewDefaultTestServerOptions(), nil, framework.SharedEtcd())
if err != nil {
t.Fatal(err)
}
defer server.TearDownFn()
config := server.ClientConfig
apiExtensionClient, err := clientset.NewForConfig(config)
if err != nil {
t.Fatal(err)
}
dynamicClient, err := dynamic.NewForConfig(config)
if err != nil {
t.Fatal(err)
}
t.Run("Structural schema", func(t *testing.T) {
structuralWithValidators := crdWithSchema(t, "Structural", structuralSchemaWithBlockingErr)
crd, err := fixtures.CreateNewV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient, dynamicClient)
if err != nil {
t.Fatal(err)
}
gvr := schema.GroupVersionResource{
Group: crd.Spec.Group,
Version: crd.Spec.Versions[0].Name,
Resource: crd.Spec.Names.Plural,
}
crClient := dynamicClient.Resource(gvr)
t.Run("CRD creation MUST allow data that is valid according to x-kubernetes-validations", func(t *testing.T) {
name1 := names.SimpleNameGenerator.GenerateName("cr-1")
_, err = crClient.Create(context.TODO(), &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name1,
},
"spec": map[string]interface{}{
"x": int64(2),
"y": int64(2),
"limit": int64(123),
},
}}, metav1.CreateOptions{})
if err != nil {
t.Errorf("Failed to create custom resource: %v", err)
}
})
t.Run("custom resource create and update MUST NOT allow data if failed validation", func(t *testing.T) {
name1 := names.SimpleNameGenerator.GenerateName("cr-1")
// a spec create that is invalid MUST fail validation
cr := &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name1,
},
"spec": map[string]interface{}{
"x": int64(-1),
"y": int64(0),
},
}}
// a spec create that is invalid MUST fail validation
_, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
if err == nil {
t.Fatal("Expected create of invalid custom resource to fail")
} else {
if !strings.Contains(err.Error(), "failed rule: self.spec.x + self.spec.y") {
t.Fatalf("Expected error to contain %s but got %v", "failed rule: self.spec.x + self.spec.y", err.Error())
}
}
})
t.Run("custom resource create and update MUST NOT allow data if there is blocking error of MaxLength", func(t *testing.T) {
name2 := names.SimpleNameGenerator.GenerateName("cr-2")
// a spec create that has maxLengh err MUST fail validation
cr := &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name2,
},
"spec": map[string]interface{}{
"x": int64(2),
"y": int64(2),
"extra": "skipValidation?",
"floatMap": map[string]interface{}{
"key1": 0.2,
"key2": 0.3,
},
"limit": nil,
},
}}
_, err := crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
if err == nil || !strings.Contains(err.Error(), "some validation rules were not checked because the object was invalid; correct the existing errors to complete validation") {
t.Fatalf("expect error to contain \"some validation rules were not checked because the object was invalid; correct the existing errors to complete validation\" but get: %v", err)
}
})
t.Run("custom resource create and update MUST NOT allow data if there is blocking error of MaxItems", func(t *testing.T) {
name2 := names.SimpleNameGenerator.GenerateName("cr-2")
// a spec create that has maxItem err MUST fail validation
cr := &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name2,
},
"spec": map[string]interface{}{
"x": int64(2),
"y": int64(2),
"floatMap": map[string]interface{}{
"key1": 0.2,
"key2": 0.3,
},
"assocList": []interface{}{
map[string]interface{}{
"k": "a",
"v": "1",
},
map[string]interface{}{
"a": "a",
},
},
"limit": nil,
},
}}
_, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
if err == nil || !strings.Contains(err.Error(), "some validation rules were not checked because the object was invalid; correct the existing errors to complete validation") {
t.Fatalf("expect error to contain \"some validation rules were not checked because the object was invalid; correct the existing errors to complete validation\" but get: %v", err)
}
})
t.Run("custom resource create and update MUST NOT allow data if there is blocking error of MaxProperties", func(t *testing.T) {
name2 := names.SimpleNameGenerator.GenerateName("cr-2")
// a spec create that has maxItem err MUST fail validation
cr := &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name2,
},
"spec": map[string]interface{}{
"x": int64(2),
"y": int64(2),
"floatMap": map[string]interface{}{
"key1": 0.2,
"key2": 0.3,
"key3": 0.4,
},
"assocList": []interface{}{
map[string]interface{}{
"k": "a",
"v": "1",
},
},
"limit": nil,
},
}}
_, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
if err == nil || !strings.Contains(err.Error(), "some validation rules were not checked because the object was invalid; correct the existing errors to complete validation") {
t.Fatalf("expect error to contain \"some validation rules were not checked because the object was invalid; correct the existing errors to complete validation\" but get: %v", err)
}
})
t.Run("custom resource create and update MUST NOT allow data if there is blocking error of missing required field", func(t *testing.T) {
name2 := names.SimpleNameGenerator.GenerateName("cr-2")
// a spec create that has required err MUST fail validation
cr := &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name2,
},
"spec": map[string]interface{}{
"x": int64(2),
"y": int64(2),
"floatMap": map[string]interface{}{
"key1": 0.2,
"key2": 0.3,
},
"assocList": []interface{}{
map[string]interface{}{
"k": "1",
},
},
"limit": nil,
},
}}
_, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
if err == nil || !strings.Contains(err.Error(), "some validation rules were not checked because the object was invalid; correct the existing errors to complete validation") {
t.Fatalf("expect error to contain \"some validation rules were not checked because the object was invalid; correct the existing errors to complete validation\" but get: %v", err)
}
})
t.Run("custom resource create and update MUST NOT allow data if there is blocking error of type", func(t *testing.T) {
name2 := names.SimpleNameGenerator.GenerateName("cr-2")
// a spec create that has required err MUST fail validation
cr := &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name2,
},
"spec": map[string]interface{}{
"x": int64(2),
"y": int64(2),
"floatMap": map[string]interface{}{
"key1": 0.2,
"key2": 0.3,
},
"assocList": []interface{}{
map[string]interface{}{
"k": "a",
"v": true,
},
},
"limit": nil,
},
}}
_, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
if err == nil || !strings.Contains(err.Error(), "some validation rules were not checked because the object was invalid; correct the existing errors to complete validation") {
t.Fatalf("expect error to contain \"some validation rules were not checked because the object was invalid; correct the existing errors to complete validation\" but get: %v", err)
}
})
})
}
func nonStructuralCrdWithValidations() *apiextensionsv1beta1.CustomResourceDefinition {
return &apiextensionsv1beta1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
@ -556,6 +791,91 @@ var structuralSchemaWithValidators = []byte(`
}
}`)
var structuralSchemaWithBlockingErr = []byte(`
{
"openAPIV3Schema": {
"description": "CRD with CEL validators",
"type": "object",
"x-kubernetes-validations": [
{
"rule": "self.spec.x + self.spec.y >= (has(self.status) ? self.status.z : 0)"
}
],
"properties": {
"spec": {
"type": "object",
"properties": {
"x": {
"type": "integer",
"default": 0
},
"y": {
"type": "integer",
"default": 0
},
"extra": {
"type": "string",
"maxLength": 0,
"x-kubernetes-validations": [
{
"rule": "self.startsWith('anything')"
}
]
},
"floatMap": {
"type": "object",
"maxProperties": 2,
"additionalProperties": { "type": "number" },
"x-kubernetes-validations": [
{
"rule": "self.all(k, self[k] >= 0.2)"
}
]
},
"assocList": {
"type": "array",
"maxItems": 1,
"items": {
"type": "object",
"maxItems": 1,
"properties": {
"k": { "type": "string" },
"v": { "type": "string" }
},
"required": ["k", "v"]
},
"x-kubernetes-list-type": "map",
"x-kubernetes-list-map-keys": ["k"],
"x-kubernetes-validations": [
{
"rule": "self.exists(e, e.k == 'a' && e.v == '1')"
}
]
},
"limit": {
"nullable": true,
"x-kubernetes-validations": [
{
"rule": "type(self) == int && self == 123"
}
],
"x-kubernetes-int-or-string": true
}
}
},
"status": {
"type": "object",
"properties": {
"z": {
"type": "integer",
"default": 0
}
}
}
}
}
}`)
var structuralSchemaWithValidMetadataValidators = []byte(`
{
"openAPIV3Schema": {