Validate reconcilers at test-time w/ helpful error messages

[mamachanko]

When a (sub)reconciler is invalid, because, say, it missing ChildObjectManager, then its (sub)reconciler test suite fails with a panic. Tracing the panic back to the missing ChildObjectManager is possible but tedious.

❯ go test ./controllers -v -run TestApplyServiceBindingSecret
=== RUN   TestApplyServiceBindingSecret
=== RUN   TestApplyServiceBindingSecret/creates_service_binding_secret_with_config_properties
--- FAIL: TestApplyServiceBindingSecret (0.00s)
    --- FAIL: TestApplyServiceBindingSecret/creates_service_binding_secret_with_config_properties (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x4ec7ce2]

goroutine 70 [running]:
testing.tRunner.func1.2({0x5418380, 0x661f600})
        /usr/local/Cellar/go/1.23.5/libexec/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
        /usr/local/Cellar/go/1.23.5/libexec/src/testing/testing.go:1635 +0x35e
panic({0x5418380?, 0x661f600?})
        /usr/local/Cellar/go/1.23.5/libexec/src/runtime/panic.go:785 +0x132
reconciler.io/runtime/reconcilers.(*ChildReconciler[...]).reconcile(0x5701040, {0x56e4490, 0xc000137b30}, 0xc00002c1a0)
        /Users/*****/workspace/reconcilerio-runtime/reconcilers/child.go:314 +0x582
reconciler.io/runtime/reconcilers.(*ChildReconciler[...]).Reconcile(0x5701040, {0x56e4490, 0xc000137ad0}, 0xc00002c1a0)
        /Users/*****/workspace/reconcilerio-runtime/reconcilers/child.go:229 +0x4f2
reconciler.io/runtime/reconcilers.(*Advice[...]).init.func1.2(0x5418b00?, {0x56cbd60?, 0xc000285810?})
        /Users/*****/workspace/reconcilerio-runtime/reconcilers/advice.go:92 +0x34
reconciler.io/runtime/reconcilers.(*Advice[...]).Reconcile(0x55baf80, {0x56e4490?, 0xc00002a6f0}, 0xc00002c1a0)
        /Users/*****/workspace/reconcilerio-runtime/reconcilers/advice.go:150 +0x19e
reconciler.io/runtime/testing.(*SubReconcilerTestCase[...]).Run.func3(0x56e4490?, {0x56cbd88?, 0xc00050ebe0?}, 0xc000582c40?, {0x56e4490?, 0xc00002a6f0?}, 0x0?)
        /Users/*****/workspace/reconcilerio-runtime/testing/subreconciler.go:287 +0x9b
reconciler.io/runtime/testing.(*SubReconcilerTestCase[...]).Run(0x56eea40, 0xc0002ab040, 0xc000284af0, 0x56bbaf8)
        /Users/*****/workspace/reconcilerio-runtime/testing/subreconciler.go:288 +0x15d2
reconciler.io/runtime/testing.SubReconcilerTestSuite[...].Run.func1()
        /Users/*****/workspace/reconcilerio-runtime/testing/subreconciler.go:349 +0x5f
testing.tRunner(0xc0002ab040, 0xc000453e60)
        /usr/local/Cellar/go/1.23.5/libexec/src/testing/testing.go:1690 +0xf4
created by testing.(*T).Run in goroutine 69
        /usr/local/Cellar/go/1.23.5/libexec/src/testing/testing.go:1743 +0x390
FAIL    ***************/controller/controllers  1.009s
FAIL

It would be nice if invalid (sub)reconcilers would be reported with the same error message one sees when running the controller.

Since validate is not exported it's not easy for reconciler authors to validate their reconcilers in their tests.

[scothis]

Wonder if we should call validate() from within init() that way the validation would apply to the first Reconcile() call rather than just setup (which is hard to unit test). The downside is that the logger wouldn't be configured for the reconciler, but I don't see a lot of logging happening within the validation method.

I'd also be ok exporting the validate() method, but I don't think it should be part of the SubReconciler interface. SubReconcilerTestCase could sniff for an exported Validate() method and call it.

Thoughts?

---

edit: validate() returns an error which is annoying to handle within init() as it's work is wrapped in a sync.Once. Because of that and the logger issue, I opted to export the validate() method and call it explicitly within each (Sub)ReconcilerTestCase.