Build per-tenant ClickHouse write-routing for ingest (Tantivy still deferred)
ingest tags every record with a tenant_id Kafka header (built previously), but nothing consumed it to actually route the write. This closes that for ClickHouse: enterprise/cmd/enterprise-ingest (a second binary, mirroring enterprise-api) reuses ingest/consumer's own flush loop unchanged, with enterprise/internal/chwriter.Registry -- a per-tenant clickhousewriter.Writer registry -- swapped in as the writer. A batch pulled from the single shared Redpanda topic can mix records from many tenants, so WriteBatch groups by TenantID and dispatches each group to its own tenant's connection, fail- closed on an empty or unrecognized tenant_id. ingest/consumer and ingest/clickhousewriter move out of internal/ (same reason api/internal/* moved earlier this phase: enterprise/ can't import anything under another module's internal/). Their New() constructors now take small local Config structs instead of ingest/internal/config types, so enterprise/ doesn't need that import either. Building this surfaced a real bug: tenantprovision.ProvisionClickHouse only granted SELECT on a tenant's ClickHouse user, correct for chrunner's read-only use but not enough for chwriter reusing the same credential to write -- every real per-tenant write would have failed closed with a permission error. Fixed by widening the grant to SELECT, INSERT; no cross-tenant boundary is crossed by also allowing INSERT within a tenant's own database. Helm gates enterprise-ingest's Deployment on the same ingest.requireTenantCredential flag that already gates tag validation -- write-routing is meaningless without tagging already being required, so they're one decision, not two. docker-compose.yml's version is a disclosed, weaker approximation: it can't achieve Helm's genuine -mode=server/-mode=consumer split, so with the enterprise profile active both ingest and enterprise-ingest independently consume every message via different consumer groups -- harmless duplication for local verification only. Not built: Tantivy's independent Redpanda consumer (search/src/consumer.rs) still doesn't read the tenant_id header at all -- every record still lands in the one shared index regardless of tenant. Not run: the live-ClickHouse- gated tests (chwriter's cross-tenant routing test, tenantprovision's INSERT regression test) -- no Docker/database access in this environment; they're correct Go that has never executed, disclosed as such in docs/security/ threat-model.md and docs/phase-4-runbook.md §14.
This commit is contained in:
@@ -0,0 +1,125 @@
|
||||
// Package chwriter is enterprise/internal/chrunner's write-side
|
||||
// counterpart -- the tenant-scoped implementation ingest/consumer.
|
||||
// Consumer needs to route each batch's records into their own tenant's
|
||||
// dedicated ClickHouse database, instead of the one shared table
|
||||
// ingest/cmd/ingest's single-tenant mode always writes to. Requires
|
||||
// importing ingest/clickhousewriter and ingest/consumer directly (see
|
||||
// enterprise/go.mod's replace directive) -- same allowed
|
||||
// "enterprise -> core" import direction chrunner uses for
|
||||
// api/querylang/executor, just against a different core module.
|
||||
//
|
||||
// Design mirrors chrunner.Registry closely: one fully separate
|
||||
// *clickhousewriter.Writer (and the driver.Conn under it) per tenant,
|
||||
// built once at construction from an immutable map -- never a shared
|
||||
// pool with session-level USE, for the same concurrency reasons
|
||||
// chrunner's doc comment explains. The one real difference:
|
||||
// chrunner.RunSQL resolves exactly one tenant per call from ctx (a
|
||||
// single request always belongs to one identity); WriteBatch resolves
|
||||
// per *record*, since one Kafka batch pulled off the shared
|
||||
// sentry.logs.raw topic can freely mix records from many different
|
||||
// tenants -- see ingest/internal/grpcserver's doc comment for why
|
||||
// there's one shared topic, not topic-per-tenant.
|
||||
package chwriter
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"github.com/sentry/sentry/ingest/clickhousewriter"
|
||||
"github.com/sentry/sentry/ingest/consumer"
|
||||
logsv1 "github.com/sentry/sentry/proto/sentry/logs/v1"
|
||||
)
|
||||
|
||||
// DataSource mirrors chrunner.DataSource -- deliberately not
|
||||
// enterprise/internal/rbacstore.DataSource itself, so this package
|
||||
// doesn't need to import rbacstore just to describe "an address and a
|
||||
// credential." Callers (enterprise-ingest's main.go) adapt rbacstore
|
||||
// rows into this.
|
||||
type DataSource struct {
|
||||
TenantID string
|
||||
Database string
|
||||
Username string
|
||||
Password string
|
||||
}
|
||||
|
||||
// Registry implements ingest/consumer's chWriter interface
|
||||
// (WriteBatch(ctx, []consumer.Record) error) by routing each record to
|
||||
// its tenant's dedicated connection. Immutable after New returns -- see
|
||||
// this file's doc comment.
|
||||
type Registry struct {
|
||||
writers map[string]*clickhousewriter.Writer
|
||||
closers []func()
|
||||
}
|
||||
|
||||
// New opens one real ClickHouse connection per DataSource (same native
|
||||
// address for all of them, different per-tenant credentials -- tenants
|
||||
// sharing a physical ClickHouse server today, same as chrunner). Fails
|
||||
// closed: if any one tenant's connection can't be opened, the whole
|
||||
// Registry fails to construct rather than silently running with a
|
||||
// partial tenant set.
|
||||
func New(ctx context.Context, addr string, sources []DataSource) (*Registry, error) {
|
||||
reg := &Registry{writers: make(map[string]*clickhousewriter.Writer, len(sources))}
|
||||
for _, src := range sources {
|
||||
w, err := clickhousewriter.New(ctx, clickhousewriter.Config{
|
||||
Addr: addr, Database: src.Database, Username: src.Username, Password: src.Password,
|
||||
})
|
||||
if err != nil {
|
||||
reg.Close()
|
||||
return nil, fmt.Errorf("chwriter: opening connection for tenant %q: %w", src.TenantID, err)
|
||||
}
|
||||
reg.writers[src.TenantID] = w
|
||||
reg.closers = append(reg.closers, func() { _ = w.Close() })
|
||||
}
|
||||
return reg, nil
|
||||
}
|
||||
|
||||
// Close releases every underlying connection -- call once at process
|
||||
// shutdown, same lifecycle as chrunner.Registry.Close.
|
||||
func (r *Registry) Close() {
|
||||
for _, c := range r.closers {
|
||||
c()
|
||||
}
|
||||
}
|
||||
|
||||
// WriteBatch implements ingest/consumer's chWriter interface. Groups
|
||||
// records by TenantID and writes each tenant's group through its own
|
||||
// dedicated connection -- fails the *whole* call (matching
|
||||
// ingest/consumer's existing all-or-nothing batch contract: a failed
|
||||
// WriteBatch means no offsets are committed and the entire batch is
|
||||
// redelivered, never partial credit) if any record's tenant is empty
|
||||
// (no TenantResolver was configured for the PushBatch call that
|
||||
// produced it -- a multi-tenant deployment must never silently write an
|
||||
// untagged record somewhere) or unrecognized (not yet provisioned, or
|
||||
// provisioning failed). Fail closed, same reasoning
|
||||
// chrunner.Registry.RunSQL's doc comment gives for the read side.
|
||||
//
|
||||
// A permanently-unprovisioned or permanently-mistagged tenant would
|
||||
// stall this consumer's offset progress entirely (every redelivery of
|
||||
// that batch fails the same way) -- a real, disclosed limitation of
|
||||
// reusing ingest/consumer's existing all-or-nothing contract rather
|
||||
// than building new partial-batch-success semantics nothing else in
|
||||
// this codebase has either. See /docs/phase-4-runbook.md.
|
||||
func (r *Registry) WriteBatch(ctx context.Context, records []consumer.Record) error {
|
||||
byTenant := make(map[string][]consumer.Record, len(records))
|
||||
for _, rec := range records {
|
||||
byTenant[rec.TenantID] = append(byTenant[rec.TenantID], rec)
|
||||
}
|
||||
|
||||
for tenantID, group := range byTenant {
|
||||
if tenantID == "" {
|
||||
return fmt.Errorf("chwriter: %d record(s) in this batch have no tenant_id, refusing to write any of it", len(group))
|
||||
}
|
||||
writer, ok := r.writers[tenantID]
|
||||
if !ok {
|
||||
return fmt.Errorf("chwriter: tenant %q has no provisioned ClickHouse connection, refusing to write %d record(s)", tenantID, len(group))
|
||||
}
|
||||
plain := make([]*logsv1.LogRecord, len(group))
|
||||
for i, rec := range group {
|
||||
plain[i] = rec.Record
|
||||
}
|
||||
if err := writer.WriteBatch(ctx, plain); err != nil {
|
||||
return fmt.Errorf("chwriter: writing batch for tenant %q: %w", tenantID, err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,157 @@
|
||||
// Fail-closed behavior (empty/unknown tenant_id) needs no live
|
||||
// ClickHouse at all -- Registry.WriteBatch returns before ever touching
|
||||
// a connection for those cases, so those tests run unconditionally.
|
||||
// Everything that actually writes data is a real integration test
|
||||
// against a live ClickHouse (same CHWRITER_TEST_CLICKHOUSE_ADDR
|
||||
// convention as enterprise/internal/chrunner's own tests), skipped
|
||||
// unless that's set; run via:
|
||||
//
|
||||
// docker run --rm --network sentry_default -v $(pwd)/../../..:/src -w /src/enterprise \
|
||||
// -e CHWRITER_TEST_CLICKHOUSE_ADDR=clickhouse:9000 \
|
||||
// -e CHWRITER_TEST_CLICKHOUSE_PASSWORD=sentry-dev-only \
|
||||
// golang:1.25-alpine go test ./internal/chwriter/... -v
|
||||
package chwriter
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
chdriver "github.com/ClickHouse/clickhouse-go/v2"
|
||||
"github.com/google/uuid"
|
||||
|
||||
"github.com/sentry/sentry/enterprise/internal/tenantprovision"
|
||||
"github.com/sentry/sentry/ingest/clickhousewriter"
|
||||
"github.com/sentry/sentry/ingest/consumer"
|
||||
logsv1 "github.com/sentry/sentry/proto/sentry/logs/v1"
|
||||
)
|
||||
|
||||
// TestWriteBatchRefusesEmptyTenantID and
|
||||
// TestWriteBatchRefusesUnknownTenantWithEmptyRegistry construct a
|
||||
// Registry directly (bypassing New, which would dial ClickHouse) so
|
||||
// they genuinely run without Docker: WriteBatch's fail-closed checks
|
||||
// happen before ever touching a real connection, purely a map lookup.
|
||||
func TestWriteBatchRefusesEmptyTenantID(t *testing.T) {
|
||||
reg := &Registry{writers: map[string]*clickhousewriter.Writer{}}
|
||||
err := reg.WriteBatch(context.Background(), []consumer.Record{
|
||||
{TenantID: "", Record: &logsv1.LogRecord{Message: "untagged"}},
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected WriteBatch to refuse a record with no tenant_id, not silently drop the tag")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWriteBatchRefusesUnknownTenantWithEmptyRegistry(t *testing.T) {
|
||||
reg := &Registry{writers: map[string]*clickhousewriter.Writer{}}
|
||||
err := reg.WriteBatch(context.Background(), []consumer.Record{
|
||||
{TenantID: "acme", Record: &logsv1.LogRecord{Message: "m"}},
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected WriteBatch to refuse a tenant with no entry in the registry")
|
||||
}
|
||||
}
|
||||
|
||||
func testAddr(t *testing.T) string {
|
||||
t.Helper()
|
||||
addr := os.Getenv("CHWRITER_TEST_CLICKHOUSE_ADDR")
|
||||
if addr == "" {
|
||||
t.Skip("CHWRITER_TEST_CLICKHOUSE_ADDR not set -- skipping live-ClickHouse integration test")
|
||||
}
|
||||
return addr
|
||||
}
|
||||
|
||||
func provisionTestTenant(t *testing.T, addr string) (tenantID string, creds tenantprovision.Credentials) {
|
||||
t.Helper()
|
||||
admin, err := chdriver.Open(&chdriver.Options{
|
||||
Addr: []string{addr},
|
||||
Auth: chdriver.Auth{Database: "default", Username: "default", Password: os.Getenv("CHWRITER_TEST_CLICKHOUSE_PASSWORD")},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("opening admin connection: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { admin.Close() })
|
||||
|
||||
tenantID = "cw" + uuid.NewString()[:8]
|
||||
creds, err = tenantprovision.New(admin).ProvisionClickHouse(context.Background(), tenantID)
|
||||
if err != nil {
|
||||
t.Fatalf("provisioning tenant %s: %v", tenantID, err)
|
||||
}
|
||||
if err := admin.Exec(context.Background(), fmt.Sprintf(
|
||||
"CREATE TABLE `%s`.logs (timestamp DateTime64(9), host String, service String, severity String, message String, attributes Map(String, String), record_id UUID) ENGINE = MergeTree ORDER BY timestamp",
|
||||
tenantID)); err != nil {
|
||||
t.Fatalf("creating logs table for tenant %s: %v", tenantID, err)
|
||||
}
|
||||
return tenantID, creds
|
||||
}
|
||||
|
||||
// TestRegistryWritesEachTenantToItsOwnDatabase is the core adversarial
|
||||
// probe for the write side, complementing chrunner's own read-side
|
||||
// version: two tenants, two connections inside one Registry, one
|
||||
// WriteBatch call mixing records from both, and a direct check (via an
|
||||
// admin connection, not through Registry) that each tenant's row landed
|
||||
// only in its own database.
|
||||
func TestRegistryWritesEachTenantToItsOwnDatabase(t *testing.T) {
|
||||
addr := testAddr(t)
|
||||
ctx := context.Background()
|
||||
tenantA, credsA := provisionTestTenant(t, addr)
|
||||
tenantB, credsB := provisionTestTenant(t, addr)
|
||||
|
||||
reg, err := New(ctx, addr, []DataSource{
|
||||
{TenantID: tenantA, Database: tenantA, Username: credsA.Username, Password: credsA.Password},
|
||||
{TenantID: tenantB, Database: tenantB, Username: credsB.Username, Password: credsB.Password},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
defer reg.Close()
|
||||
|
||||
err = reg.WriteBatch(ctx, []consumer.Record{
|
||||
{TenantID: tenantA, Record: &logsv1.LogRecord{Host: "h1", Message: "for-a", RecordId: uuid.NewString()}},
|
||||
{TenantID: tenantB, Record: &logsv1.LogRecord{Host: "h1", Message: "for-b", RecordId: uuid.NewString()}},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("WriteBatch: %v", err)
|
||||
}
|
||||
|
||||
admin, err := chdriver.Open(&chdriver.Options{
|
||||
Addr: []string{addr},
|
||||
Auth: chdriver.Auth{Database: "default", Username: "default", Password: os.Getenv("CHWRITER_TEST_CLICKHOUSE_PASSWORD")},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("opening admin connection: %v", err)
|
||||
}
|
||||
defer admin.Close()
|
||||
|
||||
for tenantID, wantMessage := range map[string]string{tenantA: "for-a", tenantB: "for-b"} {
|
||||
row := admin.QueryRow(ctx, fmt.Sprintf("SELECT message FROM `%s`.logs", tenantID))
|
||||
var got string
|
||||
if err := row.Scan(&got); err != nil {
|
||||
t.Fatalf("querying %s's logs: %v", tenantID, err)
|
||||
}
|
||||
if got != wantMessage {
|
||||
t.Fatalf("tenant %s's logs.message = %q, want %q", tenantID, got, wantMessage)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegistryRefusesUnprovisionedTenant(t *testing.T) {
|
||||
addr := testAddr(t)
|
||||
ctx := context.Background()
|
||||
tenantA, credsA := provisionTestTenant(t, addr)
|
||||
|
||||
reg, err := New(ctx, addr, []DataSource{
|
||||
{TenantID: tenantA, Database: tenantA, Username: credsA.Username, Password: credsA.Password},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("New: %v", err)
|
||||
}
|
||||
defer reg.Close()
|
||||
|
||||
err = reg.WriteBatch(ctx, []consumer.Record{
|
||||
{TenantID: "some-other-tenant-never-provisioned", Record: &logsv1.LogRecord{Host: "h1", Message: "m", RecordId: uuid.NewString()}},
|
||||
})
|
||||
if err == nil {
|
||||
t.Fatal("expected WriteBatch to refuse a tenant with no provisioned connection, not silently drop or misroute it")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user