-
Notifications
You must be signed in to change notification settings - Fork 80
Update tls guides for cert-manager 1.4.0 and coder 1.20 #472
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
Fixed a bunch of little issues related to newer cert-manager, k8s, coder, etc. Also the secret for DNS01 challenges needs to be in cert-manager namespace, not coder.
✨ Coder.com for PR #472 deployed! It will be updated on every commit.
|
@bpmct and @ericpaulsen can you sanity check these updates before I request polishing? |
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.
Thanks for this! Can you also go through and make sure we're not using any v1alpha API versions too?
guides/ssl-certificates/cloudDNS.md
Outdated
@@ -21,7 +21,7 @@ configure your Coder hostname and dev URLs. | |||
|
|||
You must have: | |||
|
|||
- A Kubernetes cluster with internet connectivity | |||
- A Kubernetes cluster [of a supported version](https://kubernetes.io/releases/version-skew-policy/#supported-version-skew) with internet connectivity |
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.
suggest we link to our internal Kubernetes page instead of directly to the upstream docs, as the versions Coder supports may differ from the versions Kubernetes upstream supports
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.
These are guides so I'd like folks to start with a recommendation that makes the most sense. Even this should probably just say "latest" probably since it's not a document that expresses the limits but tries to get people started from the best point.
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.
Well, the latest version of Kubernetes may not be a version that Coder (both the company & the product) supports. We currently follow the same support policy so it does't make a difference, but in the future it might. I think in general we want to keep people in our docs.
To clarify, I'm suggesting that we link to: https://github.com/cdr/docs/blob/main/setup/kubernetes/index.md rather than the Kubernetes documentation, or omit the new link completely here.
Of course, the content right now is not complete, and will be better once I finish/merge #232
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.
That page does make more sense than the current one. I'll leave it to @khorne3 and others which link makes more sense from a holistic docs management perspective.
Good call. I updated those and made sure the examples matched the |
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.
Caught an error in the CloudFlare docs. Other than that, LGTM.
guides/ssl-certificates/azureDNS.md
Outdated
@@ -31,7 +31,7 @@ are the same regardless of which option you choose. | |||
|
|||
You must have: | |||
|
|||
- A Kubernetes cluster with internet connectivity | |||
- A Kubernetes cluster [of a supported version](https://kubernetes.io/releases/version-skew-policy/#supported-version-skew) with internet connectivity |
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.
FYI: now that #232 was merged, you can link to that page instead https://coder.com/docs/coder/v1.20/setup/kubernetes#supported-kubernetes-versions
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.
I think we should just stick with ClusterIssuer for now, we can revisit and change later.
While we're on the topic of cert-manager though, we shouldn't be using the ingress annotation anymore and should instead be creating the Certificate object manually because 1.21.0 does not ship with an Ingress by default. I think this will still work for now, but the included Ingress is deprecated so it won't work for much longer.
Co-authored-by: Ben Potter <me@bpmct.net>
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.
good for 1.20.0
Fixed a bunch of little issues related to newer cert-manager, k8s, coder, etc.
Also the secret for DNS01 challenges needs to be in cert-manager namespace, not coder.
We may want to change the whole directory name to "TLS" since "SSL" is a dead protocol.