-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[chart] Refactor chart to only use cert-manager #4592
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
Conversation
8883629
to
764f63b
Compare
764f63b
to
e65b98f
Compare
e65b98f
to
8157c86
Compare
8157c86
to
0861494
Compare
@@ -17,7 +16,6 @@ metadata: | |||
data: | |||
init.sql: |- | |||
{{- $root := . }} | |||
{{- range $path, $bytes := .Files.Glob "config/db/init/**.sql" }} | |||
{{- range $path, $bytes := .Files.Glob "config/db/init/02-create-and-init-sessions-db.sql" }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you elaborate on this change? Why not are the other scripts not needed anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@geropl 03-recreate-gitpod-db
drops the db. Does it seem the scripts expect the chart will be used for testing?
If you provide a connection to MySQL we should only assume the user, password, and database are valid. Everything else should be done by typeorm (even the creation of the sessions db)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it seem the scripts expect the chart will be used for testing?
They are used for both use cases: initialization (because it's straight-forward to mount into any DB container) and testing.
If you provide a connection to MySQL we should only assume the user, password, and database are valid. Everything else should be done by typeorm (even the creation of the sessions db)
That's certainly another way to do it. What would be the advantage over the current approach?
@aledbf Although the PR is in Draft-mode I already have a comment: Is it still possible to (easily) manually pass in certs if that is necessary? |
You mean, you already have a secret containing the SSL certificate to be used in proxy? |
@geropl for context, I still have pending changes to this PR. |
Yes.
Did not meant to steal your time, sorry! :-D Will come back once it's "green". |
/werft run 👍 started the job as gitpod-build-aledbf-cert-manager.10 |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Refactor the helm chart to remove self-generated SSL certificates using helm to certificates from
cert-manager.
The chart already contains a section for
cert-manager
, something that is being used in staging and prod.By using
cert-manager
, we can use anIssuer
(namespaces) to generate the certificates using only one internal CA.Install the CA certificate in
registry-facade
to allow pulls from the internal docker registry (if enabled) without trust issues.Simplified configuration of components affinity.
New Job to ensure the creation of
gitpod-sessions
database.