refactor(x402): drive verifier deployment from helmfile, not Go-side kubectl apply

internal/x402/setup.go

@@ -4,15 +4,32 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"regexp"
+	"os/exec"
+	"path/filepath"
 	"strings"
 
 	"github.com/ObolNetwork/obol-stack/internal/config"
+	stackdefaults "github.com/ObolNetwork/obol-stack/internal/defaults"
 	"github.com/ObolNetwork/obol-stack/internal/embed"
+	"github.com/ObolNetwork/obol-stack/internal/helmcmd"
 	"github.com/ObolNetwork/obol-stack/internal/kubectl"
 	"gopkg.in/yaml.v3"
 )
 
+// x402Manifest is the raw embedded x402.yaml. It is no longer applied
+// directly via kubectl — helmfile renders the same file via the `base`
+// release (see EnsureVerifier). Retained as a package-level value so
+// shape/content tests can assert invariants about the embedded source.
+var x402Manifest = mustReadX402Manifest()
+
+func mustReadX402Manifest() []byte {
+	data, err := embed.ReadInfrastructureFile("base/templates/x402.yaml")
+	if err != nil {
+		panic(fmt.Sprintf("read embedded x402 manifest: %v", err))
+	}
+	return data
+}
+
 const (
 	x402Namespace    = "x402"
 	pricingConfigMap = "x402-pricing"
@@ -37,77 +54,88 @@ const (
 	// Used only as a hint in error messages; the actual chain is taken
 	// from the seller's 402 response by buy.py.
 	DefaultBuySellerChain = "base-sepolia"
-)
 
-var x402Manifest = mustReadX402Manifest()
+	// baseReleaseName matches the helmfile release in
+	// internal/embed/infrastructure/helmfile.yaml whose `chart: ./base`
+	// renders the x402 manifests. EnsureVerifier targets this release
+	// via --selector so the verifier deployment is reconciled the same
+	// way `obol stack up` deploys it — single source of truth.
+	baseReleaseName = "base"
+)
 
