Skip to content

Conversation

Pothulapati
Copy link
Contributor

@Pothulapati Pothulapati commented Jul 8, 2022

Description

Due to the way docker works in non-native platforms, It is
very hard to have a consistent experience across all platforms
as we can't just use the docker bridge netwrok IP's in non-native
platforms
. This means that users have to search
their Host IP, and use It to get up and working
which we tried, but understand that it's not a good UX.

But users can use 127-0-0-1.nip.io as the DOMAIN which resolves to
127.0.0.1 and is available in all platforms as its localhost. This
works well and good for all user communication
but internal communication fails as 127-0-0-1.nip.io for them
is something else.

So, This PR fixes that by adding new coredns gitpod.db coredns
config essentially asking to route all 127-0-0-1.nip.io to
proxy.default.svc.cluster.local. As k3s does not yet support
overriding coredns config in a sane-way
,We instead skip the
default coredns by adding coredns.yaml.skip file, and adding our own
custom-coredns.yaml which is just plain coredns.yaml
that comes with k3s, added with gitpod config.

Signed-off-by: Tarun Pothulapati [email protected]

Related Issue(s)

Fixes #

How to test

Release Notes

[local-preview] Support `127-0-0-1.nip.io` for `DOMAIN`

Documentation

Werft options:

  • /werft with-preview

@Pothulapati Pothulapati requested a review from a team July 8, 2022 15:08
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Jul 8, 2022
@Pothulapati Pothulapati force-pushed the tar/localhost-preview branch from 4e6fd5f to dbbb214 Compare July 8, 2022 15:09
@corneliusludmann
Copy link
Contributor

Great! That looks good.

Shouldn't we set the 127-0-0-1 domain here as default?

DOMAIN="${DOMAIN_STRING}.nip.io"

@Pothulapati Pothulapati force-pushed the tar/localhost-preview branch from dbbb214 to 9cb5c34 Compare July 8, 2022 15:21
@Pothulapati
Copy link
Contributor Author

Pothulapati commented Jul 8, 2022

@corneliusludmann Agreed, and Updated! We should worry about flexibility later, as we are aiming for consistency across platforms now!

