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

Allow CA dirs to be specified beyond /custom/ca/ #5859

Merged
merged 1 commit into from
Jun 18, 2024
Merged

Allow CA dirs to be specified beyond /custom/ca/ #5859

merged 1 commit into from
Jun 18, 2024

Conversation

joelsmith
Copy link
Contributor

@joelsmith joelsmith commented Jun 4, 2024 •
edited
Loading

Add --ca-dir flag to KEDA operator to specify directories with CA certificates for scalers to authenticate TLS connections (defaults to /custom/ca)

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Tests have been added
  • Changelog has been updated and is aligned with our changelog requirements
  • 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)
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #5860

See kedacore/charts#650
See kedacore/keda-docs#1402

@joelsmith joelsmith requested a review from a team as a code owner June 4, 2024 15:44
@zroubalik
Copy link
Member

@joelsmith could you please create an issue to track this feature and link it in the changelog & docs/helm PRs then? Thanks

semgrep-app[bot]
semgrep-app bot reviewed Jun 4, 2024
View reviewed changes
pkg/util/certificates_test.go Outdated Show resolved Hide resolved
semgrep-app[bot]
semgrep-app bot reviewed Jun 4, 2024
View reviewed changes
pkg/util/certificates_test.go Outdated Show resolved Hide resolved
semgrep-app[bot]
semgrep-app bot reviewed Jun 4, 2024
View reviewed changes
pkg/util/certificates_test.go Outdated Show resolved Hide resolved
@joelsmith joelsmith mentioned this pull request Jun 4, 2024
Merged
4 tasks
@joelsmith joelsmith force-pushed the kedamain branch 3 times, most recently from f676473 to 73bf0b3 Compare June 4, 2024 22:18
@joelsmith
Copy link
Contributor Author

Hi @zroubalik, I have created the issue and the docs & chart PRs. I think they are ready for a review.
Thanks!

@zroubalik
Copy link
Member

zroubalik commented Jun 5, 2024 •
edited by github-actions bot
Loading

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

@joelsmith
Copy link
Contributor Author

joelsmith commented Jun 5, 2024 •
edited
Loading

BTW, because of the added loop and resulting indent increase, the patch is easier to read with ?w=1 added to the URL, like this:
https://github.com/kedacore/keda/pull/5859/files?w=1
I'm not sure why the e2e failed... the error shows that the KEDA operator did not become ready fast enough, but looking at the operator's log, it did become ready. So maybe it was just too slow.

@zroubalik
Copy link
Member

zroubalik commented Jun 6, 2024 •
edited by github-actions bot
Loading

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

@joelsmith
Copy link
Contributor Author

@zroubalik Thanks for re-running the test -- it passed this time. Thanks!

zroubalik
zroubalik approved these changes Jun 7, 2024
View reviewed changes
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

I would like @JorTurFer to check this one as well

JorTurFer
JorTurFer reviewed Jun 18, 2024
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! Only small comment inline

pkg/util/tls_config.go Outdated Show resolved Hide resolved
@joelsmith
Allow CA dirs to be specified beyond /custom/ca/
56ff645 
Signed-off-by: Joel Smith <joelsmith@redhat.com>
@JorTurFer
Copy link
Member

JorTurFer commented Jun 18, 2024 •
edited by github-actions bot
Loading

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

JorTurFer
JorTurFer approved these changes Jun 18, 2024
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! Nice improvement ❤️

@JorTurFer JorTurFer enabled auto-merge (squash) June 18, 2024 16:44
@JorTurFer JorTurFer merged commit 00a11d2 into kedacore:main Jun 18, 2024
21 checks passed
This was referenced Jun 18, 2024
Merged
Merged
uucloud pushed a commit to uucloud/keda that referenced this pull request Jul 11, 2024
@joelsmith @uucloud
Allow CA dirs to be specified beyond /custom/ca/ (kedacore#5859)
dce5efd 
Signed-off-by: Joel Smith <joelsmith@redhat.com>
Signed-off-by: uucloud <uucloud@qq.com>
@jkyros jkyros mentioned this pull request Aug 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@semgrep-app semgrep-app[bot] semgrep-app[bot] left review comments

@JorTurFer JorTurFer JorTurFer approved these changes

@zroubalik zroubalik zroubalik approved these changes

Assignees

@JorTurFer JorTurFer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow multiple directories of CA certificates
3 participants
@joelsmith @zroubalik @JorTurFer