diff --git a/internal/x402/manifest_devmode_test.go b/internal/x402/manifest_devmode_test.go deleted file mode 100644 index 7bcc3e52..00000000 --- a/internal/x402/manifest_devmode_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package x402 - -import ( - "strings" - "testing" -) - -func TestX402Manifest_DevModeRewritesPins(t *testing.T) { - t.Setenv("OBOL_DEVELOPMENT", "true") - out := string(x402ManifestForApply()) - - for _, want := range []string{ - "ghcr.io/obolnetwork/x402-verifier:latest", - "ghcr.io/obolnetwork/serviceoffer-controller:latest", - } { - if !strings.Contains(out, want) { - t.Errorf("dev mode did not rewrite to %q", want) - } - } - for _, bad := range []string{ - "ghcr.io/obolnetwork/x402-verifier:b13254e", - "ghcr.io/obolnetwork/serviceoffer-controller:b13254e", - } { - if strings.Contains(out, bad) && !strings.Contains(out, ":latest@sha256:") { - // b13254e in a *comment* would be acceptable, but the regex doesn't - // match comments preceded by '#' — flag any unrewritten image: line. - for _, line := range strings.Split(out, "\n") { - trim := strings.TrimSpace(line) - if strings.HasPrefix(trim, "image:") && strings.Contains(trim, bad) { - t.Errorf("dev mode left immutable pin on image line: %q", line) - } - } - } - } -} - -func TestX402Manifest_ProductionPreservesPins(t *testing.T) { - t.Setenv("OBOL_DEVELOPMENT", "") - out := string(x402ManifestForApply()) - if !strings.Contains(out, "ghcr.io/obolnetwork/x402-verifier:b13254e") { - t.Error("production manifest should preserve x402-verifier:b13254e pin") - } -} diff --git a/internal/x402/setup.go b/internal/x402/setup.go index 562da993..4811ceb6 100644 --- a/internal/x402/setup.go +++ b/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 } diff --git a/internal/x402/setup_structure_test.go b/internal/x402/setup_structure_test.go new file mode 100644 index 00000000..4353a0fd --- /dev/null +++ b/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 +}