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

fix: mount ca cert also for letsencrypt-staging and parameterize secret key #49

Merged
merged 5 commits into from
Jun 2, 2023

Conversation

hbollon
Copy link
Member

@hbollon hbollon commented May 31, 2023

Description of the changes

Currently, ArgoCD's OIDC won't work with letsencrypt-staging issuer since it's not issued by a known certificate authority and so ArgoCD return a x509 certificate signed by unknown authority error when trying to login via Keycloak.
To fix that, I added the certificate volume mount to ArgoCD if the issuer is letsencrypt-staging. I also parameterized the secret key used in it since in my case it's not ca.crt but tls.crt (the variable is still default to ca.crt)

Breaking change

  • No

Tests executed on which distribution(s)

  • SKS (Exoscale)

@hbollon hbollon requested a review from a team as a code owner May 31, 2023 11:20
@hbollon hbollon self-assigned this May 31, 2023
jbarascut
jbarascut previously approved these changes May 31, 2023
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
@lentidas
Copy link
Contributor

lentidas commented Jun 1, 2023

Can I have the time to better test this tomorrow before merging? I would like us to consider a bit more before adding a variable to set this, since the thing won't be activated unless one of the two defined cluster issuers is set. So in my view it would be better if each one of the cases has its on secret key instead of allowing the user to set it up.

Like having a ternary operator: subPath = var.cluster_issuer == "letsencrypt-staging" ? "tls.crt" : "ca.crt"

@hbollon hbollon force-pushed the fix/oidc-ca-certificate-staging branch from 99d02cd to 05f4da9 Compare June 1, 2023 12:05
variables.tf Outdated Show resolved Hide resolved
locals.tf Outdated Show resolved Hide resolved
@lentidas lentidas requested review from ckaenzig and removed request for ckaenzig June 2, 2023 07:49
@hbollon hbollon merged commit 06a180d into main Jun 2, 2023
@hbollon hbollon deleted the fix/oidc-ca-certificate-staging branch June 2, 2023 14:09
@github-actions github-actions bot mentioned this pull request Jun 2, 2023
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.

4 participants