feat: enable specifying root ca for oidc (#6712) #6712
Conversation
clive-jevons
changed the title
Codecov Report
@@ Coverage Diff @@
## master #6712 +/- ##
==========================================
- Coverage 41.12% 41.10% -0.03%
==========================================
Files 157 157
Lines 21010 21021 +11
==========================================
Hits 8640 8640
- Misses 11141 11152 +11
Partials 1229 1229
Continue to review full report at Codecov.
|
clive-jevons
force-pushed
the
enable-specifying-root-ca-for-oidc
branch
from
bacc7cb
to
2cc3377
Compare
util/settings/settings.go
Outdated
| if !ok { | ||
| panic("bad oidc root ca cert") | ||
| } |
There was a problem hiding this comment.
I think this does not justify a panic. My suggestion here would be to log an error, and just proceed returning an empty &tls.Config{}.
There was a problem hiding this comment.
Makes sense 😁 Have updated accordingly 👍 Thanks!
There was a problem hiding this comment.
@jannfis <- have you had a chance to check out the newest version? 😬
clive-jevons
force-pushed
the
enable-specifying-root-ca-for-oidc
branch
from
2cc3377
to
6124d21
Compare
clive-jevons
force-pushed
the
enable-specifying-root-ca-for-oidc
branch
from
6124d21
to
a69e725
Compare
|
So, I'm just a random user who just spent a significant number of hours beating my head against the wall trying to figure out why I couldn't get ArgoCD to integrate with my cluster's Keycloak...I suppose I should have looked in the issues sooner! Thank you for this! Reading through your changes, it looks like the way this would be implemented is that you have to specify the root CA cert in the OIDC.config block as raw text? |
|
@dcarlet yes, the PR, as it stands, would allow you to simply include the PEM block of the root CA to be used for trusting the Keycloak's cert. |
|
Sorry for coming back so late to this! Just one minor thing: Would this maybe require some documentation, so that it's not a hidden feature? |
jannfis
added
the
cherry-pick/2.1
|
Also, can you please rebase to |
When configuring an external OIDC provider which uses a private PKI for its certificates it was not possible to properly verify the certificate being served. Also, when using ArgoCD in insecure mode, e.g. when running behind istio for providing mTLS, this resulted in errors. Signed-off-by: Clive Jevons <clive@jevons-it.net>
clive-jevons
force-pushed
the
enable-specifying-root-ca-for-oidc
branch
from
a69e725
to
c48c569
Compare
|
@jannfis I've rebased and also added a section to the docs 👍 Thnx 😁 |
There was a problem hiding this comment.
LGTM @clive-jevons
Sincere apologies for the very long cycle on this one. Somehow, this PR has slipped off my focus :(
jannfis
changed the title
jannfis
removed
the
cherry-pick/2.1
@jannfis <- Thanks for accepting and using the PR 😁🙏🏻 |
When configuring an external OIDC provider which uses a private PKI for its certificates it was not possible to properly verify the certificate being served. Also, when using ArgoCD in insecure mode, e.g. when running behind istio for providing mTLS, this resulted in errors. Signed-off-by: Clive Jevons <clive@jevons-it.net>
When configuring an external OIDC provider which uses a private PKI
for its certificates it was not possible to properly verify the certificate
being served. Also, when using ArgoCD in insecure mode, e.g. when running
behind istio for providing mTLS, this resulted in errors.
Closes [6713]
Issue: #6713
Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: