Closes a gap named across CLAUDE.md/docs/architecture.md/deploy/README.md
since early Phase 4: the operator's Tenant CRD and -provision-tenant
were two disconnected mechanisms. The operator's reconciler generated a
K8s Secret with a locally-generated random password that authenticated
against nothing (nothing ever called ClickHouse to create a matching
user), and unconditionally claimed status.phase=Active the moment a
Tenant object existed -- actively misleading, not just incomplete.
Two unification shapes were considered (surfaced to the user via
AskUserQuestion, given the real difference in blast radius): the
operator's reconcile loop becoming a second real actor (new Postgres +
ClickHouse admin credentials flowing into the K8s controller, plus real
reconcile-loop idempotency/retry design for an inherently one-shot
external side effect), or keeping -provision-tenant as the sole real
actor and having it also sync its result into the CRD. Went with the
lighter option.
enterprise/internal/tenantcrd (new): a Syncer using the K8s dynamic
client (unstructured.Unstructured + a GroupVersionResource, not
deploy/operator's typed Tenant struct -- avoids a cross-module Go
dependency between two independently-versioned modules for one type).
Upserts the Tenant object, creates/updates a Secret with the *real*
ClickHouse credentials owned by that Tenant via an OwnerReference, then
patches status.{clickHouseDatabaseName,clickHouseSecretRef,
tantivyIndexPath}. Idempotent and safe to retry: never rotates a
credential across a re-sync, never overwrites a pre-existing
spec.displayName a human/GitOps process set.
cmd/enterprise-api/main.go's runProvisionTenant calls Sync when
TENANT_CRD_NAMESPACE is set (empty = no-op, same shape as every other
optional dependency in this codebase). Its "already active" refusal is
now split: ClickHouse re-provisioning is still refused (rotating a live
credential would break every open connection for no benefit), but CR
sync alone is now retryable using the credentials already on file in
rbacstore -- needed for retrying a previously-failed sync, or
backfilling CR sync for a tenant provisioned before this existed.
deploy/operator's reconciler rewritten to match: it never claims
PhaseActive on its own initiative anymore, only once
status.ClickHouseDatabaseName is non-empty (the field -provision-tenant,
and only -provision-tenant, sets). Phase is now a pure function of
{spec.suspended, status.ClickHouseDatabaseName != ""} recomputed every
reconcile, not toggled in place -- fixes a related bug the old code
would have hit once suspension was involved: un-suspending an
already-provisioned tenant needs to return straight to Active, which
isn't derivable from "last observed phase was Suspended" alone. The
reconciler no longer creates or manages any Secret, dropped its
`secrets` RBAC grant entirely, and gained zero new dependencies.
Helm chart: enterprise-api gets its own ServiceAccount/Role/RoleBinding
(get/list/create tenants, get/update/patch tenants/status, get/create/
update secrets -- least-privilege, scoped to the release namespace, not
a ClusterRole) and a TENANT_CRD_NAMESPACE env var, both gated on
tenantOperator.enabled. tenant-operator's ClusterRole loses the
secrets grant it no longer needs.
Verified in this environment: enterprise/internal/tenantcrd's tests run
against k8s.io/client-go's fake dynamic + typed clientsets (real client
library, fake transport, no cluster needed); deploy/operator's rewritten
tenant_controller_test.go runs against controller-runtime's fake
client, including new regression tests for the "must not claim Active
without confirmation" and "un-suspending returns to Active, not
Provisioning" properties; helm template + parsing the rendered YAML
confirms the RBAC split renders exactly as designed under both
tenantOperator.enabled=true/false. Not verified: an actual
-provision-tenant run against a real cluster with the operator watching
(no live cluster in this environment, same disclosed limitation as the
rest of /deploy). Docs updated in lockstep: CLAUDE.md, docs/architecture.md,
deploy/README.md, deploy/helm/sentry/README.md (including a corrected
"Trying the two-tenant example" walkthrough), phase-4-runbook.md (new
§11), enterprise/README.md. Also fixed two unrelated stale claims found
along the way: docs/architecture.md still said docker-compose.yml ran
plain api unconditionally (fixed in an earlier commit, doc not updated
then), and enterprise-api's own main.go doc comment still said Helm/
docker-compose wiring wasn't built yet.
159 lines
6.5 KiB
Go
159 lines
6.5 KiB
Go
// Tests use k8s.io/client-go's fake dynamic and typed clientsets, not a
|
|
// real or in-cluster API server -- genuinely runnable in an environment
|
|
// with no Kubernetes access at all, same "real client library, fake
|
|
// transport" shape as enterprise/internal/searchclient's in-process gRPC
|
|
// tests. What a fake client can't exercise: real admission/defaulting,
|
|
// or that deploy/operator's actual CRD schema accepts what this package
|
|
// writes (see /docs/phase-4-runbook.md's verification-status notes for
|
|
// the Helm/kubeconform-based schema check that covers that instead).
|
|
package tenantcrd
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
|
|
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
|
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
|
|
"k8s.io/apimachinery/pkg/runtime"
|
|
"k8s.io/apimachinery/pkg/runtime/schema"
|
|
dynamicfake "k8s.io/client-go/dynamic/fake"
|
|
k8sfake "k8s.io/client-go/kubernetes/fake"
|
|
)
|
|
|
|
func newTestSyncer(t *testing.T, namespace string) *Syncer {
|
|
t.Helper()
|
|
scheme := runtime.NewScheme()
|
|
listKinds := map[schema.GroupVersionResource]string{tenantGVR: "TenantList"}
|
|
dyn := dynamicfake.NewSimpleDynamicClientWithCustomListKinds(scheme, listKinds)
|
|
clientset := k8sfake.NewSimpleClientset()
|
|
return newForTest(dyn, clientset, namespace)
|
|
}
|
|
|
|
func getTenant(t *testing.T, s *Syncer, tenantID string) *unstructured.Unstructured {
|
|
t.Helper()
|
|
obj, err := s.dynamic.Resource(tenantGVR).Namespace(s.namespace).Get(context.Background(), tenantID, metav1.GetOptions{})
|
|
if err != nil {
|
|
t.Fatalf("getting tenant object: %v", err)
|
|
}
|
|
return obj
|
|
}
|
|
|
|
func TestSyncCreatesTenantObjectWithDisplayName(t *testing.T) {
|
|
s := newTestSyncer(t, "sentry")
|
|
err := s.Sync(context.Background(), "acme", "Acme Corp", "/var/lib/sentry-search/tenants/acme", Credentials{Username: "tenant_acme", Password: "secret-pw"})
|
|
if err != nil {
|
|
t.Fatalf("Sync: %v", err)
|
|
}
|
|
|
|
obj := getTenant(t, s, "acme")
|
|
displayName, _, _ := unstructured.NestedString(obj.Object, "spec", "displayName")
|
|
if displayName != "Acme Corp" {
|
|
t.Fatalf("spec.displayName = %q, want %q", displayName, "Acme Corp")
|
|
}
|
|
}
|
|
|
|
func TestSyncSetsRealStatusFields(t *testing.T) {
|
|
s := newTestSyncer(t, "sentry")
|
|
err := s.Sync(context.Background(), "acme", "Acme Corp", "/var/lib/sentry-search/tenants/acme", Credentials{Username: "tenant_acme", Password: "secret-pw"})
|
|
if err != nil {
|
|
t.Fatalf("Sync: %v", err)
|
|
}
|
|
|
|
obj := getTenant(t, s, "acme")
|
|
dbName, _, _ := unstructured.NestedString(obj.Object, "status", "clickHouseDatabaseName")
|
|
if dbName != "acme" {
|
|
t.Fatalf("status.clickHouseDatabaseName = %q, want acme", dbName)
|
|
}
|
|
secretRef, _, _ := unstructured.NestedString(obj.Object, "status", "clickHouseSecretRef")
|
|
if secretRef != "sentry-tenant-acme-clickhouse" {
|
|
t.Fatalf("status.clickHouseSecretRef = %q, want sentry-tenant-acme-clickhouse", secretRef)
|
|
}
|
|
indexPath, _, _ := unstructured.NestedString(obj.Object, "status", "tantivyIndexPath")
|
|
if indexPath != "/var/lib/sentry-search/tenants/acme" {
|
|
t.Fatalf("status.tantivyIndexPath = %q, want /var/lib/sentry-search/tenants/acme", indexPath)
|
|
}
|
|
}
|
|
|
|
func TestSyncCreatesSecretOwnedByTenant(t *testing.T) {
|
|
s := newTestSyncer(t, "sentry")
|
|
err := s.Sync(context.Background(), "acme", "Acme Corp", "/idx", Credentials{Username: "tenant_acme", Password: "secret-pw"})
|
|
if err != nil {
|
|
t.Fatalf("Sync: %v", err)
|
|
}
|
|
|
|
secret, err := s.clientset.CoreV1().Secrets("sentry").Get(context.Background(), "sentry-tenant-acme-clickhouse", metav1.GetOptions{})
|
|
if err != nil {
|
|
t.Fatalf("getting secret: %v", err)
|
|
}
|
|
if secret.StringData["username"] != "tenant_acme" || secret.StringData["password"] != "secret-pw" || secret.StringData["database"] != "acme" {
|
|
t.Fatalf("unexpected secret data: %+v", secret.StringData)
|
|
}
|
|
if len(secret.OwnerReferences) != 1 || secret.OwnerReferences[0].Name != "acme" || secret.OwnerReferences[0].Kind != "Tenant" {
|
|
t.Fatalf("expected the secret to be owned by the Tenant object, got %+v", secret.OwnerReferences)
|
|
}
|
|
}
|
|
|
|
// TestSyncIsIdempotentAndNeverChangesCredentials is the regression test
|
|
// for the "safe to retry" property runProvisionTenant's idempotent
|
|
// CR-sync-only retry path depends on (see cmd/enterprise-api/main.go):
|
|
// running Sync twice for the same tenant must not create a duplicate
|
|
// Tenant object, must not error, and must never silently swap in
|
|
// different credentials than what was passed.
|
|
func TestSyncIsIdempotentAndNeverChangesCredentials(t *testing.T) {
|
|
s := newTestSyncer(t, "sentry")
|
|
ctx := context.Background()
|
|
creds := Credentials{Username: "tenant_acme", Password: "secret-pw"}
|
|
|
|
if err := s.Sync(ctx, "acme", "Acme Corp", "/idx", creds); err != nil {
|
|
t.Fatalf("first Sync: %v", err)
|
|
}
|
|
if err := s.Sync(ctx, "acme", "Acme Corp", "/idx", creds); err != nil {
|
|
t.Fatalf("second Sync: %v", err)
|
|
}
|
|
|
|
list, err := s.dynamic.Resource(tenantGVR).Namespace("sentry").List(ctx, metav1.ListOptions{})
|
|
if err != nil {
|
|
t.Fatalf("listing tenants: %v", err)
|
|
}
|
|
if len(list.Items) != 1 {
|
|
t.Fatalf("expected exactly one Tenant object after two Syncs, got %d", len(list.Items))
|
|
}
|
|
|
|
secret, err := s.clientset.CoreV1().Secrets("sentry").Get(ctx, "sentry-tenant-acme-clickhouse", metav1.GetOptions{})
|
|
if err != nil {
|
|
t.Fatalf("getting secret: %v", err)
|
|
}
|
|
if secret.StringData["password"] != "secret-pw" {
|
|
t.Fatalf("password changed across a re-sync: %q", secret.StringData["password"])
|
|
}
|
|
}
|
|
|
|
func TestSyncPreservesExistingTenantObjectDisplayName(t *testing.T) {
|
|
s := newTestSyncer(t, "sentry")
|
|
ctx := context.Background()
|
|
|
|
// A human/GitOps process already created this Tenant object (e.g.
|
|
// via `kubectl apply`, per TenantSpec's doc comment) before
|
|
// -provision-tenant ever ran -- Sync must not overwrite their
|
|
// chosen displayName with its own.
|
|
pre := &unstructured.Unstructured{Object: map[string]interface{}{
|
|
"apiVersion": "sentry.io/v1alpha1",
|
|
"kind": "Tenant",
|
|
"metadata": map[string]interface{}{"name": "acme", "namespace": "sentry"},
|
|
"spec": map[string]interface{}{"displayName": "Human-Chosen Name"},
|
|
}}
|
|
if _, err := s.dynamic.Resource(tenantGVR).Namespace("sentry").Create(ctx, pre, metav1.CreateOptions{}); err != nil {
|
|
t.Fatalf("pre-creating tenant: %v", err)
|
|
}
|
|
|
|
if err := s.Sync(ctx, "acme", "Some Other Name -provision-tenant Was Called With", "/idx", Credentials{Username: "u", Password: "p"}); err != nil {
|
|
t.Fatalf("Sync: %v", err)
|
|
}
|
|
|
|
obj := getTenant(t, s, "acme")
|
|
displayName, _, _ := unstructured.NestedString(obj.Object, "spec", "displayName")
|
|
if displayName != "Human-Chosen Name" {
|
|
t.Fatalf("spec.displayName = %q, want the pre-existing value preserved", displayName)
|
|
}
|
|
}
|