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

Adding changes for configuring unsafe ssl for self signed certs use case #3595

Closed
wants to merge 0 commits into from
Closed

Adding changes for configuring unsafe ssl for self signed certs use case #3595

wants to merge 0 commits into from

Conversation

mayurvaid-redvest
Copy link
Contributor

@mayurvaid-redvest mayurvaid-redvest commented Aug 25, 2022 •
edited
Loading

Signed-off-by: mayurvaid-redvest 109327850+mayurvaid-redvest@users.noreply.github.com

Provide a description of what has been changed

Checklist

  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • Tests have been added
  • A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
  • A PR is opened to update the documentation on (repo) (if applicable)
  • Changelog has been updated and is aligned with our changelog requirements

Fixes #

Relates to #2310

@mayurvaid-redvest mayurvaid-redvest requested a review from a team as a code owner August 25, 2022 03:44
JorTurFer
JorTurFer reviewed Aug 25, 2022
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 add a unit test for this?

@mayurvaid-redvest
Copy link
Contributor Author

Thanks @JorTurFer for reviewing , I have modified test case with new parameter and checked , Please let me know if anything else has to be added

@JorTurFer
Copy link
Member

JorTurFer commented Aug 25, 2022 •
edited by github-actions bot
Loading

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

@JorTurFer
Copy link
Member

Yes, please open a PR in keda-doc adding the parameter to Prometheus scaler page and update the changelog. The issue you have to refeer is this #2310
And thanks a ton for the contribution!

@mayurvaid-redvest
Copy link
Contributor Author

Docs Path : kedacore/keda-docs#913

JorTurFer
JorTurFer reviewed Aug 25, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@mayurvaid-redvest
Copy link
Contributor Author

@JorTurFer , Can you please review and let me know anything pending

@JorTurFer
Copy link
Member

JorTurFer commented Aug 26, 2022 •
edited by github-actions bot
Loading

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

JorTurFer
JorTurFer approved these changes Aug 26, 2022
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 contibution! ❤️

@JorTurFer
Copy link
Member

Hi @mayurvaid-redvest ,
This change is ready to merge IMO, but we are planing to do a bugfix release soon (v2.8.1) and we don't introduce new features in that kind of releases, only fixes and security patches. This feature will be merged after the hotfix release and released as part of v2.9.0
Sorry for the inconvenience

@mayurvaid-redvest
Copy link
Contributor Author

mayurvaid-redvest commented Aug 26, 2022 •
edited
Loading

@JorTurFer Do i have any action now , Or just wait for hotfix

@JorTurFer
Copy link
Member

I think that you don't need to do any more. I'll let you know if we need something else
Thanks!

@mayurvaid-redvest
Copy link
Contributor Author

I think that you don't need to do any more. I'll let you know if we need something else Thanks!

Thanks for the help

@JorTurFer
Copy link
Member

hey @mayurvaid-redvest
Sorry but we need one thing more. After the release, there is a conflict with changelog file. Could you rebase main and solve the changelog please?

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.
2 participants
@mayurvaid-redvest @JorTurFer