diff --git a/api/v1alpha1/connection_types.go b/api/v1alpha1/connection_types.go index 4c11bb11..f3dc1e50 100644 --- a/api/v1alpha1/connection_types.go +++ b/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 "" diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index bfc8e169..19b5f2bb 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/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) diff --git a/internal/controller/clientpool/clientpool_test.go b/internal/controller/clientpool/clientpool_test.go index 7f17e2b5..a26cf799 100644 --- a/internal/controller/clientpool/clientpool_test.go +++ b/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) } diff --git a/internal/controller/reconciler_events_test.go b/internal/controller/reconciler_events_test.go index 92b374cd..dc7fd60c 100644 --- a/internal/controller/reconciler_events_test.go +++ b/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, } } diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index c0acb5cc..a6232550 100644 --- a/internal/controller/worker_controller.go +++ b/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 {