Skip to content

issue self signed certificate #316

Merged
nimakaviani merged 2 commits intocnoe-io:mainfrom
nabuskey:tls-config
Jun 21, 2024
Merged

issue self signed certificate #316
nimakaviani merged 2 commits intocnoe-io:mainfrom
nabuskey:tls-config

Conversation

@nabuskey
Copy link
Copy Markdown
Collaborator

fixes: #137
related to: #300 #293

With this PR, idpbuilder will:

  1. Create a self signed certificate.
  2. Create a TLS secret in the ingress-nginx NS.
  3. Use it as the default TLS certificate for ingress-nginx. (you can still use other certs configured at ingress level)
  4. Update ArgoCD's CM to trust the CA.
  5. Create a CM that contains cert to the default NS.

I thought about using cert-manager but decided not to use it. For our purposes, we just need a certificate for ingress-nginx for in-cluster and incoming traffic only. Introducing cert-manager means:

  1. We need to maintain cert-manager manifests.
  2. Slower installation speed. We now need to wait for cert-manager to do its thing and be ready before anything else can continue.
  3. Introduces three pods that will do nothing for the most part.

Copy link
Copy Markdown
Contributor

@nimakaviani nimakaviani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. made a few minor comments. otherwise looks good.

I hear your points on the use of self-signed certs and it makes sense in the context of idpBuidler alone. However, if we try to move towards an AppSet strategy that also happens to apply to external clusters, then I wonder whether the idea of using CertManager has a stronger ground.

If users will have cert-manager installed to their clusters by default, and if the idpBuilder is supposed to pave the path for them to eventually transitioin from a dev / test environment to a prod env, shall we just bite the bullet and pay the extra cost of enabling them with the cert-manager in the test environment too?

that said, given the amount of work put into this, this works as an interim solution. But lets revisit this as we move towards expanding on the deployment strategy.

nabuskey added 2 commits June 20, 2024 21:46
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
Signed-off-by: Manabu McCloskey <manabu.mccloskey@gmail.com>
@nabuskey
Copy link
Copy Markdown
Collaborator Author

I think I'd rather wait for concrete use cases for cert-manager until we pull it into core. Ready for another round of review.

@jessesanford
Copy link
Copy Markdown
Contributor

I agree on waiting for more use cases before bringing in cert manager. It should be reasonably easy to roll forward to it when the time comes.

@cmoulliard
Copy link
Copy Markdown
Contributor

I think I'd rather wait for concrete use cases for cert-manager until we pull it into core. Ready for another round of review.

Cert Manager can help to deal with many use cases like:

@cmoulliard cmoulliard self-requested a review June 21, 2024 06:44
Copy link
Copy Markdown
Contributor

@nimakaviani nimakaviani 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!

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.

Ingress-nginx certificate handling

4 participants