[HYPERSHELL-78][HYPERSHELL-96] feat: gateway namespace GC and active sandbox count before deletion by squizzi · Pull Request #139 · openshift-online/hypershell

squizzi · 2026-08-17T20:49:46Z

The control plane now reclaims the openshell-* namespaces it creates for gateways: on a Gateway delete it deletes the managed namespace (letting Kubernetes cascade the in-namespace resources) and cleans up only the out-of-namespace resources it owns (cluster-scoped ClusterRoleBinding, external Keycloak client, cross-namespace credential RBAC), while a new periodic GC reconciler reaps orphaned managed namespaces after a 10-minute grace and records a durable GarbageCollected Event. A new read-only active_sandbox_count field is plumbed through the api-server Gateway model (migration, gRPC + REST, regenerated TypeScript SDK) and populated by the control-plane health reconciler, and the gateway management UI surfaces it as a non-blocking warning in the delete confirmation dialog. GC is enabled by default and configurable via GATEWAY_NAMESPACE_GC_ENABLED / _INTERVAL / _GRACE_PERIOD, is guarded to only ever touch namespaces carrying both control-plane management labels, and aborts the whole sweep if Gateways cannot be listed. The change set includes a new spec (specs/platform/openshell-gateway-namespace-gc.spec.md), unit tests for the namespace helpers/GC/sandbox counting, and an e2e test validating namespace GC on gateway delete.

Images

🤖 Generated with Claude Code

squizzi

Amber review — HYPERSHELL-78 namespace GC + active-sandbox surfacing

Submitted as a COMMENT because GitHub does not allow REQUEST_CHANGES on one's own PR — see the amber/changes-requested label; the substantive verdict below is REQUEST_CHANGES.

The feature is well-structured — env-driven GC config, grace period, abort-if-can't-list guard, namespace-delete cascade, a UI warning that surfaces (not blocks) active sandboxes, plus spec and e2e coverage. But the garbage collector builds its "live gateway" set from only the first page of ListGateways (default 20), so any fleet with more than 20 gateways will have live tenant namespaces reaped after the grace period — a data-loss blocker — and the spec-mandated GC audit event can't be written because the controller ClusterRole lacks events: create.

Overall assessment: REQUEST_CHANGES

Findings Summary

[Blocker] GC live-set is truncated to the first 20 gateways (empty ListGatewaysRequest), so gateways beyond page 1 look orphaned and their namespaces get deleted after grace → tenant data loss. — components/control-plane/internal/reconciler/namespace.go:89,96
[Major] recordGCEvent calls Events().Create, but the controller ClusterRole grants events only get/list/watch; the durable-record requirement in the spec silently fails (best-effort WARN). RBAC change is missing from this PR. — components/control-plane/internal/reconciler/namespace.go:197 / deploy/base/controller-rbac.yaml
[Minor] The GC live-set key comes from gatewayNamespace, whose openshell-<name> fallback diverges from the real openshell-<hex(id)> scheme; risky as a load-bearing derivation in a destructive path. — namespace.go:96 / reconciler.go:384
[Minor] Health sandbox-count reporting inherits the same 20-gateway list cap, so counts go stale for larger fleets — exactly where the delete warning matters most. — components/control-plane/internal/reconciler/health.go:107
[Minor] active_sandbox_count is readOnly on the Gateway schema yet writable via GatewayPatchRequest + REST handler; a client can overwrite the control-plane-owned count (and suppress the delete warning). — openapi.gateways.yaml:397 / handler.go:130

Convention Checklist

Convention	Status	Notes
No `panic()` in production	✅ Pass	Errors wrapped with `fmt.Errorf(... %w)` throughout
`IsNotFound` → skip, transient → retry	✅ Pass	`DeleteManagedNamespace` treats absent namespace as success
Reconcile, don't create-or-skip	✅ Pass	Sweep + namespace-delete cascade; `Mark/ClearGCEligible` idempotent
Never silently swallow partial failures	❌ Fail	Truncated `ListGateways` accepted as complete (Blocker); audit-event failure swallowed as WARN while spec requires a durable record (Major)
Least privilege / no destructive act on partial data	❌ Fail	Reaper deletes namespaces off an incomplete live set (Blocker)
RBAC matches the code's API calls	❌ Fail	`events: create` required by new code, not granted (Major)
Config separate from code	✅ Pass	`GATEWAY_NAMESPACE_GC_*` env vars with safe fallback
OpenAPI is generated, contract consistent	⚠️ Partial	`readOnly` Gateway field contradicted by writable `GatewayPatchRequest` (Minor)
Tests cover new behavior	⚠️ Partial	UI warn/no-warn + single-gateway e2e GC covered; no test for >20-gateway pagination or "GC must not reap a live gateway"

