Skip to content

fix(controller): clean retained workspace namespace metadata - #433

Open
Kevin Cho (chokevin) wants to merge 10 commits into
mainfrom
chokevin-clean-workspace-metadata
Open

Kevin Cho (chokevin) wants to merge 10 commits into
mainfrom
chokevin-clean-workspace-metadata

Conversation

@chokevin

@chokevin Kevin Cho (chokevin) commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix workspace finalization and namespace retargeting so retained external queue contracts remain admissible and executable while unfinished Workloads drain. Before deleting workload identity resources, the controller enumerates unfinished Workloads, resolves each surviving LocalQueue and its current or admitted ClusterQueue, verifies recorded ClusterQueue UID continuity when available, and evaluates the live namespace selector against current and post-cleanup labels. If cleanup would break the route, it keeps the namespace metadata and required execution ServiceAccount, and retains the previous resolved target until the Workload finishes.

Researcher authorization is handled independently of that execution-resource drain. During deletion, every workspace-owned researcher RoleBinding is revoked before waiting for unfinished Workloads. During retargeting, a changed researcher subject is applied immediately to the old namespace RoleBinding, and switching to cluster-wide authorization removes the old subject-specific binding. In both paths, the ServiceAccount needed by pending Jobs remains protected until the Workload drains.

Route discovery does not depend on status.queue, covering legacy deletion after ownership migration with empty queue status and Workloads using supported compatibility aliases. After relevant Workloads finish, finalization or retargeting removes stale RBAC, workload identity, queue resources, and TauGrid namespace metadata as originally intended. Unrelated, out-of-namespace, missing-queue, and selector-independent Workloads do not block cleanup. Queue and selector read failures fail closed, and external LocalQueues and ClusterQueues are never mutated or deleted.

The branch includes a normal merge of current origin/main at 0d5ad5ad; no rebase or force-push was used.

Implementation and test drafting used GitHub Copilot; the resulting diff and validation were reviewed before submission.

Related issue

N/A — verified TauGrid v0.4.5 live controller defect found during release validation.

Validation

PASS: cd controllers/tau-core && go test -race -count=1 -run 'TestWorkspaceDeletePreservesWorkloadIdentityWhileExternalQueueWorkloadDrains$|TestWorkspaceDeleteCleansWorkspaceAccess$|TestWorkspaceDeletePreservesNamespaceRoutingMetadataForExternalQueueWorkloads$' ./internal/controller
PASS: cd controllers/tau-core && make test
      go test -race -count=1 ./...
PASS: cd controllers/tau-core && make lint
      go vet ./...
      gofmt -l . | tee /dev/stderr | (! read -r _)
PASS: make toolchain-pins
      Go toolchain pins are consistent: toolchain 1.27.3, modules 1.27.2
PASS: git diff --check origin/main...HEAD
PASS: python3 scripts/check-license-headers.py
      License header check passed for 879 source files.

Validation ran locally on macOS against current origin/main. No cluster resources were mutated.

Compatibility and operational impact

No public API, CLI, CRD, configuration, image, chart, dependency, or security contract changes from this PR. Deleting or retargeting a workspace with unfinished Workloads on a retained external or compatibility queue can remain pending until the Workloads finish or queue/selector evidence becomes readable, including legacy workspaces whose queue status was never populated. During that interval the namespace retains admission routing metadata and the workspace-owned ServiceAccount needed by pending Jobs, while researcher access is revoked on deletion or reconciled to the current authorization during retargeting. Fully drained namespaces still remove stale TauGrid metadata and execution access, and external queues remain untouched.

Checklist

  • The change is focused and does not include unrelated edits.
  • Tests cover the changed behavior, or I explained why tests are not needed.
  • Documentation and examples are updated when public behavior changes.
  • Generated files are updated from their source and included in this pull request.
  • New dependencies are publicly available and their license and provenance were reviewed.
  • The diff contains no secrets, private endpoints, customer data, or Microsoft-internal information.
  • Fork-based CI can validate the change without repository secrets, or maintainer-only validation is clearly identified.

Comment thread controllers/tau-core/internal/controller/workspace_cleanup.go
Comment thread controllers/tau-core/internal/controller/workspace_cleanup.go Outdated
Comment thread controllers/tau-core/internal/controller/workspace_cleanup.go
Comment thread controllers/tau-core/internal/controller/workspace_cleanup.go
Comment thread controllers/tau-core/internal/controller/workspace_sync.go
Comment thread controllers/tau-core/internal/controller/workspace_cleanup.go
conditions = append(conditions, boolCondition(tauv1alpha1.ConditionDriftDetected, true, "RBACCleanupFailed", err.Error(), workspace.Generation))
}
if previousAccessReady {
if err := r.cleanupOwnedNamespaceWorkloadIdentity(ctx, workspace, keepServiceAccount); err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When retargeting also changes or removes workloadIdentity, this guard retains the obsolete ServiceAccount while the old namespace RoleBinding is reassigned to the current researcher. With an existing federated identity, that researcher can create new Jobs using the retained account and its previous Azure permissions, despite the identity change in the current spec. An unfinished Workload keeps this access available across reconciles. Separate obsolete-identity revocation from drain retention, or prevent new use of the old account while preserving already-submitted work; cover this combination with a changed researcher subject.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants