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

feat: Add TLS support #556

Merged
merged 4 commits into from
Mar 24, 2022
Merged

feat: Add TLS support #556

merged 4 commits into from
Mar 24, 2022

Conversation

kylecarbs
Copy link
Member

@kylecarbs kylecarbs commented Mar 24, 2022

This adds numerous flags with inspiration taken from Vault
for configuring TLS inside Coder.

This enables secure deployments without a proxy, like Cloudflare.

image

This adds numerous flags with inspiration taken from Vault
for configuring TLS inside Coder.

This enables secure deployments without a proxy, like Cloudflare.
@codecov
Copy link

codecov bot commented Mar 24, 2022

Codecov Report

Merging #556 (aa38753) into main (565b940) will increase coverage by 0.44%.
The diff coverage is 71.87%.

@@            Coverage Diff             @@
##             main     #556      +/-   ##
==========================================
+ Coverage   62.65%   63.10%   +0.44%     
==========================================
  Files         194      191       -3     
  Lines       10977    10923      -54     
  Branches       85       85              
==========================================
+ Hits         6878     6893      +15     
+ Misses       3360     3292      -68     
+ Partials      739      738       -1     
Flag Coverage Δ
unittest-go- 62.04% <71.87%> (+0.45%) ⬆️
unittest-go-macos-latest 57.62% <71.87%> (+0.59%) ⬆️
unittest-go-ubuntu-latest 60.78% <71.87%> (+0.74%) ⬆️
unittest-go-windows-2022 ?
unittest-js 63.32% <ø> (ø)
Impacted Files Coverage Δ
codersdk/workspaceresourceauth.go 62.50% <0.00%> (ø)
cli/start.go 65.02% <71.79%> (+11.76%) ⬆️
codersdk/client.go 55.84% <100.00%> (+5.19%) ⬆️
codersdk/provisionerdaemons.go 64.61% <100.00%> (+6.15%) ⬆️
provisionersdk/transport.go 78.72% <0.00%> (-6.39%) ⬇️
pty/ptytest/ptytest.go 94.82% <0.00%> (-5.18%) ⬇️
coderd/parameter/compute.go 74.07% <0.00%> (-4.45%) ⬇️
cli/projectinit.go 56.36% <0.00%> (-3.64%) ⬇️
provisioner/echo/serve.go 54.40% <0.00%> (-2.40%) ⬇️
coderd/provisionerdaemons.go 61.15% <0.00%> (-1.03%) ⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 565b940...aa38753. Read the comment docs.

cli/start.go Show resolved Hide resolved
cli/start.go Outdated Show resolved Hide resolved
server := http.Server{
Handler: handler,
BaseContext: func(_ net.Listener) context.Context {
return shutdownConnsCtx
Copy link
Member

Choose a reason for hiding this comment

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

This is cool! When cancelling this context, is it a hard cutoff to websockets (ie just severs the connection), or does it send a close frame?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hard cutoff right now, but we technically could send a shutdown signal. Maybe I'll do that...

Copy link
Member

Choose a reason for hiding this comment

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

i don't necessarily think either is right or wrong, just asking for my own curiosity mostly!

coder.env Outdated
@@ -1,3 +1,6 @@
# Runtime variables for "coder start".
# Run "coder start --help" to vie.
Copy link
Member

Choose a reason for hiding this comment

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

seems incomplete

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch!

kylecarbs and others added 2 commits March 24, 2022 14:09
Co-authored-by: Colin Adler <colin@coder.com>
@kylecarbs kylecarbs merged commit bf00487 into main Mar 24, 2022
@kylecarbs kylecarbs deleted the tls branch March 24, 2022 19:21
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.

None yet

2 participants