Skip to content

Fix Temporal Scaler TLS RootCAs initialization for API Key authentication#7367

Merged
JorTurFer merged 4 commits intokedacore:mainfrom
dttung2905:use-proper-tls-config-temporal
Jan 13, 2026
Merged

Fix Temporal Scaler TLS RootCAs initialization for API Key authentication#7367
JorTurFer merged 4 commits intokedacore:mainfrom
dttung2905:use-proper-tls-config-temporal

Conversation

@dttung2905
Copy link
Contributor

@dttung2905 dttung2905 commented Jan 4, 2026 •
edited
Loading

Problem

When using the Temporal scaler with API key authentication to connect to Temporal Cloud, users encounter the following error:

error: "failed to create Temporal client connection: failed reaching server: connection error: desc = \"error reading server preface: remote error: tls: certificate required\""

Root Cause

In getTemporalClient(), when API key authentication is used, the code creates a minimal TLS configuration with only MinVersion set:

if meta.APIKey != "" {
    options.Credentials = sdk.NewAPIKeyStaticCredentials(meta.APIKey)
    tlsConfig = &tls.Config{
        MinVersion: kedautil.GetMinTLSVersion(),
        // RootCAs is nil - not initialized!
    }
}

The RootCAs field is left as nil. While Go's TLS library should use system certificates when RootCAs is nil, in practice this can fail in certain environments (e.g., containers where system cert pool may not be available) or when the TLS config is passed through gRPC's credential layer. Additionally, explicitly initializing RootCAs ensures consistent behavior across all environments and allows for custom CA certificates to be loaded from /custom/ca if needed.

Solution

Replace the manual TLS config creation with kedautil.CreateTLSClientConfig(), which properly initializes RootCAs via getRootCAs(). This function:

  • Loads system certificates via x509.SystemCertPool()
  • Includes custom CAs from /custom/ca directory if present
  • Ensures proper certificate verification

Checklist

Fixes #7343

@dttung2905 dttung2905 requested a review from a team as a code owner January 4, 2026 21:44
@github-actions
Copy link

github-actions bot commented Jan 4, 2026

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

  • Add an entry in our changelog in alphabetical order and link related issue
  • Update the documentation, if needed
  • Add unit & e2e tests for your changes
  • GitHub checks are passing
  • Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

@keda-automation keda-automation requested a review from a team January 4, 2026 21:44
@snyk-io
Copy link

snyk-io bot commented Jan 4, 2026 •
edited
Loading

✅ Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@rickbrouwer
Copy link
Member

rickbrouwer commented Jan 5, 2026 •
edited by github-actions bot
Loading

/run-e2e temporal
Update: You can check the progress here

@wozniakjan
Copy link
Member

wozniakjan commented Jan 5, 2026 •
edited by github-actions bot
Loading

/run-e2e temporal
Update: You can check the progress here

@wozniakjan
Copy link
Member

wozniakjan commented Jan 5, 2026

on a surface this looks good but the e2e tests are now failing, I was able to trace it to this message

failed to create Temporal client connection: failed reaching server: context deadline exceeded while waiting for connections to become ready

full error log line:

***0***6-0***-05T09:***7:***Z	ERROR	Failed to create new HPA resource	***"controller": "scaledobject", "controllerGroup": "keda.sh", "controllerKind": "ScaledObject", "ScaledObject": ***"name":"temporal-test-so","namespace":"temporal-test-ns"***, "namespace": "temporal-test-ns", "name": "temporal-test-so", "reconcileID": "a70a0c***c-7***ef-4e***6-a***6f-***a***9***9bd***6", "HPA.Namespace": "temporal-test-ns", "HPA.Name": "keda-hpa-temporal-test-so", "error": "failed to create Temporal client connection: failed reaching server: context deadline exceeded while waiting for connections to become ready"***

@wozniakjan wozniakjan requested a review from Copilot January 5, 2026 11:19
Copilot AI reviewed Jan 5, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a TLS certificate verification issue in the Temporal scaler when using API key authentication to connect to Temporal Cloud. The fix ensures that the TLS configuration properly initializes the RootCAs certificate pool by using the existing utility function instead of manually creating a minimal TLS config.

Key Changes:

  • Replaced manual TLS config creation with kedautil.CreateTLSClientConfig() for API key authentication path
  • This ensures RootCAs are properly initialized with system certificates and custom CA certificates from /custom/ca if present

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/scalers/temporal_scaler.go Fixed TLS RootCAs initialization when using API key authentication by replacing manual config creation with the utility function
CHANGELOG.md Added entry documenting the bug fix for Temporal scaler TLS configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dttung2905
Copy link
Contributor Author

dttung2905 commented Jan 5, 2026

@wozniakjan I see the failed e2e tests, but I don't think the changes in the PR cause the e2e failure because in the previous PR , this e2e tests never pass https://github.com/kedacore/keda/commits/main/pkg/scalers/temporal_scaler.go

However, looking at the log, I think there might be a race condition where the ScaleObject was applied first before the Deployment though. Let me try to decouple the kubectl apply

@keda-automation keda-automation requested a review from a team January 5, 2026 17:15
@dttung2905
Copy link
Contributor Author

dttung2905 commented Jan 5, 2026 •
edited by github-actions bot
Loading

/run-e2e temporal
Update: You can check the progress here

@dttung2905
Copy link
Contributor Author

dttung2905 commented Jan 7, 2026

The e2e test passes but Remove kubernetes step had a transient error. Let me run the e2e again
image

@dttung2905
Copy link
Contributor Author

dttung2905 commented Jan 7, 2026 •
edited by github-actions bot
Loading

/run-e2e temporal
Update: You can check the progress here

@JorTurFer
Copy link
Member

JorTurFer commented Jan 11, 2026

I think that temporal test is failing because of temporalio/omes#280

@JorTurFer JorTurFer mentioned this pull request Jan 11, 2026
Closed
44 tasks
@dttung2905 @wozniakjan
Fix how tls config is initialised for temporal scaler
92d8bce 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905 @wozniakjan
Add changelog
783721e 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905 @wozniakjan
Attempt to fix failing e2e test
f2a0e75 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905 @wozniakjan
Fix CI lint error
92c81b2 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@wozniakjan wozniakjan force-pushed the use-proper-tls-config-temporal branch from a56694e to 92c81b2 Compare January 13, 2026 16:25
@wozniakjan
Copy link
Member

wozniakjan commented Jan 13, 2026 •
edited by github-actions bot
Loading

/run-e2e temporal
Update: You can check the progress here

@JorTurFer JorTurFer merged commit a73d662 into kedacore:main Jan 13, 2026
24 checks passed
tangobango5 pushed a commit to tangobango5/keda that referenced this pull request Feb 13, 2026
@dttung2905 @tangobango5
Fix Temporal Scaler TLS RootCAs initialization for API Key authentica…
5d18543 
…tion (kedacore#7367)

* Fix how tls config is initialised for temporal scaler

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>

* Add changelog

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>

* Attempt to fix failing e2e test

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>

* Fix CI lint error

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>

---------

Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@wozniakjan wozniakjan wozniakjan approved these changes

@JorTurFer JorTurFer JorTurFer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Keda 2.18.2 Temporal ScaledObject facing error reading server preface: remote error: tls: certificate required

5 participants

@dttung2905 @rickbrouwer @wozniakjan @JorTurFer