Skip to content

feat(provisioning): discovery endpoints for external writers#966

Merged
jghoman merged 1 commit into
mainfrom
jakob/discovery-endpoints
Jul 21, 2026
Merged

feat(provisioning): discovery endpoints for external writers#966
jghoman merged 1 commit into
mainfrom
jakob/discovery-endpoints

Conversation

@jghoman

@jghoman jghoman commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What

Two read-only endpoints on the internal provisioning API — the machine-facing "which tenants exist and where do I write" surface for external writers (viaduck destination discovery; millpond's include-values poller, shipped in millpond#107):

  • GET /api/v1/warehouses — per ready/resharding warehouse: org_id, duckling name, state, writable fence flag (false while resharding — a resharding warehouse is listed, not hidden, because vanishing reads as tenant removal downstream), teams, metadata-store connection with a k8s SecretRef (no plaintext credentials anywhere in the payload), bucket, and config_generation.
  • GET /api/v1/warehouse-team-ids — bare sorted deduped JSON array of team ids, the values-only shape millpond's poller consumes.

Teams (reworked for #969/#970's duckgres_org_teams)

Teams are the org's team rows, served with resolved table locations: events_table/persons_table = <schema>.<override-or-derived>, data_imports_schema = override-or-<schema>_data_imports. The derivation lives once, in resolveTeamTables; legacy overrides are bare identifiers (never schema-qualified), now enforced at every write surface by the new ValidateOrgTeamTableName (provisioning upsert + admin PUT), with the admin UI placeholders corrected — they previously taught the qualified form the contract forbids.

enabled is passed through as information only. It is the per-team query-serving switch (migration 000024's definition, not yet enforced on the serve path), not an ingestion signal — disabled teams stay in both endpoints. Deriving "stop ingesting" from "stop serving queries" would turn a serving hold into permanent event loss; the only ingestion-stop signal is row absence. is_billing_team/backfill_enabled are deliberately not served (billing internals / not a stream-start input; adding later is wire-compatible).

Error contract (the load-bearing part)

Transient store failures fail the whole request — a polling consumer keeps its last-known-good state; a 200 with a team silently absent is never served (after consumer-side damping that would silently stop a team's ingestion). Only a warehouse with zero team rows degrades, per-warehouse, to an empty teams array, logged and counted on duckgres_discovery_broken_team_rows_total{reason}. Both endpoints project from one shared assembly.

config_generation is an opaque change token: max updated_at across all warehouse+org+org-team rows regardless of state, read before the data queries. Compare for equality only.

Bugs found in merged code, fixed here (postgres regression tests for each)

  • Creating a team with enabled:false stored TRUE on both the provisioning upsert and admin create paths — gorm omits zero-valued default-tagged fields from INSERTs, so the DB default won; the admin path additionally had gorm's RETURNING write-back masking the caller's intent. Proven by mutation against real postgres.
  • Deleting a non-billing team was invisible to the change token (a plain DELETE leaves no updated_at behind) — DeleteOrgTeamTx now touches the parent org row (DB-clock now()) in the same transaction; the coupling is documented on Org.UpdatedAt and LatestConfigChange and pinned by TestLatestConfigChangeCoversTeamsPostgres.

Known gap (documented in code, asserted as-is in the harness)

cnpg-shard rows carry only the metadata-store kind — endpoint/db/user + SecretRef have no production writer; truth lives in the Duckling CR status until the provisioner backfills the row on Ready (follow-up: sync in the provisioner watch, guarded to state == ready so the reshard runner keeps exclusive column ownership mid-flip). Team-level consumers are unaffected.

Testing

Authored with an agent; three adversarial review rounds (lead-QE + principal-SWE agents ×2, then a scoped final pass over the fix-round code) — findings incorporated include the error-channel split, the resolved-name contract, the state-classification tripwire (TestDiscoveryStateClassification: the state enum is open, and an unclassified new state would read as fleet-wide tenant removal), the two merged-code bugs above, and a clock-source fix (gorm.Expr("now()") vs CP-pod time) on the generation touch. Checks actually run:

  • just test-controlplane, just test-configstore-integration (docker postgres), just test-controlplane-k8s — all pass
  • golangci-lint 0 issues; gofmt clean; harness bash -n clean
  • New postgres-backed tests: create-disabled (both surfaces), batch-fetch ordering, generation advance on team update and delete, qualified-override rejection
  • e2e: discovery_endpoints assertion in tests/mw-dev/e2e/harness.sh (billing team with team_<id> schema + resolved names, ext metadata round-trip, cnpg kind-only, no password-ish keys, bare sorted team-ids array) — runs in the per-PR mw-dev job
  • UI change is placeholder-text only; no test asserts placeholders

Follow-ups

  • Provisioner CR-status→row backfill for cnpg connection details.
  • CLAUDE.md gained a Discovery Endpoints section per the repo's doc-sync rule.

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown

Test Impact Plan

Deterministic summary of how this PR changes tests, CI runners, and coverage-risk signals.

Summary

Area Added Changed Deleted
Test files 1 5 0
E2E/journey files 0 0 0
Workflow files 0 0 0

Signals

  • Test cases: +21 / -0
  • Assertions: +81 / -0
  • Skips or known failures added: 0
  • Workflow continue-on-error added: 0
  • Workflow path filters added: 0
  • Test commands removed from justfile: 0
  • E2E/journey retry lines added: 0

Coverage risk: neutral or increased

No coverage-reduction warnings detected.

@jghoman
jghoman marked this pull request as draft July 17, 2026 16:28
Two read-only endpoints on the internal provisioning API, the
machine-facing 'which tenants exist and where do I write' surface for
external writers (viaduck destination discovery, millpond's
include-values source):

- GET /warehouses: per ready/resharding warehouse — org_id, duckling
  name, state, writable fence flag (false while resharding; a
  resharding warehouse is LISTED, not hidden — hiding reads as tenant
  removal downstream), teams, metadata-store connection with a k8s
  SecretRef (no plaintext credentials in the payload), bucket, and
  config_generation (an opaque change token: max updated_at over ALL
  warehouse+org+org-team rows regardless of state, read BEFORE the
  data so the stamp is never newer than the rows).
- GET /warehouse-team-ids: bare sorted deduped array of team ids —
  the values-only shape millpond's generic include-list poller
  consumes. (Not nested under /warehouses/: org ids are free-form
  slugs, so a static child would squat the namespace a per-org
  GET /warehouses/:org_id needs later.)

Teams come from duckgres_org_teams with RESOLVED table locations:
events_table/persons_table = <schema>.<override-or-derived>,
data_imports_schema = override-or-<schema>_data_imports. The
derivation lives once (resolveTeamTables); legacy overrides are BARE
identifiers, now enforced at every write surface by the new
ValidateOrgTeamTableName (provisioning upsert + admin PUT), with the
admin UI placeholders corrected to teach the bare form. enabled is
passed through as information only — it is the per-team query-serving
switch (migration 000024, not yet enforced), NOT an ingestion signal;
disabled teams stay in BOTH endpoints, and the only ingestion-stop
signal is row absence.

Error contract: transient store failures fail the WHOLE request (a
polling consumer keeps last-known-good — the safe direction); only a
warehouse with zero team rows degrades, per-warehouse, to an empty
teams array, counted on duckgres_discovery_broken_team_rows_total.

Bugs found and fixed along the way (postgres regression tests each):
- creating a team with enabled=false stored TRUE on BOTH the
  provisioning upsert and admin create paths (gorm omits zero-valued
  default-tagged fields from INSERTs; the admin path additionally had
  gorm's RETURNING write-back masking the intent)
- deleting a non-billing team was invisible to the change token (a
  plain DELETE bumps no updated_at) — DeleteOrgTeamTx now touches the
  parent org row (DB-clock now()) in the same transaction

Also: ListWarehousesByStates gains ORDER BY org_id, configstore gains
ListOrgTeamsByOrgIDs + LatestConfigChange, state-classification
tripwire test (an unclassified new state would read as fleet-wide
removal), e2e harness discovery_endpoints assertion, CLAUDE.md
discovery section.
@jghoman
jghoman force-pushed the jakob/discovery-endpoints branch from 173d246 to 679ad04 Compare July 21, 2026 14:17
@jghoman
jghoman marked this pull request as ready for review July 21, 2026 14:18
@jghoman
jghoman merged commit e7af3ca into main Jul 21, 2026
29 checks passed
@jghoman
jghoman deleted the jakob/discovery-endpoints branch July 21, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant