From a7bff2b7121c19cfcf481c826b678d44707b1feb Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Thu, 25 Sep 2025 21:48:41 -0400 Subject: [PATCH 01/15] current progress --- api/v1alpha1/temporalconnection_types.go | 5 ++ api/v1alpha1/zz_generated.deepcopy.go | 5 ++ config/rbac/role.yaml | 66 +++++++++++++++++++ config/webhook/manifests.yaml | 26 ++++++++ .../crds/temporal.io_temporalconnections.yaml | 8 +++ internal/controller/clientpool/clientpool.go | 66 ++++++++++++++----- internal/controller/worker_controller.go | 29 ++++++-- internal/k8s/deployments.go | 1 + 8 files changed, 184 insertions(+), 22 deletions(-) create mode 100644 config/rbac/role.yaml create mode 100644 config/webhook/manifests.yaml diff --git a/api/v1alpha1/temporalconnection_types.go b/api/v1alpha1/temporalconnection_types.go index 2d424fe5..5336be4f 100644 --- a/api/v1alpha1/temporalconnection_types.go +++ b/api/v1alpha1/temporalconnection_types.go @@ -32,6 +32,11 @@ type TemporalConnectionSpec struct { // https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets // +optional MutualTLSSecretRef *SecretReference `json:"mutualTLSSecretRef,omitempty"` + + // APIKeyRef is the name of the Secret that contains the API key. The secret must be `type: kubernetes.io/opaque` and exist + // in the same Kubernetes namespace as the TemporalConnection resource. + // +optional + APIKeyRef *SecretReference `json:"apiKeyRef,omitempty"` } // TemporalConnectionStatus defines the observed state of TemporalConnection diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b9b11ac7..9f342348 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -332,6 +332,11 @@ func (in *TemporalConnectionSpec) DeepCopyInto(out *TemporalConnectionSpec) { *out = new(SecretReference) **out = **in } + if in.APIKeyRef != nil { + in, out := &in.APIKeyRef, &out.APIKeyRef + *out = new(SecretReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TemporalConnectionSpec. diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml new file mode 100644 index 00000000..5082552c --- /dev/null +++ b/config/rbac/role.yaml @@ -0,0 +1,66 @@ +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: manager-role +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - apps + resources: + - deployments/scale + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalconnections + verbs: + - get + - list + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/finalizers + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/status + verbs: + - get + - patch + - update diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml new file mode 100644 index 00000000..a3edd279 --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,26 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-temporal-io-temporal-io-v1alpha1-temporalworkerdeployment + failurePolicy: Fail + name: mtemporalworker.kb.io + rules: + - apiGroups: + - temporal.io.temporal.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - temporalworkers + sideEffects: None diff --git a/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml b/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml index 80414127..4a52589c 100644 --- a/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml +++ b/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml @@ -37,6 +37,14 @@ spec: type: object spec: properties: + apiKeyRef: + properties: + name: + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + required: + - name + type: object hostPort: pattern: ^[a-zA-Z0-9.-]+:[0-9]+$ type: string diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index d332df8c..6d08b213 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -21,16 +21,48 @@ import ( runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" ) +type AuthMode string + +const ( + AuthModeTLS AuthMode = "TLS" + AuthModeAPIKey AuthMode = "API_KEY" + AuthModeUnknown AuthMode = "UNKNOWN" + // Add more auth modes here as they are supported +) + type ClientPoolKey struct { - HostPort string - Namespace string - MutualTLSSecret string // Include secret name in key to invalidate cache when the secret name changes + HostPort string + Namespace string + 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 +} + +// type ClientInfo struct { +// client sdkclient.Client +// tls *tls.Config // Storing the TLS config associated with the client to check certificate expiration. If the certificate is expired, a new client will be created. +// expiryTime time.Time // Effective expiration time (cert.NotAfter - buffer) for efficient expiration checking +// } + +type MTLSAuth struct { + tlsConfig *tls.Config + expiryTime time.Time // cert NotAfter - buffer +} + +type APIKeyProvider func() (string, error) + +type APIKeyAuth struct { + provider APIKeyProvider // returns latest key; no expiry tracking +} + +type ClientAuth struct { + mode AuthMode + mTLS *MTLSAuth // non-nil when mode == AuthMTLS + apiKey *APIKeyAuth // non-nil when mode == AuthAPIKey } type ClientInfo struct { - client sdkclient.Client - tls *tls.Config // Storing the TLS config associated with the client to check certificate expiration. If the certificate is expired, a new client will be created. - expiryTime time.Time // Effective expiration time (cert.NotAfter - buffer) for efficient expiration checking + client sdkclient.Client + auth ClientAuth } type ClientPool struct { @@ -48,7 +80,7 @@ func New(l log.Logger, c runtimeclient.Client) *ClientPool { } } -func (cp *ClientPool) GetSDKClient(key ClientPoolKey, withMTLS bool) (sdkclient.Client, bool) { +func (cp *ClientPool) GetSDKClient(key ClientPoolKey) (sdkclient.Client, bool) { cp.mux.RLock() defer cp.mux.RUnlock() @@ -57,9 +89,9 @@ func (cp *ClientPool) GetSDKClient(key ClientPoolKey, withMTLS bool) (sdkclient. return nil, false } - if withMTLS { + if key.AuthMode == AuthModeTLS { // Check if any certificate is expired - expired, err := isCertificateExpired(info.expiryTime) + expired, err := isCertificateExpired(info.auth.mTLS.expiryTime) if err != nil { cp.logger.Error("Error checking certificate expiration", "error", err) return nil, false @@ -147,14 +179,18 @@ func (cp *ClientPool) UpsertClient(ctx context.Context, opts NewClientOptions) ( mutualTLSSecret = opts.Spec.MutualTLSSecretRef.Name } key := ClientPoolKey{ - HostPort: opts.Spec.HostPort, - Namespace: opts.TemporalNamespace, - MutualTLSSecret: mutualTLSSecret, + HostPort: opts.Spec.HostPort, + Namespace: opts.TemporalNamespace, + SecretName: mutualTLSSecret, + AuthMode: AuthModeTLS, } cp.clients[key] = ClientInfo{ - client: c, - tls: clientOpts.ConnectionOptions.TLS, - expiryTime: expiryTime, + client: c, + auth: ClientAuth{ + mode: AuthModeTLS, + mTLS: &MTLSAuth{tlsConfig: clientOpts.ConnectionOptions.TLS, expiryTime: expiryTime}, + apiKey: nil, + }, } return c, nil diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index ba463890..67a79ea8 100644 --- a/internal/controller/worker_controller.go +++ b/internal/controller/worker_controller.go @@ -9,6 +9,7 @@ import ( "fmt" "time" + "github.com/temporalio/temporal-worker-controller/api/v1alpha1" temporaliov1alpha1 "github.com/temporalio/temporal-worker-controller/api/v1alpha1" "github.com/temporalio/temporal-worker-controller/internal/controller/clientpool" "github.com/temporalio/temporal-worker-controller/internal/k8s" @@ -36,14 +37,24 @@ const ( buildIDLabel = "temporal.io/build-id" ) -// getMutualTLSSecretName extracts the mutual TLS secret name from a secret reference -func getMutualTLSSecretName(secretRef *temporaliov1alpha1.SecretReference) (string, bool) { +// getSecretName extracts the secret name from a secret reference +func getSecretName(secretRef *v1alpha1.SecretReference) (string, bool) { if secretRef != nil { return secretRef.Name, true } return "", false } +// TODO (Shivam): Understand if you need to move these two to some other file +func getAuthMode(temporalConnection *v1alpha1.TemporalConnection) (clientpool.AuthMode, bool) { + if temporalConnection.Spec.MutualTLSSecretRef != nil { + return clientpool.AuthModeTLS, true + } else if temporalConnection.Spec.APIKeyRef != nil { + return clientpool.AuthModeAPIKey, true + } + return clientpool.AuthModeUnknown, false +} + // TemporalWorkerDeploymentReconciler reconciles a TemporalWorkerDeployment object type TemporalWorkerDeploymentReconciler struct { client.Client @@ -127,13 +138,17 @@ func (r *TemporalWorkerDeploymentReconciler) Reconcile(ctx context.Context, req return ctrl.Result{}, err } + // Get the Auth Mode and Secret Name + authMode, _ := getAuthMode(&temporalConnection) + secretName, _ := getSecretName(temporalConnection.Spec.MutualTLSSecretRef) + // Get or update temporal client for connection - mutualTLSSecretName, hasMutualTLS := getMutualTLSSecretName(temporalConnection.Spec.MutualTLSSecretRef) temporalClient, ok := r.TemporalClientPool.GetSDKClient(clientpool.ClientPoolKey{ - HostPort: temporalConnection.Spec.HostPort, - Namespace: workerDeploy.Spec.WorkerOptions.TemporalNamespace, - MutualTLSSecret: mutualTLSSecretName, - }, hasMutualTLS) + HostPort: temporalConnection.Spec.HostPort, + Namespace: workerDeploy.Spec.WorkerOptions.TemporalNamespace, + SecretName: secretName, + AuthMode: authMode, + }) if !ok { c, err := r.TemporalClientPool.UpsertClient(ctx, clientpool.NewClientOptions{ K8sNamespace: workerDeploy.Namespace, diff --git a/internal/k8s/deployments.go b/internal/k8s/deployments.go index ac5da9c3..5013d438 100644 --- a/internal/k8s/deployments.go +++ b/internal/k8s/deployments.go @@ -268,6 +268,7 @@ func NewDeploymentWithOwnerRef( for k, v := range spec.Template.Annotations { podAnnotations[k] = v } + // TODO (Shivam): Add API key hash annotation podAnnotations[ConnectionSpecHashAnnotation] = ComputeConnectionSpecHash(connection) blockOwnerDeletion := true From 3d90fbcf8334a0273bce8fc0fb90483610dc9f85 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Fri, 26 Sep 2025 13:02:00 -0400 Subject: [PATCH 02/15] fixing bug --- internal/demo/helloworld/helm/helloworld/values.yaml | 6 +++--- internal/planner/planner.go | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/demo/helloworld/helm/helloworld/values.yaml b/internal/demo/helloworld/helm/helloworld/values.yaml index 7e10f445..2b6e2b52 100644 --- a/internal/demo/helloworld/helm/helloworld/values.yaml +++ b/internal/demo/helloworld/helm/helloworld/values.yaml @@ -3,12 +3,12 @@ image: tag: latest temporal: - namespace: "" # e.g. default + namespace: ss-worker-controller-with-api-keys.a2dd6 # e.g. default # Use existing connection (leave empty to create new one) connectionName: "" # e.g. dev-server # Connection details (required if connectionName is empty) - address: "" # e.g. .tmprl.cloud:7233 - mtlsSecretName: "" # e.g. temporal-cloud-mtls + address: ss-worker-controller-with-api-keys.a2dd6.tmprl.cloud:7233 # e.g. .tmprl.cloud:7233 + mtlsSecretName: temporal-cloud-mtls # e.g. temporal-cloud-mtls rollout: strategy: Progressive diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 19dfd6e0..955129c8 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -390,11 +390,12 @@ func getVersionConfigDiff( } // Do nothing if the test workflows have not completed successfully - if strategy.Gate != nil { + if strategy.Gate != nil && status.CurrentVersion != nil { if len(status.TargetVersion.TaskQueues) == 0 { return nil } if len(status.TargetVersion.TestWorkflows) < len(status.TargetVersion.TaskQueues) { + l.Info("not enough test workflows running to start gate workflow", "buildID", status.TargetVersion.BuildID, "taskQueues", status.TargetVersion.TaskQueues, "testWorkflows", status.TargetVersion.TestWorkflows) return nil } for _, wf := range status.TargetVersion.TestWorkflows { From 8b01be726259fcc094143faf2034a7ba6146eaa3 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Fri, 26 Sep 2025 13:16:58 -0400 Subject: [PATCH 03/15] fix bootstrap bug --- internal/planner/planner.go | 2 +- internal/planner/planner_test.go | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 19dfd6e0..9df49f93 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -390,7 +390,7 @@ func getVersionConfigDiff( } // Do nothing if the test workflows have not completed successfully - if strategy.Gate != nil { + if strategy.Gate != nil && status.CurrentVersion != nil { if len(status.TargetVersion.TaskQueues) == 0 { return nil } diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index ccc62469..db7d8270 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -1014,6 +1014,28 @@ func TestGetTestWorkflows(t *testing.T) { }, expectWorkflows: 0, }, + { + name: "gate workflow without current version", + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "123", + Status: temporaliov1alpha1.VersionStatusInactive, + TaskQueues: []temporaliov1alpha1.TaskQueue{ + {Name: "queue1"}, + }, + }, + }, + CurrentVersion: nil, + }, + config: &Config{ + RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ + Gate: &temporaliov1alpha1.GateWorkflowConfig{WorkflowType: "TestWorkflow"}, + }, + }, + // should not start gate workflows if current version is not set. This happens when there is no initial deployment version present. + expectWorkflows: 0, + }, { name: "all test workflows already running", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ @@ -1204,6 +1226,33 @@ func TestGetVersionConfigDiff(t *testing.T) { expectSetCurrent: true, expectRampPercent: func() *int32 { f := int32(0); return &f }(), }, + { + name: "gate configured with no current version should set current immediately", + strategy: temporaliov1alpha1.RolloutStrategy{ + Strategy: temporaliov1alpha1.UpdateProgressive, + Gate: &temporaliov1alpha1.GateWorkflowConfig{WorkflowType: "TestWorkflow"}, + Steps: []temporaliov1alpha1.RolloutStep{ + {RampPercentage: 1, PauseDuration: metav1Duration(30 * time.Second)}, + }, + }, + status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ + TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ + BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ + BuildID: "123", + Status: temporaliov1alpha1.VersionStatusInactive, + HealthySince: &metav1.Time{Time: time.Now()}, + }, + }, + // CurrentVersion intentionally nil to simulate bootstrap + }, + state: &temporal.TemporalWorkerState{ + Versions: map[string]*temporal.VersionInfo{ + "123": {BuildID: "123", AllTaskQueuesHaveUnversionedPoller: false}, + }, + }, + expectConfig: true, + expectSetCurrent: true, + }, } for _, tc := range testCases { From 7de351c2d2517ff042c2fc94608a54f98f38a7a6 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Fri, 26 Sep 2025 14:04:07 -0400 Subject: [PATCH 04/15] updated bandaid --- config/rbac/role.yaml | 66 ++++++++++++++++++++++++++++++++ config/webhook/manifests.yaml | 26 +++++++++++++ internal/planner/planner.go | 5 +-- internal/planner/planner_test.go | 33 +++++----------- 4 files changed, 104 insertions(+), 26 deletions(-) create mode 100644 config/rbac/role.yaml create mode 100644 config/webhook/manifests.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml new file mode 100644 index 00000000..5082552c --- /dev/null +++ b/config/rbac/role.yaml @@ -0,0 +1,66 @@ +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: manager-role +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - apps + resources: + - deployments/scale + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalconnections + verbs: + - get + - list + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/finalizers + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/status + verbs: + - get + - patch + - update diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml new file mode 100644 index 00000000..a3edd279 --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,26 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-temporal-io-temporal-io-v1alpha1-temporalworkerdeployment + failurePolicy: Fail + name: mtemporalworker.kb.io + rules: + - apiGroups: + - temporal.io.temporal.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - temporalworkers + sideEffects: None diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 9df49f93..b65f4a8c 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -339,8 +339,7 @@ func getTestWorkflows( // Skip if there's no gate workflow defined, if the target version is already the current, or if the target // version is not yet registered in temporal if config.RolloutStrategy.Gate == nil || - status.CurrentVersion == nil || - status.CurrentVersion.BuildID == status.TargetVersion.BuildID || + (status.CurrentVersion != nil && status.CurrentVersion.BuildID == status.TargetVersion.BuildID) || status.TargetVersion.Status == temporaliov1alpha1.VersionStatusNotRegistered { return nil } @@ -390,7 +389,7 @@ func getVersionConfigDiff( } // Do nothing if the test workflows have not completed successfully - if strategy.Gate != nil && status.CurrentVersion != nil { + if strategy.Gate != nil { if len(status.TargetVersion.TaskQueues) == 0 { return nil } diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index db7d8270..2a74254d 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -1014,28 +1014,6 @@ func TestGetTestWorkflows(t *testing.T) { }, expectWorkflows: 0, }, - { - name: "gate workflow without current version", - status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ - TargetVersion: temporaliov1alpha1.TargetWorkerDeploymentVersion{ - BaseWorkerDeploymentVersion: temporaliov1alpha1.BaseWorkerDeploymentVersion{ - BuildID: "123", - Status: temporaliov1alpha1.VersionStatusInactive, - TaskQueues: []temporaliov1alpha1.TaskQueue{ - {Name: "queue1"}, - }, - }, - }, - CurrentVersion: nil, - }, - config: &Config{ - RolloutStrategy: temporaliov1alpha1.RolloutStrategy{ - Gate: &temporaliov1alpha1.GateWorkflowConfig{WorkflowType: "TestWorkflow"}, - }, - }, - // should not start gate workflows if current version is not set. This happens when there is no initial deployment version present. - expectWorkflows: 0, - }, { name: "all test workflows already running", status: &temporaliov1alpha1.TemporalWorkerDeploymentStatus{ @@ -1241,9 +1219,18 @@ func TestGetVersionConfigDiff(t *testing.T) { BuildID: "123", Status: temporaliov1alpha1.VersionStatusInactive, HealthySince: &metav1.Time{Time: time.Now()}, + TaskQueues: []temporaliov1alpha1.TaskQueue{ + {Name: "queue1"}, + }, + }, + TestWorkflows: []temporaliov1alpha1.WorkflowExecution{ + { + TaskQueue: "queue1", + Status: temporaliov1alpha1.WorkflowExecutionStatusCompleted, + }, }, }, - // CurrentVersion intentionally nil to simulate bootstrap + CurrentVersion: nil, }, state: &temporal.TemporalWorkerState{ Versions: map[string]*temporal.VersionInfo{ From d6440ca4d685460faabd36913104decd2d128420 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Fri, 26 Sep 2025 14:05:34 -0400 Subject: [PATCH 05/15] remove config folder --- config/rbac/role.yaml | 66 ----------------------------------- config/webhook/manifests.yaml | 26 -------------- 2 files changed, 92 deletions(-) delete mode 100644 config/rbac/role.yaml delete mode 100644 config/webhook/manifests.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml deleted file mode 100644 index 5082552c..00000000 --- a/config/rbac/role.yaml +++ /dev/null @@ -1,66 +0,0 @@ ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: manager-role -rules: -- apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch -- apiGroups: - - apps - resources: - - deployments - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - apps - resources: - - deployments/scale - verbs: - - update -- apiGroups: - - temporal.io - resources: - - temporalconnections - verbs: - - get - - list - - watch -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments/finalizers - verbs: - - update -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments/status - verbs: - - get - - patch - - update diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml deleted file mode 100644 index a3edd279..00000000 --- a/config/webhook/manifests.yaml +++ /dev/null @@ -1,26 +0,0 @@ ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - name: mutating-webhook-configuration -webhooks: -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-temporal-io-temporal-io-v1alpha1-temporalworkerdeployment - failurePolicy: Fail - name: mtemporalworker.kb.io - rules: - - apiGroups: - - temporal.io.temporal.io - apiVersions: - - v1alpha1 - operations: - - CREATE - - UPDATE - resources: - - temporalworkers - sideEffects: None From 23e856f80f973bf8e5fe7fbbbf7967e87ff3c8d1 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Tue, 30 Sep 2025 12:03:18 -0400 Subject: [PATCH 06/15] API key support + updated demo README --- Makefile | 34 +++- config/rbac/role.yaml | 66 ------- config/webhook/manifests.yaml | 26 --- internal/controller/clientpool/clientpool.go | 161 +++++++++++------- internal/controller/worker_controller.go | 27 +-- internal/demo/README.md | 40 ++++- .../helm/helloworld/templates/connection.yaml | 9 +- .../helm/helloworld/values.schema.json | 7 + .../helloworld/helm/helloworld/values.yaml | 4 +- internal/demo/helloworld/worker.go | 21 ++- internal/demo/util/client.go | 6 +- internal/k8s/deployments.go | 20 ++- internal/k8s/deployments_test.go | 72 +++++++- internal/planner/planner.go | 2 +- skaffold.example.env | 7 +- skaffold.yaml | 1 + 16 files changed, 306 insertions(+), 197 deletions(-) delete mode 100644 config/rbac/role.yaml delete mode 100644 config/webhook/manifests.yaml diff --git a/Makefile b/Makefile index 65065f3e..bff8f4c3 100644 --- a/Makefile +++ b/Makefile @@ -158,20 +158,31 @@ manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and Cust generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations. GOWORK=off GO111MODULE=on $(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths=./api/... paths=./internal/... paths=./cmd/... -# source secret.env && make start-sample-workflow TEMPORAL_CLOUD_API_KEY=$TEMPORAL_CLOUD_API_KEY .PHONY: start-sample-workflow +.SILENT: start-sample-workflow start-sample-workflow: ## Start a sample workflow. - @$(TEMPORAL) workflow start --type "HelloWorld" --task-queue "default/helloworld" \ - --tls-cert-path certs/client.pem \ - --tls-key-path certs/client.key \ - --address "worker-controller-test.a2dd6.tmprl.cloud:7233" \ - -n "worker-controller-test.a2dd6" -# --address replay-2025.ktasd.tmprl.cloud:7233 \ -# --api-key $(TEMPORAL_CLOUD_API_KEY) + @set -e; \ + # Load env vars from skaffold.env if present so address/namespace aren't hardcoded + if [ -f skaffold.env ]; then set -a; . skaffold.env; set +a; fi; \ + if [ -n "$$TEMPORAL_API_KEY" ]; then \ + $(TEMPORAL) workflow start --type "HelloWorld" --task-queue "default/helloworld" \ + --address "$$TEMPORAL_ADDRESS" \ + -n "$$TEMPORAL_NAMESPACE"; \ + else \ + $(TEMPORAL) workflow start --type "HelloWorld" --task-queue "default/helloworld" \ + --tls-cert-path certs/client.pem \ + --tls-key-path certs/client.key \ + --address "$$TEMPORAL_ADDRESS" \ + -n "$$TEMPORAL_NAMESPACE"; \ + fi .PHONY: apply-load-sample-workflow +.SILENT: apply-load-sample-workflow apply-load-sample-workflow: ## Start a sample workflow every 15 seconds - watch --interval 0.1 -- $(TEMPORAL) workflow start --type "HelloWorld" --task-queue "default/helloworld" + @while true; do \ + $(MAKE) -s start-sample-workflow; \ + sleep 15; \ + done .PHONY: list-workflow-build-ids list-workflow-build-ids: ## List workflow executions and their build IDs. @@ -300,6 +311,11 @@ create-cloud-mtls-secret: --cert=certs/client.pem \ --key=certs/client.key +.PHONY: create-api-key-secret +create-api-key-secret: + kubectl create secret generic temporal-api-key --namespace default \ + --from-file=api-key=certs/api-key.txt + ##### Checks ##### goimports: fmt-imports $(GOIMPORTS) @printf $(COLOR) "Run goimports for all files..." diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml deleted file mode 100644 index 5082552c..00000000 --- a/config/rbac/role.yaml +++ /dev/null @@ -1,66 +0,0 @@ ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: manager-role -rules: -- apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch -- apiGroups: - - apps - resources: - - deployments - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - apps - resources: - - deployments/scale - verbs: - - update -- apiGroups: - - temporal.io - resources: - - temporalconnections - verbs: - - get - - list - - watch -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments/finalizers - verbs: - - update -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments/status - verbs: - - get - - patch - - update diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml deleted file mode 100644 index a3edd279..00000000 --- a/config/webhook/manifests.yaml +++ /dev/null @@ -1,26 +0,0 @@ ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - name: mutating-webhook-configuration -webhooks: -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-temporal-io-temporal-io-v1alpha1-temporalworkerdeployment - failurePolicy: Fail - name: mtemporalworker.kb.io - rules: - - apiGroups: - - temporal.io.temporal.io - apiVersions: - - v1alpha1 - operations: - - CREATE - - UPDATE - resources: - - temporalworkers - sideEffects: None diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index 6d08b213..96fad714 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -37,27 +37,14 @@ type ClientPoolKey struct { AuthMode AuthMode // Include auth mode in key to invalidate cache when the auth mode changes for the secret } -// type ClientInfo struct { -// client sdkclient.Client -// tls *tls.Config // Storing the TLS config associated with the client to check certificate expiration. If the certificate is expired, a new client will be created. -// expiryTime time.Time // Effective expiration time (cert.NotAfter - buffer) for efficient expiration checking -// } - type MTLSAuth struct { tlsConfig *tls.Config expiryTime time.Time // cert NotAfter - buffer } -type APIKeyProvider func() (string, error) - -type APIKeyAuth struct { - provider APIKeyProvider // returns latest key; no expiry tracking -} - type ClientAuth struct { - mode AuthMode - mTLS *MTLSAuth // non-nil when mode == AuthMTLS - apiKey *APIKeyAuth // non-nil when mode == AuthAPIKey + mode AuthMode + mTLS *MTLSAuth // non-nil when mode == AuthMTLS, nil when mode == AuthAPIKey } type ClientInfo struct { @@ -111,56 +98,41 @@ type NewClientOptions struct { Spec v1alpha1.TemporalConnectionSpec } -func (cp *ClientPool) UpsertClient(ctx context.Context, opts NewClientOptions) (sdkclient.Client, error) { +func (cp *ClientPool) fetchClientUsingMTLSSecret(secret corev1.Secret, opts NewClientOptions) (sdkclient.Client, error) { + clientOpts := sdkclient.Options{ Logger: cp.logger, HostPort: opts.Spec.HostPort, Namespace: opts.TemporalNamespace, - // TODO(jlegrone): Make API Keys work } var pemCert []byte var expiryTime time.Time - // Get the connection secret if it exists - if opts.Spec.MutualTLSSecretRef != nil { - var secret corev1.Secret - if err := cp.k8sClient.Get(ctx, types.NamespacedName{ - Name: opts.Spec.MutualTLSSecretRef.Name, - Namespace: opts.K8sNamespace, - }, &secret); err != nil { - return nil, err - } - if secret.Type != corev1.SecretTypeTLS { - err := fmt.Errorf("secret %s must be of type kubernetes.io/tls", secret.Name) - return nil, err - } - - // Extract the certificate to calculate the effective expiration time - pemCert = secret.Data["tls.crt"] + // Extract the certificate to calculate the effective expiration time + pemCert = secret.Data["tls.crt"] - // Check if certificate is expired before creating the client - exp, err := calculateCertificateExpirationTime(pemCert, 5*time.Minute) - if err != nil { - return nil, fmt.Errorf("failed to check certificate expiration: %v", err) - } - expired, err := isCertificateExpired(exp) - if err != nil { - return nil, fmt.Errorf("failed to check certificate expiration: %v", err) - } - if expired { - return nil, fmt.Errorf("certificate is expired or is going to expire soon") - } + // Check if certificate is expired before creating the client + exp, err := calculateCertificateExpirationTime(pemCert, 5*time.Minute) + if err != nil { + return nil, fmt.Errorf("failed to check certificate expiration: %v", err) + } + expired, err := isCertificateExpired(exp) + if err != nil { + return nil, fmt.Errorf("failed to check certificate expiration: %v", err) + } + if expired { + return nil, fmt.Errorf("certificate is expired or is going to expire soon") + } - cert, err := tls.X509KeyPair(secret.Data["tls.crt"], secret.Data["tls.key"]) - if err != nil { - return nil, err - } - clientOpts.ConnectionOptions.TLS = &tls.Config{ - Certificates: []tls.Certificate{cert}, - } - expiryTime = exp + cert, err := tls.X509KeyPair(secret.Data["tls.crt"], secret.Data["tls.key"]) + if err != nil { + return nil, err } + clientOpts.ConnectionOptions.TLS = &tls.Config{ + Certificates: []tls.Certificate{cert}, + } + expiryTime = exp c, err := sdkclient.Dial(clientOpts) if err != nil { @@ -174,28 +146,95 @@ func (cp *ClientPool) UpsertClient(ctx context.Context, opts NewClientOptions) ( cp.mux.Lock() defer cp.mux.Unlock() - var mutualTLSSecret string - if opts.Spec.MutualTLSSecretRef != nil { - mutualTLSSecret = opts.Spec.MutualTLSSecretRef.Name - } key := ClientPoolKey{ HostPort: opts.Spec.HostPort, Namespace: opts.TemporalNamespace, - SecretName: mutualTLSSecret, + SecretName: opts.Spec.MutualTLSSecretRef.Name, AuthMode: AuthModeTLS, } cp.clients[key] = ClientInfo{ client: c, auth: ClientAuth{ - mode: AuthModeTLS, - mTLS: &MTLSAuth{tlsConfig: clientOpts.ConnectionOptions.TLS, expiryTime: expiryTime}, - apiKey: nil, + mode: AuthModeTLS, + mTLS: &MTLSAuth{tlsConfig: clientOpts.ConnectionOptions.TLS, expiryTime: expiryTime}, + }, + } + + return c, nil +} + +func (cp *ClientPool) fetchClientUsingAPIKeySecret(secret corev1.Secret, opts NewClientOptions) (sdkclient.Client, error) { + clientOpts := sdkclient.Options{ + Logger: cp.logger, + HostPort: opts.Spec.HostPort, + Namespace: opts.TemporalNamespace, + ConnectionOptions: sdkclient.ConnectionOptions{ + TLS: &tls.Config{}, + }, + } + + clientOpts.Credentials = sdkclient.NewAPIKeyDynamicCredentials(func(ctx context.Context) (string, error) { + return string(secret.Data["api-key"]), nil + }) + + c, err := sdkclient.Dial(clientOpts) + if err != nil { + return nil, err + } + + cp.mux.Lock() + defer cp.mux.Unlock() + + key := ClientPoolKey{ + HostPort: opts.Spec.HostPort, + Namespace: opts.TemporalNamespace, + SecretName: opts.Spec.APIKeyRef.Name, + AuthMode: AuthModeAPIKey, + } + cp.clients[key] = ClientInfo{ + client: c, + auth: ClientAuth{ + mode: AuthModeAPIKey, + mTLS: nil, }, } return c, nil } +func (cp *ClientPool) UpsertClient(ctx context.Context, secretName string, authMode AuthMode, opts NewClientOptions) (sdkclient.Client, error) { + + // Fetch the secret + var secret corev1.Secret + if err := cp.k8sClient.Get(ctx, types.NamespacedName{ + Name: secretName, + Namespace: opts.K8sNamespace, + }, &secret); err != nil { + return nil, err + } + + // Check the secret type + switch authMode { + case AuthModeTLS: + if secret.Type != corev1.SecretTypeTLS { + err := fmt.Errorf("secret %s must be of type kubernetes.io/tls", secret.Name) + return nil, err + } + return cp.fetchClientUsingMTLSSecret(secret, opts) + + case AuthModeAPIKey: + if secret.Type != corev1.SecretTypeOpaque { + err := fmt.Errorf("secret %s must be of type kubernetes.io/opaque", secret.Name) + return nil, err + } + return cp.fetchClientUsingAPIKeySecret(secret, opts) + + default: + return nil, fmt.Errorf("invalid auth mode: %s", authMode) + } + +} + func (cp *ClientPool) Close() { cp.mux.Lock() defer cp.mux.Unlock() diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index 67a79ea8..6d4c2995 100644 --- a/internal/controller/worker_controller.go +++ b/internal/controller/worker_controller.go @@ -38,21 +38,20 @@ const ( ) // getSecretName extracts the secret name from a secret reference -func getSecretName(secretRef *v1alpha1.SecretReference) (string, bool) { +func getSecretName(secretRef *v1alpha1.SecretReference) string { if secretRef != nil { - return secretRef.Name, true + return secretRef.Name } - return "", false + return "" } -// TODO (Shivam): Understand if you need to move these two to some other file -func getAuthMode(temporalConnection *v1alpha1.TemporalConnection) (clientpool.AuthMode, bool) { +func getAuthMode(temporalConnection *v1alpha1.TemporalConnection) clientpool.AuthMode { if temporalConnection.Spec.MutualTLSSecretRef != nil { - return clientpool.AuthModeTLS, true + return clientpool.AuthModeTLS } else if temporalConnection.Spec.APIKeyRef != nil { - return clientpool.AuthModeAPIKey, true + return clientpool.AuthModeAPIKey } - return clientpool.AuthModeUnknown, false + return clientpool.AuthModeUnknown } // TemporalWorkerDeploymentReconciler reconciles a TemporalWorkerDeployment object @@ -139,8 +138,14 @@ func (r *TemporalWorkerDeploymentReconciler) Reconcile(ctx context.Context, req } // Get the Auth Mode and Secret Name - authMode, _ := getAuthMode(&temporalConnection) - secretName, _ := getSecretName(temporalConnection.Spec.MutualTLSSecretRef) + authMode := getAuthMode(&temporalConnection) + var secretName string + switch authMode { + case clientpool.AuthModeTLS: + secretName = getSecretName(temporalConnection.Spec.MutualTLSSecretRef) + case clientpool.AuthModeAPIKey: + secretName = getSecretName(temporalConnection.Spec.APIKeyRef) + } // Get or update temporal client for connection temporalClient, ok := r.TemporalClientPool.GetSDKClient(clientpool.ClientPoolKey{ @@ -150,7 +155,7 @@ func (r *TemporalWorkerDeploymentReconciler) Reconcile(ctx context.Context, req AuthMode: authMode, }) if !ok { - c, err := r.TemporalClientPool.UpsertClient(ctx, clientpool.NewClientOptions{ + c, err := r.TemporalClientPool.UpsertClient(ctx, secretName, authMode, clientpool.NewClientOptions{ K8sNamespace: workerDeploy.Namespace, TemporalNamespace: workerDeploy.Spec.WorkerOptions.TemporalNamespace, Spec: temporalConnection.Spec, diff --git a/internal/demo/README.md b/internal/demo/README.md index 8a726f98..4a09c1a9 100644 --- a/internal/demo/README.md +++ b/internal/demo/README.md @@ -8,7 +8,7 @@ This guide will help you set up and run the Temporal Worker Controller locally u - [Helm](https://helm.sh/docs/intro/install/) - [Skaffold](https://skaffold.dev/docs/install/) - [kubectl](https://kubernetes.io/docs/tasks/tools/install-kubectl/) -- Temporal Cloud account with mTLS certificates +- Temporal Cloud account with API key or mTLS certificates - Understanding of [Worker Versioning concepts](https://docs.temporal.io/production-deployment/worker-deployments/worker-versioning) (Pinned and Auto-Upgrade versioning behaviors) > **Note**: This demo specifically showcases **Pinned** workflow behavior. All workflows in the demo will remain on the worker version where they started, demonstrating how the controller safely manages multiple worker versions simultaneously during deployments. @@ -20,7 +20,7 @@ This guide will help you set up and run the Temporal Worker Controller locally u minikube start ``` -2. Set up Temporal Cloud mTLS certificates and update `skaffold.env`: +2. Set up Temporal Cloud Authentication: - Create a `certs` directory in the project root - Save your Temporal Cloud mTLS client certificates as: - `certs/client.pem` @@ -29,20 +29,48 @@ This guide will help you set up and run the Temporal Worker Controller locally u ```bash make create-cloud-mtls-secret ``` - - Create a `skaffold.env` file: + - In `skaffold.example.env`, set: + ```env + TEMPORAL_API_KEY_SECRET_NAME="" + TEMPORAL_MTLS_SECRET_NAME=temporal-cloud-mtls-secret + ``` + + NOTE: Alternatively, if you are using API keys, follow the steps below instead of mTLS: + + #### Using API Keys (alternative to mTLS) + - Create a `certs` directory in the project root if not already present + - Save your Temporal Cloud API key in a file (single line, no newline): + ```bash + echo -n "" > certs/api-key.txt + ``` + - Create the Kubernetes Secret (trims any accidental newlines): + ```bash + make create-api-key-secret + ``` + - In `skaffold.example.env`, set: + ```env + TEMPORAL_API_KEY_SECRET_NAME=temporal-api-key + TEMPORAL_MTLS_SECRET_NAME="" + ``` + - Note: Do not set both mTLS and API key for the same connection. If both present, the controller + connects to the namespace using mTLS. + +3. Create the `skaffold.env` file: + - Update the value of `TEMPORAL_NAMESPACE`, `TEMPORAL_ADDRESS` in `skaffold.example.env` to match your configuration. + + - Then run: ```bash cp skaffold.example.env skaffold.env ``` - - Update the value of `TEMPORAL_NAMESPACE` in `skaffold.env` to match your Temporal cloud namespace. -3. Build and deploy the Controller image to the local k8s cluster: +4. Build and deploy the Controller image to the local k8s cluster: ```bash skaffold run --profile worker-controller ``` ### Testing Progressive Deployments -4. **Deploy the v1 worker**: +5. **Deploy the v1 worker**: ```bash skaffold run --profile helloworld-worker ``` diff --git a/internal/demo/helloworld/helm/helloworld/templates/connection.yaml b/internal/demo/helloworld/helm/helloworld/templates/connection.yaml index 37a51433..6e2082ab 100644 --- a/internal/demo/helloworld/helm/helloworld/templates/connection.yaml +++ b/internal/demo/helloworld/helm/helloworld/templates/connection.yaml @@ -8,6 +8,13 @@ metadata: {{- include "helloworld.labels" . | nindent 4 }} spec: hostPort: "{{ .Values.temporal.address }}" + {{- $apiKey := .Values.temporal.apiKeySecretName | default "" | trim }} + {{- if and $apiKey (ne $apiKey "null") }} + apiKeyRef: + name: {{ $apiKey | quote }} + {{- end }} + {{- if .Values.temporal.mtlsSecretName }} mutualTLSSecretRef: - name: {{ .Values.temporal.mtlsSecretName }} + name: {{ .Values.temporal.mtlsSecretName | quote }} + {{- end }} {{- end }} diff --git a/internal/demo/helloworld/helm/helloworld/values.schema.json b/internal/demo/helloworld/helm/helloworld/values.schema.json index dfffaa9c..520ccbd4 100644 --- a/internal/demo/helloworld/helm/helloworld/values.schema.json +++ b/internal/demo/helloworld/helm/helloworld/values.schema.json @@ -35,6 +35,10 @@ "mtlsSecretName": { "type": "string", "description": "Name of the secret containing mTLS certificates (required if connectionName is empty)" + }, + "apiKeySecretName": { + "type": "string", + "description": "Name of the secret containing API key (required if connectionName is empty)" } }, "required": ["namespace"], @@ -51,6 +55,9 @@ }, { "required": ["address", "mtlsSecretName"] + }, + { + "required": ["address", "apiKeySecretName"] } ] }, diff --git a/internal/demo/helloworld/helm/helloworld/values.yaml b/internal/demo/helloworld/helm/helloworld/values.yaml index 2b6e2b52..7ceeda8c 100644 --- a/internal/demo/helloworld/helm/helloworld/values.yaml +++ b/internal/demo/helloworld/helm/helloworld/values.yaml @@ -8,7 +8,7 @@ temporal: connectionName: "" # e.g. dev-server # Connection details (required if connectionName is empty) address: ss-worker-controller-with-api-keys.a2dd6.tmprl.cloud:7233 # e.g. .tmprl.cloud:7233 - mtlsSecretName: temporal-cloud-mtls # e.g. temporal-cloud-mtls - + apiKeySecretName: temporal-api-key + rollout: strategy: Progressive diff --git a/internal/demo/helloworld/worker.go b/internal/demo/helloworld/worker.go index 5f190df8..df0efa32 100644 --- a/internal/demo/helloworld/worker.go +++ b/internal/demo/helloworld/worker.go @@ -7,7 +7,6 @@ package helloworld import ( "context" "fmt" - "math/rand" "strings" "time" @@ -19,20 +18,26 @@ func HelloWorld(ctx workflow.Context) (string, error) { ctx = util.SetActivityTimeout(ctx, 5*time.Minute) // Get a subject - var subject string + var subject GetSubjectResponse if err := workflow.ExecuteActivity(ctx, GetSubject).Get(ctx, &subject); err != nil { return "", err } // Return the greeting - return fmt.Sprintf("Hello %s", subject), nil + return fmt.Sprintf("Hello %s", subject.Name), nil } -func GetSubject(ctx context.Context) (string, error) { - // Simulate activity execution latency - time.Sleep(time.Duration(rand.Intn(30)) * time.Second) - // Return a hardcoded subject - return "World", nil +func GetSubject(ctx context.Context) (*GetSubjectResponse, error) { + // Send heartbeats + go util.AutoHeartbeat(ctx) + + // Get user via API + subject, err := fetchUser(ctx, "https://jsonplaceholder.typicode.com") + if err != nil { + return nil, err + } + + return &subject, nil } func RolloutGate(ctx workflow.Context) error { diff --git a/internal/demo/util/client.go b/internal/demo/util/client.go index 348928eb..6fa90e22 100644 --- a/internal/demo/util/client.go +++ b/internal/demo/util/client.go @@ -38,9 +38,9 @@ func newClient(buildID string, metricsPort int) (c client.Client, stopFunc func( panic(err) } - if _, err := c.CheckHealth(context.Background(), &client.CheckHealthRequest{}); err != nil { - panic(err) - } + // if _, err := c.CheckHealth(context.Background(), &client.CheckHealthRequest{}); err != nil { + // panic(err) + // } if _, err := c.ListWorkflow(context.Background(), &workflowservice.ListWorkflowExecutionsRequest{ Namespace: opts.Namespace, diff --git a/internal/k8s/deployments.go b/internal/k8s/deployments.go index 5013d438..b29a2243 100644 --- a/internal/k8s/deployments.go +++ b/internal/k8s/deployments.go @@ -32,6 +32,7 @@ const ( ResourceNameSeparator = "-" MaxBuildIdLen = 63 ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash" + APISecretKey = "api-key" ) // DeploymentState represents the Kubernetes state of all deployments for a temporal worker deployment @@ -261,6 +262,21 @@ func NewDeploymentWithOwnerRef( }, }, }) + } else if connection.APIKeyRef != nil { + for i, container := range podSpec.Containers { + container.Env = append(container.Env, + corev1.EnvVar{ + Name: "TEMPORAL_API_KEY", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: connection.APIKeyRef.Name}, + Key: APISecretKey, + }, + }, + }, + ) + podSpec.Containers[i] = container + } } // Build pod annotations @@ -268,7 +284,6 @@ func NewDeploymentWithOwnerRef( for k, v := range spec.Template.Annotations { podAnnotations[k] = v } - // TODO (Shivam): Add API key hash annotation podAnnotations[ConnectionSpecHashAnnotation] = ComputeConnectionSpecHash(connection) blockOwnerDeletion := true @@ -307,6 +322,7 @@ func NewDeploymentWithOwnerRef( } } +// TODO (Shivam): Change hash when secret name is updated as well. func ComputeConnectionSpecHash(connection temporaliov1alpha1.TemporalConnectionSpec) string { // HostPort is required, but MutualTLSSecret can be empty for non-mTLS connections if connection.HostPort == "" { @@ -319,6 +335,8 @@ func ComputeConnectionSpecHash(connection temporaliov1alpha1.TemporalConnectionS _, _ = hasher.Write([]byte(connection.HostPort)) if connection.MutualTLSSecretRef != nil { _, _ = hasher.Write([]byte(connection.MutualTLSSecretRef.Name)) + } else if connection.APIKeyRef != nil { + _, _ = hasher.Write([]byte(connection.APIKeyRef.Name)) } return hex.EncodeToString(hasher.Sum(nil)) diff --git a/internal/k8s/deployments_test.go b/internal/k8s/deployments_test.go index f11581dd..6e6efdb9 100644 --- a/internal/k8s/deployments_test.go +++ b/internal/k8s/deployments_test.go @@ -8,6 +8,7 @@ import ( "context" "crypto/rand" "crypto/rsa" + "crypto/tls" "crypto/x509" "crypto/x509/pkix" "encoding/pem" @@ -562,6 +563,54 @@ func TestComputeConnectionSpecHash(t *testing.T) { assert.NotEqual(t, hash1, hash2, "Empty vs non-empty mTLS secret should produce different hashes") assert.NotEmpty(t, hash1, "Hash should still be generated even with empty mTLS secret") }) + + t.Run("different API key secrets produce different hashes", func(t *testing.T) { + spec1 := temporaliov1alpha1.TemporalConnectionSpec{ + HostPort: "localhost:7233", + APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "secret1"}, + } + spec2 := temporaliov1alpha1.TemporalConnectionSpec{ + HostPort: "localhost:7233", + APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "secret2"}, + } + hash1 := k8s.ComputeConnectionSpecHash(spec1) + hash2 := k8s.ComputeConnectionSpecHash(spec2) + + assert.NotEqual(t, hash1, hash2, "Different API key secrets should produce different hashes") + }) + + t.Run("empty API key secret vs non-empty produce different hashes", func(t *testing.T) { + spec1 := temporaliov1alpha1.TemporalConnectionSpec{ + HostPort: "localhost:7233", + APIKeyRef: nil, + } + spec2 := temporaliov1alpha1.TemporalConnectionSpec{ + HostPort: "localhost:7233", + APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "some-secret"}, + } + hash1 := k8s.ComputeConnectionSpecHash(spec1) + hash2 := k8s.ComputeConnectionSpecHash(spec2) + + assert.NotEqual(t, hash1, hash2, "Empty vs non-empty API key secret should produce different hashes") + assert.NotEmpty(t, hash1, "Hash should still be generated even with empty API key secret") + }) + + t.Run("same API key secret name produce the same hash", func(t *testing.T) { + spec1 := temporaliov1alpha1.TemporalConnectionSpec{ + HostPort: "localhost:7233", + APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "some-secret"}, + } + spec2 := temporaliov1alpha1.TemporalConnectionSpec{ + HostPort: "localhost:7233", + APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "some-secret"}, + } + + hash1 := k8s.ComputeConnectionSpecHash(spec1) + hash2 := k8s.ComputeConnectionSpecHash(spec2) + + assert.Equal(t, hash1, hash2, "Same API key secret name should produce the same hash") + }) + } func TestNewDeploymentWithOwnerRef_EnvironmentVariablesAndVolumes(t *testing.T) { @@ -726,6 +775,13 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { }, namespace: "test-namespace-with-tls", }, + "with API key": { + connection: temporaliov1alpha1.TemporalConnectionSpec{ + HostPort: "test.temporal.example:9999", + APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "test-api-key-secret"}, + }, + namespace: "test-namespace-with-api-key", + }, } for name, tt := range tests { @@ -777,6 +833,14 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { container.Env[i].Value = keyPath } } + } else if tt.connection.APIKeyRef != nil { + for i := range container.Env { + if container.Env[i].Name == "TEMPORAL_API_KEY" { + assert.Equal(t, tt.connection.APIKeyRef.Name, container.Env[i].ValueFrom.SecretKeyRef.Name, "API key secret name should match") + assert.Equal(t, "api-key", container.Env[i].ValueFrom.SecretKeyRef.Key, "API key secret key should be 'api-key'") + container.Env[i].Value = "dummy-token.abc.def" + } + } } // Set environment variables using t.Setenv() to simulate the runtime environment @@ -791,6 +855,9 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { // Verify that the parsed client options match our expectations assert.Equal(t, tt.connection.HostPort, clientOptions.HostPort, "HostPort should be parsed from TEMPORAL_ADDRESS") assert.Equal(t, tt.namespace, clientOptions.Namespace, "Namespace should be parsed from TEMPORAL_NAMESPACE") + if tt.connection.APIKeyRef != nil { + assert.Equal(t, "dummy-token.abc.def", os.Getenv("TEMPORAL_API_KEY"), "API key should be parsed from TEMPORAL_API_KEY") + } // Verify other client option fields that should have default/empty values assert.Empty(t, clientOptions.Identity, "Identity should be empty when not set via env vars") @@ -800,8 +867,11 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { if expectTLS { assert.NotNil(t, clientOptions.ConnectionOptions.TLS, "TLS should be configured for mTLS connection") + } else if tt.connection.APIKeyRef != nil { + // An empty TLS config is configured when an API key is used without mTLS secrets + assert.Equal(t, clientOptions.ConnectionOptions.TLS, &tls.Config{}, "Empty TLS config should be configured for API key connection") } else { - assert.Nil(t, clientOptions.ConnectionOptions.TLS, "TLS should not be configured for non-mTLS connection") + assert.Nil(t, clientOptions.ConnectionOptions.TLS, "TLS config should not be configured for non-mTLS connection") } // Note: TEMPORAL_DEPLOYMENT_NAME and TEMPORAL_WORKER_BUILD_ID are not part of client options diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 7ae1e99f..8bc2e300 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -389,7 +389,7 @@ func getVersionConfigDiff( } // Do nothing if the test workflows have not completed successfully - if strategy.Gate != nil && status.CurrentVersion != nil { + if strategy.Gate != nil { if len(status.TargetVersion.TaskQueues) == 0 { return nil } diff --git a/skaffold.example.env b/skaffold.example.env index dcb492b4..675ca389 100644 --- a/skaffold.example.env +++ b/skaffold.example.env @@ -13,8 +13,13 @@ TEMPORAL_NAMESPACE=your-namespace-here # Temporal server address (optionally templated from namespace, if using Temporal Cloud) TEMPORAL_ADDRESS=${TEMPORAL_NAMESPACE}.tmprl.cloud:7233 +# Set one of mTLS or API key names + # Temporal mTLS secret name -TEMPORAL_MTLS_SECRET_NAME=temporal-cloud-mtls +TEMPORAL_MTLS_SECRET_NAME="" + +# Temporal API key name +TEMPORAL_API_KEY_SECRET_NAME="" # Kubernetes context to use for deployment SKAFFOLD_KUBE_CONTEXT=minikube diff --git a/skaffold.yaml b/skaffold.yaml index d9660546..cd3a6148 100644 --- a/skaffold.yaml +++ b/skaffold.yaml @@ -58,6 +58,7 @@ profiles: temporal.namespace: "{{ .TEMPORAL_NAMESPACE }}" temporal.address: "{{ .TEMPORAL_ADDRESS }}" temporal.mtlsSecretName: "{{ .TEMPORAL_MTLS_SECRET_NAME }}" + temporal.apiKeySecretName: "{{ .TEMPORAL_API_KEY_SECRET_NAME }}" valuesFiles: - internal/demo/helloworld/helm/helloworld/values.yaml deploy: From 726635be9c2d91922ac54744db1e0875daf6affb Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Tue, 30 Sep 2025 12:13:32 -0400 Subject: [PATCH 07/15] quick fix ups --- internal/demo/README.md | 10 ++++----- .../helloworld/helm/helloworld/values.yaml | 6 +++--- internal/demo/helloworld/worker.go | 21 +++++++------------ 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/internal/demo/README.md b/internal/demo/README.md index 4a09c1a9..d734aeb5 100644 --- a/internal/demo/README.md +++ b/internal/demo/README.md @@ -53,7 +53,7 @@ This guide will help you set up and run the Temporal Worker Controller locally u TEMPORAL_MTLS_SECRET_NAME="" ``` - Note: Do not set both mTLS and API key for the same connection. If both present, the controller - connects to the namespace using mTLS. + connects to the namespace using the API key you have configured. 3. Create the `skaffold.env` file: - Update the value of `TEMPORAL_NAMESPACE`, `TEMPORAL_ADDRESS` in `skaffold.example.env` to match your configuration. @@ -76,26 +76,26 @@ This guide will help you set up and run the Temporal Worker Controller locally u ``` This deploys a TemporalWorkerDeployment and TemporalConnection Custom Resource using the **Progressive strategy**. Note that when there is no current version (as in an initial versioned worker deployment), the progressive steps are skipped and v1 becomes the current version immediately. All new workflow executions will now start on v1. -5. Watch the deployment status: +6. Watch the deployment status: ```bash watch kubectl get twd ``` -6. **Apply load** to the v1 worker to simulate production traffic: +7. **Apply load** to the v1 worker to simulate production traffic: ```bash make apply-load-sample-workflow ``` #### **Progressive Rollout of v2** (Non-Replay-Safe Change) -7. **Deploy a non-replay-safe workflow change**: +8. **Deploy a non-replay-safe workflow change**: ```bash git apply internal/demo/helloworld/changes/no-version-gate.patch skaffold run --profile helloworld-worker ``` This applies a **non-replay-safe change** (switching an activity response type from string to a struct). -8. **Observe the progressive rollout managing incompatible versions**: +9. **Observe the progressive rollout managing incompatible versions**: - New workflow executions gradually shift from v1 to v2 following the configured rollout steps (1% → 5% → 10% → 50% → 100%) - **Both worker versions run simultaneously** - this is critical since the code changes are incompatible - v1 workers continue serving existing workflows (which would fail to replay on v2) diff --git a/internal/demo/helloworld/helm/helloworld/values.yaml b/internal/demo/helloworld/helm/helloworld/values.yaml index 7ceeda8c..eaa35ae7 100644 --- a/internal/demo/helloworld/helm/helloworld/values.yaml +++ b/internal/demo/helloworld/helm/helloworld/values.yaml @@ -3,12 +3,12 @@ image: tag: latest temporal: - namespace: ss-worker-controller-with-api-keys.a2dd6 # e.g. default + namespace: "" # e.g. default # Use existing connection (leave empty to create new one) connectionName: "" # e.g. dev-server # Connection details (required if connectionName is empty) - address: ss-worker-controller-with-api-keys.a2dd6.tmprl.cloud:7233 # e.g. .tmprl.cloud:7233 - apiKeySecretName: temporal-api-key + address: "" # e.g. .tmprl.cloud:7233 + apiKeySecretName: "" # e.g. temporal-api-key rollout: strategy: Progressive diff --git a/internal/demo/helloworld/worker.go b/internal/demo/helloworld/worker.go index df0efa32..5f190df8 100644 --- a/internal/demo/helloworld/worker.go +++ b/internal/demo/helloworld/worker.go @@ -7,6 +7,7 @@ package helloworld import ( "context" "fmt" + "math/rand" "strings" "time" @@ -18,26 +19,20 @@ func HelloWorld(ctx workflow.Context) (string, error) { ctx = util.SetActivityTimeout(ctx, 5*time.Minute) // Get a subject - var subject GetSubjectResponse + var subject string if err := workflow.ExecuteActivity(ctx, GetSubject).Get(ctx, &subject); err != nil { return "", err } // Return the greeting - return fmt.Sprintf("Hello %s", subject.Name), nil + return fmt.Sprintf("Hello %s", subject), nil } -func GetSubject(ctx context.Context) (*GetSubjectResponse, error) { - // Send heartbeats - go util.AutoHeartbeat(ctx) - - // Get user via API - subject, err := fetchUser(ctx, "https://jsonplaceholder.typicode.com") - if err != nil { - return nil, err - } - - return &subject, nil +func GetSubject(ctx context.Context) (string, error) { + // Simulate activity execution latency + time.Sleep(time.Duration(rand.Intn(30)) * time.Second) + // Return a hardcoded subject + return "World", nil } func RolloutGate(ctx workflow.Context) error { From 49b33377c966b300fc321e4baf88fee0f28371ab Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Tue, 30 Sep 2025 13:02:59 -0400 Subject: [PATCH 08/15] add in-memory connection auth mode --- .gitignore | 1 + config/rbac/role.yaml | 66 ++++++++++++++++++++ config/webhook/manifests.yaml | 26 ++++++++ internal/controller/clientpool/clientpool.go | 52 ++++++++++++--- internal/controller/worker_controller.go | 2 +- internal/demo/util/client.go | 4 -- internal/planner/planner.go | 1 - 7 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 config/rbac/role.yaml create mode 100644 config/webhook/manifests.yaml diff --git a/.gitignore b/.gitignore index 90717c04..d19960ea 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ certs .DS_Store .claude +.config \ No newline at end of file diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml new file mode 100644 index 00000000..5082552c --- /dev/null +++ b/config/rbac/role.yaml @@ -0,0 +1,66 @@ +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: manager-role +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - apps + resources: + - deployments/scale + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalconnections + verbs: + - get + - list + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/finalizers + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/status + verbs: + - get + - patch + - update diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml new file mode 100644 index 00000000..a3edd279 --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,26 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-temporal-io-temporal-io-v1alpha1-temporalworkerdeployment + failurePolicy: Fail + name: mtemporalworker.kb.io + rules: + - apiGroups: + - temporal.io.temporal.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - temporalworkers + sideEffects: None diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index 96fad714..e565af72 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -24,9 +24,9 @@ import ( type AuthMode string const ( - AuthModeTLS AuthMode = "TLS" - AuthModeAPIKey AuthMode = "API_KEY" - AuthModeUnknown AuthMode = "UNKNOWN" + AuthModeTLS AuthMode = "TLS" + AuthModeAPIKey AuthMode = "API_KEY" + AuthModeInMemory AuthMode = "IN_MEMORY" // Add more auth modes here as they are supported ) @@ -202,15 +202,46 @@ func (cp *ClientPool) fetchClientUsingAPIKeySecret(secret corev1.Secret, opts Ne return c, nil } +func (cp *ClientPool) fetchClientUsingInMemoryConnection(opts NewClientOptions) (sdkclient.Client, error) { + clientOpts := sdkclient.Options{ + Logger: cp.logger, + HostPort: opts.Spec.HostPort, + Namespace: opts.TemporalNamespace, + } + + c, err := sdkclient.Dial(clientOpts) + if err != nil { + return nil, err + } + + key := ClientPoolKey{ + HostPort: opts.Spec.HostPort, + Namespace: opts.TemporalNamespace, + SecretName: "", // no secret name for in-memory connection + AuthMode: AuthModeInMemory, + } + cp.clients[key] = ClientInfo{ + client: c, + auth: ClientAuth{ + mode: AuthModeInMemory, + mTLS: nil, + }, + } + + return c, nil +} + func (cp *ClientPool) UpsertClient(ctx context.Context, secretName string, authMode AuthMode, opts NewClientOptions) (sdkclient.Client, error) { - // Fetch the secret + // Fetch the secret from k8s cluster, if it exists. Otherwise, use an in-memory connection to the server. var secret corev1.Secret - if err := cp.k8sClient.Get(ctx, types.NamespacedName{ - Name: secretName, - Namespace: opts.K8sNamespace, - }, &secret); err != nil { - return nil, err + if secretName != "" { + if err := cp.k8sClient.Get(ctx, types.NamespacedName{ + Name: secretName, + Namespace: opts.K8sNamespace, + }, &secret); err != nil { + return nil, err + } } // Check the secret type @@ -229,6 +260,9 @@ func (cp *ClientPool) UpsertClient(ctx context.Context, secretName string, authM } return cp.fetchClientUsingAPIKeySecret(secret, opts) + case AuthModeInMemory: + return cp.fetchClientUsingInMemoryConnection(opts) + default: return nil, fmt.Errorf("invalid auth mode: %s", authMode) } diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index 6d4c2995..572efa7e 100644 --- a/internal/controller/worker_controller.go +++ b/internal/controller/worker_controller.go @@ -51,7 +51,7 @@ func getAuthMode(temporalConnection *v1alpha1.TemporalConnection) clientpool.Aut } else if temporalConnection.Spec.APIKeyRef != nil { return clientpool.AuthModeAPIKey } - return clientpool.AuthModeUnknown + return clientpool.AuthModeInMemory } // TemporalWorkerDeploymentReconciler reconciles a TemporalWorkerDeployment object diff --git a/internal/demo/util/client.go b/internal/demo/util/client.go index 6fa90e22..6a1b7f38 100644 --- a/internal/demo/util/client.go +++ b/internal/demo/util/client.go @@ -38,10 +38,6 @@ func newClient(buildID string, metricsPort int) (c client.Client, stopFunc func( panic(err) } - // if _, err := c.CheckHealth(context.Background(), &client.CheckHealthRequest{}); err != nil { - // panic(err) - // } - if _, err := c.ListWorkflow(context.Background(), &workflowservice.ListWorkflowExecutionsRequest{ Namespace: opts.Namespace, }); err != nil { diff --git a/internal/planner/planner.go b/internal/planner/planner.go index 8bc2e300..b65f4a8c 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -394,7 +394,6 @@ func getVersionConfigDiff( return nil } if len(status.TargetVersion.TestWorkflows) < len(status.TargetVersion.TaskQueues) { - l.Info("not enough test workflows running to start gate workflow", "buildID", status.TargetVersion.BuildID, "taskQueues", status.TargetVersion.TaskQueues, "testWorkflows", status.TargetVersion.TestWorkflows) return nil } for _, wf := range status.TargetVersion.TestWorkflows { From 5f1ca93b10cd8419d57d31e953a666fb401825ff Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Tue, 30 Sep 2025 13:03:37 -0400 Subject: [PATCH 09/15] remove config dir --- config/rbac/role.yaml | 66 ----------------------------------- config/webhook/manifests.yaml | 26 -------------- 2 files changed, 92 deletions(-) delete mode 100644 config/rbac/role.yaml delete mode 100644 config/webhook/manifests.yaml diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml deleted file mode 100644 index 5082552c..00000000 --- a/config/rbac/role.yaml +++ /dev/null @@ -1,66 +0,0 @@ ---- -apiVersion: rbac.authorization.k8s.io/v1 -kind: ClusterRole -metadata: - name: manager-role -rules: -- apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch -- apiGroups: - - apps - resources: - - deployments - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - apps - resources: - - deployments/scale - verbs: - - update -- apiGroups: - - temporal.io - resources: - - temporalconnections - verbs: - - get - - list - - watch -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments/finalizers - verbs: - - update -- apiGroups: - - temporal.io - resources: - - temporalworkerdeployments/status - verbs: - - get - - patch - - update diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml deleted file mode 100644 index a3edd279..00000000 --- a/config/webhook/manifests.yaml +++ /dev/null @@ -1,26 +0,0 @@ ---- -apiVersion: admissionregistration.k8s.io/v1 -kind: MutatingWebhookConfiguration -metadata: - name: mutating-webhook-configuration -webhooks: -- admissionReviewVersions: - - v1 - clientConfig: - service: - name: webhook-service - namespace: system - path: /mutate-temporal-io-temporal-io-v1alpha1-temporalworkerdeployment - failurePolicy: Fail - name: mtemporalworker.kb.io - rules: - - apiGroups: - - temporal.io.temporal.io - apiVersions: - - v1alpha1 - operations: - - CREATE - - UPDATE - resources: - - temporalworkers - sideEffects: None From e18f521523bcddee9afe8a325ebaf6505802b39e Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Tue, 30 Sep 2025 13:15:50 -0400 Subject: [PATCH 10/15] add API in values.yaml --- internal/demo/helloworld/helm/helloworld/values.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/demo/helloworld/helm/helloworld/values.yaml b/internal/demo/helloworld/helm/helloworld/values.yaml index eaa35ae7..b9913af5 100644 --- a/internal/demo/helloworld/helm/helloworld/values.yaml +++ b/internal/demo/helloworld/helm/helloworld/values.yaml @@ -8,6 +8,7 @@ temporal: connectionName: "" # e.g. dev-server # Connection details (required if connectionName is empty) address: "" # e.g. .tmprl.cloud:7233 + mtlsSecretName: "" # e.g. temporal-cloud-mtls apiKeySecretName: "" # e.g. temporal-api-key rollout: From 1ab0c99f3e2bdd6b3383c83295a6aed362cf0b7f Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Thu, 9 Oct 2025 10:44:43 -0400 Subject: [PATCH 11/15] incorporating changes --- internal/controller/clientpool/clientpool.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index e565af72..556d2685 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -26,7 +26,7 @@ type AuthMode string const ( AuthModeTLS AuthMode = "TLS" AuthModeAPIKey AuthMode = "API_KEY" - AuthModeInMemory AuthMode = "IN_MEMORY" + AuthModeInMemory AuthMode = "IN_MEMORY" // Local in‑memory backend; no TLS/API keys (dev/test only) // Add more auth modes here as they are supported ) @@ -39,7 +39,7 @@ type ClientPoolKey struct { type MTLSAuth struct { tlsConfig *tls.Config - expiryTime time.Time // cert NotAfter - buffer + expiryTime time.Time // Time we consider the cert expired (NotAfter minus safety buffer) } type ClientAuth struct { From 6347359f7968cccd5dd2a2073a242404b749139b Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Thu, 9 Oct 2025 21:54:30 -0400 Subject: [PATCH 12/15] current work --- internal/controller/clientpool/clientpool.go | 18 +++++++++--------- internal/controller/worker_controller.go | 7 +++---- internal/demo/README.md | 20 ++++++++++---------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index 556d2685..b5c6c38d 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -24,9 +24,9 @@ import ( type AuthMode string const ( - AuthModeTLS AuthMode = "TLS" - AuthModeAPIKey AuthMode = "API_KEY" - AuthModeInMemory AuthMode = "IN_MEMORY" // Local in‑memory backend; no TLS/API keys (dev/test only) + AuthModeTLS AuthMode = "TLS" + AuthModeAPIKey AuthMode = "API_KEY" + AuthModeNoCredentials AuthMode = "NO_CREDENTIALS" // no TLS/API keys // Add more auth modes here as they are supported ) @@ -202,7 +202,7 @@ func (cp *ClientPool) fetchClientUsingAPIKeySecret(secret corev1.Secret, opts Ne return c, nil } -func (cp *ClientPool) fetchClientUsingInMemoryConnection(opts NewClientOptions) (sdkclient.Client, error) { +func (cp *ClientPool) fetchClientUsingNoCredentials(opts NewClientOptions) (sdkclient.Client, error) { clientOpts := sdkclient.Options{ Logger: cp.logger, HostPort: opts.Spec.HostPort, @@ -217,13 +217,13 @@ func (cp *ClientPool) fetchClientUsingInMemoryConnection(opts NewClientOptions) key := ClientPoolKey{ HostPort: opts.Spec.HostPort, Namespace: opts.TemporalNamespace, - SecretName: "", // no secret name for in-memory connection - AuthMode: AuthModeInMemory, + SecretName: "", + AuthMode: AuthModeNoCredentials, } cp.clients[key] = ClientInfo{ client: c, auth: ClientAuth{ - mode: AuthModeInMemory, + mode: AuthModeNoCredentials, mTLS: nil, }, } @@ -260,8 +260,8 @@ func (cp *ClientPool) UpsertClient(ctx context.Context, secretName string, authM } return cp.fetchClientUsingAPIKeySecret(secret, opts) - case AuthModeInMemory: - return cp.fetchClientUsingInMemoryConnection(opts) + case AuthModeNoCredentials: + return cp.fetchClientUsingNoCredentials(opts) default: return nil, fmt.Errorf("invalid auth mode: %s", authMode) diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index 572efa7e..dee4787e 100644 --- a/internal/controller/worker_controller.go +++ b/internal/controller/worker_controller.go @@ -9,7 +9,6 @@ import ( "fmt" "time" - "github.com/temporalio/temporal-worker-controller/api/v1alpha1" temporaliov1alpha1 "github.com/temporalio/temporal-worker-controller/api/v1alpha1" "github.com/temporalio/temporal-worker-controller/internal/controller/clientpool" "github.com/temporalio/temporal-worker-controller/internal/k8s" @@ -38,20 +37,20 @@ const ( ) // getSecretName extracts the secret name from a secret reference -func getSecretName(secretRef *v1alpha1.SecretReference) string { +func getSecretName(secretRef *temporaliov1alpha1.SecretReference) string { if secretRef != nil { return secretRef.Name } return "" } -func getAuthMode(temporalConnection *v1alpha1.TemporalConnection) clientpool.AuthMode { +func getAuthMode(temporalConnection *temporaliov1alpha1.TemporalConnection) clientpool.AuthMode { if temporalConnection.Spec.MutualTLSSecretRef != nil { return clientpool.AuthModeTLS } else if temporalConnection.Spec.APIKeyRef != nil { return clientpool.AuthModeAPIKey } - return clientpool.AuthModeInMemory + return clientpool.AuthModeNoCredentials } // TemporalWorkerDeploymentReconciler reconciles a TemporalWorkerDeployment object diff --git a/internal/demo/README.md b/internal/demo/README.md index d734aeb5..55add8f2 100644 --- a/internal/demo/README.md +++ b/internal/demo/README.md @@ -20,6 +20,14 @@ This guide will help you set up and run the Temporal Worker Controller locally u minikube start ``` +2. Create the `skaffold.env` file: + - Run: + ```bash + cp skaffold.example.env skaffold.env + ``` + + - Update the value of `TEMPORAL_NAMESPACE`, `TEMPORAL_ADDRESS` in `skaffold.example.env` to match your configuration. + 2. Set up Temporal Cloud Authentication: - Create a `certs` directory in the project root - Save your Temporal Cloud mTLS client certificates as: @@ -29,7 +37,7 @@ This guide will help you set up and run the Temporal Worker Controller locally u ```bash make create-cloud-mtls-secret ``` - - In `skaffold.example.env`, set: + - In `skaffold.env`, set: ```env TEMPORAL_API_KEY_SECRET_NAME="" TEMPORAL_MTLS_SECRET_NAME=temporal-cloud-mtls-secret @@ -47,7 +55,7 @@ This guide will help you set up and run the Temporal Worker Controller locally u ```bash make create-api-key-secret ``` - - In `skaffold.example.env`, set: + - In `skaffold.env`, set: ```env TEMPORAL_API_KEY_SECRET_NAME=temporal-api-key TEMPORAL_MTLS_SECRET_NAME="" @@ -55,14 +63,6 @@ This guide will help you set up and run the Temporal Worker Controller locally u - Note: Do not set both mTLS and API key for the same connection. If both present, the controller connects to the namespace using the API key you have configured. -3. Create the `skaffold.env` file: - - Update the value of `TEMPORAL_NAMESPACE`, `TEMPORAL_ADDRESS` in `skaffold.example.env` to match your configuration. - - - Then run: - ```bash - cp skaffold.example.env skaffold.env - ``` - 4. Build and deploy the Controller image to the local k8s cluster: ```bash skaffold run --profile worker-controller From e518342a0dc46482e2d27acb6d1773b2f63cdab7 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Fri, 10 Oct 2025 16:38:28 -0400 Subject: [PATCH 13/15] addressed all comments --- Makefile | 10 +++- api/v1alpha1/temporalconnection_types.go | 4 +- api/v1alpha1/zz_generated.deepcopy.go | 8 +-- .../crds/temporal.io_temporalconnections.yaml | 14 ++++- internal/controller/clientpool/clientpool.go | 6 +- internal/controller/worker_controller.go | 45 ++++++++++---- internal/demo/README.md | 8 +-- .../helm/helloworld/templates/connection.yaml | 12 ++-- .../helm/helloworld/values.schema.json | 24 ++++++-- .../helloworld/helm/helloworld/values.yaml | 6 +- internal/k8s/deployments.go | 12 ++-- internal/k8s/deployments_test.go | 59 ++++++++++++------- .../tests/internal/deployment_controller.go | 4 +- internal/tests/internal/validation_helpers.go | 4 +- skaffold.example.env | 5 +- skaffold.yaml | 3 +- 16 files changed, 148 insertions(+), 76 deletions(-) diff --git a/Makefile b/Makefile index bff8f4c3..2ac274e8 100644 --- a/Makefile +++ b/Makefile @@ -164,16 +164,20 @@ start-sample-workflow: ## Start a sample workflow. @set -e; \ # Load env vars from skaffold.env if present so address/namespace aren't hardcoded if [ -f skaffold.env ]; then set -a; . skaffold.env; set +a; fi; \ - if [ -n "$$TEMPORAL_API_KEY" ]; then \ + API_KEY_VAL=""; \ + if [ -n "$$TEMPORAL_API_KEY" ]; then API_KEY_VAL="$$TEMPORAL_API_KEY"; \ + elif [ -f certs/api-key.txt ]; then API_KEY_VAL="$$(tr -d '\r\n' < certs/api-key.txt)"; fi; \ + if [ -n "$$API_KEY_VAL" ]; then \ $(TEMPORAL) workflow start --type "HelloWorld" --task-queue "default/helloworld" \ --address "$$TEMPORAL_ADDRESS" \ - -n "$$TEMPORAL_NAMESPACE"; \ + --namespace "$$TEMPORAL_NAMESPACE" \ + --api-key "$$API_KEY_VAL"; \ else \ $(TEMPORAL) workflow start --type "HelloWorld" --task-queue "default/helloworld" \ --tls-cert-path certs/client.pem \ --tls-key-path certs/client.key \ --address "$$TEMPORAL_ADDRESS" \ - -n "$$TEMPORAL_NAMESPACE"; \ + --namespace "$$TEMPORAL_NAMESPACE"; \ fi .PHONY: apply-load-sample-workflow diff --git a/api/v1alpha1/temporalconnection_types.go b/api/v1alpha1/temporalconnection_types.go index 5336be4f..b27f7bc8 100644 --- a/api/v1alpha1/temporalconnection_types.go +++ b/api/v1alpha1/temporalconnection_types.go @@ -5,6 +5,7 @@ package v1alpha1 import ( + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -19,6 +20,7 @@ type SecretReference struct { } // TemporalConnectionSpec defines the desired state of TemporalConnection +// +kubebuilder:validation:XValidation:rule="!(has(self.mutualTLSSecretRef) && has(self.apiKeySecretRef))",message="Only one of mutualTLSSecretRef or apiKeySecretRef may be set" type TemporalConnectionSpec struct { // The host and port of the Temporal server. // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9.-]+:[0-9]+$` @@ -36,7 +38,7 @@ type TemporalConnectionSpec struct { // APIKeyRef is the name of the Secret that contains the API key. The secret must be `type: kubernetes.io/opaque` and exist // in the same Kubernetes namespace as the TemporalConnection resource. // +optional - APIKeyRef *SecretReference `json:"apiKeyRef,omitempty"` + APIKeySecretRef *corev1.SecretKeySelector `json:"apiKeySecretRef,omitempty"` } // TemporalConnectionStatus defines the observed state of TemporalConnection diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 9f342348..1decf431 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -332,10 +332,10 @@ func (in *TemporalConnectionSpec) DeepCopyInto(out *TemporalConnectionSpec) { *out = new(SecretReference) **out = **in } - if in.APIKeyRef != nil { - in, out := &in.APIKeyRef, &out.APIKeyRef - *out = new(SecretReference) - **out = **in + if in.APIKeySecretRef != nil { + in, out := &in.APIKeySecretRef, &out.APIKeySecretRef + *out = new(v1.SecretKeySelector) + (*in).DeepCopyInto(*out) } } diff --git a/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml b/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml index d493a4d5..e2a21715 100644 --- a/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml +++ b/helm/temporal-worker-controller/templates/crds/temporal.io_temporalconnections.yaml @@ -37,14 +37,19 @@ spec: type: object spec: properties: - apiKeyRef: + apiKeySecretRef: properties: + key: + type: string name: - pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + default: "" type: string + optional: + type: boolean required: - - name + - key type: object + x-kubernetes-map-type: atomic hostPort: pattern: ^[a-zA-Z0-9.-]+:[0-9]+$ type: string @@ -59,6 +64,9 @@ spec: required: - hostPort type: object + x-kubernetes-validations: + - message: Only one of mutualTLSSecretRef or apiKeySecretRef may be set + rule: '!(has(self.mutualTLSSecretRef) && has(self.apiKeySecretRef))' status: type: object type: object diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index b5c6c38d..03cf2ec2 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -174,7 +174,7 @@ func (cp *ClientPool) fetchClientUsingAPIKeySecret(secret corev1.Secret, opts Ne } clientOpts.Credentials = sdkclient.NewAPIKeyDynamicCredentials(func(ctx context.Context) (string, error) { - return string(secret.Data["api-key"]), nil + return string(secret.Data[opts.Spec.APIKeySecretRef.Key]), nil }) c, err := sdkclient.Dial(clientOpts) @@ -188,7 +188,7 @@ func (cp *ClientPool) fetchClientUsingAPIKeySecret(secret corev1.Secret, opts Ne key := ClientPoolKey{ HostPort: opts.Spec.HostPort, Namespace: opts.TemporalNamespace, - SecretName: opts.Spec.APIKeyRef.Name, + SecretName: opts.Spec.APIKeySecretRef.Name, AuthMode: AuthModeAPIKey, } cp.clients[key] = ClientInfo{ @@ -233,7 +233,7 @@ func (cp *ClientPool) fetchClientUsingNoCredentials(opts NewClientOptions) (sdkc func (cp *ClientPool) UpsertClient(ctx context.Context, secretName string, authMode AuthMode, opts NewClientOptions) (sdkclient.Client, error) { - // Fetch the secret from k8s cluster, if it exists. Otherwise, use an in-memory connection to the server. + // Fetch the secret from k8s cluster, if it exists. Otherwise, create a connection with the server without using any credentials. var secret corev1.Secret if secretName != "" { if err := cp.k8sClient.Get(ctx, types.NamespacedName{ diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index dee4787e..fec3f080 100644 --- a/internal/controller/worker_controller.go +++ b/internal/controller/worker_controller.go @@ -14,6 +14,7 @@ import ( "github.com/temporalio/temporal-worker-controller/internal/k8s" "github.com/temporalio/temporal-worker-controller/internal/temporal" appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -36,18 +37,41 @@ const ( buildIDLabel = "temporal.io/build-id" ) -// getSecretName extracts the secret name from a secret reference -func getSecretName(secretRef *temporaliov1alpha1.SecretReference) string { +// getAPIKeySecretName extracts the secret name from a SecretKeySelector +func getAPIKeySecretName(secretRef *corev1.SecretKeySelector) (string, error) { if secretRef != nil { - return secretRef.Name + return secretRef.Name, nil + } + + return "", fmt.Errorf("API key secret name is not set") +} + +func getTLSSecretName(secretRef *temporaliov1alpha1.SecretReference) (string, error) { + if secretRef != nil { + return secretRef.Name, nil + } + + return "", fmt.Errorf("TLS secret name is not set") +} + +func resolveAuthSecretName(tc *temporaliov1alpha1.TemporalConnection) (clientpool.AuthMode, string, error) { + auth := getAuthMode(tc) + switch auth { + case clientpool.AuthModeTLS: + name, err := getTLSSecretName(tc.Spec.MutualTLSSecretRef) + return auth, name, err + case clientpool.AuthModeAPIKey: + name, err := getAPIKeySecretName(tc.Spec.APIKeySecretRef) + return auth, name, err + default: + return auth, "", nil } - return "" } func getAuthMode(temporalConnection *temporaliov1alpha1.TemporalConnection) clientpool.AuthMode { if temporalConnection.Spec.MutualTLSSecretRef != nil { return clientpool.AuthModeTLS - } else if temporalConnection.Spec.APIKeyRef != nil { + } else if temporalConnection.Spec.APIKeySecretRef != nil { return clientpool.AuthModeAPIKey } return clientpool.AuthModeNoCredentials @@ -137,13 +161,10 @@ func (r *TemporalWorkerDeploymentReconciler) Reconcile(ctx context.Context, req } // Get the Auth Mode and Secret Name - authMode := getAuthMode(&temporalConnection) - var secretName string - switch authMode { - case clientpool.AuthModeTLS: - secretName = getSecretName(temporalConnection.Spec.MutualTLSSecretRef) - case clientpool.AuthModeAPIKey: - secretName = getSecretName(temporalConnection.Spec.APIKeyRef) + authMode, secretName, err := resolveAuthSecretName(&temporalConnection) + if err != nil { + l.Error(err, "unable to resolve auth secret name") + return ctrl.Result{}, err } // Get or update temporal client for connection diff --git a/internal/demo/README.md b/internal/demo/README.md index 55add8f2..44a5adf8 100644 --- a/internal/demo/README.md +++ b/internal/demo/README.md @@ -26,7 +26,7 @@ This guide will help you set up and run the Temporal Worker Controller locally u cp skaffold.example.env skaffold.env ``` - - Update the value of `TEMPORAL_NAMESPACE`, `TEMPORAL_ADDRESS` in `skaffold.example.env` to match your configuration. + - Update the value of `TEMPORAL_NAMESPACE`, `TEMPORAL_ADDRESS` in `skaffold.env` to match your configuration. 2. Set up Temporal Cloud Authentication: - Create a `certs` directory in the project root @@ -51,7 +51,7 @@ This guide will help you set up and run the Temporal Worker Controller locally u ```bash echo -n "" > certs/api-key.txt ``` - - Create the Kubernetes Secret (trims any accidental newlines): + - Create the Kubernetes Secret: ```bash make create-api-key-secret ``` @@ -60,8 +60,8 @@ This guide will help you set up and run the Temporal Worker Controller locally u TEMPORAL_API_KEY_SECRET_NAME=temporal-api-key TEMPORAL_MTLS_SECRET_NAME="" ``` - - Note: Do not set both mTLS and API key for the same connection. If both present, the controller - connects to the namespace using the API key you have configured. + - Note: Do not set both mTLS and API key for the same connection. If both present, the TemporalConnection Custom Resource + Instance will not get installed in the k8s environment. 4. Build and deploy the Controller image to the local k8s cluster: ```bash diff --git a/internal/demo/helloworld/helm/helloworld/templates/connection.yaml b/internal/demo/helloworld/helm/helloworld/templates/connection.yaml index 6e2082ab..c6277834 100644 --- a/internal/demo/helloworld/helm/helloworld/templates/connection.yaml +++ b/internal/demo/helloworld/helm/helloworld/templates/connection.yaml @@ -8,13 +8,15 @@ metadata: {{- include "helloworld.labels" . | nindent 4 }} spec: hostPort: "{{ .Values.temporal.address }}" - {{- $apiKey := .Values.temporal.apiKeySecretName | default "" | trim }} - {{- if and $apiKey (ne $apiKey "null") }} - apiKeyRef: - name: {{ $apiKey | quote }} + {{- $apiSecretName := .Values.temporal.apiKey.name | trim }} + {{- $apiSecretKey := .Values.temporal.apiKey.key | trim }} + {{- if $apiSecretName }} + apiKeySecretRef: + name: {{ $apiSecretName | quote }} + key: {{ $apiSecretKey | quote }} {{- end }} {{- if .Values.temporal.mtlsSecretName }} mutualTLSSecretRef: name: {{ .Values.temporal.mtlsSecretName | quote }} {{- end }} -{{- end }} +{{- end }} \ No newline at end of file diff --git a/internal/demo/helloworld/helm/helloworld/values.schema.json b/internal/demo/helloworld/helm/helloworld/values.schema.json index 520ccbd4..790edd62 100644 --- a/internal/demo/helloworld/helm/helloworld/values.schema.json +++ b/internal/demo/helloworld/helm/helloworld/values.schema.json @@ -36,9 +36,19 @@ "type": "string", "description": "Name of the secret containing mTLS certificates (required if connectionName is empty)" }, - "apiKeySecretName": { - "type": "string", - "description": "Name of the secret containing API key (required if connectionName is empty)" + "apiKey": { + "type": "object", + "additionalProperties": false, + "properties": { + "name": { + "type": "string", + "description": "Name of the secret containing API key (required if connectionName is empty)" + }, + "key": { + "type": "string", + "description": "Data key inside the API key secret" + } + } } }, "required": ["namespace"], @@ -57,7 +67,13 @@ "required": ["address", "mtlsSecretName"] }, { - "required": ["address", "apiKeySecretName"] + "properties": { + "apiKey": { + "type": "object", + "required": ["name", "key"] + } + }, + "required": ["address", "apiKey"] } ] }, diff --git a/internal/demo/helloworld/helm/helloworld/values.yaml b/internal/demo/helloworld/helm/helloworld/values.yaml index b9913af5..7143e741 100644 --- a/internal/demo/helloworld/helm/helloworld/values.yaml +++ b/internal/demo/helloworld/helm/helloworld/values.yaml @@ -3,13 +3,15 @@ image: tag: latest temporal: - namespace: "" # e.g. default + namespace: "default" # e.g. default # Use existing connection (leave empty to create new one) connectionName: "" # e.g. dev-server # Connection details (required if connectionName is empty) address: "" # e.g. .tmprl.cloud:7233 mtlsSecretName: "" # e.g. temporal-cloud-mtls - apiKeySecretName: "" # e.g. temporal-api-key + apiKey: + name: "" # e.g. temporal-api-key + key: "" # data key inside the secret rollout: strategy: Progressive diff --git a/internal/k8s/deployments.go b/internal/k8s/deployments.go index b29a2243..9e5d069f 100644 --- a/internal/k8s/deployments.go +++ b/internal/k8s/deployments.go @@ -32,7 +32,6 @@ const ( ResourceNameSeparator = "-" MaxBuildIdLen = 63 ConnectionSpecHashAnnotation = "temporal.io/connection-spec-hash" - APISecretKey = "api-key" ) // DeploymentState represents the Kubernetes state of all deployments for a temporal worker deployment @@ -262,16 +261,13 @@ func NewDeploymentWithOwnerRef( }, }, }) - } else if connection.APIKeyRef != nil { + } else if connection.APIKeySecretRef != nil { for i, container := range podSpec.Containers { container.Env = append(container.Env, corev1.EnvVar{ Name: "TEMPORAL_API_KEY", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: connection.APIKeyRef.Name}, - Key: APISecretKey, - }, + SecretKeyRef: connection.APIKeySecretRef, }, }, ) @@ -335,8 +331,8 @@ func ComputeConnectionSpecHash(connection temporaliov1alpha1.TemporalConnectionS _, _ = hasher.Write([]byte(connection.HostPort)) if connection.MutualTLSSecretRef != nil { _, _ = hasher.Write([]byte(connection.MutualTLSSecretRef.Name)) - } else if connection.APIKeyRef != nil { - _, _ = hasher.Write([]byte(connection.APIKeyRef.Name)) + } else if connection.APIKeySecretRef != nil { + _, _ = hasher.Write([]byte(connection.APIKeySecretRef.Name)) } return hex.EncodeToString(hasher.Sum(nil)) diff --git a/internal/k8s/deployments_test.go b/internal/k8s/deployments_test.go index 6e6efdb9..ae1bafd9 100644 --- a/internal/k8s/deployments_test.go +++ b/internal/k8s/deployments_test.go @@ -566,12 +566,16 @@ func TestComputeConnectionSpecHash(t *testing.T) { t.Run("different API key secrets produce different hashes", func(t *testing.T) { spec1 := temporaliov1alpha1.TemporalConnectionSpec{ - HostPort: "localhost:7233", - APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "secret1"}, + HostPort: "localhost:7233", + APIKeySecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret1"}, + Key: "api-key1"}, } spec2 := temporaliov1alpha1.TemporalConnectionSpec{ - HostPort: "localhost:7233", - APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "secret2"}, + HostPort: "localhost:7233", + APIKeySecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret2"}, + Key: "api-key2"}, } hash1 := k8s.ComputeConnectionSpecHash(spec1) hash2 := k8s.ComputeConnectionSpecHash(spec2) @@ -581,12 +585,14 @@ func TestComputeConnectionSpecHash(t *testing.T) { t.Run("empty API key secret vs non-empty produce different hashes", func(t *testing.T) { spec1 := temporaliov1alpha1.TemporalConnectionSpec{ - HostPort: "localhost:7233", - APIKeyRef: nil, + HostPort: "localhost:7233", + APIKeySecretRef: nil, } spec2 := temporaliov1alpha1.TemporalConnectionSpec{ - HostPort: "localhost:7233", - APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "some-secret"}, + HostPort: "localhost:7233", + APIKeySecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret"}, + Key: "api-key"}, } hash1 := k8s.ComputeConnectionSpecHash(spec1) hash2 := k8s.ComputeConnectionSpecHash(spec2) @@ -597,12 +603,16 @@ func TestComputeConnectionSpecHash(t *testing.T) { t.Run("same API key secret name produce the same hash", func(t *testing.T) { spec1 := temporaliov1alpha1.TemporalConnectionSpec{ - HostPort: "localhost:7233", - APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "some-secret"}, + HostPort: "localhost:7233", + APIKeySecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret"}, + Key: "api-key"}, } spec2 := temporaliov1alpha1.TemporalConnectionSpec{ - HostPort: "localhost:7233", - APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "some-secret"}, + HostPort: "localhost:7233", + APIKeySecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "secret"}, + Key: "api-key"}, } hash1 := k8s.ComputeConnectionSpecHash(spec1) @@ -777,8 +787,10 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { }, "with API key": { connection: temporaliov1alpha1.TemporalConnectionSpec{ - HostPort: "test.temporal.example:9999", - APIKeyRef: &temporaliov1alpha1.SecretReference{Name: "test-api-key-secret"}, + HostPort: "test.temporal.example:9999", + APIKeySecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "test-api-key-secret"}, + Key: "api-key"}, }, namespace: "test-namespace-with-api-key", }, @@ -815,7 +827,7 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { container := deployment.Spec.Template.Spec.Containers[0] // Infer whether TLS is expected from connection spec - expectTLS := tt.connection.MutualTLSSecretRef != nil + expectTLS := tt.connection.MutualTLSSecretRef != nil || tt.connection.APIKeySecretRef != nil if expectTLS { // Create temporary test certificate files @@ -833,18 +845,23 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { container.Env[i].Value = keyPath } } - } else if tt.connection.APIKeyRef != nil { + } else if tt.connection.APIKeySecretRef != nil { for i := range container.Env { if container.Env[i].Name == "TEMPORAL_API_KEY" { - assert.Equal(t, tt.connection.APIKeyRef.Name, container.Env[i].ValueFrom.SecretKeyRef.Name, "API key secret name should match") + assert.Equal(t, tt.connection.APIKeySecretRef.Name, container.Env[i].ValueFrom.SecretKeyRef.Name, "API key secret name should match") assert.Equal(t, "api-key", container.Env[i].ValueFrom.SecretKeyRef.Key, "API key secret key should be 'api-key'") - container.Env[i].Value = "dummy-token.abc.def" } } } // Set environment variables using t.Setenv() to simulate the runtime environment for _, env := range container.Env { + if env.Name == "TEMPORAL_API_KEY" { + // setting a dummy value here since API values are read from an actual secret object in runtime + // moreover, env.Value is nil for TEMPORAL_API_KEY since it used the ValueFrom field (check deployments.go) + t.Setenv(env.Name, "test-api-key-value") + continue + } t.Setenv(env.Name, env.Value) } @@ -855,8 +872,8 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { // Verify that the parsed client options match our expectations assert.Equal(t, tt.connection.HostPort, clientOptions.HostPort, "HostPort should be parsed from TEMPORAL_ADDRESS") assert.Equal(t, tt.namespace, clientOptions.Namespace, "Namespace should be parsed from TEMPORAL_NAMESPACE") - if tt.connection.APIKeyRef != nil { - assert.Equal(t, "dummy-token.abc.def", os.Getenv("TEMPORAL_API_KEY"), "API key should be parsed from TEMPORAL_API_KEY") + if tt.connection.APIKeySecretRef != nil { + assert.Equal(t, "test-api-key-value", os.Getenv("TEMPORAL_API_KEY"), "API key should be parsed from TEMPORAL_API_KEY") } // Verify other client option fields that should have default/empty values @@ -867,7 +884,7 @@ func TestNewDeploymentWithOwnerRef_EnvConfigSDKCompatibility(t *testing.T) { if expectTLS { assert.NotNil(t, clientOptions.ConnectionOptions.TLS, "TLS should be configured for mTLS connection") - } else if tt.connection.APIKeyRef != nil { + } else if tt.connection.APIKeySecretRef != nil { // An empty TLS config is configured when an API key is used without mTLS secrets assert.Equal(t, clientOptions.ConnectionOptions.TLS, &tls.Config{}, "Empty TLS config should be configured for API key connection") } else { diff --git a/internal/tests/internal/deployment_controller.go b/internal/tests/internal/deployment_controller.go index c44b4fe1..b009d12a 100644 --- a/internal/tests/internal/deployment_controller.go +++ b/internal/tests/internal/deployment_controller.go @@ -91,8 +91,8 @@ func applyDeployment(t *testing.T, ctx context.Context, k8sClient client.Client, go testhelpers.RunHelloWorldWorker(ctx, deployment.Spec.Template, workerCallback(i)) } - // wait 10s for all expected workers to be healthy - timedOut := waitTimeout(&wg, 10*time.Second) + // wait 30s for all expected workers to be healthy (under suite load startup can be slower) + timedOut := waitTimeout(&wg, 30*time.Second) if timedOut { t.Fatalf("could not start workers, errors were: %+v", workerErrors) diff --git a/internal/tests/internal/validation_helpers.go b/internal/tests/internal/validation_helpers.go index 76719af8..b76e762f 100644 --- a/internal/tests/internal/validation_helpers.go +++ b/internal/tests/internal/validation_helpers.go @@ -49,7 +49,7 @@ func waitForVersionRegistrationInDeployment( deploymentHandler := ts.GetDefaultClient().WorkerDeploymentClient().GetHandle(version.DeploymentName) - eventually(t, 30*time.Second, time.Second, func() error { + eventually(t, 60*time.Second, time.Second, func() error { resp, err := deploymentHandler.Describe(ctx, sdkclient.WorkerDeploymentDescribeOptions{}) if err != nil { return fmt.Errorf("unable to describe worker deployment %s: %w", version.DeploymentName, err) @@ -77,7 +77,7 @@ func setCurrentVersion( }) } deploymentHandler := ts.GetDefaultClient().WorkerDeploymentClient().GetHandle(workerDeploymentName) - eventually(t, 30*time.Second, time.Second, func() error { + eventually(t, 60*time.Second, time.Second, func() error { _, err := deploymentHandler.SetCurrentVersion(ctx, sdkclient.WorkerDeploymentSetCurrentVersionOptions{ BuildID: buildID, Identity: defaults.ControllerIdentity, diff --git a/skaffold.example.env b/skaffold.example.env index 675ca389..a29be8ed 100644 --- a/skaffold.example.env +++ b/skaffold.example.env @@ -18,8 +18,11 @@ TEMPORAL_ADDRESS=${TEMPORAL_NAMESPACE}.tmprl.cloud:7233 # Temporal mTLS secret name TEMPORAL_MTLS_SECRET_NAME="" -# Temporal API key name +# K8s secret storing the Temporal API key TEMPORAL_API_KEY_SECRET_NAME="" +# Secret key whose corresponding value is the API key +TEMPORAL_API_KEY_SECRET_KEY="" + # Kubernetes context to use for deployment SKAFFOLD_KUBE_CONTEXT=minikube diff --git a/skaffold.yaml b/skaffold.yaml index cd3a6148..64ba7096 100644 --- a/skaffold.yaml +++ b/skaffold.yaml @@ -58,7 +58,8 @@ profiles: temporal.namespace: "{{ .TEMPORAL_NAMESPACE }}" temporal.address: "{{ .TEMPORAL_ADDRESS }}" temporal.mtlsSecretName: "{{ .TEMPORAL_MTLS_SECRET_NAME }}" - temporal.apiKeySecretName: "{{ .TEMPORAL_API_KEY_SECRET_NAME }}" + temporal.apiKey.name: "{{ .TEMPORAL_API_KEY_SECRET_NAME }}" + temporal.apiKey.key: "{{ .TEMPORAL_API_KEY_SECRET_KEY }}" valuesFiles: - internal/demo/helloworld/helm/helloworld/values.yaml deploy: From da04aa617059e35716c962412b3a6ae9262db770 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Fri, 10 Oct 2025 20:21:23 -0400 Subject: [PATCH 14/15] some more cleanup --- api/v1alpha1/temporalconnection_types.go | 2 +- internal/controller/clientpool/clientpool.go | 10 ++++++---- internal/controller/worker_controller.go | 5 +++-- internal/demo/helloworld/helm/helloworld/values.yaml | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/api/v1alpha1/temporalconnection_types.go b/api/v1alpha1/temporalconnection_types.go index b27f7bc8..8f938aec 100644 --- a/api/v1alpha1/temporalconnection_types.go +++ b/api/v1alpha1/temporalconnection_types.go @@ -35,7 +35,7 @@ type TemporalConnectionSpec struct { // +optional MutualTLSSecretRef *SecretReference `json:"mutualTLSSecretRef,omitempty"` - // APIKeyRef is the name of the Secret that contains the API key. The secret must be `type: kubernetes.io/opaque` and exist + // APIKeySecretRef is the name of the Secret that contains the API key. The secret must be `type: kubernetes.io/opaque` and exist // in the same Kubernetes namespace as the TemporalConnection resource. // +optional APIKeySecretRef *corev1.SecretKeySelector `json:"apiKeySecretRef,omitempty"` diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index 03cf2ec2..5344c534 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -9,6 +9,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/pem" + "errors" "fmt" "sync" "time" @@ -26,7 +27,7 @@ type AuthMode string const ( AuthModeTLS AuthMode = "TLS" AuthModeAPIKey AuthMode = "API_KEY" - AuthModeNoCredentials AuthMode = "NO_CREDENTIALS" // no TLS/API keys + AuthModeNoCredentials AuthMode = "NO_CREDENTIALS" // Add more auth modes here as they are supported ) @@ -98,6 +99,7 @@ type NewClientOptions struct { Spec v1alpha1.TemporalConnectionSpec } +//nolint:revive func (cp *ClientPool) fetchClientUsingMTLSSecret(secret corev1.Secret, opts NewClientOptions) (sdkclient.Client, error) { clientOpts := sdkclient.Options{ @@ -115,14 +117,14 @@ func (cp *ClientPool) fetchClientUsingMTLSSecret(secret corev1.Secret, opts NewC // Check if certificate is expired before creating the client exp, err := calculateCertificateExpirationTime(pemCert, 5*time.Minute) if err != nil { - return nil, fmt.Errorf("failed to check certificate expiration: %v", err) + return nil, errors.New("failed to check certificate expiration: " + err.Error()) } expired, err := isCertificateExpired(exp) if err != nil { - return nil, fmt.Errorf("failed to check certificate expiration: %v", err) + return nil, errors.New("failed to check certificate expiration: " + err.Error()) } if expired { - return nil, fmt.Errorf("certificate is expired or is going to expire soon") + return nil, errors.New("certificate is expired or is going to expire soon") } cert, err := tls.X509KeyPair(secret.Data["tls.crt"], secret.Data["tls.key"]) diff --git a/internal/controller/worker_controller.go b/internal/controller/worker_controller.go index fec3f080..8b4b17b3 100644 --- a/internal/controller/worker_controller.go +++ b/internal/controller/worker_controller.go @@ -6,6 +6,7 @@ package controller import ( "context" + "errors" "fmt" "time" @@ -43,7 +44,7 @@ func getAPIKeySecretName(secretRef *corev1.SecretKeySelector) (string, error) { return secretRef.Name, nil } - return "", fmt.Errorf("API key secret name is not set") + return "", errors.New("API key secret name is not set") } func getTLSSecretName(secretRef *temporaliov1alpha1.SecretReference) (string, error) { @@ -51,7 +52,7 @@ func getTLSSecretName(secretRef *temporaliov1alpha1.SecretReference) (string, er return secretRef.Name, nil } - return "", fmt.Errorf("TLS secret name is not set") + return "", errors.New("TLS secret name is not set") } func resolveAuthSecretName(tc *temporaliov1alpha1.TemporalConnection) (clientpool.AuthMode, string, error) { diff --git a/internal/demo/helloworld/helm/helloworld/values.yaml b/internal/demo/helloworld/helm/helloworld/values.yaml index 7143e741..5aa3f047 100644 --- a/internal/demo/helloworld/helm/helloworld/values.yaml +++ b/internal/demo/helloworld/helm/helloworld/values.yaml @@ -3,7 +3,7 @@ image: tag: latest temporal: - namespace: "default" # e.g. default + namespace: "" # e.g. default # Use existing connection (leave empty to create new one) connectionName: "" # e.g. dev-server # Connection details (required if connectionName is empty) From 26f744e2247bc94b574e4147b27ed4d0d2033ed3 Mon Sep 17 00:00:00 2001 From: Shivam Saraf Date: Fri, 10 Oct 2025 20:58:48 -0400 Subject: [PATCH 15/15] better comment --- api/v1alpha1/temporalconnection_types.go | 7 ++- config/rbac/role.yaml | 66 ++++++++++++++++++++ config/webhook/manifests.yaml | 26 ++++++++ internal/controller/clientpool/clientpool.go | 1 - 4 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 config/rbac/role.yaml create mode 100644 config/webhook/manifests.yaml diff --git a/api/v1alpha1/temporalconnection_types.go b/api/v1alpha1/temporalconnection_types.go index 8f938aec..f085b07f 100644 --- a/api/v1alpha1/temporalconnection_types.go +++ b/api/v1alpha1/temporalconnection_types.go @@ -35,8 +35,11 @@ type TemporalConnectionSpec struct { // +optional MutualTLSSecretRef *SecretReference `json:"mutualTLSSecretRef,omitempty"` - // APIKeySecretRef is the name of the Secret that contains the API key. The secret must be `type: kubernetes.io/opaque` and exist - // in the same Kubernetes namespace as the TemporalConnection resource. + // APIKeySecretRef selects the Secret key that contains the API key used for authentication. + // The Secret must be `type: kubernetes.io/opaque` and exist in the same Kubernetes namespace as + // the TemporalConnection resource. This is a corev1.SecretKeySelector and encodes both: + // - LocalObjectReference.Name: the name of the Secret resource + // - Key: the data key within Secret.Data whose value is the API key token // +optional APIKeySecretRef *corev1.SecretKeySelector `json:"apiKeySecretRef,omitempty"` } diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml new file mode 100644 index 00000000..5082552c --- /dev/null +++ b/config/rbac/role.yaml @@ -0,0 +1,66 @@ +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: manager-role +rules: +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - apps + resources: + - deployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - apps + resources: + - deployments/scale + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalconnections + verbs: + - get + - list + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/finalizers + verbs: + - update +- apiGroups: + - temporal.io + resources: + - temporalworkerdeployments/status + verbs: + - get + - patch + - update diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml new file mode 100644 index 00000000..a3edd279 --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,26 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-temporal-io-temporal-io-v1alpha1-temporalworkerdeployment + failurePolicy: Fail + name: mtemporalworker.kb.io + rules: + - apiGroups: + - temporal.io.temporal.io + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - temporalworkers + sideEffects: None diff --git a/internal/controller/clientpool/clientpool.go b/internal/controller/clientpool/clientpool.go index 5344c534..816cf47f 100644 --- a/internal/controller/clientpool/clientpool.go +++ b/internal/controller/clientpool/clientpool.go @@ -99,7 +99,6 @@ type NewClientOptions struct { Spec v1alpha1.TemporalConnectionSpec } -//nolint:revive func (cp *ClientPool) fetchClientUsingMTLSSecret(secret corev1.Secret, opts NewClientOptions) (sdkclient.Client, error) { clientOpts := sdkclient.Options{