Add initialCRDPatches option to onUpdate tests

Author: JoelSpeed

tests/README.md

@@ -237,3 +237,35 @@ A full example is included below:
         immutableField: bar
     expectedStatusError: "status.immutableField: Invalid value: \"string\": immutableField is immutable"
 ```
+
+#### Testing validation ratcheting
+
+Kubernetes now supports [validation ratcheting][validation-ratcheting].
+This means that we can now evolve the validations of APIs over time, without immediately breaking stored APIs.
+Note, any changes to validations that may be breaking still need careful consideration, this is not a 'get out of jail free' card.
+
+Ratcheting can be tested using `onUpdate` tests and the `initialCRDPatches` option.
+`initialCRDPatches` is a list of JSON Object Patches ([RFC6902][rfc6902]) that will be applied temporarily to the CRD prior to the
+initial object being created.
+This allows you to revert a newer change to an API validation, apply an object that would be invalid with the newer validation,
+and then test how the object behaves with the new, current schema.
+
+For example, if a field does not include a maximum, and we decide to enforce a new maximum, a patch such like below could be used
+to remove the maximum temporarily to then create an object which exceeds the new maximum.
+
+```
+onUpdate:
+- name: ...
+  initialCRDPatches:
+      - op: remove
+        path: /spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/fieldWithNewMaxLength/maximum # Maximum was not originally set
+  ...
+```
+
+Once the patch is applied, three tests should be added for each ratcheting validation:
+1. Test that other values can be updated, while the invalid value is persisted
+1. Test that the value itself cannot be updated to an alternative, newly invalid value (e.g. making the value longer in this case)
+1. Test that the value can be updated to a valid value (e.g. in this case, a value shorted than the new maximum)
+
+[validation-ratcheting]: https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#validation-ratcheting
+[rfc6902]: https://datatracker.ietf.org/doc/html/rfc6902

tests/generator.go

@@ -1,13 +1,15 @@
 package tests
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	yamlpatch "github.com/vmware-archive/yaml-patch"
 
 	"github.com/ghodss/yaml"
 
@@ -139,7 +141,7 @@ func GenerateTestSuite(suiteSpec SuiteSpec) {
 			})
 
 			generateOnCreateTable(suiteSpec.Tests.OnCreate)
-			generateOnUpdateTable(suiteSpec.Tests.OnUpdate)
+			generateOnUpdateTable(suiteSpec.Tests.OnUpdate, crdFilename)
 		})
 	}
 }
@@ -200,9 +202,10 @@ func generateOnCreateTable(onCreateTests []OnCreateTestSpec) {
 
 // generateOnUpdateTable generates a table of tests from the defined OnUpdate tests
 // within the test suite test spec.
-func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec) {
+func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec, crdFileName string) {
 	type onUpdateTableInput struct {
 		featureGate         string
+		crdPatches          []Patch
 		initial             []byte
 		updated             []byte
 		expected            []byte
@@ -211,6 +214,24 @@ func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec) {
 	}
 
 	var assertOnUpdate interface{} = func(in onUpdateTableInput) {
+		var originalCRDObjectKey client.ObjectKey
+		var originalCRDSpec apiextensionsv1.CustomResourceDefinitionSpec
+
+		if len(in.crdPatches) > 0 {
+			patchedCRD, err := getPatchedCRD(crdFileName, in.crdPatches)
+			Expect(err).ToNot(HaveOccurred(), "could not load patched crd")
+
+			originalCRDObjectKey = objectKey(patchedCRD)
+
+			originalCRD := &apiextensionsv1.CustomResourceDefinition{}
+			Expect(k8sClient.Get(ctx, originalCRDObjectKey, originalCRD))
+
+			originalCRDSpec = *originalCRD.Spec.DeepCopy()
+			originalCRD.Spec = patchedCRD.Spec
+
+			Expect(k8sClient.Update(ctx, originalCRD)).To(Succeed(), "failed updating patched CRD schema")
+		}
+
 		initialObj, err := newUnstructuredFrom(in.initial)
 		Expect(err).ToNot(HaveOccurred(), "initial data should be a valid Kubernetes YAML resource")
 
@@ -227,6 +248,15 @@ func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec) {
 			Expect(k8sClient.Status().Update(ctx, initialObj)).ToNot(HaveOccurred(), "initial object status should update successfully")
 		}
 
+		if len(in.crdPatches) > 0 {
+			originalCRD := &apiextensionsv1.CustomResourceDefinition{}
+			Expect(k8sClient.Get(ctx, originalCRDObjectKey, originalCRD))
+
+			originalCRD.Spec = originalCRDSpec
+
+			Expect(k8sClient.Update(ctx, originalCRD)).To(Succeed())
+		}
+
 		// Fetch the object we just created from the API.
 		gotObj := newEmptyUnstructuredFrom(initialObj)
 		Expect(k8sClient.Get(ctx, objectKey(initialObj), gotObj))
@@ -250,7 +280,7 @@ func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec) {
 			Expect(err).To(MatchError(ContainSubstring(in.expectedError)))
 			return
 		}
-		Expect(err).ToNot(HaveOccurred())
+		Expect(err).ToNot(HaveOccurred(), "unexpected error updating spec")
 
 		if updatedObjStatus != nil {
 			Expect(unstructured.SetNestedField(updatedObj.Object, updatedObjStatus, "status")).To(Succeed(), "should be able to restore updated status")
@@ -260,7 +290,7 @@ func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec) {
 				Expect(err).To(MatchError(ContainSubstring(in.expectedStatusError)))
 				return
 			}
-			Expect(err).ToNot(HaveOccurred())
+			Expect(err).ToNot(HaveOccurred(), "unexpected error updating status")
 		}
 
 		Expect(k8sClient.Get(ctx, objectKey(initialObj), gotObj))
@@ -282,6 +312,7 @@ func generateOnUpdateTable(onUpdateTests []OnUpdateTestSpec) {
 	// Convert the test specs into table entries
 	for _, testEntry := range onUpdateTests {
 		tableEntries = append(tableEntries, Entry(testEntry.Name, onUpdateTableInput{
+			crdPatches:          testEntry.InitialCRDPatches,
 			initial:             []byte(testEntry.Initial),
 			updated:             []byte(testEntry.Updated),
 			expected:            []byte(testEntry.Expected),
@@ -453,3 +484,35 @@ func getSuiteSpecTestVersion(suiteSpec SuiteSpec) (string, error) {
 
 	return version, nil
 }
+
+func getPatchedCRD(crdFileName string, patches []Patch) (*apiextensionsv1.CustomResourceDefinition, error) {
+	patch := yamlpatch.Patch{}
+
+	for _, p := range patches {
+		patch = append(patch, yamlpatch.Operation{
+			Op:    yamlpatch.Op(p.Op),
+			Path:  yamlpatch.OpPath(p.Path),
+			Value: yamlpatch.NewNode(p.Value),
+		})
+	}
+
+	baseDoc, err := os.ReadFile(crdFileName)
+	if err != nil {
+		return nil, fmt.Errorf("could not read file %q: %w", crdFileName, err)
+	}
+
+	patchedDoc, err := patch.Apply(baseDoc)
+	if err != nil {
+		return nil, fmt.Errorf("could not apply patch: %w", err)
+	}
+
+	placeholderWrapper := yamlpatch.NewPlaceholderWrapper("{{", "}}")
+	patchedData := bytes.NewBuffer(placeholderWrapper.Unwrap(patchedDoc))
+
+	crd := &apiextensionsv1.CustomResourceDefinition{}
+	if err := yaml.Unmarshal(patchedData.Bytes(), crd); err != nil {
+		return nil, fmt.Errorf("could not unmarshal CRD: %w", err)
+	}
+
+	return crd, nil
+}

tests/types.go

@@ -67,6 +67,13 @@ type OnUpdateTestSpec struct {
 	// Name is the name of this test case.
 	Name string `json:"name"`
 
+	// InitialCRDPatches is a list of YAML patches to apply to the CRD before applying
+	// the initial version of the resource.
+	// Once the initial version has been applied, the CRD will be restored to its
+	// original state before the updated object is applied.
+	// This can be used to test ratcheting validation of CRD schema changes over time.
+	InitialCRDPatches []Patch `json:"initialCRDPatches"`
+
 	// Initial is a literal string containing the initial YAML content from which to
 	// create the resource.
 	// Note `apiVersion` and `kind` fields are required though `metadata` can be omitted.
@@ -93,3 +100,18 @@ type OnUpdateTestSpec struct {
 	// Typically this will vary in `spec` only test to test.
 	Expected string `json:"expected"`
 }
+
+// Patch represents a single operation to be applied to a YAML document.
+// It follows the JSON Patch format as defined in RFC 6902.
+// Each patch operation is atomic and can be used to modify the structure
+// or content of a YAML document.
+type Patch struct {
+	// Op is the operation to be performed. Common operations include "add", "remove", "replace", "move", "copy", and "test".
+	Op string `json:"op"`
+
+	// Path is a JSON Pointer that indicates the location in the YAML document where the operation is to be performed.
+	Path string `json:"path"`
+
+	// Value is the value to be used within the operation. This field is required for operations like "add" and "replace".
+	Value *interface{} `json:"value"`
+}