-func mustReadX402Manifest() []byte {
-	data, err := embed.ReadInfrastructureFile("base/templates/x402.yaml")
-	if err != nil {
-		panic(fmt.Sprintf("read embedded x402 manifest: %v", err))
+// EnsureVerifier deploys the x402 verifier subsystem if it doesn't exist.
+// Idempotent — helmfile sync is safe to run multiple times.
+//
+// Historical note: this used to read embed.FS x402.yaml directly and
+// `kubectl apply` it, which fought helmfile's field manager and forced
+// us to duplicate the dev-mode image-pin rewrite (formerly in this file,
+// now lives canonically in internal/defaults/defaults.go). Driving the
+// deployment through helmfile against the already-populated
+// $OBOL_CONFIG_DIR/defaults/ tree picks up the canonical dev rewrite
+// for free and removes the entire footgun. See CLAUDE.md pitfall #9.
+func EnsureVerifier(cfg *config.Config) error {
+	if err := kubectl.EnsureCluster(cfg); err != nil {
+		return err
 	}
-	return data
-}
 
-// devLocallyBuiltImageBases mirrors internal/defaults.devLocallyBuiltImageBases
-// — duplicated here to avoid a defaults → x402 → defaults import cycle.
-// Must stay in lockstep with the canonical list there.
-var devLocallyBuiltImageBases = []string{
-	"ghcr.io/obolnetwork/x402-verifier",
-	"ghcr.io/obolnetwork/serviceoffer-controller",
-	"ghcr.io/obolnetwork/x402-buyer",
-	"ghcr.io/obolnetwork/demo-server",
-	"ghcr.io/obolnetwork/obol-stack-public-storefront",
-}
+	// Refresh the defaults tree so the helmfile sync below reads the
+	// most recent embedded manifests. Under OBOL_DEVELOPMENT=true this
+	// also applies the canonical digest-pin -> :latest rewrite via
+	// defaults.rewriteDevDigestPins so freshly built local images are
+	// honored. No-op when the stamp is up to date.
+	backendName := stackdefaults.DetectedBackendName(cfg)
+	stackID := stackdefaults.StackID(cfg)
+	if stackID == "" {
+		return fmt.Errorf("stack ID not found, run 'obol stack init' first")
+	}
+	if _, err := stackdefaults.RefreshInfrastructureIfChanged(cfg, backendName, stackID); err != nil {
+		return fmt.Errorf("refresh infrastructure defaults: %w", err)
+	}
 
-// rewriteDevImagePinsInManifest applies the same `:tag@sha256:digest` /
-// `@sha256:digest` / `:tag` → `:latest` rewrite the defaults pipeline uses,
-// so kubectl-applied manifests inside EnsureVerifier honor the local-build
-// path under OBOL_DEVELOPMENT=true. Without this rewrite, the embedded
-// x402.yaml carrying `:b13254e` pins beats the helmfile-rendered :latest
-// deployment, and the cluster runs the stale registry image regardless of
-// OBOL_FORCE_REBUILD_LOCAL_DEV_IMAGES (root cause of the missing
-// HandleProxy debug-log saga during flow-11 step 43 chase, May 2026).
-//
-// Pattern parity with internal/defaults.rewriteDevDigestPins is enforced
-// by the regression test in TestX402Manifest_DevModeRewritesPins.
-func rewriteDevImagePinsInManifest(data []byte) []byte {
-	out := data
-	for _, base := range devLocallyBuiltImageBases {
-		re := regexp.MustCompile(regexp.QuoteMeta(base) +
-			`(:[a-f0-9]{7,40}@sha256:[a-f0-9]{64}|@sha256:[a-f0-9]{64}|:[a-f0-9]{7,40})`)
-		out = re.ReplaceAll(out, []byte(base+":latest"))
+	if err := helmfileSyncBaseRelease(cfg); err != nil {
+		return fmt.Errorf("helmfile sync %s: %w", baseReleaseName, err)
 	}
-	return out
+
+	// Populate the CA bundle after deploying the verifier so TLS verification
+	// of the facilitator works immediately. Idempotent — safe to call multiple times.
+	bin, kc := kubectl.Paths(cfg)
+	populateCABundle(bin, kc)
+	return nil
 }
 
-// x402ManifestForApply returns the kubectl-apply-ready bytes, rewriting
-// immutable image pins to `:latest` when OBOL_DEVELOPMENT=true so the
-// in-cluster verifier/controller uses the freshly-built local image.
-// In production (OBOL_DEVELOPMENT unset/false) returns the embedded
-// manifest verbatim — the pins are intentional and immutable.
-func x402ManifestForApply() []byte {
-	if os.Getenv("OBOL_DEVELOPMENT") != "true" {
-		return x402Manifest
+// helmfileSyncBaseRelease runs `helmfile --selector name=base sync`
+// against the defaults helmfile rendered into $OBOL_CONFIG_DIR/defaults.
+// This is the same invocation pattern used by `internal/stack.syncDefaults`
+// and `internal/update.ApplyUpgrades`, scoped to the single release that
+// owns the x402 manifests.
+func helmfileSyncBaseRelease(cfg *config.Config) error {
+	kubeconfigPath := filepath.Join(cfg.ConfigDir, "kubeconfig.yaml")
+	helmfilePath := filepath.Join(cfg.ConfigDir, "defaults", "helmfile.yaml")
+
+	if _, err := os.Stat(helmfilePath); err != nil {
+		return fmt.Errorf("defaults helmfile not found at %s (run 'obol stack init' first): %w", helmfilePath, err)
 	}
-	return rewriteDevImagePinsInManifest(x402Manifest)
-}
 
-// EnsureVerifier deploys the x402 verifier subsystem if it doesn't exist.
-// Idempotent — kubectl apply is safe to run multiple times.
-func EnsureVerifier(cfg *config.Config) error {
-	if err := kubectl.EnsureCluster(cfg); err != nil {
-		return err
+	helmfileBin := filepath.Join(cfg.BinDir, "helmfile")
+	helmBin := filepath.Join(cfg.BinDir, "helm")
+
+	args := []string{
+		"--file", helmfilePath,
+		"--kubeconfig", kubeconfigPath,
+		"--selector", "name=" + baseReleaseName,
+		"sync",
 	}
-	bin, kc := kubectl.Paths(cfg)
+	args = append(args, helmcmd.SyncFlagsForVersion(helmBin)...)
 
-	fmt.Println("Applying x402 payment components...")
-	if err := kubectl.Apply(bin, kc, x402ManifestForApply()); err != nil {
-		return err
+	cmd := exec.Command(helmfileBin, args...)
+	cmd.Env = append(os.Environ(),
+		"KUBECONFIG="+kubeconfigPath,
+		"STACK_DATA_DIR="+cfg.DataDir,
+	)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("%w: %s", err, strings.TrimSpace(string(out)))
 	}
-	// Populate the CA bundle after deploying the verifier so TLS verification
-	// of the facilitator works immediately. Idempotent — safe to call multiple times.
-	populateCABundle(bin, kc)
 	return nil
 }
 

internal/x402/setup_structure_test.go

@@ -0,0 +1,82 @@
+package x402
+
+import (
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestEnsureVerifier_NoInlineRegex enforces CLAUDE.md pitfall #9 at the
+// structural level: setup.go must not carry its own image-pin rewrite
+// regex. The canonical rewrite lives in internal/defaults/defaults.go,
+// applied to the helmfile-rendered tree under $OBOL_CONFIG_DIR/defaults.
+// Driving the verifier deployment through helmfile (not kubectl apply
+// of embed.FS) means any duplicated regex is dead code at best and a
+// silent-bypass footgun at worst.
+//
+// If this test fires, either:
+//   - delete the duplicate regex from internal/x402/setup.go, or
+//   - if the duplicate is genuinely needed (it almost never is), move
+//     it behind a shared helper in internal/defaults and call that.
+func TestEnsureVerifier_NoInlineRegex(t *testing.T) {
+	setupPath := mustResolveFile(t, "setup.go")
+
+	data, err := os.ReadFile(setupPath)
+	if err != nil {
+		t.Fatalf("read setup.go: %v", err)
+	}
+	src := string(data)
+
+	// Cheap textual guard first — surfaces a clear error message even when
+	// the AST parse below would also catch it.
+	if strings.Contains(src, `"regexp"`) {
+		t.Fatalf("internal/x402/setup.go must not import the regexp package; " +
+			"the image-pin rewrite belongs in internal/defaults (see CLAUDE.md pitfall #9)")
+	}
+	if strings.Contains(src, "regexp.MustCompile") || strings.Contains(src, "regexp.Compile") {
+		t.Fatalf("internal/x402/setup.go must not compile regexes inline; " +
+			"the duplicated rewrite was deleted in favor of helmfile-driven deploy")
+	}
+
+	// AST-level guard: catches aliased imports (e.g. `re "regexp"`) and is
+	// resilient to comments that happen to contain the word "regexp".
+	fset := token.NewFileSet()
+	file, err := parser.ParseFile(fset, setupPath, data, parser.ImportsOnly)
+	if err != nil {
+		t.Fatalf("parse setup.go: %v", err)
+	}
+	for _, imp := range file.Imports {
+		path := strings.Trim(imp.Path.Value, `"`)
+		if path == "regexp" {
+			t.Fatalf("internal/x402/setup.go imports %q; remove the duplicated rewrite", path)
+		}
+	}
+}
+
+// mustResolveFile locates a source file relative to this test file. Works
+// whether `go test` is run from the package directory or from the repo root.
+func mustResolveFile(t *testing.T, name string) string {
+	t.Helper()
+	// First try working directory (default for `go test ./...`).
+	if _, err := os.Stat(name); err == nil {
+		abs, err := filepath.Abs(name)
+		if err != nil {
+			t.Fatalf("abs %q: %v", name, err)
+		}
+		return abs
+	}
+	t.Fatalf("could not locate %q from %q", name, mustGetwd(t))
+	return ""
+}
+
+func mustGetwd(t *testing.T) string {
+	t.Helper()
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("getwd: %v", err)
+	}
+	return wd
+}