diff --git a/.env b/.env new file mode 100644 index 0000000..83e1103 --- /dev/null +++ b/.env @@ -0,0 +1,10 @@ +# Selects which query-serving binary docker-compose.yml runs by +# default: "single-tenant" (api, Phase 0-3 behavior) or "enterprise" +# (enterprise-api, Phase 4 multi-tenant) -- mutually exclusive, the same +# choice Helm's enterprise.enabled flag makes for a real cluster +# (deploy/helm/sentry/templates/api.yaml vs enterprise-api.yaml). See +# the api/enterprise-api service definitions in docker-compose.yml. +# +# Override per-invocation without editing this file: +# COMPOSE_PROFILES=enterprise docker compose up +COMPOSE_PROFILES=single-tenant diff --git a/CLAUDE.md b/CLAUDE.md index 5bba796..836cc83 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -190,14 +190,20 @@ the fix verified Docker-free (`chrunner_test.go`'s and `searchclient_test.go`'s `TestSearchRefusesMidProvisioningTenant`-shaped tests) — see `api/queryapi/tenant_isolation_gap_test.go` for the full accounting of all four probes, now all closed. The deployment- -topology gap that briefly was the largest one is now closed for Helm: -`deploy/helm/sentry/templates/api.yaml`/`enterprise-api.yaml` are -mutually exclusive on the same `enterprise.enabled` flag that turns on -RBAC/audit/SSO, rendering to the same Service name/port either way — a -Helm-deployed cluster can't accidentally run the wrong one. -`docker-compose.yml` still runs plain `api` unconditionally, though -(local/dev parity with the Helm chart's enforcement is real remaining -work). Per-resource dashboard grants (the RBAC matrix's "(own/granted)" +topology gap that briefly was the largest one is now closed for both +Helm and docker-compose: `deploy/helm/sentry/templates/api.yaml`/ +`enterprise-api.yaml` are mutually exclusive on the same +`enterprise.enabled` flag that turns on RBAC/audit/SSO, rendering to the +same Service name/port either way — a Helm-deployed cluster can't +accidentally run the wrong one. `docker-compose.yml`'s `api`/ +`enterprise-api` services are now the same mutually-exclusive choice, +gated behind `COMPOSE_PROFILES` (`.env` checks in `single-tenant` as the +zero-config default) and sharing a host port/network-alias trick so +`alerting`/`web` need no conditional logic either way — verified via +`docker compose config` (renders/validates without a daemon, confirms +the two never both appear for one profile selection), not an actual +`docker compose up` in this environment. Per-resource dashboard grants +(the RBAC matrix's "(own/granted)" qualifier) are now enforced too: `api/dashboards.PermissionStore` (core interface) implemented by `enterprise/internal/rbacstore. DashboardPermissions`, wired in only by `enterprise-api` — an Editor can diff --git a/docker-compose.yml b/docker-compose.yml index 9f656fb..7d4b469 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -180,7 +180,17 @@ services: volumes: - search-index-data:/var/lib/sentry-search + # Mutually exclusive with enterprise-api below, same choice Helm makes + # via enterprise.enabled (deploy/helm/sentry/templates/api.yaml vs + # enterprise-api.yaml) -- selected by the COMPOSE_PROFILES value in + # .env (checked in as "single-tenant", the zero-config default) or an + # override on the command line, e.g. `COMPOSE_PROFILES=enterprise + # docker compose up`. `docker compose run api ...` (as the manual RBAC + # testing steps in enterprise/README.md/phase-4-runbook.md §4 use) + # still works regardless of the active profile -- an explicit service + # reference on the command line bypasses profile filtering. api: + profiles: ["single-tenant"] build: context: . # needs both api/ and proto/ (gRPC client to search) dockerfile: api/Dockerfile @@ -220,8 +230,17 @@ services: depends_on: metadata-migrate: condition: service_completed_successfully + # Both optional (required: false): whichever of api/enterprise-api + # is actually in the active profile set is the one this waits on + # -- the other isn't defined for this run at all, and without + # `required: false` compose would error on the inactive one rather + # than just skipping it. See api's doc comment above. api: condition: service_healthy + required: false + enterprise-api: + condition: service_healthy + required: false ports: - "8081:8081" environment: @@ -229,6 +248,10 @@ services: POSTGRES_DATABASE: "sentry_metadata" POSTGRES_USERNAME: "sentry" POSTGRES_PASSWORD: "sentry-dev-only" + # Resolves to whichever of api/enterprise-api is actually active -- + # enterprise-api declares a `default.aliases: [api]` network alias + # below specifically so this never needs to change based on which + # profile is selected. API_QUERY_URL: "http://api:8080" healthcheck: test: ["CMD", "/alerting", "-healthcheck"] @@ -283,15 +306,21 @@ services: # Multi-tenant-aware alternative to `api` (Phase 4) -- see # enterprise/cmd/enterprise-api/main.go's doc comment for why this is - # a second binary rather than a flag on `api`. NOT part of the default - # traffic path: `web`'s VITE_API_BASE_URL still points at `api` - # (localhost:8080), and nothing here provisions any tenants (see that - # binary's -provision-tenant flag) -- included so it can be - # built/run/curled directly, same "available, not defaulted in" shape - # as enterprise-auth above. CLICKHOUSE_ADMIN_USERNAME/PASSWORD reuse - # the same admin credential `clickhouse-migrate` uses, since - # tenantprovision needs access_management, not a tenant-scoped grant. + # a second binary rather than a flag on `api`. Mutually exclusive with + # `api` above via COMPOSE_PROFILES (see that service's doc comment); + # when the "enterprise" profile is active this replaces `api` in the + # traffic path transparently, same as Helm: HTTP_LISTEN_ADDR is + # overridden to :8080 (this binary's own default is :8083) and the + # `default.aliases` entry below makes this reachable at the hostname + # `api` too, so alerting's API_QUERY_URL and web's VITE_API_BASE_URL + # need zero conditional logic -- whichever binary is actually running + # transparently answers on the same name/port either way. Nothing here + # provisions any tenants on its own (see -provision-tenant). + # CLICKHOUSE_ADMIN_USERNAME/PASSWORD reuse the same admin credential + # `clickhouse-migrate` uses, since tenantprovision needs + # access_management, not a tenant-scoped grant. enterprise-api: + profiles: ["enterprise"] build: context: . dockerfile: enterprise/cmd/enterprise-api/Dockerfile @@ -301,9 +330,14 @@ services: condition: service_completed_successfully metadata-migrate: condition: service_completed_successfully + networks: + default: + aliases: + - api ports: - - "8083:8083" + - "8080:8080" environment: + HTTP_LISTEN_ADDR: ":8080" CLICKHOUSE_ADDR: "clickhouse:9000" CLICKHOUSE_ADMIN_USERNAME: "default" CLICKHOUSE_ADMIN_PASSWORD: "sentry-dev-only" @@ -329,14 +363,26 @@ services: # localhost:8080/8081 -- fetched from the *browser*, which # resolves against the host's mapped ports, not the compose # network's service DNS names. + # localhost:8080 works unchanged regardless of which profile is + # active -- enterprise-api maps the same host port api does when + # it's the one running (see that service's doc comment). VITE_API_BASE_URL: "http://localhost:8080" VITE_ALERTING_API_BASE_URL: "http://localhost:8081" VITE_ENTERPRISE_AUTH_BASE_URL: "http://localhost:8082" container_name: sentry-web depends_on: - - api - - alerting - - enterprise-auth + # api/enterprise-api optional, same reasoning as alerting's + # depends_on above -- only one is ever in the active profile set. + api: + condition: service_started + required: false + enterprise-api: + condition: service_started + required: false + alerting: + condition: service_started + enterprise-auth: + condition: service_started ports: - "3000:3000" diff --git a/docs/phase-4-runbook.md b/docs/phase-4-runbook.md index d9516c4..a00ad99 100644 --- a/docs/phase-4-runbook.md +++ b/docs/phase-4-runbook.md @@ -329,17 +329,25 @@ close the ClickHouse half of the headline gap §"Known gaps" below used to describe as completely unbuilt. It's still a second binary you have to choose to run, though — see `/docs/security/threat-model.md`'s "Read this first" section. With OIDC login now built (§3a), a real -`curl -X POST http://localhost:8083/query` walkthrough as a logged-in +`curl -X POST http://localhost:8080/query` walkthrough as a logged-in tenant is *possible* now, but still needs the manual `tenant_memberships` bootstrap from §3a — a full end-to-end curl walkthrough isn't included here yet. +`api`/`enterprise-api` are now mutually exclusive via `COMPOSE_PROFILES` +(checked in as `single-tenant` in `.env`, i.e. plain `api` runs by +default) — see §10a below for why, and confirmation that it actually +holds. `-provision-tenant` itself doesn't bind a port, so it runs fine +regardless of the active profile; actually serving traffic on +`enterprise-api` needs the `enterprise` profile active, since it now +binds the same host port (8080) plain `api` does: + ```sh -docker compose build enterprise-api -docker compose run --rm enterprise-api -provision-tenant=acme -display-name="Acme Corp" -docker compose run --rm enterprise-api -provision-tenant=globex -display-name="Globex Corporation" -docker compose up -d enterprise-api -curl -s http://localhost:8083/healthz +COMPOSE_PROFILES=enterprise docker compose build enterprise-api +COMPOSE_PROFILES=enterprise docker compose run --rm enterprise-api -provision-tenant=acme -display-name="Acme Corp" +COMPOSE_PROFILES=enterprise docker compose run --rm enterprise-api -provision-tenant=globex -display-name="Globex Corporation" +COMPOSE_PROFILES=enterprise docker compose up -d enterprise-api +curl -s http://localhost:8080/healthz ``` Confirm isolation end to end against the live stack (this is the same @@ -436,18 +444,56 @@ each set of values and confirm `kubectl get deploy sentry-api -o jsonpath='{.spec.template.spec.containers[0].image}'` matches, and that `kubectl get svc sentry-api` routes to whichever one is actually running. +## 10a. Confirm `docker-compose.yml` now enforces the same binary swap + +Local/dev parity with §10 above was a named gap ("`docker-compose.yml` +still runs plain `api` unconditionally") -- closed via `COMPOSE_PROFILES` +(`api`/`enterprise-api` are each gated behind a profile, `.env` checks in +`single-tenant` as the zero-config default) plus the same "same host +port, `enterprise-api` gets a `default.aliases: [api]` network alias" +trick §10's Helm chart uses at the Service-name level. Verified in this +environment via `docker compose config` (no daemon needed -- it renders +and validates the merged YAML without starting anything): + +```sh +docker compose config --quiet && echo "config is valid" + +# exactly one of api/enterprise-api per profile, never both or neither: +docker compose config --services +# expect: ... api ... (no enterprise-api) +COMPOSE_PROFILES=enterprise docker compose config --services +# expect: ... enterprise-api ... (no api) + +# enterprise-api really does take over api's name/port when active: +COMPOSE_PROFILES=enterprise docker compose config \ + | python3 -c "import yaml,sys,json; d=yaml.safe_load(sys.stdin)['services']['enterprise-api']; print(json.dumps({'ports': d['ports'], 'aliases': d['networks']['default']['aliases'], 'HTTP_LISTEN_ADDR': d['environment']['HTTP_LISTEN_ADDR']}, indent=2))" +# expect port 8080 (not enterprise-api's own default 8083), alias +# ["api"], and HTTP_LISTEN_ADDR ":8080" +``` + +`docker compose run`/`build enterprise-api` (§8's provisioning steps) +work regardless of the active profile -- explicit service references on +the command line bypass profile filtering, confirmed in this +environment (the commands got past client-side profile resolution and +failed only on `permission denied ... docker.sock`, this environment's +already-disclosed no-Docker-daemon-access limitation, not a +profile-related error). **Not verified**: an actual `docker compose up` +against a real daemon in this environment — the `config` rendering above +proves the compose file's *shape* is correct, not that containers +actually start and route traffic correctly end to end. + ## Known gaps (do not treat this phase as done without reading these) Full accounting: `/docs/security/threat-model.md`. Headline items: -- **Both storage engines' isolation exists, and the Helm chart now - enforces which binary runs.** `deploy/helm/sentry/templates/api.yaml`/ - `enterprise-api.yaml` are mutually exclusive on `enterprise.enabled` - (§10) -- a Helm-deployed cluster can't accidentally run the - non-isolated binary once that flag is set. `docker-compose.yml` still - runs plain `api` unconditionally alongside a separately-started - `enterprise-api` (§8), so this enforcement doesn't extend to local/dev - yet. +- **Both storage engines' isolation exists, and both Helm and + docker-compose now enforce which binary runs.** + `deploy/helm/sentry/templates/api.yaml`/`enterprise-api.yaml` are + mutually exclusive on `enterprise.enabled` (§10) -- a Helm-deployed + cluster can't accidentally run the non-isolated binary once that flag + is set. `docker-compose.yml`'s `api`/`enterprise-api` services are now + the same mutually-exclusive choice via `COMPOSE_PROFILES` (§8, §10a), + closing the local/dev parity gap this bullet used to name. - The `Tenant` CRD (`deploy/operator`) and `enterprise-api -provision-tenant` are still two independent provisioning mechanisms -- running both for the same tenant ID today takes two separate diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index b7d9329..c613df3 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -44,20 +44,26 @@ searchclient`'s tests run a real in-process gRPC server and confirm the wire-level `SearchRequest` carries the right `tenant_id`. All pass, for real, no disclaimer needed for this specific claim. -**The Helm chart now closes this for K8s deployments; `docker-compose.yml` -still doesn't.** `deploy/helm/sentry/templates/api.yaml` and -`enterprise-api.yaml` are mutually exclusive, gated on opposite sides of -the same `enterprise.enabled` flag, rendering to the same Service -name/port — so a Helm-deployed cluster runs exactly one of the two -binaries, chosen by the same flag that turns on RBAC/audit/SSO, not a -second independently-forgettable decision. Verified by parsing (not +**Both Helm and docker-compose now close this.** +`deploy/helm/sentry/templates/api.yaml` and `enterprise-api.yaml` are +mutually exclusive, gated on opposite sides of the same +`enterprise.enabled` flag, rendering to the same Service name/port — so +a Helm-deployed cluster runs exactly one of the two binaries, chosen by +the same flag that turns on RBAC/audit/SSO, not a second +independently-forgettable decision. Verified by parsing (not eyeballing) the rendered YAML under both values: exactly one `sentry-api` -Deployment either way, with the right image. **`docker-compose.yml` -still runs plain `api` unconditionally** and includes `enterprise-api` -as an extra, separately-started service — local/dev parity with the Helm -chart's enforcement is real remaining work. And this only constrains -*deployment*, not *operation*: nothing stops an operator from manually -running plain `api`'s image against a cluster that has tenants +Deployment either way, with the right image. `docker-compose.yml`'s +`api`/`enterprise-api` services are now the analogous mutually-exclusive +choice, gated behind `COMPOSE_PROFILES` (`.env` checks in +`single-tenant`, i.e. plain `api`, as the zero-config default) and +sharing the same host port/network-alias trick to stay transparent to +`alerting`/`web` either way — verified via `docker compose config` +(renders and validates the merged YAML without a daemon; confirms +`api`/`enterprise-api` never both appear in `--services` output for the +same profile selection) — see `/docs/phase-4-runbook.md` §10a. This +still only constrains *deployment*, not *operation*: nothing stops an +operator from manually running plain `api`'s image against a cluster +(or compose project) that has tenants provisioned, pointing at the same ClickHouse/Postgres. The Helm chart makes the *default*, chart-managed path correct; it isn't a runtime guard against misconfiguration. @@ -404,7 +410,7 @@ terms: | Tantivy tenant_id resolution (`enterprise/internal/searchclient`) | **Enforced, verified live** — real gRPC wire-level test | | Ingest tenant-awareness (ClickHouse and Tantivy both) | **Not implemented, undesigned** — every ingested record lands in the single shared database/index regardless of tenant | | Deployment actually routing traffic to `enterprise-api` (Helm) | **Enforced** — `api`/`enterprise-api` are mutually exclusive, same flag as RBAC/audit/SSO | -| Deployment actually routing traffic to `enterprise-api` (docker-compose) | **Not implemented** — `docker-compose.yml` runs plain `api` unconditionally | +| Deployment actually routing traffic to `enterprise-api` (docker-compose) | **Enforced** — `api`/`enterprise-api` are mutually exclusive via `COMPOSE_PROFILES`, same flag choice as Helm's `enterprise.enabled`; verified via `docker compose config`, not an actual `docker compose up` in this environment | | Human SSO login — OIDC | **Built, verified with a real fake IdP** (not yet tried against a real external IdP) | | Human SSO login — SAML | **Built, verified with a real fake IdP** (not yet tried against a real external IdP) | | Multi-tenant-membership login (tenant picker) | **Not implemented** — refused with a clear error, not guessed | diff --git a/enterprise/README.md b/enterprise/README.md index 872e5a2..ae2bd44 100644 --- a/enterprise/README.md +++ b/enterprise/README.md @@ -284,21 +284,29 @@ membership, listing a tenant's members, or a flag for ## Provisioning a tenant and running `enterprise-api` +`api`/`enterprise-api` are mutually exclusive in `docker-compose.yml`, +gated behind `COMPOSE_PROFILES` (`.env` checks in `single-tenant`, i.e. +plain `api`, as the zero-config default -- mirrors Helm's +`enterprise.enabled` flag). `-provision-tenant` itself doesn't bind a +port, so it runs fine regardless of the active profile; actually +serving traffic on `enterprise-api` needs the `enterprise` profile +active, since it now binds the same host port (8080) plain `api` does +(it gets a `default.aliases: [api]` network alias too, so `alerting`'s +`API_QUERY_URL`/`web`'s `VITE_API_BASE_URL` need zero changes either +way): + ```sh -docker compose build enterprise-api # context is the repo root, not enterprise/ -- see cmd/enterprise-api/Dockerfile -docker compose run --rm enterprise-api -provision-tenant=acme -display-name="Acme Corp" -docker compose up -d enterprise-api -curl -s http://localhost:8083/healthz +COMPOSE_PROFILES=enterprise docker compose build enterprise-api # context is the repo root, not enterprise/ -- see cmd/enterprise-api/Dockerfile +COMPOSE_PROFILES=enterprise docker compose run --rm enterprise-api -provision-tenant=acme -display-name="Acme Corp" +COMPOSE_PROFILES=enterprise docker compose up -d enterprise-api +curl -s http://localhost:8080/healthz ``` `-provision-tenant` creates the tenant/data_source rows in rbacstore if they don't exist, provisions ClickHouse, persists the credentials, and marks the tenant active -- refuses to run twice for the same tenant (re-provisioning would either rotate a live credential or silently fail -to, see `tenantprovision.ProvisionClickHouse`'s doc comment). `web` -still points at plain `api` by default (`VITE_API_BASE_URL`) -- -pointing it at `enterprise-api` instead is a manual `docker-compose.yml` -edit today, not a supported flag. +to, see `tenantprovision.ProvisionClickHouse`'s doc comment). ## Environment variables (`enterprise-auth`)