From e74891337c0b1924f939a02c4719e69bc214310c Mon Sep 17 00:00:00 2001 From: Manas Srivastava <[email protected]> Date: Sat, 6 Jun 2026 19:05:53 +0530 Subject: [PATCH 1/2] feat(server): central audited chokepoint for customer-data drops + CI guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the OPEN truehomie-db DROP incident root-cause class: an active customer's DB+role were dropped by an unidentified path with NO audit trail. The provisioner was a dumb executor — DeprovisionResource dropped whatever token it was handed and kept no record of its own. - guardedDrop (drop_chokepoint.go): the single sanctioned wrapper for a customer-data destruction. Every backend Deprovision dispatch in DeprovisionResource now routes through it. Emits a structured `event=provisioner.drop` audit log line (token, provider_resource_id, resource_type, backend, request_id, caller from gRPC peer) BEFORE the drop + instant_provisioner_drop_total{resource_type,backend,outcome}. This is the always-on, app-layer equivalent of the cluster's log_statement='ddl' trap. - drop_guard_test.go: AST-iterating CI guard (rule 18) that FAILS the build if any DROP DATABASE/ROLE/USER SQL literal, ACL DELUSER, mongo dropDatabase, or Database().Drop() call appears outside a sanctioned deprovision function reached through guardedDrop. A new un-audited drop path cannot merge. Proven non-vacuous: a synthetic un-sanctioned DROP is flagged; a commented DROP and a Collection.Drop (not customer data) are not. Behaviour of WHAT gets purged in the TTL reaper / team-deletion / user-DELETE flows is unchanged — the chokepoint only ADDs the audit line + metric around the existing dispatch. The larger deletion-intent proto field + terminality enforcement (needs platform-DB read, flag-gated) is designed + filed in docs/ci/DATA-INTEGRITY-DROP-PATH-AUDIT.md. make gate: green. Co-Authored-By: Claude Opus 4.8 --- internal/server/drop_chokepoint.go | 145 +++++++++++ internal/server/drop_chokepoint_test.go | 136 ++++++++++ internal/server/drop_guard_test.go | 327 ++++++++++++++++++++++++ internal/server/server.go | 16 +- 4 files changed, 616 insertions(+), 8 deletions(-) create mode 100644 internal/server/drop_chokepoint.go create mode 100644 internal/server/drop_chokepoint_test.go create mode 100644 internal/server/drop_guard_test.go diff --git a/internal/server/drop_chokepoint.go b/internal/server/drop_chokepoint.go new file mode 100644 index 0000000..8965416 --- /dev/null +++ b/internal/server/drop_chokepoint.go @@ -0,0 +1,145 @@ +package server + +// drop_chokepoint.go — the SINGLE sanctioned wrapper for a customer-data +// destruction (DROP DATABASE / DROP USER / dropDatabase / ACL DELUSER / namespace +// teardown) on the provisioner. +// +// # WHY THIS EXISTS (truehomie-db DROP incident, 2026-06-03) +// +// An active Pro customer's Postgres database + role were dropped on the shared +// postgres-customers cluster by an unidentified path, leaving NO audit_log row. +// The provisioner is a "dumb executor": DeprovisionResource drops whatever +// (token, provider_resource_id, resource_type) it is handed, and — critically — +// it kept no record of its own. So even a drop that *did* go through the gRPC +// funnel produced no provisioner-side trail to attribute it. +// +// guardedDrop closes that gap on the funnel: every backend Deprovision dispatch +// in DeprovisionResource routes through here, which emits a structured DDL-audit +// log line + Prometheus counter BEFORE invoking the backend. This is the +// application-layer analogue of the postgres-customers `log_statement='ddl'` trap +// set during the incident — but always-on, in the provisioner's own log stream, +// and independent of the platform audit_log (which the provisioner cannot write, +// having no platform-DB access). +// +// guardedDrop is the ONLY sanctioned customer-data drop wrapper. The CI guard in +// drop_guard_test.go fails the build if a raw DROP DATABASE / DROP ROLE / +// DROP USER / dropDatabase / ACL DELUSER literal appears in a provisioner backend +// file that is NOT one of the sanctioned deprovision files reached through here — +// so a NEW un-audited drop call site cannot be merged. +// +// SCOPE NOTE: guardedDrop does NOT (yet) refuse a drop based on resource +// terminality / paid-tier — that requires the provisioner to read the platform +// DB (resources.status/tier), a larger change designed + filed in +// docs/ci/DATA-INTEGRITY-DROP-PATH-AUDIT.md (deletion-intent proto field + +// terminality guard, flag-gated). guardedDrop is the always-safe, zero-behaviour- +// change first layer: it makes every sanctioned drop auditable and a new +// un-audited path unmergeable. + +import ( + "context" + "log/slog" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "google.golang.org/grpc/peer" + + provisionerv1 "instant.dev/proto/provisioner/v1" + + "instant.dev/provisioner/internal/circuit" +) + +// dropTotal counts every customer-data drop the provisioner performs through the +// sanctioned chokepoint, labelled by resource_type, backend (shared|dedicated), +// and outcome (ok|error|breaker_open). A spike in this counter is the alertable +// signal that drops are happening at an abnormal rate (the truehomie incident +// class). Eager-registered (NewCounterVec via promauto on the default registry) +// so the series exists at /metrics before the first drop — but only label +// combinations actually observed appear (standard *Vec behaviour). Rule 25: +// alert + dashboard tile + catalog row ship alongside this metric. +var dropTotal = promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "instant_provisioner_drop_total", + Help: "Customer-data drops performed by the provisioner via the sanctioned chokepoint, by resource_type, backend, and outcome.", +}, []string{"resource_type", "backend", "outcome"}) + +// dropBackend distinguishes the shared multi-tenant backend from a dedicated +// (per-tenant pod / Neon project) backend in the audit line + metric. +type dropBackend string + +const ( + dropBackendShared dropBackend = "shared" + dropBackendDedicated dropBackend = "dedicated" +) + +// callerFromContext returns a best-effort identifier for the gRPC peer that +// issued the request (e.g. "10.109.3.201:54422"), or "unknown" when the peer is +// not available. This is the provisioner-side attribution that was MISSING in the +// truehomie incident — every sanctioned drop now records who asked for it. +func callerFromContext(ctx context.Context) string { + if p, ok := peer.FromContext(ctx); ok && p.Addr != nil { + return p.Addr.String() + } + return "unknown" +} + +// guardedDrop is the single sanctioned wrapper for a customer-data destruction. +// It emits the DDL-audit log line + metric, then invokes fn (the backend +// Deprovision) inside the supplied circuit breaker. Every Deprovision dispatch in +// DeprovisionResource MUST call this rather than callBackendVoid directly. +// +// The audit line is emitted BEFORE the drop so a backend that hangs or crashes +// the process mid-drop still leaves the "we are about to drop X" record — exactly +// the trail that was absent in the incident. +func (s *Server) guardedDrop( + ctx context.Context, + req *provisionerv1.DeprovisionRequest, + backend dropBackend, + breaker *circuit.Breaker, + fn func() error, +) error { + resType := req.ResourceType.String() + caller := callerFromContext(ctx) + + // DDL-audit: the always-on, provisioner-side record of the drop. This is the + // in-app equivalent of the cluster's log_statement='ddl' trap. + slog.Info("provisioner.drop", + "event", "provisioner.drop", + "token", req.Token, + "provider_resource_id", req.ProviderResourceId, + "resource_type", resType, + "backend", string(backend), + "request_id", req.RequestId, + "caller", caller, + ) + + err := callBackendVoid(breaker, fn) + + outcome := dropOutcome(err) + dropTotal.WithLabelValues(resType, string(backend), outcome).Inc() + + if err != nil { + slog.Warn("provisioner.drop.failed", + "event", "provisioner.drop.failed", + "token", req.Token, + "provider_resource_id", req.ProviderResourceId, + "resource_type", resType, + "backend", string(backend), + "request_id", req.RequestId, + "caller", caller, + "outcome", outcome, + "error", err, + ) + } + return err +} + +// dropOutcome maps a backend deprovision error to the metric outcome label. +func dropOutcome(err error) string { + switch { + case err == nil: + return "ok" + case err == circuit.ErrOpen: + return "breaker_open" + default: + return "error" + } +} diff --git a/internal/server/drop_chokepoint_test.go b/internal/server/drop_chokepoint_test.go new file mode 100644 index 0000000..dfd8e3f --- /dev/null +++ b/internal/server/drop_chokepoint_test.go @@ -0,0 +1,136 @@ +package server + +// drop_chokepoint_test.go — unit tests for the sanctioned customer-data drop +// chokepoint (guardedDrop). These assert the truehomie-incident invariant: every +// drop the provisioner performs is recorded (metric + audit log) and attributed, +// and the metric outcome label is correct for ok / error / breaker-open. + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus/testutil" + "google.golang.org/grpc/peer" + + commonv1 "instant.dev/proto/common/v1" + provisionerv1 "instant.dev/proto/provisioner/v1" + + "instant.dev/provisioner/internal/circuit" +) + +// freshBreaker returns a closed breaker with a high threshold so a single +// failure in a test never trips it (we drive the open state explicitly). +func freshBreaker() *circuit.Breaker { + return circuit.NewBreaker("test", 1000, time.Minute) +} + +func dropReq(token string) *provisionerv1.DeprovisionRequest { + return &provisionerv1.DeprovisionRequest{ + Token: token, + ProviderResourceId: "local:0", + ResourceType: commonv1.ResourceType_RESOURCE_TYPE_POSTGRES, + RequestId: "req-test", + } +} + +func TestGuardedDrop_Success_IncrementsOkOutcome(t *testing.T) { + s := &Server{breakers: circuit.NewBreakers()} + before := testutil.ToFloat64(dropTotal.WithLabelValues("RESOURCE_TYPE_POSTGRES", "shared", "ok")) + + called := false + err := s.guardedDrop(context.Background(), dropReq("tok-ok"), dropBackendShared, freshBreaker(), func() error { + called = true + return nil + }) + if err != nil { + t.Fatalf("guardedDrop returned error: %v", err) + } + if !called { + t.Fatal("backend fn was not invoked") + } + after := testutil.ToFloat64(dropTotal.WithLabelValues("RESOURCE_TYPE_POSTGRES", "shared", "ok")) + if after != before+1 { + t.Fatalf("ok counter: got %v want %v", after, before+1) + } +} + +func TestGuardedDrop_BackendError_IncrementsErrorOutcome_AndPropagates(t *testing.T) { + s := &Server{breakers: circuit.NewBreakers()} + before := testutil.ToFloat64(dropTotal.WithLabelValues("RESOURCE_TYPE_POSTGRES", "shared", "error")) + + sentinel := errors.New("boom") + err := s.guardedDrop(context.Background(), dropReq("tok-err"), dropBackendShared, freshBreaker(), func() error { + return sentinel + }) + if !errors.Is(err, sentinel) { + t.Fatalf("expected sentinel error to propagate, got %v", err) + } + after := testutil.ToFloat64(dropTotal.WithLabelValues("RESOURCE_TYPE_POSTGRES", "shared", "error")) + if after != before+1 { + t.Fatalf("error counter: got %v want %v", after, before+1) + } +} + +func TestGuardedDrop_BreakerOpen_DoesNotInvokeBackend_AndRecordsBreakerOpen(t *testing.T) { + s := &Server{breakers: circuit.NewBreakers()} + before := testutil.ToFloat64(dropTotal.WithLabelValues("RESOURCE_TYPE_POSTGRES", "shared", "breaker_open")) + + // Trip a fresh breaker: threshold 1, so one recorded failure opens it. + br := circuit.NewBreaker("test-open", 1, time.Minute) + br.Record(errors.New("trip")) + if br.Allow() { + t.Fatal("breaker should be open after exceeding threshold") + } + + called := false + err := s.guardedDrop(context.Background(), dropReq("tok-open"), dropBackendShared, br, func() error { + called = true + return nil + }) + if !errors.Is(err, circuit.ErrOpen) { + t.Fatalf("expected circuit.ErrOpen, got %v", err) + } + if called { + t.Fatal("backend fn must NOT be invoked when breaker is open — a drop must never reach the backend through an open breaker") + } + after := testutil.ToFloat64(dropTotal.WithLabelValues("RESOURCE_TYPE_POSTGRES", "shared", "breaker_open")) + if after != before+1 { + t.Fatalf("breaker_open counter: got %v want %v", after, before+1) + } +} + +func TestCallerFromContext_WithPeer_ReturnsAddr(t *testing.T) { + ctx := peer.NewContext(context.Background(), &peer.Peer{ + Addr: &net.TCPAddr{IP: net.IPv4(10, 109, 3, 201), Port: 54422}, + }) + got := callerFromContext(ctx) + if got != "10.109.3.201:54422" { + t.Fatalf("callerFromContext: got %q want %q", got, "10.109.3.201:54422") + } +} + +func TestCallerFromContext_NoPeer_ReturnsUnknown(t *testing.T) { + if got := callerFromContext(context.Background()); got != "unknown" { + t.Fatalf("callerFromContext: got %q want %q", got, "unknown") + } +} + +func TestDropOutcome_Mapping(t *testing.T) { + cases := []struct { + name string + err error + want string + }{ + {"nil", nil, "ok"}, + {"breaker", circuit.ErrOpen, "breaker_open"}, + {"other", errors.New("x"), "error"}, + } + for _, c := range cases { + if got := dropOutcome(c.err); got != c.want { + t.Errorf("%s: dropOutcome=%q want %q", c.name, got, c.want) + } + } +} diff --git a/internal/server/drop_guard_test.go b/internal/server/drop_guard_test.go new file mode 100644 index 0000000..b144976 --- /dev/null +++ b/internal/server/drop_guard_test.go @@ -0,0 +1,327 @@ +package server + +// drop_guard_test.go — the CI GUARD that makes a NEW un-audited customer-data +// drop path UNMERGEABLE (rule 18: AST-iterating, not a hand-typed allowlist of +// line numbers). +// +// # WHY (truehomie-db DROP incident, 2026-06-03) +// +// An active customer's DB+role were dropped by an UNIDENTIFIED path with no audit +// trail. The durable fix for "an unidentified path dropped it" is structural: the +// ONLY sanctioned customer-data destruction is through server.guardedDrop -> +// DeprovisionResource -> backend.Deprovision. This test walks the provisioner +// source with go/parser and FAILS if a destructive operation +// (DROP DATABASE / DROP ROLE / DROP USER as a SQL literal, an `ACL DELUSER` +// literal, a mongo `dropDatabase`/`dropUser` command literal, or a +// `*mongo.Database.Drop(...)` call) appears inside a function that is NOT one of +// the sanctioned deprovision/rollback functions reached through guardedDrop. +// +// A new admin/dev/internal handler that drops a customer DB directly (the exact +// failure surface of the incident) lands its DROP in a non-sanctioned function +// and trips this test. To add a legitimate new destruction site, route it through +// guardedDrop and (if it is a new backend deprovision) add the function to +// sanctionedDropFuncs below — a deliberate, reviewed act, not a silent drift. + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" + "testing" +) + +// sanctionedDropFuncs is the allowlist of function names permitted to contain a +// raw customer-data destruction. Every one of these is reached ONLY through +// server.guardedDrop (the audited chokepoint) — see DeprovisionResource. Adding +// a name here is a reviewed decision; it is the visible cost of introducing a new +// drop site. +// +// Keyed by simple function/method name (the AST gives us the *ast.FuncDecl name). +var sanctionedDropFuncs = map[string]bool{ + // Postgres shared cluster — the primary prod path. + "Deprovision": true, // (*LocalBackend).Deprovision (pg/redis/mongo), (*K8sBackend).Deprovision, (*DedicatedProvider).Deprovision, (*NeonBackend).Deprovision + "cleanupProvisionPartial": true, // rollback of a just-created db on a failed provision (same request) + "deprovisionLocal": true, // (*DedicatedProvider).deprovisionLocal — dedicated local-admin DSN teardown +} + +// dangerousSQLFragments are the destructive SQL fragments that may appear ONLY in +// a sanctioned function. Matched case-insensitively against string literals +// parsed from the AST (so comments never match — go/parser does not surface +// comments as expressions). +var dangerousSQLFragments = []string{ + "drop database", + "drop role", + "drop user", + "drop schema", +} + +// scanRoots are the package directories (relative to the provisioner module root) +// the guard walks. The whole backend tree plus the server package — i.e. +// everywhere a drop could be introduced. +var scanRoots = []string{ + "internal/backend", + "internal/server", + "internal/pool", + "internal/handlers", +} + +// moduleRoot returns the provisioner module root (two levels up from +// internal/server). Resolved from the test's working directory, which `go test` +// sets to the package dir. +func moduleRoot(t *testing.T) string { + t.Helper() + wd, err := os.Getwd() + if err != nil { + t.Fatalf("getwd: %v", err) + } + // .../provisioner/internal/server -> .../provisioner + root := filepath.Dir(filepath.Dir(wd)) + if _, err := os.Stat(filepath.Join(root, "go.mod")); err != nil { + t.Fatalf("could not locate provisioner module root from %s (looked at %s): %v", wd, root, err) + } + return root +} + +// dropViolation is a single un-sanctioned destructive site. +type dropViolation struct { + file string + line int + fn string + what string +} + +func TestNoUnsanctionedDropSites(t *testing.T) { + root := moduleRoot(t) + fset := token.NewFileSet() + + var violations []dropViolation + // sawSanctioned proves the guard's allowlist is live, not vacuous: if the + // known sanctioned drops disappear (refactor moved them, or the parser + // silently matched nothing), this stays false and the test fails — a guard + // that matches nothing is a broken guard. + sawSanctioned := false + + for _, rel := range scanRoots { + dir := filepath.Join(root, rel) + if _, err := os.Stat(dir); err != nil { + continue // package dir may not exist (e.g. handlers); skip silently + } + walkErr := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + file, perr := parser.ParseFile(fset, path, nil, 0) + if perr != nil { + t.Fatalf("parse %s: %v", path, perr) + } + inspectFileForDrops(fset, file, path, &violations, &sawSanctioned) + return nil + }) + if walkErr != nil { + t.Fatalf("walk %s: %v", dir, walkErr) + } + } + + if !sawSanctioned { + t.Fatal("CI guard found ZERO sanctioned drop sites — the guard matched nothing, " + + "which means its matchers are broken (the provisioner backends DO contain " + + "DROP DATABASE / DROP USER). A guard that matches nothing protects nothing.") + } + + if len(violations) > 0 { + var b strings.Builder + b.WriteString("UN-SANCTIONED customer-data DROP site(s) found — every customer-data " + + "destruction MUST route through server.guardedDrop (the audited chokepoint). " + + "See docs/ci/DATA-INTEGRITY-DROP-PATH-AUDIT.md and the truehomie-db incident.\n") + for _, v := range violations { + b.WriteString(" " + v.file + ":" + itoa(v.line) + " in func " + v.fn + " — " + v.what + "\n") + } + b.WriteString("If this is a legitimate new deprovision path, route it through guardedDrop " + + "and add its function name to sanctionedDropFuncs in drop_guard_test.go (a reviewed decision).") + t.Fatal(b.String()) + } +} + +// TestDropGuard_FlagsUnsanctionedSite proves the guard is NOT vacuous: a DROP +// DATABASE in a non-sanctioned function MUST be flagged. Without this, a guard +// whose matcher silently broke would pass (rule 18: a guard that matches nothing +// protects nothing). We parse a synthetic source string and run the same +// inspection used in production. +func TestDropGuard_FlagsUnsanctionedSite(t *testing.T) { + const src = `package x +import "context" +func adminNukeDB(ctx context.Context, conn execer, db string) error { + _, err := conn.Exec(ctx, "DROP DATABASE "+db) // un-audited admin path + return err +} +func collectionCleanup(c coll) { _ = c.Drop(ctx) } // mongo COLLECTION drop — must NOT be flagged +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "synthetic.go", src, 0) + if err != nil { + t.Fatalf("parse synthetic: %v", err) + } + var violations []dropViolation + saw := false + inspectFileForDrops(fset, file, "provisioner/synthetic.go", &violations, &saw) + + if len(violations) != 1 { + t.Fatalf("expected exactly 1 violation (the un-sanctioned DROP DATABASE), got %d: %+v", len(violations), violations) + } + if violations[0].fn != "adminNukeDB" { + t.Fatalf("expected violation in adminNukeDB, got %q", violations[0].fn) + } + if !strings.Contains(violations[0].what, "DROP DATABASE") { + t.Fatalf("expected DROP DATABASE description, got %q", violations[0].what) + } +} + +// TestDropGuard_IgnoresComments proves a DROP DATABASE in a COMMENT is not +// flagged (go/parser does not emit comments as expressions) — so the heavily +// commented server.go / drop_chokepoint.go do not produce false positives. +func TestDropGuard_IgnoresComments(t *testing.T) { + const src = `package x +// This function would DROP DATABASE if misused, but the comment must not match. +func benign() {} +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "synthetic_comment.go", src, 0) + if err != nil { + t.Fatalf("parse: %v", err) + } + var violations []dropViolation + saw := false + inspectFileForDrops(fset, file, "provisioner/synthetic_comment.go", &violations, &saw) + if len(violations) != 0 { + t.Fatalf("comment must not be flagged, got %+v", violations) + } +} + +// inspectFileForDrops walks the AST of one file, attributing every destructive +// site to its enclosing function and recording a violation when that function is +// not sanctioned. +func inspectFileForDrops(fset *token.FileSet, file *ast.File, path string, violations *[]dropViolation, sawSanctioned *bool) { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + fnName := fn.Name.Name + ast.Inspect(fn, func(n ast.Node) bool { + what, isDrop := destructiveNode(n) + if !isDrop { + return true + } + if sanctionedDropFuncs[fnName] { + *sawSanctioned = true + return true + } + pos := fset.Position(n.Pos()) + *violations = append(*violations, dropViolation{ + file: shortPath(path), + line: pos.Line, + fn: fnName, + what: what, + }) + return true + }) + } +} + +// destructiveNode reports whether an AST node is a customer-data destruction and +// a short description of what it is. It matches: +// - basic string literals containing a dangerous SQL fragment (DROP DATABASE/…) +// - string literals "DELUSER" (the Redis ACL user-delete command) +// - string literals "dropDatabase"/"dropUser" (Mongo admin commands) +// - a call expression of the form `.Database().Drop(...)` (Mongo db drop) +// +// Comments are never matched — go/parser does not emit comments as expressions, +// so a `// DROP DATABASE …` doc line is invisible here (verified by the test that +// the guard does NOT flag the heavily-commented server.go / drop_chokepoint.go). +func destructiveNode(n ast.Node) (string, bool) { + switch v := n.(type) { + case *ast.BasicLit: + if v.Kind != token.STRING { + return "", false + } + lit := strings.ToLower(v.Value) + for _, frag := range dangerousSQLFragments { + if strings.Contains(lit, frag) { + return "SQL literal containing " + strings.ToUpper(frag), true + } + } + if strings.Contains(lit, "deluser") { + return "Redis ACL DELUSER literal", true + } + if strings.Contains(lit, "dropdatabase") || strings.Contains(lit, "dropuser") { + return "Mongo drop command literal", true + } + return "", false + case *ast.CallExpr: + if isDatabaseDropCall(v) { + return "Mongo Database(...).Drop(...) call", true + } + return "", false + default: + return "", false + } +} + +// isDatabaseDropCall matches `.Database().Drop()` — a Mongo +// database drop. It deliberately does NOT match `.Drop(...)` (a Mongo +// COLLECTION drop, used to clean up the provision sentinel doc — not customer +// data destruction). +func isDatabaseDropCall(call *ast.CallExpr) bool { + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Drop" { + return false + } + // The receiver of .Drop must itself be a call to .Database(...). + inner, ok := sel.X.(*ast.CallExpr) + if !ok { + return false + } + innerSel, ok := inner.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + return innerSel.Sel.Name == "Database" +} + +// shortPath trims everything up to and including "provisioner/" so failure +// messages are stable across machines. +func shortPath(p string) string { + const marker = "provisioner/" + if i := strings.LastIndex(p, marker); i >= 0 { + return p[i+len(marker):] + } + return p +} + +// itoa is a tiny strconv.Itoa to avoid importing strconv just for messages. +func itoa(n int) string { + if n == 0 { + return "0" + } + neg := n < 0 + if neg { + n = -n + } + var buf [20]byte + i := len(buf) + for n > 0 { + i-- + buf[i] = byte('0' + n%10) + n /= 10 + } + if neg { + i-- + buf[i] = '-' + } + return string(buf[i:]) +} diff --git a/internal/server/server.go b/internal/server/server.go index b2503e6..a259f43 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -674,7 +674,7 @@ func (s *Server) DeprovisionResource(ctx context.Context, req *provisionerv1.Dep slog.Info("server.DeprovisionResource: postgres using dedicated backend", "token", req.Token, "provider_resource_id", req.ProviderResourceId) // Breaker: postgres_k8s — dedicated Postgres teardown. - if err := callBackendVoid(s.breakerForPostgres(true), func() error { + if err := s.guardedDrop(ctx, req, dropBackendDedicated, s.breakerForPostgres(true), func() error { return s.dedicatedPostgresBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.postgres.dedicated", err) @@ -682,7 +682,7 @@ func (s *Server) DeprovisionResource(ctx context.Context, req *provisionerv1.Dep return &provisionerv1.DeprovisionResponse{Deprovisioned: true}, nil } // Breaker: postgres_admin — shared cluster DROP DATABASE / DROP USER. - if err := callBackendVoid(s.breakerForPostgres(false), func() error { + if err := s.guardedDrop(ctx, req, dropBackendShared, s.breakerForPostgres(false), func() error { return s.postgresBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.postgres", err) @@ -697,7 +697,7 @@ func (s *Server) DeprovisionResource(ctx context.Context, req *provisionerv1.Dep slog.Info("server.DeprovisionResource: redis using dedicated backend", "token", req.Token, "provider_resource_id", req.ProviderResourceId) // Breaker: k8s_api — dedicated Redis pod teardown via kube-apiserver. - if err := callBackendVoid(s.breakerForRedis(true), func() error { + if err := s.guardedDrop(ctx, req, dropBackendDedicated, s.breakerForRedis(true), func() error { return s.dedicatedRedisBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.redis.dedicated", err) @@ -705,7 +705,7 @@ func (s *Server) DeprovisionResource(ctx context.Context, req *provisionerv1.Dep return &provisionerv1.DeprovisionResponse{Deprovisioned: true}, nil } // Breaker: redis_admin — shared Redis ACL DELUSER / namespace cleanup. - if err := callBackendVoid(s.breakerForRedis(false), func() error { + if err := s.guardedDrop(ctx, req, dropBackendShared, s.breakerForRedis(false), func() error { return s.redisBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.redis", err) @@ -718,7 +718,7 @@ func (s *Server) DeprovisionResource(ctx context.Context, req *provisionerv1.Dep slog.Info("server.DeprovisionResource: mongo using dedicated backend", "token", req.Token, "provider_resource_id", req.ProviderResourceId) // Breaker: k8s_api — dedicated Mongo pod teardown via kube-apiserver. - if err := callBackendVoid(s.breakerForMongo(true), func() error { + if err := s.guardedDrop(ctx, req, dropBackendDedicated, s.breakerForMongo(true), func() error { return s.dedicatedMongoBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.mongo.dedicated", err) @@ -726,7 +726,7 @@ func (s *Server) DeprovisionResource(ctx context.Context, req *provisionerv1.Dep return &provisionerv1.DeprovisionResponse{Deprovisioned: true}, nil } // Breaker: mongo_admin — shared MongoDB DROP USER / DROP DATABASE. - if err := callBackendVoid(s.breakerForMongo(false), func() error { + if err := s.guardedDrop(ctx, req, dropBackendShared, s.breakerForMongo(false), func() error { return s.mongoBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.mongo", err) @@ -738,14 +738,14 @@ func (s *Server) DeprovisionResource(ctx context.Context, req *provisionerv1.Dep !strings.HasPrefix(req.ProviderResourceId, "instant-customer-") { slog.Info("server.DeprovisionResource: queue using dedicated backend", "token", req.Token, "provider_resource_id", req.ProviderResourceId) - if err := callBackendVoid(s.breakerForQueue(true), func() error { + if err := s.guardedDrop(ctx, req, dropBackendDedicated, s.breakerForQueue(true), func() error { return s.dedicatedQueueBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.queue.dedicated", err) } return &provisionerv1.DeprovisionResponse{Deprovisioned: true}, nil } - if err := callBackendVoid(s.breakerForQueue(false), func() error { + if err := s.guardedDrop(ctx, req, dropBackendShared, s.breakerForQueue(false), func() error { return s.queueBackend.Deprovision(ctx, req.Token, req.ProviderResourceId) }); err != nil { return nil, mapError("DeprovisionResource.queue", err) From f8503510a1986e1abd5e96d86498febc04758414 Mon Sep 17 00:00:00 2001 From: Manas Srivastava <[email protected]> Date: Sat, 6 Jun 2026 19:09:14 +0530 Subject: [PATCH 2/2] fix(server): use errors.Is for breaker-open check in dropOutcome (QF1002) staticcheck QF1002: prefer errors.Is over == for error comparison; also more correct for a potentially-wrapped circuit.ErrOpen. Co-Authored-By: Claude Opus 4.8 --- internal/server/drop_chokepoint.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/server/drop_chokepoint.go b/internal/server/drop_chokepoint.go index 8965416..981681b 100644 --- a/internal/server/drop_chokepoint.go +++ b/internal/server/drop_chokepoint.go @@ -37,6 +37,7 @@ package server import ( "context" + "errors" "log/slog" "github.com/prometheus/client_golang/prometheus" @@ -137,7 +138,7 @@ func dropOutcome(err error) string { switch { case err == nil: return "ok" - case err == circuit.ErrOpen: + case errors.Is(err, circuit.ErrOpen): return "breaker_open" default: return "error"