@@ -133,6 +127,9 @@ for f in /var/lib/rancher/k3s/server/manifests/gitpod/*StatefulSet*.yaml; do yq
# removing init container from ws-daemon (systemd and Ubuntu)
yq eval-all -i 'del(.spec.template.spec.initContainers[0])' /var/lib/rancher/k3s/server/manifests/gitpod/*_DaemonSet_ws-daemon.yaml

touch /var/lib/rancher/k3s/server/manifests/coredns.yaml.skip
Copy link
Contributor Author

Choose a reason for hiding this comment

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

name: coredns
namespace: kube-system
data:
gitpod.db: |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

coredns magic that makes the internal communication work!

@corneliusludmann
Copy link
Contributor

My suggestion would be to keep the flexibility that allows overriding the domain with an env var but set 127-0-0-1.nip.io as default instead of the cluster IP domain. That would allow using a domain with the host IP to make it accessible in the network for colleagues but also to use the 127-0-0-1 approach. What do you think?

Due to the way docker works in non-native platforms, It is
very hard to have a consistent experience across all platforms
as we can't just use the [docker bridge netwrok IP's in non-native
platforms](https://docs.docker.com/desktop/networking/). This means that
users have to search their Host IP, and use It
to get up and working [which we tried, but understand that it's not
a good UX](https://github.com/gitpod-io/website/pull/2349).

But users can use `127-0-0-1.nip.io` as the DOMAIN which resolves to
 `127.0.0.1` and is available in all platforms as its `localhost`. This
works well and good for all user communication
but internal communication fails as `127-0-0-1.nip.io` for them
is something else.

So, This PR fixes that by adding new coredns
`gitpod.db` coredns config essentially asking to route all
`127-0-0-1.nip.io` to `proxy.default.svc.cluster.local`. [As k3s does
not yet support overriding coredns config in a sane-way](k3s-io/k3s#462)
,We instead skip the default coredns by adding `coredns.yaml.skip` file,
and adding our own `custom-coredns.yaml` which is just plain
`coredns.yaml` that comes with `k3s`, added with gitpod config.

Signed-off-by: Tarun Pothulapati <[email protected]>
@Pothulapati Pothulapati force-pushed the tar/localhost-preview branch from 9cb5c34 to f80fd43 Compare July 8, 2022 15:28
@Pothulapati
Copy link
Contributor Author

Pothulapati commented Jul 8, 2022

@corneliusludmann Makes sense, and is simple enough! Updated!

Also, I haven't toggled the coredns settings based on DOMAIN as it's non-intrusive in the other case i.e when DOMAIN is set 🤔

@Pothulapati
Copy link
Contributor Author

Tested this, and can confirm that this works on Linux and Windows. Intel Mac testing on the way.

@Pothulapati
Copy link
Contributor Author

This also has been tested on a Intel Mac

Copy link
Contributor

@adrienthebo adrienthebo left a comment

Choose a reason for hiding this comment

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

Verified on an Intel mac with the following:

Verification procedure ``` docker run -p 8443:443 --privileged --name gitpod --rm -it -v /tmp/gitpod:/var/gitpod eu.gcr.io/gitpod-core-dev/build/local-preview:tar-localhost-preview.3 ```
curl -vkL https://127-0-0-1.nip.io:8443 2>&1|tee -a /dev/stderr|pbcopy
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0*   Trying 127.0.0.1:8443...
* Connected to 127-0-0-1.nip.io (127.0.0.1) port 8443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: /etc/ssl/cert.pem
*  CApath: none
* (304) (OUT), TLS handshake, Client hello (1):
} [321 bytes data]
* (304) (IN), TLS handshake, Server hello (2):
{ [122 bytes data]
* (304) (IN), TLS handshake, Unknown (8):
{ [15 bytes data]
* (304) (IN), TLS handshake, Certificate (11):
{ [1023 bytes data]
* (304) (IN), TLS handshake, CERT verify (15):
{ [264 bytes data]
* (304) (IN), TLS handshake, Finished (20):
{ [36 bytes data]
* (304) (OUT), TLS handshake, Finished (20):
} [36 bytes data]
* SSL connection using TLSv1.3 / AEAD-AES128-GCM-SHA256
* ALPN, server accepted to use h2
* Server certificate:
*  subject: [NONE]
*  start date: Jul 11 22:29:43 2022 GMT
*  expire date: Oct  9 22:29:43 2022 GMT
*  issuer: O=mkcert development CA; OU=6d80fa04b8ea; CN=mkcert 6d80fa04b8ea
*  SSL certificate verify result: unable to get local issuer certificate (20), continuing anyway.

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0* Using HTTP2, server supports multiplexing
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x7fdb2280e400)
> GET / HTTP/2
> Host: 127-0-0-1.nip.io:8443
> user-agent: curl/7.79.1
> accept: */*
> 
* Connection state changed (MAX_CONCURRENT_STREAMS == 250)!
< HTTP/2 200 
< accept-ranges: bytes
< cache-control: no-cache, no-transform, must-revalidate
< content-security-policy: frame-ancestors 'self' https://*.127-0-0-1.nip.io https://127-0-0-1.nip.io
< content-type: text/html; charset=utf-8
< date: Mon, 11 Jul 2022 22:34:54 GMT
< etag: "ren1nu5pb"
< last-modified: Thu, 07 Jul 2022 06:50:18 GMT
< referrer-policy: no-referrer-when-downgrade
< strict-transport-security: max-age=31536000
< vary: Accept-Encoding
< x-content-type-options: nosniff
< x-xss-protection: 1; mode=block
< 
{ [5921 bytes data]

100  7391    0  7391    0     0  29595      0 --:--:-- --:--:-- --:--:-- 30668
* Connection #0 to host 127-0-0-1.nip.io left intact
[...gitpod index HTML...]
</details>

LGTM! 

@roboquat roboquat merged commit a65c946 into main Jul 11, 2022
@roboquat roboquat deleted the tar/localhost-preview branch July 11, 2022 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/L team: delivery Issue belongs to the self-hosted team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants