Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions components/control-plane/internal/gateway/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"log"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
Expand All @@ -13,15 +14,31 @@ import (
"k8s.io/client-go/kubernetes"
)

// Labels createNamespace stamps on every gateway namespace. Garbage collection
// only ever deletes namespaces carrying BOTH labels, so a Gateway pointed at a
// pre-existing shared namespace can never cause that namespace to be reaped.
// Labels createNamespace stamps on managed namespaces. Gateway namespace garbage
// collection sweeps managed namespaces whose names match the gateway prefix
// (openshell-<hex>) and excludes ManagedDatabase namespaces (openshell-db-<hex>).
const (
ManagedByLabel = "app.kubernetes.io/managed-by"
ManagedByValue = "hypershell-control-plane"
ManagedLabel = "hypershell.redhat.io/managed"
ManagedLabelValue = "true"

// GatewayNamespacePrefix and DatabaseNamespacePrefix mirror the namespace names
// the API server assigns in its BeforeCreate hooks (gatewayNamespacePrefix in
// components/api-server/plugins/gateways/model.go and dbNamespacePrefix in
// components/api-server/plugins/managedDatabases/model.go). Both produce
// "<prefix><16 hex chars>", and both namespace kinds carry the same management
// labels, so GC cannot tell them apart by label alone and falls back to the name.
//
// The trailing dash in DatabaseNamespacePrefix is load-bearing: a gateway hash
// may legitimately begin with the hex letters "db" (e.g. openshell-db1a2b...),
// but never with "openshell-db-" because the character after "db" is always a
// hex digit, never a dash. Keep these two constants in sync with the API server;
// if that naming ever changes, gateway GC would silently start reaping (or
// sparing) the wrong namespaces.
GatewayNamespacePrefix = "openshell-"
DatabaseNamespacePrefix = "openshell-db-"

// GCEligibleSinceAnnotation records, in RFC3339, when a managed namespace was
// first observed orphaned (no live Gateway). The grace period is measured
// from this timestamp so it survives control-plane restarts.
Expand All @@ -39,10 +56,24 @@ func IsManagedNamespace(ns *corev1.Namespace) bool {
ns.Labels[ManagedByLabel] == ManagedByValue
}

// IsGatewayNamespaceForGC reports whether ns is a gateway workload namespace
// subject to gateway namespace garbage collection. ManagedDatabase CNPG
// namespaces (openshell-db-*) carry the same management labels but are owned by
// the ManagedDatabase reconciler. Name-prefix matching keeps pre-existing orphaned
// gateway namespaces eligible for periodic GC without a label migration.
func IsGatewayNamespaceForGC(ns *corev1.Namespace) bool {
if !IsManagedNamespace(ns) {
return false
}
return strings.HasPrefix(ns.Name, GatewayNamespacePrefix) &&
!strings.HasPrefix(ns.Name, DatabaseNamespacePrefix)
}

// DeleteManagedNamespace deletes a gateway namespace, best-effort and
// idempotent. It only deletes namespaces this control plane manages (see
// IsManagedNamespace): an unmanaged or already-absent namespace is treated as a
// no-op success. It returns deleted=true only when a delete call was issued.
// idempotent. It only deletes namespaces subject to gateway namespace GC (see
// IsGatewayNamespaceForGC): an unmanaged, non-gateway, or already-absent
// namespace is treated as a no-op success. It returns deleted=true only when a
// delete call was issued.
func DeleteManagedNamespace(ctx context.Context, client kubernetes.Interface, namespace string) (bool, error) {
ns, err := client.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{})
if err != nil {
Expand All @@ -52,8 +83,8 @@ func DeleteManagedNamespace(ctx context.Context, client kubernetes.Interface, na
}
return false, fmt.Errorf("get namespace %s: %w", namespace, err)
}
if !IsManagedNamespace(ns) {
log.Printf("INFO namespace %s is not managed by hypershell-control-plane, skipping deletion", namespace)
if !IsGatewayNamespaceForGC(ns) {
log.Printf("INFO namespace %s is not a gateway workload namespace, skipping deletion", namespace)
return false, nil
}
if ns.DeletionTimestamp != nil {
Expand Down
41 changes: 41 additions & 0 deletions components/control-plane/internal/gateway/namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,33 @@ func TestIsManagedNamespace(t *testing.T) {
}
}

func TestIsGatewayNamespaceForGC(t *testing.T) {
tests := []struct {
name string
ns string
want bool
}{
{"gateway namespace", "openshell-a14873d1631f1b74", true},
{"e2e orphan", "openshell-e2e-orphan-123", true},
// A gateway hash may begin with the hex letters "db"; the trailing dash in
// the database prefix keeps it classified as a gateway namespace.
{"gateway hash starting with db", "openshell-db1a2b3c4d5e6f70", true},
{"managed database namespace", "openshell-db-a1b2c3d4e5f67890", false},
{"unmanaged", "openshell-gw", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ns := managedNamespace(tt.ns, nil)
if tt.name == "unmanaged" {
ns.Labels = nil
}
if got := IsGatewayNamespaceForGC(ns); got != tt.want {
t.Errorf("IsGatewayNamespaceForGC() = %v, want %v", got, tt.want)
}
})
}
}

func TestDeleteManagedNamespace(t *testing.T) {
ctx := context.Background()

Expand Down Expand Up @@ -78,6 +105,20 @@ func TestDeleteManagedNamespace(t *testing.T) {
}
})