Clearing the Blocker (paginate the gateway list before building the live set, or abort when the assembled set is smaller than Metadata.Total) and the Major (add events: create/patch RBAC) is required before merge; the Minors are cheap follow-ups worth folding in now.

squizzi · 2026-08-17T21:00:07Z

+	// gateways we must abort the whole sweep: an empty or failed list would make
+	// every managed namespace look orphaned and risk reaping live ones.
+	client := pb.NewGatewayServiceClient(r.grpcConn)
+	resp, err := client.ListGateways(ctx, &pb.ListGatewaysRequest{})


[Blocker] GC builds its live-gateway set from only the first page of gateways → reaps live namespaces.

ListGateways is called with an empty ListGatewaysRequest{}. Server-side pagination (grpcutil.NormalizePagination) defaults an unset page/size to page 1, DefaultPageSize=20. So resp.GetItems() returns at most 20 gateways, and the live set built below (line 96) is silently truncated. With more than 20 gateways in the fleet, every gateway beyond the first page has its namespace treated as orphaned; once past the grace period this reaper calls DeleteManagedNamespace on a live gateway's namespace → tenant data loss / outage.

The existing "abort the sweep if we cannot list gateways" guard does not protect against this, because the list succeeds — it just returns a partial page. The e2e test only creates one gateway, so it will never catch this.

Fix: page through all gateways until exhausted before building the live set (accumulate across page=1..n), or read resp.Metadata.Total and abort the sweep if the assembled live set is smaller than the reported total. A destructive reaper must operate on the complete set or not at all.

Confidence: high.

squizzi · 2026-08-17T21:00:18Z

+	}
+	live := make(map[string]struct{})
+	for _, gw := range resp.GetItems() {
+		live[gatewayNamespace(gw)] = struct{}{}


[Minor] Live-set key derivation depends on gatewayNamespace, whose fallback diverges from the real namespace scheme.

The live set is keyed on gatewayNamespace(gw). That helper (reconciler.go) falls back to openshell-<name> when gw.GetNamespace() is empty, but real gateway namespaces are openshell-<hex(id)> (see plugins/gateways/model.go BeforeCreate). Today gw.Namespace is always populated at create time, so the fallback is effectively dead — but this is now load-bearing in a destructive path: if a gateway ever arrives without a namespace (proto default, partial record), the live set is keyed on the wrong string and the reaper won't see the real namespace as live.

Fix: in a delete/GC path, don't guess. Treat an empty gw.Namespace as an error/skip (exclude that gateway and refuse to reap anything, or abort the sweep) rather than synthesizing a namespace that can't match reality.

Confidence: medium.

active_sandbox_count is a control-plane-owned observability signal written only over gRPC UpdateGateway by the health reconciler. It was already readOnly on the REST Gateway schema, yet REST PATCH still accepted and persisted a client-supplied value via GatewayPatchRequest, letting any caller clobber the reconciler's count. Drop active_sandbox_count from GatewayPatchRequest (OpenAPI spec, generated client, hand-written patch struct, and the handler write block) so it can no longer be set over REST. The field stays gRPC-writable, gorm-persisted, and readable on GET. Also pass --module to the Go SDK generator in `make generate-sdk-go`; without it the generated client imported "/types" and failed to build. Regenerated the Go and TypeScript SDKs. Addresses PR #139 review comments (active_sandbox_count readOnly vs writable on GatewayPatchRequest, and REST PATCH persisting a client-supplied count). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

squizzi

Amber re-review — PR #139 (self-review)

Overall assessment: APPROVE — submitted as a COMMENT event because GitHub does not permit approving your own pull request; the approval verdict is carried by the amber/approved label.

The author has addressed every finding from the first review — the fleet-wide pagination Blocker, the events RBAC Major, and all three Minors are resolved, and each fix is correct on inspection rather than merely present. Only one low-severity, currently-unreachable Minor remains (the gatewayNamespace fallback on the non-GC paths), so this is safe to merge.

Previously-raised findings — all resolved

[Blocker → Resolved] GC/health only saw the first 20 gateways (unpaged list). listAllGateways (reconciler.go:400) now pages the full inventory with a provably-terminating loop; both the GC live-set and the health reconciler consume it.
[Major → Resolved] Namespace reaper could not write its audit Event. deploy/base/controller-rbac.yaml now grants events: get,list,watch,create,patch in a dedicated rule.
[Minor → Resolved] Health reconciler was also capped at 20 gateways — now uses listAllGateways.
[Minor → Resolved] active_sandbox_count was writable over REST — removed from GatewayPatchRequest, marked readOnly: true in openapi.gateways.yaml, and the handler no longer patches it; it is written only over gRPC UpdateGateway.
[Minor → Resolved] GC live-set no longer trusts a synthesized namespace — it keys on gw.GetNamespace() and aborts the sweep if any live gateway lacks one.

Remaining (non-blocking)

[Minor] gatewayNamespace fallback divergence (reconciler.go:388) — see inline comment. Unreachable today; optional hardening of the destructive delete/health paths.
[Informational] The PR bundles regenerated SDK drift unrelated to this feature (sdk-go GatewayBuilder gains CredentialDriver, loses Oidc; header Source path + spec SHA normalized). This is correct make generate output and the removed Oidc builder has no callers, so it is safe — noted only so reviewers understand why the generated files changed.

Findings Summary

[Minor] gatewayNamespace openshell-<name> fallback still used by delete/health paths - Correctness/Robustness (reconciler.go:388)
[Informational] Regenerated SDK drift bundled with feature - Hygiene (components/sdk-go/types/gateway.go)

Convention Checklist

Convention	Status	Notes
No `panic()` in production	Pass	Error paths return wrapped errors
Error wrapping with context (`fmt.Errorf %w`)	Pass	Delete/GC paths wrap consistently
Reconcile, don't create-or-skip	Pass	`DeleteManagedNamespace` idempotent; GC re-evaluates each sweep
Never silently swallow partial failures	Pass	List failures abort the sweep; summarize/count errors logged, not fatal
No destructive action on partial data	Pass	Sweep aborts if gateways can't be listed or a live gateway lacks a namespace
RBAC matches code	Pass	`events: create,patch` granted for the audit Event
Config separate from code	Pass	GC toggle/interval/grace via env with WARN-on-invalid fallback
OpenAPI generated & consistent	Pass	`active_sandbox_count` readOnly across model/openapi/handler; SDKs regenerated
Tests present	Pass	Unit tests for helpers/GC/sandbox count; e2e validates GC on delete

squizzi

Submitted as COMMENT because GitHub does not allow APPROVE on one's own PR; the verdict is APPROVE via amber/approved.

Prior blockers on pagination, events RBAC, REST-writable count, and namespace guessing are resolved and hold up under re-inspection. Remaining notes are Minor only; overall APPROVE.

Findings Summary (ordered by severity, highest first):

[Minor] REST PATCH comment still says count is written via gRPC UpdateGateway; it is Adjust/Set only - Docs (handler.go:128-130)
[Minor] selfHeal Set RPCs lack sandboxCountRPCTimeout used by applyDelta - Reliability (sandboxcount.go:224-227)
[Minor] Unmanaged namespace skips delete and in-ns labeled cleanup is gone, so AlreadyExists without management labels can orphan workloads - Correctness (gateway/reconciler.go delete path)
[Minor] Adjust/Set inherit coarse gRPC RBAC (any owner/creator, any namespace); consider SA-only when allowlists exist - Least privilege (grpc_handler.go)

Convention Checklist (omit conventions not applicable to the diff):

Convention	Result
No `panic()` in production	Pass
Error wrapping with context (`fmt.Errorf %w`)	Pass
Reconcile, don't create-or-skip	Pass
Never silently swallow partial failures	Pass
No destructive action on partial data	Pass
RBAC matches code	Pass
Config separate from code	Pass
OpenAPI generated & consistent	Pass
Tests present	Pass

squizzi · 2026-08-18T20:41:25Z

+			// active_sandbox_count is a control-plane-owned observability signal
+			// written only over gRPC UpdateGateway (readOnly on the REST Gateway
+			// schema); it is intentionally not settable via the public REST PATCH.


[Minor] Comment still says active_sandbox_count is written via gRPC UpdateGateway; it is not.

The REST PATCH omission is correct (readOnly on the schema, field not applied here). After the dedicated Adjust/Set RPCs, UpdateGateway also refuses to mutate the count (grpc_handler.go around the Replace path). Only this comment is stale.

Fix: Say the count is written only via AdjustActiveSandboxCount / SetActiveSandboxCount.

Confidence: 95

squizzi · 2026-08-18T20:41:28Z

+	for _, ns := range namespaces {
+		if err := r.set(ctx, ns, active[ns]); err != nil {
+			log.Printf("WARN sandbox count: set %s to %d: %v", ns, active[ns], err)
+		}


[Minor] selfHeal Set RPCs lack the sandboxCountRPCTimeout used by applyDelta.

applyDelta wraps each Adjust with context.WithTimeout(r.baseCtx, sandboxCountRPCTimeout) (10s). This loop calls r.set(ctx, ...) on the parent context, so a hung Set can stall the whole heal pass.

Fix: Apply the same per-RPC timeout around each r.set (or a single budget for the loop, if that is preferred).

Confidence: 90

squizzi · 2026-08-18T20:41:31Z

+// DeleteGatewayResources cleans up the resources a gateway owns that live
+// OUTSIDE its namespace and are therefore not reclaimed when the namespace is
+// deleted. Everything inside the gateway's namespace (Deployments, Services,
+// Secrets, ConfigMaps, PVCs, Jobs, Roles, RoleBindings, and cert-manager /
+// Gateway API objects) is garbage-collected by Kubernetes as a side effect of
+// deleting the namespace itself, so those are not enumerated here. The
+// out-of-namespace resources handled below are:
+//   - the cluster-scoped ClusterRoleBinding created for the gateway,
+//   - the gateway's external Keycloak client, and
+//   - any credential RBAC the gateway created in a separate credential namespace.
 func DeleteGatewayResources(


[Minor] Unmanaged namespace skip plus removal of labeled in-namespace cleanup can orphan workloads.

This path no longer enumerates labeled in-namespace objects; it relies on namespace deletion to cascade. DeleteManagedNamespace is a no-op when the namespace lacks both management labels, and createNamespace treats AlreadyExists as success without adopting/labeling the existing namespace. If a gateway lands on a pre-existing namespace without those labels, delete/GC will skip the namespace and nothing will clean the labeled workloads that used to be deleted in-namespace.

Fix: Keep skipping truly shared namespaces, but either fall back to labeled in-namespace cleanup when DeleteManagedNamespace returns deleted=false, or adopt/label on AlreadyExists when that is safe.

Confidence: 80

squizzi · 2026-08-18T20:41:31Z

+// AdjustActiveSandboxCount applies a relative delta to the active_sandbox_count
+// of the gateway backing the given namespace. The adjustment is atomic and
+// floored at zero. A namespace with no live gateway is a no-op (count 0).
+func (h *gatewayGRPCHandler) AdjustActiveSandboxCount(ctx context.Context, req *pb.AdjustActiveSandboxCountRequest) (*pb.AdjustActiveSandboxCountResponse, error) {
+	if err := grpcutil.ValidateStringField("namespace", req.Namespace, true); err != nil {
+		return nil, err
+	}
+	count, svcErr := h.service.AdjustActiveSandboxCount(ctx, req.Namespace, int(req.Delta))
+	if svcErr != nil {
+		return nil, grpcutil.ServiceErrorToGRPC(svcErr)
+	}
+	return &pb.AdjustActiveSandboxCountResponse{ActiveSandboxCount: int32(count)}, nil
+}
+
+// SetActiveSandboxCount sets the active_sandbox_count of the gateway backing the
+// given namespace to an absolute observed value (self-heal). The value is
+// floored at zero. A namespace with no live gateway is a no-op (count 0).
+func (h *gatewayGRPCHandler) SetActiveSandboxCount(ctx context.Context, req *pb.SetActiveSandboxCountRequest) (*pb.SetActiveSandboxCountResponse, error) {
+	if err := grpcutil.ValidateStringField("namespace", req.Namespace, true); err != nil {
+		return nil, err
+	}
+	count, svcErr := h.service.SetActiveSandboxCount(ctx, req.Namespace, int(req.Count))
+	if svcErr != nil {
+		return nil, grpcutil.ServiceErrorToGRPC(svcErr)
+	}
+	return &pb.SetActiveSandboxCountResponse{ActiveSandboxCount: int32(count)}, nil
+}


[Minor] Adjust/Set inherit coarse gRPC RBAC (any gateway:owner / gateway:creator, any namespace).

isGRPCAuthorized grants those roles every non-read RPC with no namespace or resource binding. These new methods can therefore be invoked by any such principal against any gateway namespace, not only the control-plane service account. When RBAC_SERVICE_ACCOUNTS is configured, consider restricting Adjust/Set to that allowlist (or a dedicated role).

Fix: SA-only (or method-specific role) when the service-account allowlist is non-empty; keep current behavior as a documented fallback otherwise.

Confidence: 85

Delete a gateway's namespace when the gateway is deleted, and periodically reap managed namespaces orphaned by a missed delete event or a failed bootstrap. Closes HYPERSHELL-96; addresses HYPERSHELL-78. - reconciler EventDeleted now deletes the namespace after cleaning up resources. Best-effort and idempotent: only namespaces carrying the hypershell-control-plane managed labels are ever deleted, and an already-absent namespace is treated as success. - NamespaceGCReconciler sweeps managed namespaces on a ticker, deleting those with no live Gateway after a grace period. The grace deadline is persisted on the namespace via a gc-eligible-since annotation so it survives control-plane restarts. Before reaping it records a Kubernetes Event in the control-plane namespace with a pod-state summary (Running/CrashLoopBackOff/Completed/...) and the active sandbox count. - Configurable via GATEWAY_NAMESPACE_GC_ENABLED (default true), GATEWAY_NAMESPACE_GC_INTERVAL (default 5m), and GATEWAY_NAMESPACE_GC_GRACE_PERIOD (default 10m). - Management labels are now shared constants (single source of truth with createNamespace). New helpers take kubernetes.Interface so they are unit tested with a fake clientset: pod classification, active-sandbox counting, managed-namespace deletion, grace-annotation lifecycle, and the GC decision state machine. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

Expose the number of active (Running or Pending) agent sandboxes observed in a gateway's namespace as a read-only, control-plane-populated field on the Gateway resource. This surfaces the running-sandbox count so it can be shown before a gateway is deleted (HYPERSHELL-96). The field is plumbed through the proto definition, OpenAPI spec, Gorm model (with migration 2026081712000006), the gRPC and REST presenters/handlers, and the patch path. It is omitted from the create request since it is only ever populated by the control plane. Regenerates the OpenAPI client and the TypeScript SDK. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

The gateway health reconciler now counts active (Running or Pending) agent sandbox pods in each gateway namespace on every cycle and reports the value via UpdateGateway alongside phase/status. An UpdateGateway call is issued whenever the phase/status OR the observed count changes, keeping the count reasonably fresh so it can be surfaced before a gateway is deleted (HYPERSHELL-96). The count is an observability signal only and never gates a reconcile decision. A transient pod-list error is logged and leaves the last reported count untouched rather than clobbering it with zero. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

Add specs/platform/openshell-gateway-namespace-gc.spec.md covering both the delete-driven namespace cleanup (HYPERSHELL-96) and periodic garbage collection of orphaned managed namespaces (HYPERSHELL-78). The spec documents the both-label managed-namespace guard, the 10-minute grace period persisted via the gc-eligible-since annotation (surviving control-plane restarts), aborting the sweep when gateways cannot be listed, the durable GarbageCollected Kubernetes Event recorded in the control-plane namespace before deletion, best-effort/idempotent deletion, and the control-plane-populated active_sandbox_count surfaced in the delete confirmation as a warning that never gates deletion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

Add step 10 to the OpenShell e2e: after the gateway-dependent checks, delete the gateway through the HyperShell API and assert the control plane garbage collects its managed namespace. The step confirms the namespace exists before deletion, sends DELETE and expects 204, polls until the gateway record is gone from the API (404), then polls until the managed namespace disappears from the cluster (default E2E_GC_TIMEOUT=180s), dumping the namespace YAML and controller logs on failure. It clears GW_ID afterward so the EXIT-trap cleanup does not attempt a redundant delete, and is skipped under E2E_SKIP_CLEANUP=1 to preserve resources for inspection (mirroring the sandbox cleanup gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

…ay deletion HYPERSHELL-96 asks the UI to show the number of running sandboxes before a gateway is deleted. Plumb the control-plane-populated active_sandbox_count through the host adapter into the port DTO, carry it onto the presentation GatewayConnection, and surface it as an advisory warning Alert in the delete confirmation dialog. The warning is display-only: it never blocks deletion, matching the locked design where sandbox count is an operator signal rather than a delete gate. Gated on count > 0 with an ICU-plural message; wired from both the row-action and detail-header call sites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

Address review feedback on the namespace-GC spec: - Remove Jira ticket references from the spec body (not needed in specs). - Clarify that deleting the gateway namespace cascades removal of all in-namespace resources, so they are not deleted individually for their own sake. Explicit cleanup remains only for resources that outlive the namespace: the cluster-scoped ClusterRoleBinding, the external Keycloak client, and any cross-namespace credential RBAC. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

…gateway resources Deleting a gateway's namespace makes the Kubernetes namespace controller garbage-collect every namespaced object inside it (Deployments, Services, Secrets, ConfigMaps, PVCs, Jobs, Roles, RoleBindings, and cert-manager / Gateway API objects). Enumerating and deleting each of those individually in DeleteGatewayResources before deleting the namespace was redundant work. Trim DeleteGatewayResources to only clean up the resources that outlive the namespace and are therefore not reclaimed by namespace deletion: - the cluster-scoped ClusterRoleBinding created for the gateway, - the gateway's external Keycloak client, and - credential RBAC created in a separate credential namespace. Also drops the now-unused clientset parameter and updates the caller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

The regenerated SDK marks Gateway.active_sandbox_count as a required number, so the adapter test fixture must supply it; add a concrete value. Regenerate locales/en.json to include the gateway delete active sandbox warning message, which was in code but never extracted. Both gaps failed the web-console quality gates (typecheck, then i18n:check). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

The sandbox pod can report Running while the Sandbox CR is still phase=Provisioning; the openshell CLI gates `sandbox exec` on the CR reaching Ready, so the interaction section raced the controller and failed intermittently with "not ready (phase: Provisioning)". Poll a no-op `sandbox exec -- true` until it succeeds (bounded by the existing sandbox timeout) before running the interaction commands, in both the tests/e2e and pr-test copies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

The namespace reaper and health reconciler both listed gateways with an unpaged ListGatewaysRequest{}, which the API server caps at the first 20 (default page size). For the GC live-set this is a correctness Blocker: gateways beyond page 1 were absent from the live set, so their live namespaces looked orphaned and were at risk of being reaped. For health it meant active_sandbox_count was only ever refreshed for the first 20 gateways. Add a shared listAllGateways helper that pages through the whole fleet (bounded by the authoritative Metadata.Total) and use it in both reconcilers. In the destructive GC path, key the live set on the real gw.Namespace and abort the whole sweep if any live gateway has an empty namespace, rather than synthesizing one via gatewayNamespace (whose openshell-<name> fallback cannot match the real openshell-<hex(id)> scheme). recordGCEvent writes a durable Kubernetes Event as the spec's audit record, but the controller ClusterRole granted events only get/list/watch, so every reap was silently unaudited in-cluster. Split the pods/events rule and grant events create/patch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

active_sandbox_count is a control-plane-owned observability signal written only over gRPC UpdateGateway by the health reconciler. It was already readOnly on the REST Gateway schema, yet REST PATCH still accepted and persisted a client-supplied value via GatewayPatchRequest, letting any caller clobber the reconciler's count. Drop active_sandbox_count from GatewayPatchRequest (OpenAPI spec, generated client, hand-written patch struct, and the handler write block) so it can no longer be set over REST. The field stays gRPC-writable, gorm-persisted, and readable on GET. Also pass --module to the Go SDK generator in `make generate-sdk-go`; without it the generated client imported "/types" and failed to build. Regenerated the Go and TypeScript SDKs. Addresses PR #139 review comments (active_sandbox_count readOnly vs writable on GatewayPatchRequest, and REST PATCH persisting a client-supplied count). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

Removing ActiveSandboxCount from GatewayPatchRequest left the struct tag columns padded for the old longest field name; gofmt re-aligns them so 'gofmt -l' passes the API server lint gate. The TS SDK had been regenerated with the root 'make generate-sdk-go' target, which uses a relative --spec path and skips the header normalization. Regenerate via the api-server 'make generate generate-sdk' target so the '// Source:' path drops its '../../' prefix and the '// Generated:' line is stripped, matching the OpenAPI SDK drift check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

gatewayNamespace synthesized an 'openshell-<name>' namespace when a Gateway carried no namespace. That guess diverges from the real 'openshell-<hex(ksuid)>' scheme assigned in Gateway.BeforeCreate, so on the delete path it could hand a wrong (possibly live) namespace to the destructive DeleteManagedNamespace. Make gatewayNamespace return an error instead of guessing, and have the three callers refuse to act on it: the delete path logs and skips (the NamespaceGCReconciler is the backstop), the provisioning path fails the reconcile, and the health path logs and returns. In practice gw.Namespace is always populated at create, so this only hardens an unreachable edge. Add unit coverage for both branches of the hardened helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

The hot-reload dev server (`react-router dev`) runs Vite without the BFF, so /api requests reached the OIDC-enforcing API server unauthenticated and came back 401 ("Gateways could not be loaded"). Add a dev-only Vite plugin that mints a Keycloak token via the resource-owner password grant, caches and refreshes it, and attaches it as `Authorization: Bearer` on proxied /api requests. The dev identity is configurable via KIND_DEV_USER/KIND_DEV_PASSWORD (default admin/admin), exported by swap-component.sh, so other roles can be exercised by restarting hot-reload. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

The openshell CLI validates the OIDC issuer over HTTPS against Keycloak's self-signed certificate, which the system trust store does not include, so `openshell gateway add` failed on OIDC discovery (.well-known/openid-configuration). Add a helper that extracts the cert-manager hypershell-ca from the cluster and prints an `export SSL_CERT_FILE=...` line -- the same mechanism the e2e suite uses -- so the CLI (rustls) trusts the dev CA for OIDC and gateway TLS alike, with no `--gateway-insecure`. Diagnostics go to stderr and the sole stdout line is the export, so `eval "$(make kind-gateway-trust)"` sets it in the current shell. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

Migrate active-sandbox accounting from a periodic full-namespace pod LIST to an event-driven, label-selected pod informer with atomic DB updates and a periodic self-heal. - api-server: AdjustActiveSandboxCount/SetActiveSandboxCount gRPC RPCs with atomic single-column SQL (floored at zero, NULL-as-zero, IS DISTINCT FROM guard); active_sandbox_count is now a nullable read-only column excluded from the patch request. - control-plane: SandboxCountReconciler informer on agents.x-k8s.io/sandbox-name-hash with a synced-gate and periodic self-heal (drift-to-zero + restart recovery); removed the health reconciler's LIST-based counting; dropped the legacy openshell.ai/managed-by sandbox label. - web-console: non-sortable Active sandboxes column adjacent to the gateway name with a localized not-available fallback for unset counts. - specs/skills: add openshell-gateway-sandbox-count spec, finalize openshell-gateway-namespace-gc spec, and record the reconciliation in RECONCILE.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The e2e-testing.spec requires the suite to validate the gateway's read-only active_sandbox_count field on sandbox create/delete, but the assertion was never added. Fold it into the existing sandbox interaction step: reuse the running sandbox (count 1), create a second (count 2), delete it (back to 1), polling GET /gateways/<id> for each transition since the field is control-plane-owned and advisory. Record the closed gap as row E2 in RECONCILE.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Both web-console teardown paths (hot-reload EXIT cleanup and swap_down) restored the deployment with `kubectl apply -f deploy/base/web-console.yaml`. The base manifest has no OIDC env, so the apply stripped OIDC_ISSUER/OIDC_CLIENT_ID/SESSION_SECRET off the running deployment. The next dev swap then found no OIDC vars to hand the Vite /api proxy, so it forwarded requests without a bearer token and the OIDC-enforcing API server returned 401 (surfacing as "Gateways could not be loaded"). Restore from the Kind overlay instead: render deploy/kind and apply only the web-console's own resources (Deployment, Service, ServiceAccount) so the OIDC env is preserved and a concurrently swapped api-server or control-plane is never reset to its baseline image. Falls back to the base manifest if kustomize rendering yields nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…list active_sandbox_count is a control-plane-owned observability signal. The coarse gRPC role check grants any gateway:creator/owner every non-read method in any namespace, so an ordinary owner could call AdjustActiveSandboxCount / SetActiveSandboxCount and forge the count. When a service-account allowlist is configured, both the unary and stream interceptors now deny these two methods for any principal that is not an allowlisted service account, rather than falling through to the role check. With no allowlist configured the standard role check still applies, as a documented single-tenant/dev fallback. Also corrects the handler.go comment, which claimed the field was written via gRPC UpdateGateway: it is written only via the two sandbox-count RPCs and is readOnly on the REST schema. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

applyDelta already wraps each adjust RPC in sandboxCountRPCTimeout, but the self-heal loop issued its set RPCs with no deadline. A single wedged API server could therefore stall the entire self-heal pass. Each set RPC now runs under its own sandboxCountRPCTimeout, cancelled before the next iteration so cancels are not leaked until the loop ends. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…e survives Kubernetes only garbage-collects a gateway's in-namespace objects as a side effect of deleting the namespace. On the delete path, when DeleteManagedNamespace leaves the namespace in place (most importantly a pre-existing namespace this control plane does not manage, which the NamespaceGCReconciler also skips) those workloads would be orphaned. DeleteLabeledNamespaceResources now sweeps only resources carrying hypershell.redhat.io/managed=true across the gateway's GVR set (plus the optional cert-manager / Gateway API kinds), so co-tenant workloads in a shared namespace are never touched and the namespace itself is never deleted. It runs only when DeleteManagedNamespace reports the namespace was not reaped, preserving the no-reap-shared-namespace guarantee. It is best-effort with per-resource WARN logs, matching DeleteGatewayResources. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

poll_active_sandbox_count echoes its last observed value on stdout so callers can capture it via command substitution, but the per-iteration progress line went to stdout too and polluted that capture. Send the progress line to stderr so only the final value reaches the caller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The web-console hot-reload path runs pnpm install plus three dependency builds before Vite starts, and Vite's first run pre-bundles the large PatternFly-heavy graph with no output for up to ~1 minute, which reads as a hang. Announce each phase (install, dependency build, dev-server start) and note the silent first-run optimization so the wait is legible. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Align map literal keys in TestDeleteLabeledNamespaceResources so the control-plane gofmt lint check passes. Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

squizzi commented Aug 17, 2026

View reviewed changes

squizzi added amber/self-review This PR was reviewed by the Amber review agent by one of the contributors to the PR. amber/changes-requested Amber requested changes on this PR labels Aug 17, 2026

squizzi commented Aug 17, 2026

View reviewed changes

Comment thread components/control-plane/internal/reconciler/reconciler.go

Comment thread components/control-plane/internal/reconciler/reconciler.go Outdated

squizzi added amber/approved The Amber review agent has approved this PR. and removed amber/changes-requested Amber requested changes on this PR labels Aug 17, 2026

squizzi commented Aug 18, 2026

View reviewed changes

squizzi and others added 22 commits August 18, 2026 15:59

squizzi and others added 3 commits August 18, 2026 16:02

chore(control-plane): gofmt reconciler_test.go for CI

df5a675

Align map literal keys in TestDeleteLabeledNamespaceResources so the control-plane gofmt lint check passes. Signed-off-by: Kyle Squizzato <kysquizz@redhat.com>

squizzi force-pushed the squizzi/hypershell-78/namespace-gc branch from 05f7d1c to df5a675 Compare August 18, 2026 23:03

squizzi changed the title ~~feat: gateway namespace GC and active sandbox count before deletion~~ [HYPERSHELL-78][HYPERSHELL-96] feat: gateway namespace GC and active sandbox count before deletion Aug 18, 2026

squizzi marked this pull request as ready for review August 18, 2026 23:08

squizzi enabled auto-merge August 18, 2026 23:18

squizzi added this pull request to the merge queue Aug 18, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 18, 2026

Navigation Menu

Uh oh!

[HYPERSHELL-78][HYPERSHELL-96] feat: gateway namespace GC and active sandbox count before deletion - #139

[HYPERSHELL-78][HYPERSHELL-96] feat: gateway namespace GC and active sandbox count before deletion#139
squizzi wants to merge 25 commits into
mainfrom
squizzi/hypershell-78/namespace-gc

squizzi commented Aug 17, 2026 •

edited

Loading

Uh oh!

squizzi left a comment

Uh oh!

squizzi Aug 17, 2026

Uh oh!

squizzi Aug 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

squizzi left a comment

Uh oh!

Uh oh!

Uh oh!

squizzi left a comment

Uh oh!

squizzi Aug 18, 2026

Uh oh!

squizzi Aug 18, 2026

Uh oh!

squizzi Aug 18, 2026

Uh oh!

squizzi Aug 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

squizzi commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Images

Uh oh!

squizzi left a comment

Choose a reason for hiding this comment

Amber review — HYPERSHELL-78 namespace GC + active-sandbox surfacing

Findings Summary

Convention Checklist

Uh oh!

squizzi Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

squizzi Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

squizzi left a comment

Choose a reason for hiding this comment

Amber re-review — PR #139 (self-review)

Previously-raised findings — all resolved

Remaining (non-blocking)

Findings Summary

Convention Checklist

Uh oh!

Uh oh!

Uh oh!

squizzi left a comment

Choose a reason for hiding this comment

Uh oh!

squizzi Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

squizzi Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

squizzi Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

squizzi Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

squizzi commented Aug 17, 2026 •

edited

Loading