WIP: Update golden test framework

* Move to envtest by default (instead of mock-kubeapiserver)

* Use a more convention env var

* Support object rewriting

Author: justinsb

docs/addon/walkthrough/tests.md

@@ -33,7 +33,7 @@ This means that the output is materialized and checked in to the repo; this
 proves to be very handy for understanding the impact of a change.
 
 There's also a helpful "cheat" function, which rewrite the output when you run
-the tests locally - set the HACK_AUTOFIX_EXPECTED_OUTPUT env var to a non-empty
+the tests locally - set the WRITE_GOLDEN_OUTPUT env var to a non-empty
 string.  This is useful when you have a big set of changes; it's just as easy to
 review the changes yourself in the diff and there's not a ton of value in typing
 them out.
@@ -87,7 +87,7 @@ the env-var cheat code.
    ```bash
    cd pkg/controller/{{operator}}/tests
    touch tests/simple.out.yaml
-   HACK_AUTOFIX_EXPECTED_OUTPUT=1 go test ./...
+   WRITE_GOLDEN_OUTPUT=1 go test ./...
    ```
 
 1. Verify the output is reproducible

pkg/test/golden/validator.go

@@ -27,14 +27,15 @@ import (
 	"strings"
 
 	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/diff"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
-	"sigs.k8s.io/controller-runtime/pkg/scheme"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/mockkubeapiserver"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon"
 	"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/addon/pkg/loaders"
@@ -61,37 +62,59 @@ type T interface {
 	TempDir() string
 }
 
-func NewValidator(t T, b *scheme.Builder) *validator {
+type AddToSchemeFunc func(s *runtime.Scheme) error
+
+func NewValidator(t T, env *envtest.Environment, addToSchemeFuncs ...AddToSchemeFunc) *validator {
+	ctx := context.TODO()
+	ctx, cancel := context.WithCancel(ctx)
+
 	v := &validator{T: t, scheme: runtime.NewScheme()}
-	if err := b.AddToScheme(v.scheme); err != nil {
-		t.Fatalf("error from AddToScheme: %v", err)
+	for _, addToSchemeFunc := range addToSchemeFuncs {
+		if err := addToSchemeFunc(v.scheme); err != nil {
+			t.Fatalf("error from AddToScheme: %v", err)
+		}
 	}
 
 	v.T.Helper()
 	addon.Init()
 	v.findChannelsPath()
 
-	k8s, err := mockkubeapiserver.NewMockKubeAPIServer(":0")
-	if err != nil {
-		t.Fatalf("error building mock kube-apiserver: %v", err)
-	}
+	useEnvtest := true
+	var restConfig *rest.Config
+	if useEnvtest {
+		rc, err := env.Start()
+		if err != nil {
+			t.Fatalf("failed to start envtest kube-apiserver: %v", err)
+		}
+		restConfig = rc
+		t.Cleanup(func() {
+			if err := env.Stop(); err != nil {
+				t.Errorf("error stopping envtest: %v", err)
+			}
+		})
 
-	addr, err := k8s.StartServing()
-	if err != nil {
-		t.Errorf("error starting mock kube-apiserver: %v", err)
-	}
-	v.k8s = k8s
+	} else {
+		k8s, err := mockkubeapiserver.NewMockKubeAPIServer(":0")
+		if err != nil {
+			t.Fatalf("error building mock kube-apiserver: %v", err)
+		}
 
-	t.Cleanup(func() {
-		if err := k8s.Stop(); err != nil {
-			t.Errorf("error stopping mock kube-apiserver: %v", err)
+		addr, err := k8s.StartServing()
+		if err != nil {
+			t.Errorf("error starting mock kube-apiserver: %v", err)
 		}
-	})
+		v.k8s = k8s
 
-	restConfig := &rest.Config{
-		Host: addr.String(),
-	}
+		t.Cleanup(func() {
+			if err := k8s.Stop(); err != nil {
+				t.Errorf("error stopping mock kube-apiserver: %v", err)
+			}
+		})
 
+		restConfig = &rest.Config{
+			Host: addr.String(),
+		}
+	}
 	mgr, err := manager.New(restConfig, manager.Options{
 		Scheme: v.scheme,
 	})
@@ -100,6 +123,25 @@ func NewValidator(t T, b *scheme.Builder) *validator {
 	}
 	v.client = mgr.GetClient()
 	v.mgr = mgr
+
+	managerError := make(chan error)
+	go func() {
+		err := v.mgr.Start(ctx)
+		managerError <- err
+	}()
+
+	// Wait for the manager to start
+	if !v.mgr.GetCache().WaitForCacheSync(ctx) {
+		t.Fatalf("error waiting for cache sync")
+	}
+
+	t.Cleanup(func() {
+		// Cancel the context so the manager exits
+		cancel()
+		// Wait for manager to exit
+		<-managerError
+	})
+
 	return v
 }
 
@@ -186,7 +228,13 @@ func (v *validator) Client() client.Client {
 	return v.client
 }
 
-func (v *validator) Validate(r declarative.Reconciler) {
+type ValidateOptions struct {
+	RewriteObjects func(o *unstructured.Unstructured)
+}
+
+func (v *validator) Validate(r *declarative.Reconciler, options ValidateOptions) {
+	ctx := context.TODO()
+
 	t := v.T
 	t.Helper()
 
@@ -205,8 +253,6 @@ func (v *validator) Validate(r declarative.Reconciler) {
 		t.Fatalf("error reading dir %s: %v", basedir, err)
 	}
 
-	ctx := context.TODO()
-
 	for _, f := range files {
 		p := filepath.Join(basedir, f.Name())
 		t.Logf("Filepath: %s", p)
@@ -217,7 +263,7 @@ func (v *validator) Validate(r declarative.Reconciler) {
 		}
 
 		if strings.HasSuffix(p, "~") {
-			// Ignore editor temp files (for sanity)
+			// Ignore editor temp files (this makes development easier)
 			t.Logf("ignoring editor temp file %s", p)
 			continue
 		}
@@ -258,6 +304,7 @@ func (v *validator) Validate(r declarative.Reconciler) {
 				if err := v.client.Create(ctx, obj); err != nil {
 					t.Errorf("error creating resource in %s: %v", p, err)
 				}
+				t.Logf("created object %v %v/%v", obj.GetObjectKind().GroupVersionKind().Kind, obj.GetNamespace(), obj.GetName())
 				objectsToCleanup = append(objectsToCleanup, obj)
 			}
 		}
@@ -291,6 +338,15 @@ func (v *validator) Validate(r declarative.Reconciler) {
 			continue
 		}
 
+		{
+			obj := cr.(client.Object)
+			if err := v.client.Create(ctx, obj); err != nil {
+				t.Errorf("error creating resource in %s: %v", p, err)
+			}
+			t.Logf("created object %v %v/%v", obj.GetObjectKind().GroupVersionKind().Kind, obj.GetNamespace(), obj.GetName())
+			objectsToCleanup = append(objectsToCleanup, obj)
+		}
+
 		namespace, err := metadataAccessor.Namespace(cr)
 		if err != nil {
 			t.Errorf("error getting namespace in %s: %v", p, err)
@@ -324,6 +380,9 @@ func (v *validator) Validate(r declarative.Reconciler) {
 					b.WriteString("---\n")
 				}
 				u := o.UnstructuredObject()
+				if options.RewriteObjects != nil {
+					options.RewriteObjects(u)
+				}
 				if err := yamlizer.Encode(u, &b); err != nil {
 					t.Fatalf("error encoding to yaml: %v", err)
 				}
@@ -336,15 +395,19 @@ func (v *validator) Validate(r declarative.Reconciler) {
 		{
 			b, err := os.ReadFile(expectedPath)
 			if err != nil {
-				t.Errorf("error reading file %s: %v", expectedPath, err)
-				continue
+				if os.IsNotExist(err) && ShouldWriteGoldenOutput(t) {
+					// We'll create the file below
+				} else {
+					t.Errorf("error reading file %s: %v", expectedPath, err)
+					continue
+				}
 			}
 			expectedYAML = string(b)
 		}
 
 		if actualYAML != expectedYAML {
-			if os.Getenv("HACK_AUTOFIX_EXPECTED_OUTPUT") != "" {
-				t.Logf("HACK_AUTOFIX_EXPECTED_OUTPUT is set; replacing expected output in %s", expectedPath)
+			if ShouldWriteGoldenOutput(t) {
+				t.Logf("WRITE_GOLDEN_OUTPUT is set; replacing expected output in %s", expectedPath)
 				if err := os.WriteFile(expectedPath, []byte(actualYAML), 0644); err != nil {
 					t.Fatalf("error writing expected output to %s: %v", expectedPath, err)
 				}
@@ -357,15 +420,27 @@ func (v *validator) Validate(r declarative.Reconciler) {
 			}
 
 			t.Errorf("unexpected diff between actual and expected YAML. See previous output for details.")
-			t.Logf(`To regenerate the output based on this result, rerun this test with HACK_AUTOFIX_EXPECTED_OUTPUT="true"`)
+			t.Logf(`To regenerate the output based on this result, rerun this test with WRITE_GOLDEN_OUTPUT="true"`)
 		}
 
 		for _, objectToCleanup := range objectsToCleanup {
 			if err := v.client.Delete(ctx, objectToCleanup); err != nil {
 				t.Errorf("error deleting object: %v", err)
 			}
 		}
+
+	}
+}
+
+func ShouldWriteGoldenOutput(t T) bool {
+	if os.Getenv("HACK_AUTOFIX_EXPECTED_OUTPUT") != "" {
+		t.Logf("HACK_AUTOFIX_EXPECTED_OUTPUT is set, please switch to use WRITE_GOLDEN_OUTPUT.  This may be an test failure in future versions.")
+		return true
+	}
+	if os.Getenv("WRITE_GOLDEN_OUTPUT") != "" {
+		return true
 	}
+	return false
 }
 
 func diffFiles(t T, expectedPath, actual string) error {