Refactor and fix metrics-api scaler by SpiritZhou · Pull Request #6940 · kedacore/keda

SpiritZhou · 2025-08-01T08:16:52Z

Refactor and fix metrics-api scaler

Checklist

When introducing a new scaler, I agree with the scaling governance policy
I have verified that my change is according to the deprecations & breaking changes policy
Tests have been added
Changelog has been updated and is aligned with our changelog requirements
Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #

Relates to # #6939

Signed-off-by: zhenghanzhou <[email protected]>

Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]>

wozniakjan · 2025-08-08T09:26:53Z

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

wozniakjan

lgtm

wozniakjan · 2025-08-08T09:30:25Z

+	AuthMode string `keda:"name=authMode,order=triggerMetadata;authParams,optional"`

 	// apiKeyAuth
 	enableAPIKeyAuth bool
-	method           string // way of providing auth key, either "header" (default) or "query"
+	Method           string `keda:"name=method,order=triggerMetadata;authParams,default=header,enum=header;query"`
 	// keyParamName  is either header key or query param used for passing apikey
 	// default header is "X-API-KEY", default query param is "api_key"
-	keyParamName string
-	apiKey       string
+	KeyParamName string `keda:"name=keyParamName,order=triggerMetadata;authParams,optional"`
+	APIKey       string `keda:"name=apiKey,order=authParams,optional"`

 	// base auth
 	enableBaseAuth bool
-	username       string
-	password       string // +optional
+	Username       string `keda:"name=username,order=authParams,optional"`
+	Password       string `keda:"name=password,order=authParams,optional"` // +optional

 	// client certification
 	enableTLS bool
-	cert      string
-	key       string
-	ca        string
+	Cert      string `keda:"name=cert,order=authParams,optional"`
+	Key       string `keda:"name=key,order=authParams,optional"`
+	CA        string `keda:"name=ca,order=authParams,optional"`

 	// bearer
 	enableBearerAuth bool
-	bearerToken      string
+	BearerToken      string `keda:"name=token,order=authParams,optional"`


curious if this can be further simplified by using authentication.Config like in #6969? So far each scaler had its own quirky behavior and expectations regarding auth params so if it's too problematic, we can merge as is and work on further improvements later

Is it too much work to change this as part of this PR? probably it's a task that we will forget xD
I approve it just in case, but I'd prefer to change it as part of this PR

zroubalik

What is the status here please?

Signed-off-by: Zhenghan Zhou <[email protected]>

Signed-off-by: zhenghanzhou <[email protected]>

JorTurFer · 2025-08-29T06:57:46Z

+	AuthMode string `keda:"name=authMode,order=triggerMetadata;authParams,optional"`

 	// apiKeyAuth
 	enableAPIKeyAuth bool
-	method           string // way of providing auth key, either "header" (default) or "query"
+	Method           string `keda:"name=method,order=triggerMetadata;authParams,default=header,enum=header;query"`
 	// keyParamName  is either header key or query param used for passing apikey
 	// default header is "X-API-KEY", default query param is "api_key"
-	keyParamName string
-	apiKey       string
+	KeyParamName string `keda:"name=keyParamName,order=triggerMetadata;authParams,optional"`
+	APIKey       string `keda:"name=apiKey,order=authParams,optional"`

 	// base auth
 	enableBaseAuth bool
-	username       string
-	password       string // +optional
+	Username       string `keda:"name=username,order=authParams,optional"`
+	Password       string `keda:"name=password,order=authParams,optional"` // +optional

 	// client certification
 	enableTLS bool
-	cert      string
-	key       string
-	ca        string
+	Cert      string `keda:"name=cert,order=authParams,optional"`
+	Key       string `keda:"name=key,order=authParams,optional"`
+	CA        string `keda:"name=ca,order=authParams,optional"`

 	// bearer
 	enableBearerAuth bool
-	bearerToken      string
+	BearerToken      string `keda:"name=token,order=authParams,optional"`


Is it too much work to change this as part of this PR? probably it's a task that we will forget xD
I approve it just in case, but I'd prefer to change it as part of this PR

Signed-off-by: zhenghanzhou <[email protected]>

SpiritZhou · 2025-08-29T08:47:26Z

I refactored the auth logic, but I'm considering how to make it more generic. I found that a new type of authentication needs to be added, but its parameters are not generic. Currently, the auth parameters—APIKey, Method, and KeyParamName—are only suitable for metrics APIs, such as the current CustomAuth used for the Prometheus scaler. This means that if other scalers need to refactor their authentication, the number of auth parameters will continue to increase. Do you have any suggestions? Or should we just keep adding new parameters for now, since not many scalers have their own authentication yet? @wozniakjan @JorTurFer

Signed-off-by: zhenghanzhou <[email protected]>

JorTurFer · 2025-09-01T06:38:17Z

I refactored the auth logic, but I'm considering how to make it more generic. I found that a new type of authentication needs to be added, but its parameters are not generic. Currently, the auth parameters—APIKey, Method, and KeyParamName—are only suitable for metrics APIs, such as the current CustomAuth used for the Prometheus scaler. This means that if other scalers need to refactor their authentication, the number of auth parameters will continue to increase. Do you have any suggestions? Or should we just keep adding new parameters for now, since not many scalers have their own authentication yet? @wozniakjan @JorTurFer

I'd move the scaler parameters to the generic configuration. It's true that we must avoid breaking changes, so maybe we have to deprecate the old values and move them to a generic configuration and we can use for any scaler. I agree with generalize and making things common for all the scaler

SpiritZhou · 2025-09-02T07:20:29Z

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

Signed-off-by: Zbynek Roubalik <[email protected]>

* one commit on spiritzhou/refactormetricapi Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update pkg/scalers/metrics_api_scaler.go Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> --------- Signed-off-by: zhenghanzhou <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]> Signed-off-by: Zbynek Roubalik <[email protected]> Co-authored-by: zhenghanzhou <[email protected]> Co-authored-by: Rick Brouwer <[email protected]> Co-authored-by: Zbynek Roubalik <[email protected]>

* one commit on spiritzhou/refactormetricapi Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update pkg/scalers/metrics_api_scaler.go Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> --------- Signed-off-by: zhenghanzhou <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]> Signed-off-by: Zbynek Roubalik <[email protected]> Co-authored-by: zhenghanzhou <[email protected]> Co-authored-by: Rick Brouwer <[email protected]> Co-authored-by: Zbynek Roubalik <[email protected]> Signed-off-by: Dmitriy Altuhov <[email protected]>

* one commit on spiritzhou/refactormetricapi Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update pkg/scalers/metrics_api_scaler.go Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> * Update Signed-off-by: zhenghanzhou <[email protected]> --------- Signed-off-by: zhenghanzhou <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]> Signed-off-by: Zbynek Roubalik <[email protected]> Co-authored-by: zhenghanzhou <[email protected]> Co-authored-by: Rick Brouwer <[email protected]> Co-authored-by: Zbynek Roubalik <[email protected]>

one commit on spiritzhou/refactormetricapi

1dc6321

Signed-off-by: zhenghanzhou <[email protected]>

SpiritZhou force-pushed the spiritzhou/refactormetricapi branch from 9484e89 to 1dc6321 Compare August 1, 2025 08:19

zhenghanzhou added 3 commits August 1, 2025 16:33

Update

f920812

Signed-off-by: zhenghanzhou <[email protected]>

Update

981858c

Signed-off-by: zhenghanzhou <[email protected]>

Update

e5c98f7

Signed-off-by: zhenghanzhou <[email protected]>

rickbrouwer reviewed Aug 1, 2025

View reviewed changes

Comment thread pkg/scalers/metrics_api_scaler.go Outdated

Update pkg/scalers/metrics_api_scaler.go

