Hot fix for panic on schema conversion. (#126167)

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go

@@ -24,6 +24,7 @@ import (
 	"github.com/google/cel-go/cel"
 	"github.com/google/cel-go/checker"
 	"github.com/google/cel-go/common/types"
+	"github.com/pkg/errors"
 
 	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
@@ -125,6 +126,9 @@ func Compile(s *schema.Structural, declType *apiservercel.DeclType, perCallLimit
 	if len(s.XValidations) == 0 {
 		return nil, nil
 	}
+	if declType == nil {
+		return nil, errors.New("Failed to convert to declType for CEL validation rules")
+	}
 	celRules := s.XValidations
 
 	oldSelfEnvSet, optionalOldSelfEnvSet, err := prepareEnvSet(baseEnvSet, declType)

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model/adaptor.go

@@ -62,6 +62,9 @@ func (s *Structural) Pattern() string {
 }
 
 func (s *Structural) Items() common.Schema {
+	if s.Structural.Items == nil {
+		return nil
+	}
 	return &Structural{Structural: s.Structural.Items}
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/model/schemas_test.go

@@ -274,6 +274,7 @@ func TestEstimateMaxLengthJSON(t *testing.T) {
 		Name                string
 		InputSchema         *schema.Structural
 		ExpectedMaxElements int64
+		ExpectNilType       bool
 	}
 	tests := []maxLengthTest{
 		{
@@ -499,13 +500,61 @@ func TestEstimateMaxLengthJSON(t *testing.T) {
 			// so we expect the max length to be exactly equal to the user-supplied one
 			ExpectedMaxElements: 20,
 		},
+		{
+			Name: "Property under array",
+			InputSchema: &schema.Structural{
+				Generic: schema.Generic{
+					Type: "array",
+				},
+				Properties: map[string]schema.Structural{
+					"field": {
+						Generic: schema.Generic{
+							Type:    "string",
+							Default: schema.JSON{Object: "default"},
+						},
+					},
+				},
+			},
+			// Got nil for delType
+			ExpectedMaxElements: 0,
+			ExpectNilType:       true,
+		},
+		{
+			Name: "Items under object",
+			InputSchema: &schema.Structural{
+				Generic: schema.Generic{
+					Type: "object",
+				},
+				Items: &schema.Structural{
+					Generic: schema.Generic{
+						Type: "array",
+					},
+					Properties: map[string]schema.Structural{
+						"field": {
+							Generic: schema.Generic{
+								Type:    "string",
+								Default: schema.JSON{Object: "default"},
+							},
+						},
+					},
+					ValueValidation: &schema.ValueValidation{
+						Required: []string{"field"},
+					},
+				},
+			},
+			// Skip items under object for schema conversion.
+			ExpectedMaxElements: 0,
+		},
 	}
 	for _, testCase := range tests {
 		t.Run(testCase.Name, func(t *testing.T) {
 			decl := SchemaDeclType(testCase.InputSchema, false)
-			if decl.MaxElements != testCase.ExpectedMaxElements {
+			if decl != nil && decl.MaxElements != testCase.ExpectedMaxElements {
 				t.Errorf("wrong maxElements (got %d, expected %d)", decl.MaxElements, testCase.ExpectedMaxElements)
 			}
+			if testCase.ExpectNilType && decl != nil {
+				t.Errorf("expected nil type, got %v", decl)
+			}
 		})
 	}
 }

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

@@ -100,9 +100,15 @@ func validator(validationSchema, nodeSchema *schema.Structural, isResourceRoot b
 	var itemsValidator, additionalPropertiesValidator *Validator
 	var propertiesValidators map[string]Validator
 	var allOfValidators []*Validator
+	var elemType *cel.DeclType
+	if declType != nil {
+		elemType = declType.ElemType
+	} else {
+		elemType = declType
+	}
 
 	if validationSchema.Items != nil && nodeSchema.Items != nil {
-		itemsValidator = validator(validationSchema.Items, nodeSchema.Items, nodeSchema.Items.XEmbeddedResource, declType.ElemType, perCallLimit)
+		itemsValidator = validator(validationSchema.Items, nodeSchema.Items, nodeSchema.Items.XEmbeddedResource, elemType, perCallLimit)
 	}
 
 	if len(validationSchema.Properties) > 0 {
@@ -117,6 +123,9 @@ func validator(validationSchema, nodeSchema *schema.Structural, isResourceRoot b
 
 			var fieldType *cel.DeclType
 			if escapedPropName, ok := cel.Escape(k); ok {
+				if declType == nil {
+					continue
+				}
 				if f, ok := declType.Fields[escapedPropName]; ok {
 					fieldType = f.Type
 				} else {
@@ -138,7 +147,7 @@ func validator(validationSchema, nodeSchema *schema.Structural, isResourceRoot b
 	}
 	if validationSchema.AdditionalProperties != nil && validationSchema.AdditionalProperties.Structural != nil &&
 		nodeSchema.AdditionalProperties != nil && nodeSchema.AdditionalProperties.Structural != nil {
-		additionalPropertiesValidator = validator(validationSchema.AdditionalProperties.Structural, nodeSchema.AdditionalProperties.Structural, nodeSchema.AdditionalProperties.Structural.XEmbeddedResource, declType.ElemType, perCallLimit)
+		additionalPropertiesValidator = validator(validationSchema.AdditionalProperties.Structural, nodeSchema.AdditionalProperties.Structural, nodeSchema.AdditionalProperties.Structural.XEmbeddedResource, elemType, perCallLimit)
 	}
 
 	if validationSchema.ValueValidation != nil && len(validationSchema.ValueValidation.AllOf) > 0 {

test/integration/apiserver/crd_validation_expressions_test.go

@@ -650,6 +650,103 @@ func TestCustomResourceValidatorsWithBlockingErrors(t *testing.T) {
 	})
 }
 
+// TestCustomResourceValidatorsWithSchemaConversion tests CRD replacement with schema conversion issue should not panic.
+func TestCustomResourceValidatorsWithSchemaConversion(t *testing.T) {
+	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)
+	}
+
+	// Create CRD with normal items+array schema
+	structuralWithValidators := crdWithSchema(t, "Structural", structuralSchemaWithItemsUnderArray)
+	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)
+
+	// Create a valid CR instance
+	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{}{
+			"backend": []interface{}{
+				map[string]interface{}{
+					"replicas": 8,
+				},
+			},
+		},
+	}}, metav1.CreateOptions{})
+	if err != nil {
+		t.Errorf("Failed to create custom resource: %v", err)
+	}
+	crd, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), crd.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	structuralSchemaWithItemsUnderObject := crdWithSchema(t, "Structural", structuralSchemaWithItemsUnderObject)
+	structuralSchemaWithItemsUnderObject.SetResourceVersion(crd.GetResourceVersion())
+	// Update CRD with invalid schema items under object
+	crd, err = apiExtensionClient.ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), structuralSchemaWithItemsUnderObject, metav1.UpdateOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Make an unrelated update to the previous persisted CR instance to make sure CRD handler doesn't panic
+	oldCR, err := crClient.Get(context.TODO(), name1, metav1.GetOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	oldCR.Object["metadata"].(map[string]interface{})["labels"] = map[string]interface{}{"key": "value"}
+	_, err = crClient.Update(context.TODO(), oldCR, metav1.UpdateOptions{})
+	if err == nil || !strings.Contains(err.Error(), "rule compiler initialization error: Failed to convert to declType for CEL validation rules") {
+		t.Fatalf("expect error to contain \rule compiler initialization error: Failed to convert to declType for CEL validation rules\" but get: %v", err)
+	}
+	// Create another CR instance with an array and be rejected
+	name2 := names.SimpleNameGenerator.GenerateName("cr-2")
+	_, 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": name2,
+		},
+		"spec": map[string]interface{}{
+			"backend": []interface{}{
+				map[string]interface{}{
+					"replicas": 7,
+				},
+			},
+		},
+	}}, metav1.CreateOptions{})
+	if err == nil || !strings.Contains(err.Error(), "Invalid value: \"array\": spec.backend in body must be of type object: \"array\"") {
+		t.Fatalf("expect error to contain \"Invalid value: \"array\": spec.backend in body must be of type object: \"array\"\" but get: %v", err)
+	}
+	// Delete the CRD
+	err = fixtures.DeleteV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient)
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
 func nonStructuralCrdWithValidations() *apiextensionsv1beta1.CustomResourceDefinition {
 	return &apiextensionsv1beta1.CustomResourceDefinition{
 		ObjectMeta: metav1.ObjectMeta{
@@ -880,6 +977,76 @@ var structuralSchemaWithBlockingErr = []byte(`
   }
 }`)
 
+var structuralSchemaWithItemsUnderArray = []byte(`
+{
+  "openAPIV3Schema": {
+    "description": "CRD with CEL validators",
+    "type": "object",
+    "properties": {
+      "spec": {
+        "type": "object",
+        "properties": {
+          "backend": {
+            "type": "array",
+            "maxItems": 100,
+            "items": {
+              "type": "object",
+              "properties": {
+                "replicas": {
+                  "type": "integer"
+                }
+              },
+              "required": [
+                "replicas"
+              ],
+              "x-kubernetes-validations": [
+                {
+                  "rule": "0 <= self.replicas && self.replicas <= 10"
+                }
+              ]
+            }
+          }
+        }
+      }
+    }
+  }
+}`)
+
+var structuralSchemaWithItemsUnderObject = []byte(`
+{
+  "openAPIV3Schema": {
+    "description": "CRD with CEL validators",
+    "type": "object",
+    "properties": {
+      "spec": {
+        "type": "object",
+        "properties": {
+          "backend": {
+            "type": "object",
+            "maxItems": 100,
+            "items": {
+              "type": "object",
+              "properties": {
+                "replicas": {
+                  "type": "integer"
+                }
+              },
+              "required": [
+                "replicas"
+              ],
+              "x-kubernetes-validations": [
+                {
+                  "rule": "0 <= self.replicas && self.replicas <= 10"
+                }
+              ]
+            }
+          }
+        }
+      }
+    }
+  }
+}`)
+
 var structuralSchemaWithValidMetadataValidators = []byte(`
 {
   "openAPIV3Schema": {