Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support OAuth for pulsar scaler #4709

Merged
merged 12 commits into from
Sep 17, 2023

Conversation

mingmcb
Copy link
Contributor

@mingmcb mingmcb commented Jun 19, 2023

support OAuth for pulsar scaler

Checklist

Relates to #4700

Relates to kedacore/keda-docs#1161

@mingmcb mingmcb requested a review from a team as a code owner June 19, 2023 14:40
@github-actions
Copy link

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 mingmcb force-pushed the feat-pulsar-oauth-extension branch from 8933a45 to 02c5f58 Compare June 19, 2023 15:15
@zroubalik
Copy link
Member

zroubalik commented Jun 21, 2023

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

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.

Looking great, my only concern is about exposing auth data in the trigger metadata, see: kedacore/keda-docs#1161 (review)

Signed-off-by: Ming Meng <ming.meng@collibra.com>
pkg/scalers/pulsar_scaler.go Outdated Show resolved Hide resolved
@mingmcb mingmcb force-pushed the feat-pulsar-oauth-extension branch from d8756cd to 53b26ad Compare June 21, 2023 21:28
Signed-off-by: Ming Meng <101287520+mingmcb@users.noreply.github.com>
Signed-off-by: Ming Meng <ming.meng@collibra.com>
@mingmcb mingmcb force-pushed the feat-pulsar-oauth-extension branch from 53b26ad to 9dca8af Compare June 21, 2023 21:44
@mingmcb mingmcb requested a review from zroubalik June 21, 2023 21:57
@zroubalik
Copy link
Member

zroubalik commented Jun 22, 2023

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

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.

please change the format for scope

pkg/scalers/authentication/authentication_helpers.go Outdated Show resolved Hide resolved
pkg/scalers/pulsar_scaler.go Outdated Show resolved Hide resolved
Signed-off-by: Ming Meng <ming.meng@collibra.com>
@mingmcb mingmcb requested a review from zroubalik June 22, 2023 19:05
Signed-off-by: Ming Meng <101287520+mingmcb@users.noreply.github.com>
Signed-off-by: Ming Meng <101287520+mingmcb@users.noreply.github.com>
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/scalers/authentication/authentication_helpers.go Outdated Show resolved Hide resolved
pkg/scalers/pulsar_scaler.go Outdated Show resolved Hide resolved
@mingmcb mingmcb force-pushed the feat-pulsar-oauth-extension branch 2 times, most recently from c513f2a to 1f5f9cb Compare July 6, 2023 19:30
@mingmcb mingmcb requested a review from zroubalik July 6, 2023 19:31
Signed-off-by: Ming Meng <ming.meng@collibra.com>
@mingmcb mingmcb force-pushed the feat-pulsar-oauth-extension branch from 1f5f9cb to 1f809b9 Compare July 6, 2023 19:36
@zroubalik
Copy link
Member

This is great, there's also a proposal for a generic OAuth support that I like. I wonder if we can go this direction as well for Pulsar? #4694

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.

Looking good, minor stuff.

CHANGELOG.md Outdated Show resolved Hide resolved
@zroubalik
Copy link
Member

zroubalik commented Jul 7, 2023

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

@zroubalik
Copy link
Member

Looking great, my only concern is about exposing auth data in the trigger metadata, see: kedacore/keda-docs#1161 (review)

@kedacore/keda-core-contributors WDYT?

@mingmcb
Copy link
Contributor Author

mingmcb commented Jul 7, 2023

This is great, there's also a proposal for a generic OAuth support that I like. I wonder if we can go this direction as well for Pulsar? #4694

Doesn't look like #4694 is implemented, I can convert to use it if it is ready.

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.

@mingmcb do you think you can address the review comments? We can proceed and merge the PR then.

We are back after a summer break

@mingmcb mingmcb force-pushed the feat-pulsar-oauth-extension branch 2 times, most recently from 3c805bf to c414866 Compare September 15, 2023 16:00
Signed-off-by: Ming Meng <ming.meng@collibra.com>
@zroubalik
Copy link
Member

zroubalik commented Sep 15, 2023

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

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.

LGTM

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@JorTurFer JorTurFer left a comment

Choose a reason for hiding this comment

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

LGTM

@JorTurFer JorTurFer merged commit 70e1241 into kedacore:main Sep 17, 2023
19 checks passed
@mingmcb mingmcb deleted the feat-pulsar-oauth-extension branch September 18, 2023 12:23
toniiiik pushed a commit to toniiiik/keda that referenced this pull request Jan 15, 2024
Signed-off-by: anton.lysina <alysina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants