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.

Sign up for GitHub

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

Jump to bottom

Support unsafeSsl for RabbitMQ scaler #4571

Merged
merged 6 commits into from
May 30, 2023
Merged

Support unsafeSsl for RabbitMQ scaler #4571

merged 6 commits into from
May 30, 2023

Conversation

dttung2905
Copy link
Contributor

@dttung2905 dttung2905 commented May 24, 2023 •
edited by JorTurFer

Hi team,
This PR add support for unsafeSsl as currently the default option for RabbitMQ scaler is hard coded to be unsafeSsl = false. This PR will make the RabbitMQ consistent with other scalers

I also do minor refactoring of the method parseRabbitMQMetadata due to high cyclomatic complexity flagged in the CI check link

Checklist

Fixes #4448

@dttung2905 dttung2905 requested a review from a team as a code owner May 24, 2023 14:36
@JorTurFer
Copy link
Member

Looking good but the HTTP is missing, the line 127 generated the httpClient:

s.httpClient = kedautil.CreateHTTPClient(meta.timeout, false)

That false is the parameter for skipping the cert validation in the client, could you propagate the value to it too?

@dttung2905
Copy link
Contributor Author

Thanks @JorTurFer for the pointer!

JorTurFer
JorTurFer reviewed May 28, 2023
View reviewed changes
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.

Looking good :)
Could you update the changelog please?

@JorTurFer
Copy link
Member

JorTurFer commented May 28, 2023 •
edited by github-actions bot

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

@dttung2905
Add support for unsafeSsl
4663770 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Refactor method parseRabbitMQMetadata due to high cyclomatic complexity
f1ebca0 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Fix unittest failure due a shift in indexing
be38e60 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Minor changes from code review
a2e9f74 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Fix CI build failure
8990002 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905
Add CHANGELOG file
b170615 
Signed-off-by: dttung2905 <ttdao.2015@accountancy.smu.edu.sg>
@dttung2905 dttung2905 mentioned this pull request May 29, 2023
Merged
1 task
@dttung2905 dttung2905 changed the title WIP: Support unsafeSsl for RabbitMQ scaler Support unsafeSsl for RabbitMQ scaler May 29, 2023
JorTurFer
JorTurFer approved these changes May 30, 2023
View reviewed changes
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! Thanks for the improvement

@JorTurFer
Copy link
Member

JorTurFer commented May 30, 2023 •
edited by github-actions bot

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

@JorTurFer JorTurFer merged commit 1d2e7ca into kedacore:main May 30, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JorTurFer JorTurFer JorTurFer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Support for Trust Server SSL Certificate (unsafe ssl) for RabbitMQ Scaler
2 participants
@dttung2905 @JorTurFer