Return a placeholder error for blocking failure before CEL validation.

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

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

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

@@ -79,6 +79,27 @@ 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
+				}
+				allErrs = append(allErrs, field.TooLongFail(errPath, value, err.Error()))
+
+			case openapierrors.TooManyPropertiesCode, openapierrors.MaxItemsFailCode:
+				value := interface{}("")
+				if err.Value != nil {
+					value = err.Value
+				}
+				allErrs = append(allErrs, field.TooManyFail(errPath, value, err.Error()))
+
+			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 {

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

@@ -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)
+		if has, err := hasBlockingErr(errs); has {
-		errs = append(errs, err...)
+			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
 }

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

@@ -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)
+			if has, err := hasBlockingErr(errs); has {
-			errs = append(errs, err...)
+				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)
+		if has, err := hasBlockingErr(errs); has {
-		errs = append(errs, err...)
+			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
+}

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

@@ -123,6 +123,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 +148,28 @@ 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)))
 	}
 }
 
+// TooLongFail returns a *Error indicating "MaxLength exceed".
+func TooLongFail(field *Path, value interface{}, detail string) *Error {
+	return &Error{ErrorTypeTooLong, field.String(), value, detail}
+}
+
+// TooManyFail returns a *Error indicating "MaxItems/MaxProperties exceed"
+func TooManyFail(field *Path, value interface{}, detail string) *Error {
+	return &Error{ErrorTypeTooMany, field.String(), value, detail}
+}
+
+// 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 {

test/integration/apiserver/crd_validation_expressions_test.go

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