Added doc to use oauth for pulsar scaler #1161
Conversation
✅ Deploy Preview for keda ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Thank you for your contribution! 🙏 We will review your PR as soon as possible. While you are waiting, make sure to:
Learn more about:
|
Signed-off-by: Ming Meng <ming.meng@collibra.com>
mingmcb
force-pushed
the
feat-pulsar-oauth-extension
branch
from
e3d4f0a to
2bf1ceb
Compare
content/docs/2.10/scalers/pulsar.md
Outdated
| - `oauthTokenURI` - Token endpoint for your OAuth provider | ||
| - `grantType` - only `client_credentials` is supported | ||
| - `scopes` - space delimited oauth scopes(Optional). | ||
| - `clientID` - clientID from your OAuth provider. It will be ignored if clientID is also provided from `authenticationRef` |
There was a problem hiding this comment.
I think we should move this to TriggerAuthentication. We tend to not put credentials related stuff into trigger config.
@tomkerkhove @JorTurFer WDYT
There was a problem hiding this comment.
You can check Kafka scaler: https://keda.sh/docs/2.10/scalers/apache-kafka/#authentication-parameters
There was a problem hiding this comment.
I don't think that clientID and oauthTokenURI are secrets, I mean, AFAIK, only the clientSecret is a sensitive data, clientId usually is part of the query string
There was a problem hiding this comment.
but we can move it into TriggerAuthentication as it's related with the credentials, but they aren't sensitive data
There was a problem hiding this comment.
yeah, this is what I mean. It logically belongs there imho. Kafka is done this way also.
There was a problem hiding this comment.
The clientID is already part of TirggerAuthentication. see the code example: https://github.com/kedacore/keda-docs/pull/1161/files#diff-36e3935f303a0a808c6ee2bc08e18d62f54d555079a822c101882f3aec0198a6R268-R280
The idea is to have the additional clientID in the metadata is mainly for ease of use. with OAuth RFC8705, it is possible to request the access token without the secret. In this case, developer can just have everything in the metadata without secret. see example for this case: https://github.com/kedacore/keda-docs/pull/1161/files#diff-36e3935f303a0a808c6ee2bc08e18d62f54d555079a822c101882f3aec0198a6R308-R328
There was a problem hiding this comment.
Okay make sense. Could you please then add also the scopes and oauthTokenURI to TriggerAuth as well? The same way clientID is done.
content/docs/2.10/scalers/pulsar.md
Outdated
| - `authModes` - a comma separated list of authentication modes to use. (Values: `bearer`, `tls`,`basic`, Default: `""`, Optional, `tls,bearer` or `tls,basic` are valid combinations and would indicate mutual TLS to secure the connection and then `bearer` or `basic` headers should be added to the HTTP request) | ||
| - `authModes` - a comma separated list of authentication modes to use. (Values: `bearer`, `tls`, `basic`, `oauth`, Default: `""`, Optional, `tls,bearer` or `tls,basic` are valid combinations and would indicate mutual TLS to secure the connection and then `bearer` or `basic` headers should be added to the HTTP request) | ||
| - `oauthTokenURI` - Token endpoint for your OAuth provider | ||
| - `grantType` - only `client_credentials` is supported |
There was a problem hiding this comment.
if client_credentials is the only supported type, then we can make it a default one, right? And we don't have make it configurable and add a property for it in here, am I correct in my assumption? If we add another type in the future, we can expose this config then.
There was a problem hiding this comment.
yes, I can make it as default if works better.
There was a problem hiding this comment.
yeah and remove the property from the config pls
mingmcb
force-pushed
the
feat-pulsar-oauth-extension
branch
from
9e2d4a4 to
b37416c
Compare
Signed-off-by: Ming Meng <ming.meng@collibra.com>
mingmcb
force-pushed
the
feat-pulsar-oauth-extension
branch
from
b37416c to
4b49f3d
Compare
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com> Signed-off-by: Ming Meng <101287520+mingmcb@users.noreply.github.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com> Signed-off-by: Ming Meng <101287520+mingmcb@users.noreply.github.com>
Signed-off-by: Ming Meng <ming.meng@collibra.com>
mingmcb
force-pushed
the
feat-pulsar-oauth-extension
branch
from
b23b3d1 to
7572aa4
Compare
Added doc to use oauth for pulsar scaler
Checklist
Related to kedacore/keda#4709