From 94ea968433ab827760fe4153a88d13dea4139cd0 Mon Sep 17 00:00:00 2001 From: Foreman Bot Date: Sun, 21 Jun 2026 03:01:19 -0700 Subject: [PATCH 1/2] feat: add codegen-drift check to fast in-workspace gate After the existing gofmt/vet/build/lint/test checks, regenerate manifests, CRDs, and Helm chart CRDs via `make manifests chart-crds foreman-chart-crds`, then run `git diff --quiet`. If the tree is dirty, fail the gate with feedback naming the drifted files so the coder can regenerate in-run. The check is skipped gracefully if controller-gen is not available in the workspace. This catches changes to API types, kubebuilder markers, or field doc comments that alter generated CRDs or role.yaml before they reach CI, avoiding wasted cycles where the fast gate green-lights a change that drifts generated files. Fixes #775 Signed-off-by: Foreman Bot --- pkg/foreman/agent/coder_gate.go | 49 +++++++++- pkg/foreman/agent/coder_gate_test.go | 133 +++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 5 deletions(-) diff --git a/pkg/foreman/agent/coder_gate.go b/pkg/foreman/agent/coder_gate.go index a4b624b..35a366c 100644 --- a/pkg/foreman/agent/coder_gate.go +++ b/pkg/foreman/agent/coder_gate.go @@ -82,11 +82,11 @@ type checkFailure struct { // golangci-lint binary (e.g. "./bin/golangci-lint"). run is the command // runner (production callers pass execCommandRunner). // -// The gate runs four deterministic checks in order: gofmt, go vet, -// go build, and golangci-lint. Heavy envtest or integration tests are -// intentionally out of scope; they run in a separate clean-room -// Kubernetes Job. All four checks run regardless of earlier failures so -// the feedback reports everything wrong at once. +// The gate runs five deterministic checks in order: gofmt, go vet, +// go build, golangci-lint, and a codegen-drift check. Heavy envtest or +// integration tests are intentionally out of scope; they run in a +// separate clean-room Kubernetes Job. All checks run regardless of +// earlier failures so the feedback reports everything wrong at once. func RunCoderGate(ctx context.Context, workspace, golangciPath string, run commandRunner) (pass bool, feedback string) { var failures []checkFailure @@ -130,6 +130,15 @@ func RunCoderGate(ctx context.Context, workspace, golangciPath string, run comma } } + // 6. Codegen-drift check: regenerate manifests/CRDs and fail if the + // tree is dirty. This catches changes to API types, kubebuilder + // markers, or field doc comments that alter generated CRDs or + // role.yaml before they reach CI (#775). Skipped gracefully if + // controller-gen is unavailable. + if drifted, out := checkCodegenDrift(ctx, workspace, run); drifted { + failures = append(failures, checkFailure{name: "codegen drift", output: out}) + } + if len(failures) == 0 { return true, "" } @@ -182,6 +191,36 @@ func changedTestPackages(ctx context.Context, workspace string, run commandRunne return pkgs } +// checkCodegenDrift regenerates manifests and CRDs, then checks whether +// the workspace tree is dirty. It returns (drifted, output) where output +// lists the drifted files. Skipped (returns false, "") if +// bin/controller-gen is not present in the workspace. +func checkCodegenDrift(ctx context.Context, workspace string, run commandRunner) (drifted bool, output string) { + controllerGen := filepath.Join(workspace, "bin", "controller-gen") + if _, err := run(ctx, workspace, nil, "test", "-f", controllerGen); err != nil { + // controller-gen not available; skip gracefully. + return false, "" + } + + // Regenerate manifests, CRDs, and sync to Helm charts. + if out, err := run(ctx, workspace, nil, "make", "manifests", "chart-crds", "foreman-chart-crds"); err != nil { + // If make itself fails, report it as a drift failure. + return true, "make manifests chart-crds foreman-chart-crds failed:\n" + out + } + + // Check whether the tree is dirty after regeneration. + if _, err := run(ctx, workspace, nil, "git", "diff", "--quiet"); err == nil { + return false, "" + } + + // Tree is dirty; collect the list of drifted files. + out, _ := run(ctx, workspace, nil, "git", "diff", "--name-only") + msg := "Generated files drifted after regeneration. " + + "Run 'make manifests chart-crds foreman-chart-crds' and commit the changes.\n\n" + + "Drifted files:\n" + return true, msg + out +} + // buildFeedback renders the directive and a per-check section for every // failing check, truncating each check's output to maxCheckOutputBytes. func buildFeedback(failures []checkFailure) string { diff --git a/pkg/foreman/agent/coder_gate_test.go b/pkg/foreman/agent/coder_gate_test.go index ec1bd50..71ec07f 100644 --- a/pkg/foreman/agent/coder_gate_test.go +++ b/pkg/foreman/agent/coder_gate_test.go @@ -318,3 +318,136 @@ func TestRunCoderGateTruncation(t *testing.T) { t.Errorf("output not truncated: %d x's exceed cap %d", got, maxCheckOutputBytes) } } + +// TestRunCoderGate_CodegenDrift_FailsWhenDrifted verifies the gate fails +// when regenerated manifests/CRDs differ from the committed tree (#775). +func TestRunCoderGate_CodegenDrift_FailsWhenDrifted(t *testing.T) { + const golangciPath = "./bin/golangci-lint" + run := func(_ context.Context, _ string, _ []string, name string, args ...string) (string, error) { + switch { + case name == "gofmt": + return "", nil + case name == "go": + return "", nil + case name == golangciPath: + return "", nil + case name == "git" && len(args) > 0 && args[0] == "status": + return "", nil // no changed packages for test tier + case name == "test" && len(args) > 0 && args[0] == "-f": + return "", nil // controller-gen exists + case name == "make": + return "", nil // make manifests chart-crds foreman-chart-crds succeeds + case name == "git" && len(args) > 0 && args[0] == "diff": + if len(args) > 1 && args[1] == "--quiet" { + return "", errors.New("exit status 1") // tree is dirty + } + return "config/crd/bases/inference.llmkube.dev_models.yaml\nrole.yaml\n", nil + default: + return "", nil + } + } + pass, feedback := RunCoderGate(context.Background(), "/work", golangciPath, run) + if pass { + t.Fatal("gate should fail when codegen drift is detected") + } + if !strings.Contains(feedback, "codegen drift") { + t.Errorf("feedback should cite codegen drift; got:\n%s", feedback) + } + if !strings.Contains(feedback, "config/crd/bases/inference.llmkube.dev_models.yaml") { + t.Errorf("feedback should list drifted CRD file; got:\n%s", feedback) + } +} + +// TestRunCoderGate_CodegenDrift_PassesWhenClean verifies the gate passes +// when regenerated manifests/CRDs match the committed tree (#775). +func TestRunCoderGate_CodegenDrift_PassesWhenClean(t *testing.T) { + const golangciPath = "./bin/golangci-lint" + run := func(_ context.Context, _ string, _ []string, name string, args ...string) (string, error) { + switch { + case name == "gofmt": + return "", nil + case name == "go": + return "", nil + case name == golangciPath: + return "", nil + case name == "git" && len(args) > 0 && args[0] == "status": + return "", nil // no changed packages for test tier + case name == "test" && len(args) > 0 && args[0] == "-f": + return "", nil // controller-gen exists + case name == "make": + return "", nil // make manifests chart-crds foreman-chart-crds succeeds + case name == "git" && len(args) > 0 && args[0] == "diff": + return "", nil // tree is clean + default: + return "", nil + } + } + pass, feedback := RunCoderGate(context.Background(), "/work", golangciPath, run) + if !pass { + t.Fatalf("gate should pass when codegen is clean; feedback:\n%s", feedback) + } + if feedback != "" { + t.Errorf("expected empty feedback on pass, got %q", feedback) + } +} + +// TestRunCoderGate_CodegenDrift_SkippedWhenNoControllerGen verifies the +// codegen-drift check is skipped gracefully when controller-gen is not +// available in the workspace (#775). +func TestRunCoderGate_CodegenDrift_SkippedWhenNoControllerGen(t *testing.T) { + const golangciPath = "./bin/golangci-lint" + run := func(_ context.Context, _ string, _ []string, name string, args ...string) (string, error) { + switch { + case name == "gofmt": + return "", nil + case name == "go": + return "", nil + case name == golangciPath: + return "", nil + case name == "git" && len(args) > 0 && args[0] == "status": + return "", nil // no changed packages for test tier + case name == "test" && len(args) > 0 && args[0] == "-f": + return "", errors.New("exit status 1") // controller-gen not found + default: + return "", nil + } + } + pass, feedback := RunCoderGate(context.Background(), "/work", golangciPath, run) + if !pass { + t.Fatalf("gate should pass when controller-gen is unavailable; feedback:\n%s", feedback) + } + if feedback != "" { + t.Errorf("expected empty feedback on pass, got %q", feedback) + } +} + +// TestRunCoderGate_CodegenDrift_FailsWhenMakeFails verifies the gate fails +// when make manifests chart-crds foreman-chart-crds itself errors (#775). +func TestRunCoderGate_CodegenDrift_FailsWhenMakeFails(t *testing.T) { + const golangciPath = "./bin/golangci-lint" + run := func(_ context.Context, _ string, _ []string, name string, args ...string) (string, error) { + switch { + case name == "gofmt": + return "", nil + case name == "go": + return "", nil + case name == golangciPath: + return "", nil + case name == "git" && len(args) > 0 && args[0] == "status": + return "", nil // no changed packages for test tier + case name == "test" && len(args) > 0 && args[0] == "-f": + return "", nil // controller-gen exists + case name == "make": + return "Error: controller-gen: exit status 1\n", errors.New("exit status 2") + default: + return "", nil + } + } + pass, feedback := RunCoderGate(context.Background(), "/work", golangciPath, run) + if pass { + t.Fatal("gate should fail when make manifests fails") + } + if !strings.Contains(feedback, "codegen drift") { + t.Errorf("feedback should cite codegen drift; got:\n%s", feedback) + } +} From 39d354d93c1d5e9dae273e7219a1cda5ac29a64c Mon Sep 17 00:00:00 2001 From: Christopher Maher Date: Sun, 21 Jun 2026 03:09:34 -0700 Subject: [PATCH 2/2] docs: correct RunCoderGate check count (six, incl. unit-test tier) Signed-off-by: Christopher Maher --- pkg/foreman/agent/coder_gate.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/foreman/agent/coder_gate.go b/pkg/foreman/agent/coder_gate.go index 35a366c..fcf8fef 100644 --- a/pkg/foreman/agent/coder_gate.go +++ b/pkg/foreman/agent/coder_gate.go @@ -82,11 +82,12 @@ type checkFailure struct { // golangci-lint binary (e.g. "./bin/golangci-lint"). run is the command // runner (production callers pass execCommandRunner). // -// The gate runs five deterministic checks in order: gofmt, go vet, -// go build, golangci-lint, and a codegen-drift check. Heavy envtest or -// integration tests are intentionally out of scope; they run in a -// separate clean-room Kubernetes Job. All checks run regardless of -// earlier failures so the feedback reports everything wrong at once. +// The gate runs six deterministic checks in order: gofmt, go vet, +// go build, golangci-lint, a fast unit-test tier on changed packages, +// and a codegen-drift check. Heavy envtest or integration tests are +// intentionally out of scope; they run in a separate clean-room +// Kubernetes Job. All checks run regardless of earlier failures so the +// feedback reports everything wrong at once. func RunCoderGate(ctx context.Context, workspace, golangciPath string, run commandRunner) (pass bool, feedback string) { var failures []checkFailure