diff --git a/pkg/foreman/agent/coder_gate.go b/pkg/foreman/agent/coder_gate.go index a4b624b..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 four deterministic checks in order: gofmt, go vet, -// go build, and golangci-lint. Heavy envtest or integration tests are +// 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 four checks run regardless of earlier failures so -// the feedback reports everything wrong at once. +// 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 +131,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 +192,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) + } +}