t.Run("skips a managed database namespace", func(t *testing.T) {
client := fake.NewSimpleClientset(managedNamespace("openshell-db-a1b2c3d4e5f67890", nil))
deleted, err := DeleteManagedNamespace(ctx, client, "openshell-db-a1b2c3d4e5f67890")
if err != nil {
t.Fatalf("DeleteManagedNamespace() error = %v", err)
}
if deleted {
t.Errorf("deleted = true, want false for ManagedDatabase namespace")
}
if _, err := client.CoreV1().Namespaces().Get(ctx, "openshell-db-a1b2c3d4e5f67890", metav1.GetOptions{}); err != nil {
t.Errorf("ManagedDatabase namespace should be preserved, err = %v", err)
}
})

t.Run("absent namespace is a no-op success", func(t *testing.T) {
client := fake.NewSimpleClientset()
deleted, err := DeleteManagedNamespace(ctx, client, "gone")
Expand Down
16 changes: 11 additions & 5 deletions components/control-plane/internal/reconciler/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ func (r *NamespaceGCReconciler) grpcLiveNamespaces(ctx context.Context) (map[str
// namespaces backed by a live Gateway and reaps it if it has been orphaned past
// the grace period. It is best-effort and idempotent.
func (r *NamespaceGCReconciler) reconcileNamespace(ctx context.Context, ns *corev1.Namespace, live map[string]struct{}) error {
// Defense in depth: only ever act on namespaces this control plane manages,
// even if the server-side label selector over-returns.
if !gateway.IsManagedNamespace(ns) {
// Defense in depth: only gateway workload namespaces are subject to this
// reconciler, even if the server-side label selector over-returns.
if !gateway.IsGatewayNamespaceForGC(ns) {
return nil
}
// A namespace already terminating needs no further action.
Expand Down Expand Up @@ -268,8 +268,14 @@ func (r *NamespaceGCReconciler) recordGCEvent(ctx context.Context, namespace, me
Namespace: r.cpNamespace,
},
InvolvedObject: corev1.ObjectReference{
Kind: "Namespace",
Name: namespace,
APIVersion: "v1",
Kind: "Namespace",
Name: namespace,
// The Event lives in the control-plane namespace so it outlives the
// reaped namespace. Kubernetes requires involvedObject.namespace to
// match event.namespace for namespaced Events; Name still identifies
// the gateway namespace that was garbage collected.
Namespace: r.cpNamespace,
},
Reason: "GarbageCollected",
Message: message,
Expand Down
57 changes: 56 additions & 1 deletion components/control-plane/internal/reconciler/namespace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,62 @@ func TestReconcileNamespace_OrphanPastGraceIsReaped(t *testing.T) {
t.Errorf("event reason = %q, want GarbageCollected", events.Items[0].Reason)
}
if events.Items[0].InvolvedObject.Name != "openshell-gw" {
t.Errorf("event involved object = %q, want openshell-gw", events.Items[0].InvolvedObject.Name)
t.Errorf("event involved object name = %q, want openshell-gw", events.Items[0].InvolvedObject.Name)
}
if events.Items[0].InvolvedObject.Namespace != "hypershell" {
t.Errorf("event involved object namespace = %q, want hypershell", events.Items[0].InvolvedObject.Namespace)
}
}

func TestRecordGCEvent_InvolvedObjectNamespaceMatchesEventNamespace(t *testing.T) {
ctx := context.Background()
now := time.Date(2026, 8, 17, 12, 0, 0, 0, time.UTC)
client := fake.NewSimpleClientset()
r := NewNamespaceGCReconciler(client, nil, time.Minute, 10*time.Minute, "hypershell-stage")
r.now = func() time.Time { return now }

if err := r.recordGCEvent(ctx, "openshell-gw", "test message"); err != nil {
t.Fatalf("recordGCEvent() error = %v", err)
}

events, err := client.CoreV1().Events("hypershell-stage").List(ctx, metav1.ListOptions{})
if err != nil {
t.Fatalf("list events: %v", err)
}
if len(events.Items) != 1 {
t.Fatalf("event count = %d, want 1", len(events.Items))
}
ev := events.Items[0]
if ev.InvolvedObject.APIVersion != "v1" {
t.Errorf("involvedObject.apiVersion = %q, want v1", ev.InvolvedObject.APIVersion)
}
if ev.Namespace != "hypershell-stage" {
t.Errorf("event namespace = %q, want hypershell-stage", ev.Namespace)
}
if ev.InvolvedObject.Namespace != ev.Namespace {
t.Errorf("involvedObject.namespace = %q, want %q", ev.InvolvedObject.Namespace, ev.Namespace)
}
if ev.InvolvedObject.Name != "openshell-gw" {
t.Errorf("involvedObject.name = %q, want openshell-gw", ev.InvolvedObject.Name)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

func TestReconcileNamespace_SkipsManagedDatabaseNamespace(t *testing.T) {
ctx := context.Background()
now := time.Date(2026, 8, 17, 12, 0, 0, 0, time.UTC)
// ManagedDatabase namespaces share management labels but use the openshell-db- prefix.
ns := managedNS("openshell-db-a1b2c3d4e5f67890", map[string]string{
gateway.GCEligibleSinceAnnotation: now.Add(-20 * time.Minute).Format(time.RFC3339),
})
client := fake.NewSimpleClientset(ns)
r := newTestGC(client, now)

if err := r.reconcileNamespace(ctx, ns, map[string]struct{}{}); err != nil {
t.Fatalf("reconcileNamespace() error = %v", err)
}

if !nsExists(t, client, "openshell-db-a1b2c3d4e5f67890") {
t.Fatalf("ManagedDatabase namespace deleted by gateway GC, want retained")
}
}

Expand Down
7 changes: 7 additions & 0 deletions deploy/kind/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ patches:
# production/OpenShift leaves this unset to keep them enforced.
- name: GATEWAY_SKIP_NETWORK_POLICIES
value: "true"
# Shorten periodic namespace GC for e2e (production defaults 5m/10m).
# Go time.ParseDuration accepts seconds too (e.g. 30s, 1m30s).
# tests/e2e seeds a synthetic orphan after gateway provisioning (after step 2); step 11a asserts the reaper deleted it.
- name: GATEWAY_NAMESPACE_GC_INTERVAL
value: "30s"
- name: GATEWAY_NAMESPACE_GC_GRACE_PERIOD
value: "30s"
- name: OIDC_ISSUER
value: "http://keycloak-service.keycloak.svc.cluster.local:8080/realms/hypershell"
- name: OIDC_CLIENT_ID
Expand Down
83 changes: 71 additions & 12 deletions specs/platform/e2e-testing.spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ The e2e test suite SHALL validate the following 8 areas, extending the original
5. **Sandbox lifecycle** -- create a sandbox as the admin user, wait for the pod to reach `Running` state, and verify the gateway's `active_sandbox_count` accounting reflects sandbox create and delete (see Active Sandbox Count Accounting)
6. **Sandbox interaction** -- execute commands inside the sandbox (`uname -a`, `ls /workspace`)
7. **Developer user RBAC verification** -- authenticate as the `developer` user (the `openshell-user` tier) and confirm it MAY create a sandbox but MAY NOT create a gateway via the HyperShell API (see Developer RBAC Enforcement)
8. **Gateway deletion + namespace garbage collection** -- delete the gateway through the HyperShell API and verify the control plane removes the gateway record and reaps its managed namespace (see Gateway Deletion and Namespace GC, and `openshell-gateway-namespace-gc.spec.md`)
8. **Gateway deletion + namespace garbage collection** -- validate both garbage-collection paths from `openshell-gateway-namespace-gc.spec.md`: (a) seed a synthetic orphaned managed namespace after gateway provisioning and validate periodic `NamespaceGCReconciler` reap + `GarbageCollected` Event in step 11 (while steps 3–10 run in parallel with the reaper); (b) delete-driven reap of the gateway's managed namespace (see Gateway Deletion and Namespace GC)

The admin-user OIDC flow that authenticates areas 1--6 and 8 is validated separately (see OIDC Authentication in E2E Tests).

Expand Down Expand Up @@ -292,22 +292,80 @@ The e2e test suite SHALL verify the RBAC boundary of the `openshell-user` tier b

### Requirement: Gateway Deletion and Namespace GC

The e2e test suite SHALL validate that deleting a Gateway through the HyperShell
API drives the control plane to remove the gateway record and reap its managed
namespace, per `openshell-gateway-namespace-gc.spec.md`. Before deletion the suite
SHALL confirm the gateway's managed namespace exists, so its later disappearance
is a real garbage-collection signal rather than a namespace that never existed.
After issuing `DELETE /api/hypershell/v1/gateways/<id>`, the suite SHALL poll
until the gateway record returns `404` (the delete event has been processed) and
until the managed namespace is gone, within `E2E_GC_TIMEOUT` seconds. A namespace
that is not reaped within the timeout SHALL be reported as a test failure with GC
diagnostics (the namespace's remaining state and control-plane logs).
The e2e test suite SHALL validate both garbage-collection paths described in
`openshell-gateway-namespace-gc.spec.md`:

1. **Periodic reaper** -- the `NamespaceGCReconciler` sweeps managed namespaces
with no live Gateway, respects the grace period, records a `GarbageCollected`
Kubernetes Event in the control-plane namespace via `recordGCEvent`, then
deletes the namespace.
2. **Delete-driven reap** -- deleting a Gateway through the HyperShell API drives
the control plane to remove the gateway record and reap its managed namespace
(`DeleteManagedNamespace`; this path does not emit the periodic GC Event).

#### Periodic orphan namespace GC (Kind e2e)

To exercise the periodic path without waiting for production defaults (5m sweep /
10m grace), the Kind overlay SHALL patch the control-plane deployment with
`GATEWAY_NAMESPACE_GC_INTERVAL` and `GATEWAY_NAMESPACE_GC_GRACE_PERIOD` set to
short Go duration strings (for example `30s`; any positive value accepted by
`time.ParseDuration` is valid). Immediately after gateway provisioning succeeds,
the suite SHALL seed a synthetic orphaned managed namespace (`openshell-e2e-orphan-*`)
labeled with both required management labels (`hypershell.redhat.io/managed=true`
and `app.kubernetes.io/managed-by=hypershell-control-plane`) and a name matching
the gateway prefix (not `openshell-db-*`), annotate it with a
backdated `hypershell.redhat.io/gc-eligible-since` timestamp so the next sweep can
reap without waiting a full grace period. Steps 3–10 SHALL run while the periodic
reaper may delete that namespace in the background, so the suite is not blocked
waiting on the sweep interval. In step 11 the suite SHALL validate delete-driven
gateway namespace GC first, then assert the orphan namespace was reaped and a
`GarbageCollected` Event exists in the control-plane namespace
(`E2E_HS_NAMESPACE`, default `hypershell-system`) with `involvedObject.name`
equal to the orphan namespace name. The orphan reap deadline SHALL be measured
from seed time (`E2E_ORPHAN_GC_TIMEOUT` seconds after creation); if the namespace
is already gone when step 11 runs, validation SHALL pass without additional
waiting. Failure to reap or to record the Event SHALL be reported with GC
diagnostics (namespace state and control-plane logs).

#### Delete-driven gateway namespace GC

Before deletion the suite SHALL confirm the gateway's managed namespace exists,
so its later disappearance is a real garbage-collection signal rather than a
namespace that never existed. After issuing
`DELETE /api/hypershell/v1/gateways/<id>`, the suite SHALL poll until the gateway
record returns `404` (the delete event has been processed) and until the managed
namespace is gone, within `E2E_GC_TIMEOUT` seconds. A namespace that is not reaped
within the timeout SHALL be reported as a test failure with GC diagnostics (the
namespace's remaining state and control-plane logs).

Deletion SHALL NOT be gated on the gateway's active sandbox count: even with
active sandboxes the delete is accepted and the namespace is reaped, cascading
removal of the in-namespace sandbox resources (see
`openshell-gateway-namespace-gc.spec.md` and `openshell-gateway-database.spec.md`).

#### Scenario: Periodic orphan namespace garbage collected

- GIVEN the Kind overlay has shortened `GATEWAY_NAMESPACE_GC_INTERVAL` and
`GATEWAY_NAMESPACE_GC_GRACE_PERIOD` (for example `30s`)
- AND a synthetic managed namespace was seeded after gateway provisioning with
both management labels, a gateway-style name, and a backdated
`hypershell.redhat.io/gc-eligible-since`
annotation, with no live Gateway backing it
- AND steps 3–10 have run while the periodic reaper may have deleted it
- WHEN the suite validates orphan GC in step 11 (after delete-driven GC)
- THEN the namespace SHALL be gone within `E2E_ORPHAN_GC_TIMEOUT` seconds of
seeding
- AND a namespace still present after that deadline SHALL be reported as a
failure with GC diagnostics

#### Scenario: GarbageCollected Event recorded for periodic reap

- GIVEN the periodic reaper has deleted the synthetic orphan namespace
- WHEN the suite queries Events in the control-plane namespace
- THEN a `GarbageCollected` Event SHALL exist with `involvedObject.name` equal to
the orphan namespace name
- AND the absence of such an Event SHALL be reported as a test failure

#### Scenario: Namespace present before deletion

- GIVEN a `Running` gateway whose managed namespace exists
Expand Down Expand Up @@ -485,7 +543,7 @@ The `deploy/` directory SHALL use a kustomize base/overlay structure to support
- GIVEN `deploy/kind/kustomization.yaml` references `../base` as a resource
- WHEN `kustomize build deploy/kind/` is executed
- THEN the output SHALL include all base resources (namespace, postgres, api-server, controller, controller-rbac, web-console)
- AND Kind-specific resources: networking Gateway with `gatewayClassName: cloud-provider-kind`, cert-manager certificates for `*.hypershell.localhost` and `*.gw.localhost`, HTTPRoutes for component services, CoreDNS Corefile, OIDC secrets, and Kustomize patches for OIDC configuration (JWT flags, Keycloak hostname, control-plane and web-console OIDC env vars)
- AND Kind-specific resources: networking Gateway with `gatewayClassName: cloud-provider-kind`, cert-manager certificates for `*.hypershell.localhost` and `*.gw.localhost`, HTTPRoutes for component services, CoreDNS Corefile, OIDC secrets, and Kustomize patches for OIDC configuration (JWT flags, Keycloak hostname, control-plane and web-console OIDC env vars) and shortened namespace GC timing (`GATEWAY_NAMESPACE_GC_INTERVAL` and `GATEWAY_NAMESPACE_GC_GRACE_PERIOD`, for example `30s`, so e2e can exercise the periodic reaper)

#### Scenario: OpenShift Overlay

Expand Down Expand Up @@ -581,6 +639,7 @@ deploy/
| `E2E_SANDBOX_TIMEOUT` | `120` | Seconds to wait for sandbox pod readiness |
| `E2E_PROVISION_TIMEOUT` | `180` | Seconds to wait for gateway provisioning |
| `E2E_GC_TIMEOUT` | `180` | Seconds to wait for the managed namespace to be garbage collected after a gateway delete |
| `E2E_ORPHAN_GC_TIMEOUT` | `90` | Seconds from orphan namespace seed time for the periodic reaper to delete the synthetic orphan (validated in step 11) |
| `E2E_SKIP_CLEANUP` | `0` | Set to `1` to keep test resources after run |
| `E2E_OIDC_USERNAME` | `admin` | Admin OIDC user (member of `hypershell-admins` + `hypershell-users`) used for areas 1--6 |
| `E2E_OIDC_PASSWORD` | `admin` | Password for the admin OIDC user (local dev only) |
Expand Down
Loading
Loading