clean up auth mode and secret name handling

api/v1alpha1/connection_types.go

@@ -5,10 +5,22 @@
 package v1alpha1
 
 import (
+	"errors"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+// AuthMode is the mode for authenticating to a Temporal server.
+type AuthMode string
+
+const (
+	AuthModeTLS           AuthMode = "TLS"
+	AuthModeAPIKey        AuthMode = "API_KEY"
+	AuthModeNoCredentials AuthMode = "NO_CREDENTIALS"
+	// Add more auth modes here as they are supported
+)
+
 // NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.
 
 // SecretReference contains the name of a Secret resource in the same namespace.
@@ -59,6 +71,38 @@ type ConnectionSpec struct {
 	APIKeySecretRef *corev1.SecretKeySelector `json:"apiKeySecretRef,omitempty"`
 }
 
+// AuthMode returns the authentication mode for the ConnectionSpec.
+func (s ConnectionSpec) AuthMode() AuthMode {
+	switch {
+	case s.MutualTLSSecretRef != nil:
+		return AuthModeTLS
+	case s.APIKeySecretRef != nil:
+		return AuthModeAPIKey
+	default:
+		return AuthModeNoCredentials
+	}
+}
+
+// SecretName extracts the secret name from the ConnectionSpec, returning an
+// error if the secret name is missing and the authentication mode requires it.
+func (s ConnectionSpec) SecretName() (string, error) {
+	authMode := s.AuthMode()
+	switch authMode {
+	case AuthModeTLS:
+		if s.MutualTLSSecretRef == nil || s.MutualTLSSecretRef.Name == "" {
+			return "", errors.New("TLS secret name is not set")
+		}
+		return s.MutualTLSSecretRef.Name, nil
+	case AuthModeAPIKey:
+		if s.APIKeySecretRef == nil || s.APIKeySecretRef.Name == "" {
+			return "", errors.New("API key secret name is not set")
+		}
+		return s.APIKeySecretRef.Name, nil
+	default:
+		return "", nil
+	}
+}
+
 func (s ConnectionSpec) TLSServerName() string {
 	if s.TLS == nil {
 		return ""

internal/controller/clientpool/clientpool.go

@@ -22,21 +22,12 @@ import (
 	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-type AuthMode string
-
-const (
-	AuthModeTLS           AuthMode = "TLS"
-	AuthModeAPIKey        AuthMode = "API_KEY"
-	AuthModeNoCredentials AuthMode = "NO_CREDENTIALS"
-	// Add more auth modes here as they are supported
-)
-
 type ClientPoolKey struct {
 	HostPort      string
 	TLSServerName string
-	Namespace     string   // Temporal namespace
-	SecretName    string   // Include secret name in key to invalidate cache when the secret name changes
-	AuthMode      AuthMode // Include auth mode in key to invalidate cache when the auth mode changes for the secret
+	Namespace     string            // Temporal namespace
+	SecretName    string            // Include secret name in key to invalidate cache when the secret name changes
+	AuthMode      v1alpha1.AuthMode // Include auth mode in key to invalidate cache when the auth mode changes for the secret
 }
 
 type MTLSAuth struct {
@@ -45,7 +36,7 @@ type MTLSAuth struct {
 }
 
 type ClientAuth struct {
-	mode AuthMode
+	mode v1alpha1.AuthMode
 	mTLS *MTLSAuth // non-nil when mode == AuthMTLS, nil when mode == AuthAPIKey
 }
 
@@ -100,7 +91,7 @@ func (cp *ClientPool) GetSDKClient(key ClientPoolKey) (sdkclient.Client, bool) {
 		return nil, false
 	}
 
-	if key.AuthMode == AuthModeTLS {
+	if key.AuthMode == v1alpha1.AuthModeTLS {
 		// Check if any certificate is expired
 		expired, err := isCertificateExpired(info.auth.mTLS.expiryTime)
 		if err != nil {
@@ -184,10 +175,10 @@ func (cp *ClientPool) fetchClientUsingMTLSSecret(secret corev1.Secret, opts NewC
 		TLSServerName: tlsServerName,
 		Namespace:     opts.TemporalNamespace,
 		SecretName:    opts.Spec.MutualTLSSecretRef.Name,
-		AuthMode:      AuthModeTLS,
+		AuthMode:      v1alpha1.AuthModeTLS,
 	}
 	auth := ClientAuth{
-		mode: AuthModeTLS,
+		mode: v1alpha1.AuthModeTLS,
 		mTLS: &MTLSAuth{tlsConfig: clientOpts.ConnectionOptions.TLS, expiryTime: expiryTime},
 	}
 	return &clientOpts, &key, &auth, nil
@@ -217,10 +208,10 @@ func (cp *ClientPool) fetchClientUsingAPIKeySecret(opts NewClientOptions) (*sdkc
 		TLSServerName: tlsServerName,
 		Namespace:     opts.TemporalNamespace,
 		SecretName:    opts.Spec.APIKeySecretRef.Name,
-		AuthMode:      AuthModeAPIKey,
+		AuthMode:      v1alpha1.AuthModeAPIKey,
 	}
 	auth := ClientAuth{
-		mode: AuthModeAPIKey,
+		mode: v1alpha1.AuthModeAPIKey,
 		mTLS: nil,
 	}
 
@@ -244,10 +235,10 @@ func (cp *ClientPool) fetchClientUsingNoCredentials(opts NewClientOptions) (*sdk
 		TLSServerName: tlsServerName,
 		Namespace:     opts.TemporalNamespace,
 		SecretName:    "",
-		AuthMode:      AuthModeNoCredentials,
+		AuthMode:      v1alpha1.AuthModeNoCredentials,
 	}
 	auth := ClientAuth{
-		mode: AuthModeNoCredentials,
+		mode: v1alpha1.AuthModeNoCredentials,
 		mTLS: nil,
 	}
 
@@ -257,7 +248,7 @@ func (cp *ClientPool) fetchClientUsingNoCredentials(opts NewClientOptions) (*sdk
 func (cp *ClientPool) ParseClientSecret(
 	ctx context.Context,
 	secretName string,
-	authMode AuthMode,
+	authMode v1alpha1.AuthMode,
 	opts NewClientOptions,
 ) (*sdkclient.Options, *ClientPoolKey, *ClientAuth, error) {
 	// Fetch the secret from k8s cluster, if it exists. Otherwise, create a connection with the server without using any credentials.
@@ -273,21 +264,21 @@ func (cp *ClientPool) ParseClientSecret(
 
 	// Check the secret type
 	switch authMode {
-	case AuthModeTLS:
+	case v1alpha1.AuthModeTLS:
 		if secret.Type != corev1.SecretTypeTLS && secret.Type != corev1.SecretTypeOpaque {
 			err := fmt.Errorf("secret %s must be of type kubernetes.io/tls or Opaque", secret.Name)
 			return nil, nil, nil, err
 		}
 		return cp.fetchClientUsingMTLSSecret(secret, opts)
 
-	case AuthModeAPIKey:
+	case v1alpha1.AuthModeAPIKey:
 		if secret.Type != corev1.SecretTypeOpaque {
 			err := fmt.Errorf("secret %s must be of type kubernetes.io/opaque", secret.Name)
 			return nil, nil, nil, err
 		}
 		return cp.fetchClientUsingAPIKeySecret(opts)
 
-	case AuthModeNoCredentials:
+	case v1alpha1.AuthModeNoCredentials:
 		return cp.fetchClientUsingNoCredentials(opts)
 
 	default:
@@ -305,7 +296,7 @@ func (cp *ClientPool) DialAndUpsertClient(clientOpts sdkclient.Options, clientPo
 	// (non-namespace-scoped) RPC that fails with namespace-scoped API keys
 	// on Temporal Cloud. This is safe because client.Dial already calls
 	// GetSystemInfo internally, which is a superset of CheckHealth.
-	if clientAuth.mode != AuthModeAPIKey {
+	if clientAuth.mode != v1alpha1.AuthModeAPIKey {
 		if _, err := c.CheckHealth(context.Background(), &sdkclient.CheckHealthRequest{}); err != nil {
 			c.Close()
 			return nil, fmt.Errorf("temporal server health check failed: %w", err)

internal/controller/clientpool/clientpool_test.go

@@ -230,8 +230,8 @@ func TestFetchMTLS_ValidCert_Succeeds(t *testing.T) {
 	clientOpts, key, auth, err := cp.fetchClientUsingMTLSSecret(secret, makeOpts("localhost:7233"))
 
 	require.NoError(t, err)
-	assert.Equal(t, AuthModeTLS, key.AuthMode)
-	assert.Equal(t, AuthModeTLS, auth.mode)
+	assert.Equal(t, temporaliov1alpha1.AuthModeTLS, key.AuthMode)
+	assert.Equal(t, temporaliov1alpha1.AuthModeTLS, auth.mode)
 	assert.NotNil(t, auth.mTLS)
 	assert.NotNil(t, clientOpts.ConnectionOptions.TLS)
 	assert.Len(t, clientOpts.ConnectionOptions.TLS.Certificates, 1)
@@ -306,7 +306,7 @@ func TestFetchNoCredentials_TLSServerNameOverride(t *testing.T) {
 	require.NotNil(t, clientOpts.ConnectionOptions.TLS)
 	assert.Equal(t, "temporal-cloud.example.com", clientOpts.ConnectionOptions.TLS.ServerName)
 	assert.Equal(t, "temporal-cloud.example.com", key.TLSServerName)
-	assert.Equal(t, AuthModeNoCredentials, auth.mode)
+	assert.Equal(t, temporaliov1alpha1.AuthModeNoCredentials, auth.mode)
 }
 
 func newTestPoolWithFakeClient(objects ...runtime.Object) *ClientPool {
@@ -349,8 +349,8 @@ func TestFetchAPIKey_CredentialsAndTLSSet(t *testing.T) {
 	clientOpts, key, auth, err := cp.fetchClientUsingAPIKeySecret(opts)
 
 	require.NoError(t, err)
-	assert.Equal(t, AuthModeAPIKey, key.AuthMode)
-	assert.Equal(t, AuthModeAPIKey, auth.mode)
+	assert.Equal(t, temporaliov1alpha1.AuthModeAPIKey, key.AuthMode)
+	assert.Equal(t, temporaliov1alpha1.AuthModeAPIKey, auth.mode)
 	assert.Nil(t, auth.mTLS)
 	assert.NotNil(t, clientOpts.Credentials, "API key credentials must be set")
 	assert.NotNil(t, clientOpts.ConnectionOptions.TLS, "TLS config must be non-nil for gRPC API key transport")
@@ -401,8 +401,8 @@ func TestParseClientSecret_OpaqueSecretType(t *testing.T) {
 	_, key, auth, err := cp.fetchClientUsingMTLSSecret(opaqueSecret, makeOpts("localhost:7233"))
 
 	require.NoError(t, err, "Opaque secret with tls.crt and tls.key should be accepted for mTLS auth")
-	assert.Equal(t, AuthModeTLS, key.AuthMode)
-	assert.Equal(t, AuthModeTLS, auth.mode)
+	assert.Equal(t, temporaliov1alpha1.AuthModeTLS, key.AuthMode)
+	assert.Equal(t, temporaliov1alpha1.AuthModeTLS, auth.mode)
 	assert.NotNil(t, auth.mTLS)
 }
 
@@ -414,15 +414,15 @@ func TestParseClientSecret_OpaqueSecretType(t *testing.T) {
 // Cloud, namespace-scoped API keys do not have permission to call the system-scoped
 // CheckHealth RPC, so every connection attempt failed.
 //
-// After the fix, CheckHealth is skipped for AuthModeAPIKey because client.Dial already
+// After the fix, CheckHealth is skipped for temporaliov1alpha1.AuthModeAPIKey because client.Dial already
 // calls GetSystemInfo internally (a superset of CheckHealth).
 func TestDialAndUpsert_APIKeySkipsCheckHealth(t *testing.T) {
 	mock := &mockSDKClient{}
 	cp := newTestPool()
 	cp.dialFn = func(_ sdkclient.Options) (sdkclient.Client, error) { return mock, nil }
 
-	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: AuthModeAPIKey}
-	auth := ClientAuth{mode: AuthModeAPIKey}
+	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: temporaliov1alpha1.AuthModeAPIKey}
+	auth := ClientAuth{mode: temporaliov1alpha1.AuthModeAPIKey}
 
 	c, err := cp.DialAndUpsertClient(sdkclient.Options{}, key, auth)
 
@@ -439,9 +439,9 @@ func TestDialAndUpsert_TLSCallsCheckHealth(t *testing.T) {
 	cp := newTestPool()
 	cp.dialFn = func(_ sdkclient.Options) (sdkclient.Client, error) { return mock, nil }
 
-	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: AuthModeTLS}
+	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: temporaliov1alpha1.AuthModeTLS}
 	auth := ClientAuth{
-		mode: AuthModeTLS,
+		mode: temporaliov1alpha1.AuthModeTLS,
 		mTLS: &MTLSAuth{tlsConfig: &tls.Config{}, expiryTime: time.Now().Add(time.Hour)},
 	}
 
@@ -458,8 +458,8 @@ func TestDialAndUpsert_NoCredsCallsCheckHealth(t *testing.T) {
 	cp := newTestPool()
 	cp.dialFn = func(_ sdkclient.Options) (sdkclient.Client, error) { return mock, nil }
 
-	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: AuthModeNoCredentials}
-	auth := ClientAuth{mode: AuthModeNoCredentials}
+	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: temporaliov1alpha1.AuthModeNoCredentials}
+	auth := ClientAuth{mode: temporaliov1alpha1.AuthModeNoCredentials}
 
 	c, err := cp.DialAndUpsertClient(sdkclient.Options{}, key, auth)
 
@@ -476,7 +476,7 @@ func TestEvictClient_RemovesAndClosesClient(t *testing.T) {
 		HostPort:   "localhost:7233",
 		Namespace:  "default",
 		SecretName: "my-secret",
-		AuthMode:   AuthModeAPIKey,
+		AuthMode:   temporaliov1alpha1.AuthModeAPIKey,
 	}
 	mock := &mockSDKClient{}
 	cp.SetClientForTesting(key, mock)
@@ -493,7 +493,7 @@ func TestEvictClient_RemovesAndClosesClient(t *testing.T) {
 
 func TestEvictClient_NoopWhenKeyAbsent(t *testing.T) {
 	cp := newTestPool()
-	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: AuthModeNoCredentials}
+	key := ClientPoolKey{HostPort: "localhost:7233", Namespace: "default", AuthMode: temporaliov1alpha1.AuthModeNoCredentials}
 	// Should not panic when key is not in the pool
 	cp.EvictClient(key)
 }

internal/controller/reconciler_events_test.go

@@ -283,7 +283,7 @@ func noCredsPoolKey(hostPort, temporalNamespace string) clientpool.ClientPoolKey
 		HostPort:   hostPort,
 		Namespace:  temporalNamespace,
 		SecretName: "",
-		AuthMode:   clientpool.AuthModeNoCredentials,
+		AuthMode:   temporaliov1alpha1.AuthModeNoCredentials,
 	}
 }
 

internal/controller/worker_controller.go

@@ -55,34 +55,6 @@ const (
 	finalizerName = "temporal.io/delete-protection"
 )
 
-// getAPIKeySecretName extracts the secret name from a SecretKeySelector
-func getAPIKeySecretName(secretRef *corev1.SecretKeySelector) (string, error) {
-	if secretRef != nil && secretRef.Name != "" {
-		return secretRef.Name, nil
-	}
-
-	return "", errors.New("API key secret name is not set")
-}
-
-func getTLSSecretName(secretRef *temporaliov1alpha1.SecretReference) (string, error) {
-	if secretRef != nil && secretRef.Name != "" {
-		return secretRef.Name, nil
-	}
-
-	return "", errors.New("TLS secret name is not set")
-}
-
-func resolveAuthSecretName(tc *temporaliov1alpha1.Connection) (clientpool.AuthMode, string, error) {
-	if tc.Spec.MutualTLSSecretRef != nil {
-		name, err := getTLSSecretName(tc.Spec.MutualTLSSecretRef)
-		return clientpool.AuthModeTLS, name, err
-	} else if tc.Spec.APIKeySecretRef != nil {
-		name, err := getAPIKeySecretName(tc.Spec.APIKeySecretRef)
-		return clientpool.AuthModeAPIKey, name, err
-	}
-	return clientpool.AuthModeNoCredentials, "", nil
-}
-
 // WorkerDeploymentReconciler reconciles a WorkerDeployment object
 type WorkerDeploymentReconciler struct {
 	client.Client
@@ -261,7 +233,8 @@ func (r *WorkerDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 
 	// Get the Auth Mode and Secret Name
-	authMode, secretName, err := resolveAuthSecretName(&connection)
+	authMode := connection.Spec.AuthMode()
+	secretName, err := connection.Spec.SecretName()
 	if err != nil {
 		l.Error(err, "unable to resolve auth secret name")
 		r.recordWarningAndSetBlocked(ctx, &workerDeploy,
@@ -563,22 +536,23 @@ func (r *WorkerDeploymentReconciler) handleDeletion(
 	// Resolve Connection.
 	// The Connection is guaranteed to exist because we hold a finalizer on it
 	// that prevents deletion while any WD references it.
-	var temporalConnection temporaliov1alpha1.Connection
+	var connection temporaliov1alpha1.Connection
 	if err := r.Get(ctx, types.NamespacedName{
 		Name:      workerDeploy.Spec.WorkerOptions.ConnectionRef.Name,
 		Namespace: workerDeploy.Namespace,
-	}, &temporalConnection); err != nil {
+	}, &connection); err != nil {
 		return fmt.Errorf("unable to fetch Connection: %w", err)
 	}
 
-	authMode, secretName, err := resolveAuthSecretName(&temporalConnection)
+	authMode := connection.Spec.AuthMode()
+	secretName, err := connection.Spec.SecretName()
 	if err != nil {
 		return fmt.Errorf("unable to resolve auth secret name: %w", err)
 	}
 
 	temporalClient, ok := r.TemporalClientPool.GetSDKClient(clientpool.ClientPoolKey{
-		HostPort:      temporalConnection.Spec.HostPort,
-		TLSServerName: temporalConnection.Spec.TLSServerName(),
+		HostPort:      connection.Spec.HostPort,
+		TLSServerName: connection.Spec.TLSServerName(),
 		Namespace:     workerDeploy.Spec.WorkerOptions.TemporalNamespace,
 		SecretName:    secretName,
 		AuthMode:      authMode,
@@ -587,7 +561,7 @@ func (r *WorkerDeploymentReconciler) handleDeletion(
 		clientOpts, key, clientAuth, err := r.TemporalClientPool.ParseClientSecret(ctx, secretName, authMode, clientpool.NewClientOptions{
 			K8sNamespace:      workerDeploy.Namespace,
 			TemporalNamespace: workerDeploy.Spec.WorkerOptions.TemporalNamespace,
-			Spec:              temporalConnection.Spec,
+			Spec:              connection.Spec,
 			Identity:          getControllerIdentity(),
 		})
 		if err != nil {