Skip to content

fix(azure-data-explorer): remove clientSecretFromEnv support#7554

Open
rickbrouwer wants to merge 3 commits intokedacore:mainfrom
rickbrouwer:azuredataexplorer
Open

fix(azure-data-explorer): remove clientSecretFromEnv support#7554
rickbrouwer wants to merge 3 commits intokedacore:mainfrom
rickbrouwer:azuredataexplorer

Conversation

@rickbrouwer
Copy link
Member

@rickbrouwer rickbrouwer commented Mar 17, 2026 •
edited
Loading

Refactor the Azure Data Explorer, the last one.

Further, the clientSecretFromEnv was storing the environment variable name as the secret value instead of resolving it via ResolvedEnv. This went unnoticed because the existing tests used the actual secret value as the env var name.

What is worth considering now, since clientSecretFromEnv has effectively never worked in production, this raises the question of whether we should keep it at all. Given the general preference to move away from FromEnv parameters, we could remove it entirely as part of this change. If there is a reason to keep it, please indicate so.
If we want to remove this, then in that case the documentation will likely need to be updated as well.

Checklist

  • Tests have been added (if applicable)
  • Ensure make generate-scalers-schema has been run to update any outdated generated files
  • Changelog has been updated and is aligned with our changelog requirements, only when the change impacts end users
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #5797

@rickbrouwer
fix(azure-data-explorer): resolve clientSecret from env instead of us…
531b37d 
…ing env var name

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
@rickbrouwer rickbrouwer requested a review from a team as a code owner March 17, 2026 16:23
@github-actions
Copy link

github-actions bot commented Mar 17, 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 March 17, 2026 16:24
@rickbrouwer rickbrouwer mentioned this pull request Mar 17, 2026
Open
58 tasks
@snyk-io
Copy link

snyk-io bot commented Mar 17, 2026 •
edited
Loading

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

Status Scan Engine 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 Author

rickbrouwer commented Mar 17, 2026 •
edited by github-actions bot
Loading

/run-e2e azure-data-explorer*
Update: You can check the progress here

@rickbrouwer
Copy link
Member Author

rickbrouwer commented Mar 17, 2026 •
edited by github-actions bot
Loading

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

dttung2905
dttung2905 reviewed Mar 17, 2026
View reviewed changes
Copy link
Contributor

@dttung2905 dttung2905 left a comment

Choose a reason for hiding this comment

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

Thanks so much for the PR. I did a quick search on clientSecretFromEnv and there are only two occurences which are all in azure data explorer. @JorTurFer is it considered breaking changes and need 2 version announcement beforehand ?

@rickbrouwer
Merge branch 'main' into azuredataexplorer
254525d 
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
@keda-automation keda-automation requested a review from a team March 20, 2026 07:15
zroubalik
zroubalik approved these changes Mar 20, 2026
View reviewed changes
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

+1 for the removal

@keda-automation keda-automation requested a review from a team March 21, 2026 17:06
@rickbrouwer rickbrouwer changed the title fix(azure-data-explorer): resolve clientSecret from env instead of using env var name fix(azure-data-explorer): remove clientSecret from Env Mar 21, 2026
@rickbrouwer rickbrouwer changed the title fix(azure-data-explorer): remove clientSecret from Env fix(azure-data-explorer): remove clientSecretFromEnv support Mar 21, 2026
@rickbrouwer
Copy link
Member Author

rickbrouwer commented Mar 21, 2026

+1 for the removal

Me too. Since it never worked, I have removed that part.

@rickbrouwer
Copy link
Member Author

rickbrouwer commented Mar 21, 2026 •
edited by github-actions bot
Loading

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

@rickbrouwer rickbrouwer added the Awaiting/2nd-approval This PR needs one more approval review label Mar 23, 2026
@rickbrouwer
update after feedback
dd1168d 
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
@rickbrouwer
Copy link
Member Author

rickbrouwer commented Mar 24, 2026 •
edited by github-actions bot
Loading

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zroubalik zroubalik zroubalik approved these changes

+1 more reviewer

@dttung2905 dttung2905 dttung2905 left review comments

Reviewers whose approvals may not affect merge requirements

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

Awaiting/2nd-approval This PR needs one more approval review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor Scalers with new declarative scaler config

3 participants

@rickbrouwer @zroubalik @dttung2905