From 728cc351b5617e9d1eec107ea51f19002a125f12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CNaeel=E2=80=9D?= Date: Fri, 15 May 2026 07:49:47 +0400 Subject: [PATCH] doc: add multitenant porting guide for upstream upgrade --- doc/multitenant-porting-guide.md | 313 +++++++++++++++++++++++++++++++ 1 file changed, 313 insertions(+) create mode 100644 doc/multitenant-porting-guide.md diff --git a/doc/multitenant-porting-guide.md b/doc/multitenant-porting-guide.md new file mode 100644 index 00000000..b814f42c --- /dev/null +++ b/doc/multitenant-porting-guide.md @@ -0,0 +1,313 @@ +# Fission Multi-Tenant Porting Guide + +> **Purpose:** This document exists so that any AI agent or engineer can fully +> understand what was changed to add multi-tenancy to this Fission fork, why +> each decision was made, and what needs to be ported when a new upstream +> Fission release arrives. +> +> Fork base: `github.com/fission/fission` tag `v1.22.0` (2025-12-16) +> Our branch: `feature/multitenant` + +--- + +## 1. The Problem We Solved + +In stock Fission v1.22.0 all resource namespaces must be listed in the +`FISSION_RESOURCE_NAMESPACES` environment variable **before** the process starts. +Adding a new tenant namespace requires: + +1. Patching that env var on executor, router, buildermgr deployments +2. Triggering a rolling restart of all three components (~30 s downtime each) + +At scale (hundreds of tenants created continuously) this causes a permanent +rolling-restart loop and cascading failures for existing users. + +**Our solution:** hot namespace registration without pod restarts. Any platform +(console, operator, CI/CD) creates a Kubernetes Namespace with label +`fission.io/managed=true` — all three Fission components detect it within +milliseconds via k8s Watch and register it live. + +--- + +## 2. Integration Contract (external platforms) + +The entire contract between an external platform and Fission is a single label: + +```yaml +apiVersion: v1 +kind: Namespace +metadata: + name: tenant-abc123 + labels: + fission.io/managed: "true" +``` + +No other coupling to Fission internals is required. + +To remove a tenant namespace: delete the namespace or remove the label. +Current removal strategy is `track-only` (the manager records the event but does +not actively deregister — the executor types stop receiving events for deleted +resources naturally). `dispatch-remove` strategy exists in the model but is not +wired by default (see §8). + +--- + +## 3. Backward Compatibility + +`FISSION_RESOURCE_NAMESPACES` continues to work exactly as before. Namespaces +listed there are bootstrapped at startup with source `env` and do not require the +label. The NSWatcher layer adds **on top** of the existing mechanism — nothing +was removed. + +--- + +## 4. File Map + +### New files (did not exist in v1.22.0) + +| File | Purpose | What breaks if removed | +|------|---------|------------------------| +| `pkg/utils/namespace_manager_model.go` | All types: `NamespaceRecord`, `NamespacePhase`, `NamespaceSource`, `NamespaceEvent`, `NamespaceRemovalStrategy`, `ManagedNamespaceWatcherConfig` | Everything — all other files import these types | +| `pkg/utils/namespace_manager.go` | `NamespaceManager` interface + `inMemoryNamespaceManager` implementation. `RunManagedNamespaceWatcher()` — the single entry point used by all three components. `NewNamespaceWatcherEventHandlers()` — k8s informer callbacks. `EnsureNamespaceSA` helper call site. | All NSWatcher functionality | +| `pkg/utils/namespace_manager_test.go` | Unit + integration tests for NamespaceManager | Tests only | +| `pkg/utils/namespace_manager_model_test.go` | Tests for model helpers | Tests only | +| `pkg/utils/serviceaccount.go` (was modified, `EnsureNamespaceSA` added at bottom) | `EnsureNamespaceSA(ctx, client, logger, ns)` — creates fission-fetcher SA/Role/RoleBinding in a new namespace idempotently | Pool pods in new namespaces fail to start (no SA to run fetcher) | +| `pkg/executor/multitenant/ns_watcher.go` | `StartNSWatcher()` — executor entry point. `registerNamespace()` — calls `AddNamespace` on global resolver + `EnsureNamespaceSA` + all executor types. | Executor never learns about new namespaces | +| `pkg/executor/multitenant/namespace_subscriber.go` | `NewNamespaceSubscriber()` — adapter from `NamespaceSubscriber` interface to executor `registerNamespace()` | Same as above | +| `pkg/executor/multitenant/ns_watcher_test.go` | Tests | Tests only | +| `pkg/executor/multitenant/namespace_subscriber_test.go` | Tests | Tests only | +| `pkg/router/ns_watcher.go` | `StartNSWatcher()` — router entry point, 5 lines | Router never learns about new namespaces | +| `pkg/router/namespace_subscriber.go` | `NewNamespaceSubscriber()` — adapter calling `HTTPTriggerSet.AddNamespace` | Same as above | +| `pkg/router/namespace_subscriber_test.go` | Tests | Tests only | +| `pkg/buildermgr/ns_watcher.go` | `StartNSWatcher()` — buildermgr entry point, 5 lines | Buildermgr never learns about new namespaces | +| `pkg/buildermgr/namespace_subscriber.go` | `NewNamespaceSubscriber()` — adapter calling `envWatcher.AddNamespace` + `pkgWatcher.AddNamespace` | Same as above | +| `pkg/buildermgr/namespace_subscriber_test.go` | Tests | Tests only | +| `deploy/multitenant/rbac.yaml` | ClusterRoles + ClusterRoleBindings for all three components (see §6) | Components crash at startup or fail to watch namespaces | + +### Modified files (existed in v1.22.0, we changed them) + +| File | What we added | What breaks if reverted | +|------|--------------|-------------------------| +| `pkg/utils/namespace.go` | `ManagedNamespaceLabelKey/Value` constants, `ManagedNamespaceLabelSelector()`, `IsManagedNamespace()`, `AddNamespace()` (thread-safe dedup), `Snapshot()` (sorted slice copy under read-lock), `SnapshotWithOptions()` | All callers of `Snapshot()` break — there are many; label constants used by informer filter | +| `pkg/utils/namespace_test.go` | Tests for new methods | Tests only | +| `pkg/utils/informer.go` | `NewSharedInformerFactoryForNamespaces(namespaces []string)` — creates informer factory filtered to a dynamic list of namespaces | Executor types cannot create per-NS informers for new namespaces | +| `pkg/executor/executor.go` | `StartNSWatcher(...)` call added after executor types are initialized | NSWatcher never starts in executor | +| `pkg/executor/executortype/executortype.go` | `AddNamespace(ctx, ns, mgr)` added to the `ExecutorType` interface | All three executor types must implement this; compilation fails | +| `pkg/executor/executortype/poolmgr/gpm.go` | `AddNamespace()` implementation — creates per-NS informer factory, pod lister, event handlers | poolmgr never picks up functions in new namespaces | +| `pkg/executor/executortype/poolmgr/poolpodcontroller.go` | Uses `Snapshot()` in runtime loop instead of static namespace list | Pool pods not created in new namespaces | +| `pkg/executor/executortype/newdeploy/newdeploymgr.go` | `AddNamespace()` implementation | newdeploy never picks up functions in new namespaces | +| `pkg/executor/executortype/container/containermgr.go` | `AddNamespace()` implementation | container executor never picks up functions in new namespaces | +| `pkg/router/router.go` | `StartNSWatcher(...)` call added | NSWatcher never starts in router | +| `pkg/router/httpTriggers.go` | `AddNamespace(ns string)` on `HTTPTriggerSet` — starts per-NS informers for HTTPTriggers and Functions | Router ignores HTTPTriggers in new namespaces | +| `pkg/router/functionReferenceResolver.go` | Uses `Snapshot()` in runtime loop | Router resolves functions only in statically-configured namespaces | +| `pkg/buildermgr/buildermgr.go` | `StartNSWatcher(...)` call added | NSWatcher never starts in buildermgr | +| `pkg/buildermgr/envwatcher.go` | `AddNamespace(ns string)` — starts per-NS Environment informer | Buildermgr ignores Environments in new namespaces | +| `pkg/buildermgr/pkgwatcher.go` | `AddNamespace(ns string)` — starts per-NS Package informer | Buildermgr ignores Packages in new namespaces | +| `pkg/storagesvc/archivePruner.go` | Uses `Snapshot()` instead of static namespace list | Archive pruner only cleans old namespaces | +| `.gitignore` | Added `*.token` | Minor — token files would be committed accidentally | +| `deploy/multitenant/rbac.yaml` | New file (see above) | — | + +--- + +## 5. Data Flow: from label to HTTP 200 + +``` +kubectl label ns tenant-abc123 fission.io/managed=true + │ + ▼ +k8s API server emits ADDED event on Namespace stream + │ + ▼ (within ~50ms) +utils.RunManagedNamespaceWatcher ← informer AddFunc + │ (all 3 components share this function) + ▼ +NamespaceManager.DispatchAdd(ctx, "tenant-abc123") + │ + ├──► executor subscriber: + │ registerNamespace() + │ 1. DefaultNSResolver().AddNamespace("tenant-abc123") + │ 2. EnsureNamespaceSA(ctx, client, logger, "tenant-abc123") + │ └─ creates fission-fetcher SA + Role + RoleBinding + │ 3. poolmgr.AddNamespace("tenant-abc123") + │ └─ creates per-NS InformerFactory, pod lister, handlers + │ 4. newdeploy.AddNamespace("tenant-abc123") + │ 5. container.AddNamespace("tenant-abc123") + │ + ├──► router subscriber: + │ HTTPTriggerSet.AddNamespace("tenant-abc123") + │ └─ starts watching HTTPTriggers + Functions in that NS + │ + └──► buildermgr subscriber: + envWatcher.AddNamespace("tenant-abc123") + pkgWatcher.AddNamespace("tenant-abc123") + └─ starts watching Environments + Packages in that NS + +User creates: fission env create --namespace tenant-abc123 ... + fission fn create --namespace tenant-abc123 ... + fission httptrigger create --namespace tenant-abc123 ... + +Router picks up HTTPTrigger → resolves Function → cold-starts pod in tenant-abc123 + │ + ▼ +HTTP 200 ← function response +``` + +--- + +## 6. RBAC Explained + +Three ClusterRoles in `deploy/multitenant/rbac.yaml`: + +### `fission-executor-ns-watcher` (also for router: `fission-router-ns-watcher`) +``` +namespaces: list, watch +``` +Without this: informer fails to start with "Forbidden" — components never learn +about new namespaces. + +### `fission-executor-sa-provisioner` +``` +serviceaccounts: get, list, watch, create, update, patch +events: create +localsubjectaccessreviews: create +roles: get, list, watch, create, update, patch +rolebindings: get, list, watch, create, update, patch +``` +Why `events.create`: Kubernetes forbids creating a Role that grants permissions +the caller does not currently hold. Since `fission-fetcher` gets `events.create` +in the Role we create for it, `fission-executor` must hold `events.create` itself +to be allowed to create that Role. This is a k8s RBAC escalation prevention rule. + +Why `localsubjectaccessreviews.create`: `EnsureNamespaceSA` calls +`setupSAAndRoleBindings` which first checks if the SA already has each permission +before creating it — that check uses a `LocalSubjectAccessReview`. + +--- + +## 7. Key Design Decisions and Why + +### Decision: pub/sub via `NamespaceManager`, not direct calls +**Why not:** simply call `poolmgr.AddNamespace`, `router.AddNamespace` etc. +directly from a shared goroutine. +**Why pub/sub:** executor, router and buildermgr run as separate processes +(different pod). Each process has its own copy of the watcher. Subscribers are +registered in-process. This pattern makes each component fully self-contained and +testable in isolation. No cross-process coupling. + +### Decision: `fission.io/managed=true` label as the only trigger +**Why not:** watch all namespaces, or use a CRD, or use annotations. +**Why label:** labels are the idiomatic k8s way to select resources. A label +selector in the informer factory (`fission.io/managed=true`) means the informer +only receives events for labeled namespaces — zero overhead for the hundreds of +system namespaces. + +### Decision: `EnsureNamespaceSA` in executor, not in a separate operator +**Why:** the SA must exist before the first pool pod starts. The executor is +already in the hot path — it processes the NS event and then immediately triggers +pod scheduling. Doing it in a separate controller would introduce a race. Doing it +in the executor keeps the lifecycle coupled correctly. + +### Decision: `NamespaceRemovalStrategy = track-only` (not `dispatch-remove`) +**Why:** removing a namespace in k8s is already an irreversible event — all +resources inside are cascade-deleted by k8s. The executor types detect the pod +deletions themselves. Dispatching an explicit "remove" to all subscribers would +require each subscriber to implement a cleanup path — added complexity for zero +operational benefit in our use case. Can be switched per-component via +`ManagedNamespaceWatcherConfig.RemovalStrategy`. + +### Decision: `AddNamespace` is idempotent (safe to call multiple times) +**Why:** k8s informers can deliver the same event more than once (resync). Every +`AddNamespace` call is a no-op if the namespace is already registered. No locks +held across the whole function — `NamespaceResolver.AddNamespace` uses a write +lock only for the map write, checks for existence first under the same lock. + +### Decision: global `NamespaceResolver` (`DefaultNSResolver()`) updated once in executor +**Why:** `DefaultNSResolver().AddNamespace(ns)` is called exactly once in +`registerNamespace()` — before the executor types are called. Each executor type +does NOT call it themselves. This avoids a subtle race: if executor type A adds +the NS to the global resolver first, and executor type B checks the global resolver +as its dedup mechanism, B would see it as already registered and skip — even +though B has not actually processed it yet. The correct dedup is per executor type. + +--- + +## 8. What Is NOT Done (deliberately deferred) + +| Missing feature | Why deferred | File/interface to extend | +|----------------|--------------|--------------------------| +| `dispatch-remove` full wiring | Not needed for current use case | `ManagedNamespaceWatcherConfig.RemovalStrategy` — just switch the constant | +| Buildermgr ClusterRole in rbac.yaml | Buildermgr uses the same SA as executor in our deployment; check your setup | Add a third ClusterRole/Binding to `deploy/multitenant/rbac.yaml` | +| Helm chart integration | We apply rbac.yaml manually. A proper Helm chart would include these RBAC objects | `charts/fission-all/templates/` | +| Layer 2 (tenant isolation: per-NS network policy, resource quotas) | Out of scope for Layer 1 | Not started | +| Layer 3 (per-tenant auth, billing hooks) | Out of scope | Not started | +| `NamespaceManager.DispatchRemove` subscriber wiring | Each subscriber has a `RemoveFunc` stub returning nil | Implement per subscriber | + +--- + +## 9. Porting to a New Upstream Version + +When `github.com/fission/fission` releases v1.23 or later, follow this order: + +1. **Check the upstream changelog** for any changes to: + - `pkg/utils/namespace.go` — if they renamed or refactored `NamespaceResolver`, our `AddNamespace`/`Snapshot` additions need to be reapplied + - `pkg/executor/executortype/executortype.go` — if they changed the `ExecutorType` interface, our `AddNamespace` method needs to be reapplied + - `pkg/router/httpTriggers.go` — if `HTTPTriggerSet` changed, our `AddNamespace` method on it needs to be reapplied + - `pkg/buildermgr/envwatcher.go`, `pkgwatcher.go` — same + - `pkg/utils/informer.go` — if the informer factory pattern changed + +2. **Apply in this order** (each depends on the previous): + 1. `pkg/utils/namespace_manager_model.go` — pure types, no deps on other changed files + 2. `pkg/utils/namespace.go` additions (`AddNamespace`, `Snapshot`, label constants) + 3. `pkg/utils/namespace_manager.go` — depends on model + namespace.go + 4. `pkg/utils/serviceaccount.go` — add `EnsureNamespaceSA` at the bottom + 5. `pkg/utils/informer.go` — add `NewSharedInformerFactoryForNamespaces` + 6. `pkg/executor/executortype/executortype.go` — add `AddNamespace` to interface + 7. Executor types: `poolmgr/gpm.go`, `newdeploy/newdeploymgr.go`, `container/containermgr.go` — implement `AddNamespace` + 8. `pkg/executor/multitenant/` — copy the whole package as-is + 9. `pkg/executor/executor.go` — add `StartNSWatcher` call + 10. `pkg/router/httpTriggers.go` — add `AddNamespace` method on `HTTPTriggerSet` + 11. `pkg/router/namespace_subscriber.go`, `pkg/router/ns_watcher.go` — copy as-is + 12. `pkg/router/router.go` — add `StartNSWatcher` call + 13. `pkg/buildermgr/envwatcher.go`, `pkgwatcher.go` — add `AddNamespace` method + 14. `pkg/buildermgr/namespace_subscriber.go`, `pkg/buildermgr/ns_watcher.go` — copy as-is + 15. `pkg/buildermgr/buildermgr.go` — add `StartNSWatcher` call + 16. `pkg/storagesvc/archivePruner.go` — replace static namespace list with `Snapshot()` + 17. `deploy/multitenant/rbac.yaml` — apply unchanged + +3. **Run tests:** + ```bash + go test ./pkg/utils/... ./pkg/executor/... ./pkg/router/... ./pkg/buildermgr/... + ``` + +4. **Build and deploy:** + ```bash + # on VM: + docker run --rm -v $PWD:/src -w /src golang:1.26-alpine \ + sh -c 'go build -o /src/fission-bundle-bin ./cmd/fission-bundle/' + docker build -f Dockerfile.mt -t naeel/fission-bundle:vNEW_TAG . + docker push naeel/fission-bundle:vNEW_TAG + kubectl set image deployment/executor -n fission executor=naeel/fission-bundle:vNEW_TAG + kubectl set image deployment/router -n fission router=naeel/fission-bundle:vNEW_TAG + kubectl set image deployment/buildermgr -n fission buildermgr=naeel/fission-bundle:vNEW_TAG + ``` + +5. **Run e2e test:** + ```bash + bash ~/terra/fission/scripts/test_layer1.sh + # Expected: PASS=5 FAIL=0 + ``` + +--- + +## 10. Test Coverage + +| Test file | What it covers | +|-----------|---------------| +| `pkg/utils/namespace_test.go` | `AddNamespace` dedup, `Snapshot` sorted output, `IsManagedNamespace` | +| `pkg/utils/namespace_manager_test.go` | `Bootstrap`, `DispatchAdd`/`Remove`/`Resync`, subscriber dispatch, `TestStartManagedNamespaceWatcherIntegration` — full k8s fake informer → subscriber pipeline | +| `pkg/utils/namespace_manager_model_test.go` | Model helpers, `Clone`, `IsActive`, `IsTerminal` | +| `pkg/executor/multitenant/ns_watcher_test.go` | `registerNamespace` with fake k8s client | +| `pkg/executor/multitenant/namespace_subscriber_test.go` | Subscriber adapter | +| `pkg/router/namespace_subscriber_test.go` | Router subscriber adapter | +| `pkg/buildermgr/namespace_subscriber_test.go` | Buildermgr subscriber adapter | +| `~/terra/fission/scripts/test_layer1.sh` | End-to-end: create labeled NS → executor registers it → create env/fn/trigger → call function → HTTP 200 |