fix: temporal scaler was not setting up tls for apikey auth#6781
Merged
wozniakjan merged 3 commits intokedacore:mainfrom May 16, 2025
Merged
fix: temporal scaler was not setting up tls for apikey auth#6781wozniakjan merged 3 commits intokedacore:mainfrom
wozniakjan merged 3 commits intokedacore:mainfrom
Conversation
d7c626b to
32f7687
Compare
Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com>
32f7687 to
9890a6d
Compare
Contributor
Author
|
cc @wozniakjan if we can get it into #6724 it would be great. |
JorTurFer
approved these changes
May 15, 2025
Member
|
/run-e2e temporal |
Member
unfortunately 2.17.1 is now already released, we will see if there is enough to release 2.17.2 or if this will have to wait for 2.18.0 |
wozniakjan
reviewed
May 16, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR ensures TLS is correctly configured for API Key authentication in the Temporal scaler by deferring and centralizing the client’s ConnectionOptions assignment after both API Key and certificate branches.
- Introduce
tlsConfigvariable and set default TLS for API Key auth - Override
tlsConfigwhen user-provided cert/key are present - Move
options.ConnectionOptionsassignment to the end to prevent overwriting TLS settings - Update CHANGELOG with a note on the secondary fix
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/scalers/temporal.go | Refactored TLS setup logic to preserve TLS config across branches and centralized ConnectionOptions assignment |
| CHANGELOG.md | Added entry for the secondary Temporal Scaler fix |
Comments suppressed due to low confidence (1)
pkg/scalers/temporal.go:216
- [nitpick] The interceptor function signature is split across lines and repeats parameter types on separate lines. Consider merging
reqandreplyparameters and formatting the entire signature on one line for improved readability and consistency.
func(ctx context.Context, method string, req any, reply any,
Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com>
auto-merge was automatically disabled
May 16, 2025 09:27
Head branch was pushed to by a user without write access
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
Member
|
/run-e2e temporal |
wozniakjan
approved these changes
May 16, 2025
|
Will this be a part of v2.17.2 release? |
|
Also ended up here with the same issue. Would appreciate this in a v2.17.2 but using |
wozniakjan
added a commit
to wozniakjan/keda
that referenced
this pull request
Jun 17, 2025
…#6781) * fix: temporal scaler was not setting up tls for apikey auth Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> * Update CHANGELOG.md Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> --------- Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com>
wozniakjan
added a commit
to wozniakjan/keda
that referenced
this pull request
Jun 17, 2025
…#6781) * fix: temporal scaler was not setting up tls for apikey auth Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> * Update CHANGELOG.md Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> --------- Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
wozniakjan
added a commit
that referenced
this pull request
Jun 18, 2025
* feat: grpc mTLS certificates are hot reloaded (#6756) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * fix: temporal scaler was not setting up tls for apikey auth (#6781) * fix: temporal scaler was not setting up tls for apikey auth Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> * Update CHANGELOG.md Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> --------- Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> * chore: changelog and issue template v2.17.2 Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> --------- Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: João Bastos <joaopaulosr95@gmail.com>
|
has it been fixed in am i missing something? |
Member
Yes, that should be fixed in 2.17.2. The message you show is also a bit different from the original issue. Could you share your scaledObject? Maybe it would be good to create a separate issue for that and share it there. |
dpochopsky
pushed a commit
to dpochopsky/keda
that referenced
this pull request
Sep 12, 2025
…#6781) * fix: temporal scaler was not setting up tls for apikey auth Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> * Update CHANGELOG.md Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> --------- Signed-off-by: joaopaulosr95 <joaopaulosr95@gmail.com> Signed-off-by: João Bastos <joaopaulosr95@gmail.com> Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com> Co-authored-by: Jan Wozniak <wozniak.jan@gmail.com> Signed-off-by: David Pochopsky <david.pochopsky@united.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After seeing
connection reset by peererrors in my Temporal KEDA scaler, I started digging and found out about #6724, which was released earlier today. I deployed the new v2.17.1 Helm chart to my K8S cluster and still kept seeing the same errors, then decided to dig into the scaler code to see what I could find.@rickbrouwer and I connected to explore options, and we realized that the TLS settings from #6707 were being overwritten, thus causing the error previously mentioned. This change fixed the main issue for good by rearranging the *tls.config variable, and it is already validated against a production K8S cluster.
Checklist
Fixes #
Relates to #