Include cluster UID in CONTROLLER_IDENTITY to prevent cross-cluster conflicts (#309)

Completes the two-release migration for including the cluster namespace
UID in the controller identity string.

Previously, the new `{identity}/{namespaceUID}` format was prepared
behind a "next release" comment while the bare `{identity}` string
remained active. This PR flips that: `getControllerIdentity()` now
returns the full `{identity}/{suffix}` format, and
`getDeprecatedControllerIdentity()` handles reclamation of Worker
Deployments previously claimed under the old format.

**Changes:**
- `getControllerIdentity()` now returns
`{CONTROLLER_IDENTITY}/{CONTROLLER_IDENTITY_SUFFIX}`; returns empty
string if either is unset
- `getDeprecatedControllerIdentity()` (renamed from
`getControllerIdentityWithNamespaceUID`) is used only for
backward-compatible reclamation
- Startup guard in `main()` fails fast if `CONTROLLER_IDENTITY` is unset
- Fallback guard in `Reconcile()` for library users who bypass `main()`
requires that `getControllerIdentity() != ""`
- Safety check in `claimManagerIdentity()` refuses to call
`SetManagerIdentity` with an empty string to prevent accidental field
clearing
- Renamed `ToBeDeprecatedDefaultControllerIdentity` →
`DeprecatedDefaultControllerIdentity`
- Updated tests to set both `IdentityEnvKey` and `IdentitySuffixEnvKey`

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: carlydf

cmd/main.go

@@ -121,6 +121,12 @@ func main() {
 		setupLog.Error(nil, "POD_NAMESPACE environment variable must be set")
 		os.Exit(1)
 	}
+
+	if os.Getenv(controller.IdentityEnvKey) == "" {
+		setupLog.Error(nil, "CONTROLLER_IDENTITY environment variable must be set")
+		os.Exit(1)
+	}
+
 	var ns corev1.Namespace
 	if err := mgr.GetAPIReader().Get(context.Background(), types.NamespacedName{Name: podNamespace}, &ns); err != nil {
 		setupLog.Error(err, "unable to fetch namespace UID for controller identity suffix")

internal/controller/execplan.go

@@ -188,10 +188,10 @@ func (r *TemporalWorkerDeploymentReconciler) shouldClaimManagerIdentity(vcfg *pl
 	if existing == "" {
 		return true // unclaimed
 	}
-	// In the next release, the namespace UID will be included in the controller identity.
-	// To support smooth rollback, in this release, we will detect the future format and
-	// treat that as a reclaimable claim.
-	if existing == getControllerIdentityWithNamespaceUID() {
+
+	// Handle Worker Deployments that were controller-managed before we
+	// started recording the cluster-UID in the manager identity
+	if existing == getDeprecatedControllerIdentity() {
 		return true
 	}
 	return false
@@ -205,6 +205,14 @@ func (r *TemporalWorkerDeploymentReconciler) claimManagerIdentity(
 	vcfg *planner.VersionConfig,
 ) error {
 	identity := getControllerIdentity()
+	if identity == "" {
+		// Passing an empty identity to SetManagerIdentity clears the field on the
+		// Worker Deployment, leaving it ownerless. Refuse rather than cause that.
+		// This should never happen, but this is the extra fallback in case somehow
+		// the check in main() and Reconcile() are not sufficient.
+		return errors.New(fmt.Sprintf("%s and %s are not set; refusing to call SetManagerIdentity to avoid clearing the manager identity field",
+			IdentityEnvKey, IdentitySuffixEnvKey))
+	}
 	resp, err := deploymentHandler.SetManagerIdentity(ctx, sdkclient.WorkerDeploymentSetManagerIdentityOptions{
 		Self:          true,
 		ConflictToken: vcfg.ConflictToken,

internal/controller/suite_test.go

@@ -29,6 +29,7 @@ var testEnv *envtest.Environment
 
 func TestMain(m *testing.M) {
 	_ = os.Setenv(IdentityEnvKey, "test-controller-identity")
+	_ = os.Setenv(IdentitySuffixEnvKey, "123")
 	os.Exit(m.Run())
 }
 

internal/controller/util.go

@@ -58,21 +58,24 @@ func getControllerVersion() string {
 	return "unknown"
 }
 
-// getControllerIdentity returns the identity from environment variable (set by Helm).
-// Returns empty string if unset. main() enforces this at startup, but that check is
-// bypassed if the reconciler is used as a library (e.g. embedded in another controller
-// manager or in tests). An empty return means the env var was not set before starting.
-func getControllerIdentity() string {
+// getDeprecatedControllerIdentity is used for smooth identity reclamation when rolling
+// forward to the new identity format.
+func getDeprecatedControllerIdentity() string {
 	if identity := os.Getenv(IdentityEnvKey); identity != "" {
 		return identity
 	}
-	return defaults.ToBeDeprecatedDefaultControllerIdentity
+	return defaults.DeprecatedDefaultControllerIdentity
 }
 
-// getControllerIdentityWithNamespaceUID returns the identity which will be used in the
-// next release. Used in this release for smooth rollback identity reclamation.
-func getControllerIdentityWithNamespaceUID() string {
-	return getControllerIdentity() + "/" + os.Getenv(IdentitySuffixEnvKey)
+// getControllerIdentity returns the identity with controller env var and namespace uid suffix.
+// Presence of both are ensured by main() and in Reconcile() for users of the controller as a library.
+func getControllerIdentity() string {
+	id := os.Getenv(IdentityEnvKey)
+	suffix := os.Getenv(IdentitySuffixEnvKey)
+	if id != "" && suffix != "" {
+		return id + "/" + suffix
+	}
+	return ""
 }
 
 func GetControllerMaxDeploymentVersionsIneligibleForDeletion() int32 {

internal/controller/worker_controller.go

@@ -135,6 +135,14 @@ func (r *TemporalWorkerDeploymentReconciler) Reconcile(ctx context.Context, req
 
 	l := log.FromContext(ctx)
 
+	// Fallback identity check for when the reconciler is used as a library and
+	// main() is not in the call path. main() is kept as the primary check for
+	// faster feedback in normal Helm-based deployments.
+	if getControllerIdentity() == "" {
+		return ctrl.Result{}, errors.New(fmt.Sprintf("%s and %s are not set",
+			IdentityEnvKey, IdentitySuffixEnvKey))
+	}
+
 	l.V(1).Info("Running Reconcile loop")
 
 	// Fetch the worker deployment

internal/defaults/defaults.go

@@ -12,6 +12,7 @@ const (
 	ServerMaxVersions                = 100
 	MaxVersionsIneligibleForDeletion = int32(ServerMaxVersions * 0.75)
 
-	// ToBeDeprecatedDefaultControllerIdentity will stop being used in the next release.
-	ToBeDeprecatedDefaultControllerIdentity = "temporal-worker-controller"
+	// DeprecatedDefaultControllerIdentity is no longer used, except to detect deployments previously
+	// managed by this identity for clean reclamation with the new identity format.
+	DeprecatedDefaultControllerIdentity = "temporal-worker-controller"
 )

internal/tests/internal/env_helpers.go

@@ -41,7 +41,9 @@ const (
 	testDrainageRefreshInterval          = time.Second
 	testMaxVersionsIneligibleForDeletion = 5
 	testMaxVersionsInDeployment          = 6
-	testControllerIdentity               = "test-controller-identity"
+	testControllerIdentityPrefix         = "test-controller-identity"
+	testControllerIdentitySuffix         = "123"
+	testControllerIdentity               = testControllerIdentityPrefix + "/" + testControllerIdentitySuffix
 )
 
 // setupKubebuilderAssets sets up the KUBEBUILDER_ASSETS environment variable if not already set
@@ -96,7 +98,8 @@ func getRepoRoot(t *testing.T) string {
 func setupTestEnvironment(t *testing.T) (*rest.Config, client.Client, manager.Manager, *clientpool.ClientPool, func()) {
 	// Set faster reconcile interval for testing
 	t.Setenv("RECONCILE_INTERVAL", "1s")
-	t.Setenv(controller.IdentityEnvKey, testControllerIdentity)
+	t.Setenv(controller.IdentityEnvKey, testControllerIdentityPrefix)
+	t.Setenv(controller.IdentitySuffixEnvKey, testControllerIdentitySuffix)
 	if kubeAssets := os.Getenv("KUBEBUILDER_ASSETS"); kubeAssets == "" {
 		t.Skip("Skipping because KUBEBUILDER_ASSETS not set")
 	}