Allow and enforce CEL CRD validation rules that reference oldSelf.

Co-authored-by: Joe Betz <jpbetz@google.com>
2022-02-08 15:22:01 -05:00
parent 11b3a18cca
commit fe38a414f8
10 changed files with 845 additions and 86 deletions
--- a/test/integration/apiserver/crd_validation_expressions_test.go
+++ b/test/integration/apiserver/crd_validation_expressions_test.go
@@ -300,6 +300,78 @@ func TestCustomResourceValidators(t *testing.T) {
 			t.Error("Unexpected error creating custom resource but metadata validation rule")
 		}
 	})
+	t.Run("Schema with valid transition rule", func(t *testing.T) {
+		structuralWithValidators := crdWithSchema(t, "Structural", structuralSchemaWithValidTransitionRule)
+		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("custom resource update MUST pass if a x-kubernetes-validations rule contains a valid transition rule", func(t *testing.T) {
+			name1 := names.SimpleNameGenerator.GenerateName("cr-1")
+			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{}{
+					"someImmutableThing": "original",
+					"somethingElse":      "original",
+				},
+			}}
+			cr, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
+			if err != nil {
+				t.Fatalf("Unexpected error creating custom resource: %v", err)
+			}
+			cr.Object["spec"].(map[string]interface{})["somethingElse"] = "new value"
+			_, err = crClient.Update(context.TODO(), cr, metav1.UpdateOptions{})
+			if err != nil {
+				t.Fatalf("Unexpected error updating custom resource: %v", err)
+			}
+		})
+		t.Run("custom resource update MUST fail if a x-kubernetes-validations rule contains an invalid transition rule", func(t *testing.T) {
+			name1 := names.SimpleNameGenerator.GenerateName("cr-1")
+			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{}{
+					"someImmutableThing": "original",
+					"somethingElse":      "original",
+				},
+			}}
+			cr, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
+			if err != nil {
+				t.Fatalf("Unexpected error creating custom resource: %v", err)
+			}
+			cr.Object["spec"].(map[string]interface{})["someImmutableThing"] = "new value"
+			_, err = crClient.Update(context.TODO(), cr, metav1.UpdateOptions{})
+			if err == nil {
+				t.Fatalf("Expected error updating custom resource: %v", err)
+			} else if !strings.Contains(err.Error(), "failed rule: self.someImmutableThing == oldSelf.someImmutableThing") {
+				t.Errorf("Expected error to contain %s but got %v", "failed rule: self.someImmutableThing == oldSelf.someImmutableThing", err.Error())
+			}
+		})
+	})
+
+	t.Run("CRD creation MUST fail if a x-kubernetes-validations rule contains invalid transition rule", func(t *testing.T) {
+		structuralWithValidators := crdWithSchema(t, "InvalidStructuralMetadata", structuralSchemaWithInvalidTransitionRule)
+		_, err := fixtures.CreateNewV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient, dynamicClient)
+		if err == nil {
+			t.Error("Expected error creating custom resource but got none")
+		} else if !strings.Contains(err.Error(), "oldSelf cannot be used on the uncorrelatable portion of the schema") {
+			t.Errorf("Expected error to contain %s but got %v", "oldSelf cannot be used on the uncorrelatable portion of the schema", err.Error())
+		}
+	})
 }

 func nonStructuralCrdWithValidations() *apiextensionsv1beta1.CustomResourceDefinition {
@@ -495,3 +567,59 @@ var structuralSchemaWithInvalidMetadataValidators = []byte(`
    }
  }
 }`)
+
+var structuralSchemaWithValidTransitionRule = []byte(`
+{
+  "openAPIV3Schema": {
+    "description": "CRD with CEL validators",
+    "type": "object",
+    "properties": {
+      "spec": {
+        "type": "object",
+        "properties": {
+		  "someImmutableThing": { "type": "string" },
+          "somethingElse": { "type": "string" }
+	    },
+		"x-kubernetes-validations": [
+		  {
+			"rule": "self.someImmutableThing == oldSelf.someImmutableThing"
+		  }
+		]
+      },
+      "status": {
+        "type": "object",
+        "properties": {}
+	  }
+    }
+  }
+}`)
+
+var structuralSchemaWithInvalidTransitionRule = []byte(`
+{
+  "openAPIV3Schema": {
+    "description": "CRD with CEL validators",
+    "type": "object",
+    "properties": {
+      "spec": {
+        "type": "object",
+        "properties": {
+		  "list": {
+            "type": "array",
+            "items": {
+              "type": "string",
+		      "x-kubernetes-validations": [
+		        {
+			      "rule": "self == oldSelf"
+                }
+		      ]
+            }
+          }
+	    }
+      },
+      "status": {
+        "type": "object",
+        "properties": {}
+	  }
+    }
+  }
+}`)