feat(cli): add the ocf scaffolding CLI for custom-resource wrappers - #166
Conversation
…assing Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
go test exits 0 for a package with no test files and for a test file with zero Test functions, so the gate could pass vacuously if a template regression gutted or omitted builder_test.go. Switch to `go test -json` and require at least one passing test per generated package, and assert Generate's returned file list matches the expected file set instead of discarding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ntities --version was only validated on the derivation path, and --group was never validated at all. Both land inside a Go string literal in the generated builder.go and builder_test.go, so "--version 1.0" produced a silently wrong identity that still compiled, and a --group containing a double quote could close the literal and inject an arbitrary expression into the generated file. Resolve now rejects a --group that is not a DNS subdomain the way Kubernetes defines API groups, keeping "" valid for the core API group, and checks an explicit --version against the same pattern the derivation path uses. The templates additionally emit the identity format string and the identity assertions through printf "%q", so no input can break out of the literal even if a future validation gap appears. Rendered output for well-formed input is unchanged, so no golden moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cations .golangci.yml set no run.build-tags, so internal/scaffold/gate_test.go was invisible to make lint and two noctx violations went unreported. Adding the scaffold build tag to the run section keeps the file linted from now on. The gate also shelled out to the go tool with no context, so a hung go tool in CI would hang until the job timeout instead of failing with a diagnostic. runGo and runGoTestJSON now use exec.CommandContext with the test's context. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
resource.go.tmpl branched on the variant name to build the "It implements the following component interfaces" list, contradicting VariantSpec's own contract that templates read the spec instead of branching on the name. A fifth variant would have needed edits in two places, and forgetting the template would silently produce a Resource whose GoDoc omits its lifecycle interface. The variant-specific bullets now live in VariantSpec.LifecycleInterfaces and the template ranges over them. Rendered output is byte-identical for every golden case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Generate created the output directory 0750 and wrote the four files 0600. That is the wrong default for source a user will edit, commit and share; the repo's own golden writer already uses 0644. Use 0755 and 0644. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sage testDirDisplay formats the summary header line as well as the go test hint, so the name was narrower than the job; it is now displayDir, with a doc comment that says so. SilenceUsage on the wrapper subcommand was dead: cobra consults only the executed command and the root, and the root already sets it. Error paths still print a bare error with no usage dump. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ports --type's import path was never validated. splitType checked only the type name and deriveAlias only sanitized the alias, so anything before the last dot landed verbatim inside the import literal of all four generated files. A --type whose path carried a double quote and a newline closed the literal and injected an extra import, for example a blank "os" import, into the generated package. The result was syntactically valid, so go/format accepted it and the package compiled and ran the injected package's init. splitType now rejects a path that is not shaped like a Go import path: one or more slash-separated elements, each non-empty, built only from the ASCII characters the module system permits in a path element, and neither starting nor ending in a dot. The templates additionally emit the import literal through printf "%q", the same boundary quoting the identity format string already uses, so no input can break out of the literal even if a future validation gap appears. Rendered output for well-formed input is unchanged, so no golden moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds the ocf scaffolding CLI (under cmd/ocf) plus an internal/scaffold templating engine so framework consumers can generate custom-resource wrapper packages (builder/resource/mutator + tests) that match the framework version the CLI was built from, with CI enforcing template drift via a new scaffold gate.
Changes:
- Add
ocf scaffold wrapperandocf version, backed by embedded templates and validated option resolution. - Add a scaffold CI gate (
make test-scaffold) that generates wrappers into a temp module and runsgo test -json, asserting tests actually ran per generated package. - Document the CLI (new
docs/cli.md) and link it from README, mkdocs nav, and the custom-resource wrapper docs.
Reviewed changes
Copilot reviewed 48 out of 49 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a scaffolding section and links to CLI docs. |
| mkdocs.yml | Adds CLI page to the Guides nav. |
| Makefile | Adds test-scaffold and wires it into make all. |
| internal/scaffold/variant.go | Defines wrapper variants and their generic-layer wiring. |
| internal/scaffold/templates/resource.go.tmpl | Template for generated Resource wrapper. |
| internal/scaffold/templates/mutator.go.tmpl | Template for generated Mutator with plan/apply semantics. |
| internal/scaffold/templates/builder.go.tmpl | Template for generated Builder and default handlers. |
| internal/scaffold/templates/builder_test.go.tmpl | Template for generated builder tests. |
| internal/scaffold/render.go | Renders embedded templates and gofmts output. |
| internal/scaffold/render_test.go | Golden tests + parsability tests for template rendering. |
| internal/scaffold/options.go | Validates/derives scaffold flags into TemplateData. |
| internal/scaffold/options_test.go | Table tests for validation and derivation rules. |
| internal/scaffold/data.go | TemplateData helpers (identity formatting, type names). |
| internal/scaffold/generate.go | Writes rendered files to disk with safe directory semantics. |
| internal/scaffold/generate_test.go | Tests write behavior, --force, and directory checks. |
| internal/scaffold/gate_test.go | Scaffold gate: generate into temp module, run go test -json, assert tests ran. |
| internal/scaffold/testdata/golden/static/builder.go.golden | Golden output for static builder. |
| internal/scaffold/testdata/golden/static/builder_test.go.golden | Golden output for static builder tests. |
| internal/scaffold/testdata/golden/static/mutator.go.golden | Golden output for static mutator. |
| internal/scaffold/testdata/golden/static/resource.go.golden | Golden output for static resource. |
| internal/scaffold/testdata/golden/workload/builder.go.golden | Golden output for workload builder. |
| internal/scaffold/testdata/golden/workload/builder_test.go.golden | Golden output for workload builder tests. |
| internal/scaffold/testdata/golden/workload/mutator.go.golden | Golden output for workload mutator. |
| internal/scaffold/testdata/golden/workload/resource.go.golden | Golden output for workload resource. |
| internal/scaffold/testdata/golden/task/builder.go.golden | Golden output for task builder. |
| internal/scaffold/testdata/golden/task/builder_test.go.golden | Golden output for task builder tests. |
| internal/scaffold/testdata/golden/task/mutator.go.golden | Golden output for task mutator. |
| internal/scaffold/testdata/golden/task/resource.go.golden | Golden output for task resource. |
| internal/scaffold/testdata/golden/integration/builder.go.golden | Golden output for integration builder. |
| internal/scaffold/testdata/golden/integration/builder_test.go.golden | Golden output for integration builder tests. |
| internal/scaffold/testdata/golden/integration/mutator.go.golden | Golden output for integration mutator. |
| internal/scaffold/testdata/golden/integration/resource.go.golden | Golden output for integration resource. |
| internal/scaffold/testdata/golden/static-cluster-scoped/builder.go.golden | Golden output for cluster-scoped static builder. |
| internal/scaffold/testdata/golden/static-cluster-scoped/builder_test.go.golden | Golden output for cluster-scoped static builder tests. |
| internal/scaffold/testdata/golden/static-cluster-scoped/mutator.go.golden | Golden output for cluster-scoped static mutator. |
| internal/scaffold/testdata/golden/static-cluster-scoped/resource.go.golden | Golden output for cluster-scoped static resource. |
| cmd/ocf/main.go | CLI entrypoint. |
| cmd/ocf/root.go | Root command wiring for ocf. |
| cmd/ocf/scaffold.go | Implements ocf scaffold wrapper flags + generation + summary. |
| cmd/ocf/version.go | Implements ocf version using build info. |
| cmd/ocf/cli_test.go | CLI behavior tests (generation, defaults, errors, version parsing). |
| docs/custom-resource.md | Adds a tip pointing users to generate wrappers via the CLI. |
| docs/cli.md | New CLI documentation page. |
| .github/workflows/test.yml | Runs make test-scaffold in CI. |
| .golangci.yml | Enables scaffold build tag for linting. |
| go.mod | Adds cobra dependency for the CLI. |
| go.sum | Updates sums for new/updated dependencies. |
| .github/copilot-instructions.md | Updates contributor guidance to include CLI + scaffold internals. |
| .ai/base.md | Mirrors instruction updates for the AI instruction set. |
The decode loop broke on any decoder error, so a truncated or corrupted event stream silently dropped the remaining events and surfaced later as a package that appeared to have run no tests. Only io.EOF now ends the loop; any other decode error is joined with the run error and returned, so the gate fails with the decode error and the captured output.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated no new comments.
Suppressed comments (3)
internal/scaffold/gate_test.go:179
runGoTestJSONrelies on per-test{"Action":"pass","Test":...}events to prove each generated package actually ran tests, but the invoked command does not disable the Go test cache. Ifgo testreturns cached results, it may emit only package-level pass events (noTestfield), which would makeassertEachPackageRanTestsfail on subsequent runs even when templates are correct.
Add -count=1 to force tests to run (and/or to ensure the JSON stream includes per-test events consistently).
cmd := exec.CommandContext(t.Context(), "go", "test", "-json", "./...")
internal/scaffold/templates/builder.go.tmpl:18
- The status-handler GoDoc says to “inspect the fields your reports readiness through”. This is inaccurate for non-readiness variants (e.g. the Task variant reports completion, not readiness) and will be emitted into every scaffolded wrapper.
Prefer wording that refers to the variant’s state ({{$spec.StatusNoun}}) rather than “readiness”.
// This is a scaffolded default: it reports {{$spec.StatusValue}} unconditionally, without
// reading the {{.Kind}}'s status. Replace it with logic that inspects the fields
// your {{.Kind}} reports readiness through.
internal/scaffold/templates/builder.go.tmpl:51
- The suspension-mutation GoDoc says “Replace it with the change that stops your workload”, but this handler is also generated for the integration variant (e.g. Service/Ingress) where “workload” is misleading.
Adjust the wording to be kind-neutral so generated wrappers don’t suggest workload-specific actions for non-workload objects.
// This is a scaffolded default: it records no mutation, so the {{.Kind}} is left
// untouched while suspended. Replace it with the change that stops your workload,
// for example scaling to zero or setting a suspended field.
The scaffolded status handler told every kind to inspect the fields it "reports readiness through", which is wrong for task and integration wrappers that report completion and operational state. It now names the variant's own state. The suspension handler said to replace it with the change that "stops your workload", which misdescribes an integration kind; it now says to take the object out of service. The gate's inner go test run adds -count=1 so it executes the generated tests on every run instead of replaying a cached result.
|
Addressed the three suppressed comments from the latest review in 6cfd4f0. They arrived as suppressed comments rather than threads, so the resolutions are recorded here. gate_test.go: no builder.go.tmpl: status handler GoDoc says "readiness". Correct, and it shipped into every scaffolded wrapper. The line now names the variant's own state via builder.go.tmpl: suspension handler GoDoc says "stops your workload". Correct for the same reason, since the handler is generated for the integration variant too. It now reads "the change that takes the Goldens regenerated for the workload, task, and integration variants. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated no new comments.
Suppressed comments (2)
internal/scaffold/gate_test.go:184
- Like
runGo,runGoTestJSONappends a new GOFLAGS entry without removing any existing GOFLAGS from the environment. If GOFLAGS is already set, the intended-mod=modmay be ignored, making the scaffold gate sensitive to the runner's ambient environment.
cmd := exec.CommandContext(t.Context(), "go", "test", "-count=1", "-json", "./...")
cmd.Dir = dir
cmd.Env = append(os.Environ(), "GOFLAGS=-mod=mod")
internal/scaffold/gate_test.go:153
cmd.Env = append(os.Environ(), "GOFLAGS=-mod=mod")can result in duplicate GOFLAGS entries when the parent environment already has GOFLAGS set. On many systems the first occurrence wins, so this test may not actually force-mod=mod, leading to non-reproducible gate behavior depending on the runner environment.
This issue also appears on line 181 of the same file.
cmd := exec.CommandContext(t.Context(), "go", args...)
cmd.Dir = dir
cmd.Env = append(os.Environ(), "GOFLAGS=-mod=mod")
|
The latest review's two suppressed comments both rest on the same premise, and I checked it rather than acting on it. No code change. Claim: That is not how Go resolves it. Both call sites ( Worth noting what the gate deliberately does not isolate: an ambient |
…-cli # Conflicts: # .ai/base.md # .github/copilot-instructions.md # go.sum
The plugin skill references are verbatim copies of docs/ produced by make sync-plugin, and the copilot instruction files are produced by make ai-instructions. Marking them linguist-generated collapses them in pull request diffs and keeps them out of language statistics, so a docs change reviews as the source file rather than as the source plus its copies.
The custom-resource-wrappers skill and the new-wrapper command walked through writing the wrapper package by hand, which is now the fallback rather than the first move. Both lead with ocf scaffold wrapper and keep the eight steps as the reference for what the generated code means and for extending a wrapper that already exists.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 52 out of 53 changed files in this pull request and generated no new comments.
Suppressed comments (1)
plugin/commands/new-wrapper.md:22
- The placeholder
--variant <category>is ambiguous/inaccurate: the CLI only accepts a fixed set of variant values. Using the explicit allowed set here avoids users thinking arbitrary category names are valid.
golangci-lint 2.12.2, which CI now pins through .tool-versions, flags the literal repeated across the three status-bearing variants. It is one value by definition, so it becomes a named constant.
govet under golangci-lint 2.12.2 flags reflect.Ptr, the pre-1.18 alias, in favour of reflect.Pointer. This is the only remaining use in the module and it currently fails lint on main, independently of this branch.
Description
Writing a custom-resource wrapper by hand is almost entirely mechanical: a builder with roughly eight fluent delegations, a resource with eleven to fifteen forwarding methods, a mutator, and the standard test cases. The declared-data migration stamped that pattern out 25 times inside this repo; external users writing wrappers for their own CRDs face the same boilerplate with no tooling, and hand-copied wrappers drift from the framework version they were copied from.
This adds
ocf, a CLI shipped in the module atcmd/ocfand installed withgo install github.com/sourcehawk/operator-component-framework/cmd/ocf@latest.ocf scaffold wrappergenerates a complete wrapper package that compiles and whose generated tests pass immediately. Templates are embedded in the binary, so generated code always matches the framework version the CLI was built from.Changes
ocf scaffold wrappergeneratesmutator.go,builder.go,resource.go, andbuilder_test.gofor any of the four resource categories (static, workload, task, integration), namespaced or cluster-scoped, with GoDoc on every exported symbol following the built-in primitives' wording.go testpasses on a fresh scaffold.--type,--group,--version,--kind,--alias, and--packageare validated, and version, alias, kind, and package name are derived when not given.go.mod. It prints a next-steps block with thego mod tidyhint instead, and refuses to write into a non-empty directory without--force.ocf versionprints the framework version the binary was built from.test-scaffold, wired intomake alland CI: it scaffolds every variant against real Kubernetes types into a temp module with areplacedirective pointing at the checkout, then runsgo test ./...inside it. Template drift now fails CI instead of failing users.docs/cli.mdin the mkdocs nav, a scaffolding section in the README, and a pointer fromdocs/custom-resource.mdto the CLI that generates the pattern that page describes.cobra is a new direct dependency of the module. Nothing outside
cmd/ocfimports it, sogo list -deps ./pkg/...pulls in zero cobra packages and library consumers are unaffected.Challenges
The load-bearing risk is template drift: templates that render code against an API the framework has since changed would compile in CI here and fail in a user's module.
make test-scaffoldcloses that by generating and running the real thing. Its first version only checked the go tool's exit code, which is not enough, becausego testexits 0 both for a package with no test files and for a package whose test file declares no tests. It now consumesgo test -jsonand asserts that each generated package actually ran a passing test.The other sharp edge was flag values reaching generated source.
--group,--version, and the--typeimport path are interpolated into Go string literals in the templates, so an unvalidated value could produce a silently wrong identity string or inject an extra import that still compiles. All three are validated inOptions.Resolve, and the generated literals are quoted at the template boundary so no input can break out of them.Related
Testing
make allpasses: fmt, lint, the full test suite under envtest, the new scaffold gate, example tests, and example builds.mkdocs build --strictpasses.Unit coverage is 89.9% for
internal/scaffoldand 87.5% forcmd/ocf: table tests for every validation and derivation rule, golden tests pinning the full rendered output of all four variants plus a cluster-scoped case, writer tests for the--forceand non-empty-directory semantics, and CLI tests for flag errors and the printed next-steps block.The generated code itself is covered by
make test-scaffoldrather than by assertions on strings. Worth poking at yourself: install the binary, scaffold a wrapper for a CRD you actually manage, rungo mod tidyandgo test ./<package>/..., and check whether the generated defaults and their "replace me" wording tell you what to change. Deliberately breaking a template and runningmake test-scaffoldis the quickest way to see the gate work.🤖 Generated with Claude Code