temporal: always set temporal-namespace header on requests (#7079)#7080
temporal: always set temporal-namespace header on requests (#7079)#7080zroubalik merged 4 commits intokedacore:mainfrom
Conversation
|
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:
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. |
…7079) Signed-off-by: Robert Cao <robert.cao@relativity.com>
Signed-off-by: Robert Cao <robert.cao@relativity.com>
|
/run-e2e temporal |
|
/run-e2e temporal |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the Temporal scaler to always include the temporal-namespace header on requests to the Temporal server, regardless of whether an API key is present. Previously, this header was only set when the scaler metadata contained an API key, which caused issues with the initial GetSystemInfo request.
- Moved the gRPC unary interceptor outside of the API key conditional block
- Ensures namespace header is set on all requests, not just authenticated ones
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 | Relocated the gRPC interceptor code to always execute, ensuring temporal-namespace header is set on all requests |
| CHANGELOG.md | Added entry documenting the fix for the temporal-namespace header issue |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
zroubalik
left a comment
There was a problem hiding this comment.
@robholland @Prajithp @jhecking I would love to hear your thoughts here
|
As far as I can tell, the end to end Temporal test failures are also present in the nightly tests on the main branch (https://github.com/kedacore/keda/actions/runs/17814119370). I can certainly look into them here if needed. It does look like potentially based on the failure logs, that the worker deployment may not actually be picking up the workflows, as the deployment does not scale down back to 0. I'm going to try to find some time to debug them to see if anything obvious stands out. |
|
Makes sense to me 👍 |
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
…7079) (kedacore#7080) * temporal: always set temporal-namespace header on requests (kedacore#7079) Signed-off-by: Robert Cao <robert.cao@relativity.com> * changelog Signed-off-by: Robert Cao <robert.cao@relativity.com> * fix: reorder changelog Signed-off-by: Robert Cao <robert.cao@relativity.com> --------- Signed-off-by: Robert Cao <robert.cao@relativity.com> Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com> Signed-off-by: Dmitriy Altuhov <altuhovd@gmail.com>
…7079) (kedacore#7080) * temporal: always set temporal-namespace header on requests (kedacore#7079) Signed-off-by: Robert Cao <robert.cao@relativity.com> * changelog Signed-off-by: Robert Cao <robert.cao@relativity.com> * fix: reorder changelog Signed-off-by: Robert Cao <robert.cao@relativity.com> --------- Signed-off-by: Robert Cao <robert.cao@relativity.com> Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
…7079) (kedacore#7080) * temporal: always set temporal-namespace header on requests (kedacore#7079) Signed-off-by: Robert Cao <robert.cao@relativity.com> * changelog Signed-off-by: Robert Cao <robert.cao@relativity.com> * fix: reorder changelog Signed-off-by: Robert Cao <robert.cao@relativity.com> --------- Signed-off-by: Robert Cao <robert.cao@relativity.com> Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com> Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
The temporal scaler now always attaches the
temporal-namespaceheader on requests to the temporal server. Previously, this header was only set (at the keda scaler) level on requests when the scaler metadata contained an api key.Note that namespace specific gRPC requests already contained the
temporal-namespaceheader on every request, but the initialGetSystemInforequest does not, because it is not a namespace specific request: temporalio/sdk-go#1458Checklist
Fixes #7079
I don't believe that any documentation needs to be updated, since there is no mention of the
temporal-namespaceheader on the current scaler documentation, but please correct me if I should add to the documentation.For potential tests, I could do two things:
getTemporalClientDialOptionsthat returns all of the dialOptions, updategetTemporalClientto call that method, and then assert that the interceptor is always added