605cc5d

Co-authored-by: Rick Brouwer <[email protected]> Signed-off-by: Zhenghan Zhou <[email protected]>

wozniakjan approved these changes Aug 8, 2025

View reviewed changes

rickbrouwer requested changes Aug 8, 2025

View reviewed changes

Comment thread CHANGELOG.md Outdated

zroubalik reviewed Aug 26, 2025

View reviewed changes

Merge branch 'main' into spiritzhou/refactormetricapi

fae3fe1

Signed-off-by: Zhenghan Zhou <[email protected]>

SpiritZhou requested a review from a team as a code owner August 29, 2025 06:28

keda-automation requested a review from a team August 29, 2025 06:28

Update

4735fd8

Signed-off-by: zhenghanzhou <[email protected]>

JorTurFer approved these changes Aug 29, 2025

View reviewed changes

zhenghanzhou added 2 commits August 29, 2025 16:27

Update

87c8ae3

Signed-off-by: zhenghanzhou <[email protected]>

Update

2ee037a

Signed-off-by: zhenghanzhou <[email protected]>

keda-automation requested a review from a team August 29, 2025 08:28

Update

96c69e4

Signed-off-by: zhenghanzhou <[email protected]>

Update

fba031d

Signed-off-by: zhenghanzhou <[email protected]>

JorTurFer approved these changes Sep 1, 2025

View reviewed changes

zroubalik requested a review from rickbrouwer September 18, 2025 10:00

rickbrouwer approved these changes Sep 18, 2025

View reviewed changes

rickbrouwer added the ok-to-merge This PR can be merged label Sep 18, 2025

Merge branch 'main' into spiritzhou/refactormetricapi

b0b3982

Signed-off-by: Zbynek Roubalik <[email protected]>

keda-automation requested a review from a team September 18, 2025 18:51

zroubalik merged commit 29a6d60 into kedacore:main Sep 18, 2025
16 of 23 checks passed

keda-automation requested a review from a team September 18, 2025 18:51

dttung2905 mentioned this pull request Oct 16, 2025

Refactor Scalers with new declarative scaler config #5797

Closed

58 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor and fix metrics-api scaler#6940

Refactor and fix metrics-api scaler#6940
zroubalik merged 12 commits into
kedacore:mainfrom
SpiritZhou:spiritzhou/refactormetricapi

SpiritZhou commented Aug 1, 2025

Uh oh!

Uh oh!

wozniakjan commented Aug 8, 2025 •

edited by github-actions Bot

Loading

Uh oh!

wozniakjan left a comment

Uh oh!

wozniakjan Aug 8, 2025

Uh oh!

JorTurFer Aug 29, 2025 •

edited

Loading

Uh oh!

Uh oh!

zroubalik left a comment

Uh oh!

JorTurFer Aug 29, 2025 •

edited

Loading

Uh oh!

SpiritZhou commented Aug 29, 2025

Uh oh!

JorTurFer commented Sep 1, 2025

Uh oh!

SpiritZhou commented Sep 2, 2025 •

edited by github-actions Bot

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

SpiritZhou commented Aug 1, 2025

Checklist

Uh oh!

Uh oh!

wozniakjan commented Aug 8, 2025 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wozniakjan left a comment

Choose a reason for hiding this comment

Uh oh!

wozniakjan Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

JorTurFer Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zroubalik left a comment

Choose a reason for hiding this comment

Uh oh!

JorTurFer Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SpiritZhou commented Aug 29, 2025

Uh oh!

JorTurFer commented Sep 1, 2025

Uh oh!

SpiritZhou commented Sep 2, 2025 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wozniakjan commented Aug 8, 2025 •

edited by github-actions Bot

Loading

JorTurFer Aug 29, 2025 •

edited

Loading

JorTurFer Aug 29, 2025 •

edited

Loading

SpiritZhou commented Sep 2, 2025 •

edited by github-actions Bot

Loading