feat: support multiple certificates in coder server and helm#4150
Conversation
| tlsConfig.GetCertificate = func(hi *tls.ClientHelloInfo) (*tls.Certificate, error) { | ||
| // If there's only one certificate, return it. | ||
| if len(certs) == 1 { | ||
| return &certs[0], nil | ||
| } | ||
|
|
||
| // Expensively check which certificate matches the client hello. | ||
| for _, cert := range certs { | ||
| cert := cert | ||
| if err := hi.SupportsCertificate(&cert); err == nil { | ||
| return &cert, nil | ||
| } | ||
| } | ||
|
|
||
| // Return the first certificate if we have one, or return nil so the | ||
| // server doesn't fail. | ||
| if len(certs) > 0 { | ||
| return &certs[0], nil | ||
| } | ||
| return nil, nil //nolint:nilnil | ||
| } |
There was a problem hiding this comment.
👍 This allows us to hot reload certs later.
| func configureTLS(listener net.Listener, tlsMinVersion, tlsClientAuth, tlsCertFile, tlsKeyFile, tlsClientCAFile string) (net.Listener, error) { | ||
| func loadCertificates(tlsCertFiles, tlsKeyFiles []string) ([]tls.Certificate, error) { | ||
| if len(tlsCertFiles) != len(tlsKeyFiles) { | ||
| return nil, xerrors.New("--tls-cert-file and --tls-key-file must be used the same amount of times") |
There was a problem hiding this comment.
| return nil, xerrors.New("--tls-cert-file and --tls-key-file must be used the same amount of times") | |
| return nil, xerrors.New("the number of --tls-cert-file and --tls-key-file must be the same") |
There was a problem hiding this comment.
I personally think the old message is better, what do you think @kylecarbs
| dials int64 | ||
| ) | ||
| client := codersdk.New(accessURL) | ||
| client.HTTPClient = &http.Client{ |
There was a problem hiding this comment.
Could add the certs to the client. But not a big deal as you verify hostname later 🤷
There was a problem hiding this comment.
I'm going to leave this as-is because we will get better test failure messages if the server somehow returns an unexpected certificate
kylecarbs
left a comment
There was a problem hiding this comment.
Did you manually test the Helm chart with multiple certificates? Just to confirm that it all works.
All the code looks good to me, and I'm impressed with how thorough the tests are 🥳
|
Tested with real helm deployment and found a few bugs in the Helm chart which are now fixed in the latest commit. The coder binary worked flawlessly though. |
Adds multiple certificate support to
coder serverand the Helm chart as it's likely admins will want to be able to use multiple certificates to support the new wildcard access feature.--tls-cert-fileand--tls-key-fileto be array flags instead of single string flags.coder.tls.secretNamesfor loading multiple certificates._helpers.tpl.coder.tls.secretNamein favor ofcoder.tls.secretNames.NOTES.txtthat informs users if they are using the deprecated value and says hi. :)TODO:
coder servermultiple certificate loading functionality.helm templateandhelm install --dry-runto ensure template correctness.