Skip to content

Refactor and fix metrics-api scaler#6940

Merged
zroubalik merged 12 commits intokedacore:mainfrom
SpiritZhou:spiritzhou/refactormetricapi
Sep 18, 2025
Merged

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

Conversation

@SpiritZhou
Copy link
Contributor

Refactor and fix metrics-api scaler

Checklist

Fixes #

Relates to # #6939

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
@SpiritZhou 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
Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>
@wozniakjan
Copy link
Member

wozniakjan commented Aug 8, 2025

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

Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

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

lgtm

Comment on lines +43 to +66
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"`
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

@JorTurFer JorTurFer Aug 29, 2025

Choose a reason for hiding this comment

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

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

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.

What is the status here please?

Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>
@SpiritZhou SpiritZhou requested a review from a team as a code owner August 29, 2025 06:28
@keda-automation keda-automation requested a review from a team August 29, 2025 06:28
Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Comment on lines +43 to +66
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"`
Copy link
Member

@JorTurFer JorTurFer Aug 29, 2025

Choose a reason for hiding this comment

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

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

zhenghanzhou added 2 commits August 29, 2025 16:27
Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
@keda-automation keda-automation requested a review from a team August 29, 2025 08:28
Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
@SpiritZhou
Copy link
Contributor Author

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 <zhenghanzhou@microsoft.com>
@JorTurFer
Copy link
Member

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
Copy link
Contributor Author

SpiritZhou commented Sep 2, 2025

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

@rickbrouwer rickbrouwer added the ok-to-merge This PR can be merged label Sep 18, 2025
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
@keda-automation keda-automation requested a review from a team September 18, 2025 18:51
@zroubalik zroubalik merged commit 29a6d60 into kedacore:main Sep 18, 2025
16 of 23 checks passed
@keda-automation keda-automation requested a review from a team September 18, 2025 18:51
jmickey pushed a commit to jmickey/keda that referenced this pull request Sep 30, 2025
* one commit on spiritzhou/refactormetricapi

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update pkg/scalers/metrics_api_scaler.go

Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

---------

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
Co-authored-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
alt-dima pushed a commit to alt-dima/keda that referenced this pull request Dec 13, 2025
* one commit on spiritzhou/refactormetricapi

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update pkg/scalers/metrics_api_scaler.go

Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

---------

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
Co-authored-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Signed-off-by: Dmitriy Altuhov <altuhovd@gmail.com>
tangobango5 pushed a commit to tangobango5/keda that referenced this pull request Dec 22, 2025
* one commit on spiritzhou/refactormetricapi

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update pkg/scalers/metrics_api_scaler.go

Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

---------

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
Co-authored-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
tangobango5 pushed a commit to tangobango5/keda that referenced this pull request Feb 13, 2026
* one commit on spiritzhou/refactormetricapi

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update pkg/scalers/metrics_api_scaler.go

Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

* Update

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>

---------

Signed-off-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Signed-off-by: Zhenghan Zhou <iammrzhouzhenghan@gmail.com>
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
Co-authored-by: zhenghanzhou <zhenghanzhou@microsoft.com>
Co-authored-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-merge This PR can be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants