Skip to content

docs: add note about external domain in requirements #4034

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 25, 2021
Merged

docs: add note about external domain in requirements #4034

merged 1 commit into from
Aug 25, 2021

Conversation

jsjoeio
Copy link
Contributor

@jsjoeio jsjoeio commented Aug 24, 2021

Fixes #3921

@jsjoeio jsjoeio requested a review from a team as a code owner August 24, 2021 22:56
@jsjoeio jsjoeio self-assigned this Aug 24, 2021
@jsjoeio jsjoeio added the docs Documentation related label Aug 24, 2021
@codecov
Copy link

codecov bot commented Aug 24, 2021 •
edited
Loading

Codecov Report

Merging #4034 (523856d) into main (e5a9df3) will not change coverage.
The diff coverage is n/a.

❗ Current head 523856d differs from pull request most recent head f0d3e02. Consider uploading reports for the commit f0d3e02 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4034   +/-   ##
=======================================
  Coverage   63.51%   63.51%           
=======================================
  Files          36       36           
  Lines        1872     1872           
  Branches      379      379           
=======================================
  Hits         1189     1189           
  Misses        580      580           
  Partials      103      103

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 e5a9df3...f0d3e02. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Aug 24, 2021 •
edited
Loading

✨ Coder.com for PR #4034 deployed! It will be updated on every commit.

jawnsy
jawnsy approved these changes Aug 24, 2021
View reviewed changes
docs/requirements.md Outdated
@@ -53,3 +53,4 @@ Notes:

- To lower costs, you can shut down your server when you're not using it.
- We recommend using the `gcloud cli` to avoid using the GCP Dashboard if possible.
- To enable HTTPS, you have to add an external domain.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this strictly true? In air-gapped networks, you can use self-signed certificates or certificates signed by an internal certificate authority, and if you control DNS then everything is OK.

I think this statement is fine but would suggest we soften it to "For serving code-server over HTTPS, we recommend using an external domain name along with a service such as Let's Encrypt"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahhh okay, I didn't know. This came after a GitHub Discussion. Let's go with your wording instead.

jsjoeio
jsjoeio commented Aug 24, 2021
View reviewed changes
@jsjoeio jsjoeio force-pushed the jsjoeio-docs-google-cloud branch from ad753c8 to f0d3e02 Compare August 24, 2021 23:39
@jsjoeio jsjoeio enabled auto-merge August 24, 2021 23:40
GirlBossRush
GirlBossRush approved these changes Aug 25, 2021
View reviewed changes
Copy link
Contributor

@GirlBossRush GirlBossRush left a comment

Choose a reason for hiding this comment

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

This looks great. I’m curious how we can test such domains in the future, maybe updating the health check endpoint to ping itself to see if the server is accessible from the outside.

@jsjoeio
Copy link
Contributor Author

jsjoeio commented Aug 25, 2021 •
edited
Loading

maybe updating the health check endpoint to ping itself to see if the server is accessible from the outside.

Yes, that would be really cool! I'll make a new issue.

@jsjoeio jsjoeio mentioned this pull request Aug 25, 2021
Closed
@jsjoeio jsjoeio merged commit 02d355f into main Aug 25, 2021
@jsjoeio jsjoeio deleted the jsjoeio-docs-google-cloud branch August 25, 2021 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jawnsy jawnsy jawnsy approved these changes

@GirlBossRush GirlBossRush GirlBossRush approved these changes

Assignees

@jsjoeio jsjoeio

Labels
docs Documentation related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add docs around Google Cloud https
3 participants
@jsjoeio @jawnsy @GirlBossRush