Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to simplify HTTPS setup #8983

hkolvenbach · 2018-03-02T09:04:11Z

What does this PR do?

This PR changes the behavior how DNS names for SINGLE_PORT mode are generated, if the new ENV variable CHE_SINGLEPORT_CUSTOM_DNS is set to true. It will then generate DNS names with underscores instead of dots, such that all subdomains are conforming to *.domain.tld, which is important for a HTTPS setup using a wildcard certificate (usually only *.domain.tld is supported by wildcard certificates, not *.*.domain.tld and so on). This requires a wildcard DNS entry for your domain in the form of *.domain.tld and won't use any service like nip.io.

CHE_SINGLEPORT_WILDCARD__DOMAIN_HOST needs to be set to domain.tld in that case.

Work to do:

change the Java part to enable this only if the CHE_SINGLEPORT_CUSTOM_DNS flag is set (currently hardcoded)

I would like to get feedback if this is something upstream che would want. If yes, I will solve the outstanding issues and write documentation for it. It would certainly make HTTPS setup easier for everyone and I would not need to maintain a patched fork of che.

What issues does this PR fix or reference?

Release Notes

SINGLE_PORT mode now supports custom single level DNS domains *.domain.tld

Docs PR

eclipse-che/che-docs#371

hkolvenbach · 2018-03-02T09:04:48Z

@JamesDrummond this might be interesting for you. Did you solve the HTTPS setup already?

benoitf · 2018-03-02T09:14:10Z

in che 5.x we had a template so users could specify through a property the behavior on how subdomains were concatenated (like if some ppl wanted -or _)
Adding a property is fine for me but I would give a greater choice of possibility to che6 as for che5
@mshaposhnik

hkolvenbach · 2018-03-02T09:38:25Z

@benoitf like the idea, this would be even easier to implement. by the way: is there a good reason to use dots as default or should we just switch to _ or -?

benoitf · 2018-03-02T09:40:02Z

@hkolvenbach I would say that default could be -or _ it was just easy to use .as separators

mshaposhnik · 2018-03-02T12:56:50Z

...ipse/che/workspace/infrastructure/docker/provisioner/labels/SinglePortLabelsProvisioner.java

+    //         ? host.indexOf(externalIpOfContainers)
+    //         : host.indexOf(internalIpOfContainers);
+    // return host.substring(0, idx - 1).replaceAll("\\.", "-");
+    return host.replaceAll("\\.", "-");


We don't want hostnames\IP's in the labels. So need to cut URL either until any IP or first dot in case of custom DNS.

mshaposhnik · 2018-03-02T13:21:23Z

dockerfiles/init/manifests/che.pp

@@ -40,6 +40,8 @@
  #
  $che_single_port = getValue("CHE_SINGLE_PORT","false")
  $che_single_port_wildcard_domain_host = getValue("CHE_SINGLEPORT_WILDCARD__DOMAIN_HOST","nip.io")
+  $che_single_port_custom_dns = getValue("CHE_SINGLEPORT_CUSTOM_DNS","false")


Property name doesnt looks very self-explained to me.. They're all is a DNS. The only difference is it requires explicit IP or not. So i propose something like CHE_SINGLEPORT_WILDCARD__DOMAIN_IPLESS

good suggestion!

maybe we could have done

CHE_SINGLEPORT_DOMAIN_WILDCARD "true/false" CHE_SINGLEPORT_DOMAIN "foo.com" (or xip.io/nip.io, etc)

@benoitf i tried to do it like you described, but i am not sure how to elegantly get the configuration into the SinglePortHostnameBuilder (without passing it as an argument to the SinglePortHostnameBuilder each time), so I opted for the easier method.

mshaposhnik · 2018-03-02T13:22:35Z

...ipse/che/workspace/infrastructure/docker/provisioner/labels/SinglePortLabelsProvisioner.java

@@ -94,10 +95,11 @@ public void provision(DockerEnvironment internalEnv, RuntimeIdentity identity)
   * exec-agent-http-dev-machine-workspaceao6k83hkdav975g5


Now it will be exec-agent-http_dev-machine_workspaceao6k83hkdav975g5

JamesDrummond · 2018-03-02T17:28:41Z

@hkolvenbach I have not :( . #8945 . Would be nice if there is a problem with java not trusting the cert to actually output useful information. Right now I just get broken pipe with no other information to solve the problem. Created custom image to include certs into trust store but has not fixed the problem. Would be great to get step-by-step of getting ocp to work. Would do it myself but need to get it working first ;)

garagatyi

Changing wildcard approach to something more widespread than multi-level wildcard certificates seems a good idea to me. Including default behavior.

On the other hand, adding more properties and options to it will lead, eventually, to a rule-based approach that we had in Che5. So I would opt for implementing something similar to it in Che 6. Since our code has significantly changed in Che 6 it is not easy to move that code from Che 5 and I even think it is better to reimplement it from scratch to avoid legacy baggage from the previous architecture.

hkolvenbach · 2018-03-06T13:35:11Z

@garagatyi I was thinking about porting https://github.com/codenvy/codenvy/blob/61c0113b00eaea22cd20381be6ecbe9dd0da56e0/dockerfiles/init/modules/codenvy/templates/machine.properties.erb#L67 and https://github.com/codenvy/codenvy/blob/fa4422ec95acc09f9daad591fd2f6edc08614028/plugins/plugin-hosted/codenvy-machine-hosted/src/main/java/com/codenvy/machine/UriTemplateServerProxyTransformer.java to Che 6, to be able to rewrite URLs to something like workspace_wsagent_xyz.domain.tld/node_port. Do you think this is a good idea?

Regarding the docker infrastructure, it would probably be better to do this through labeling the exposed ports for traefik, but I couldn't find a starting point in the source code for that. Do you know how exposed ports for wsagent etc are configured? Through labeling we could also treat SSH and HTTP/HTTPS differently.

garagatyi · 2018-03-06T15:15:47Z

@benoitf You have previously created another approach of URLs rewriting. WDYT about the suggested idea?

@mshaposhnik could you help with labeling ports in traefik here?

benoitf · 2018-03-07T08:32:42Z

@garagatyi yes I think than //%5$s/%3$s_%2$s/%4$s is hard to make it evolve like introducing new arguments while with ${server-reference} etc, it's easier to understand what are the new variables without breaking any existing users settings.
So I would prefer using specific variable pattern instead of 1, 2, 3

garagatyi · 2018-03-07T10:05:48Z

@hkolvenbach here are docs about previous (Che 5 specific) configuration of URLs templates https://www.eclipse.org/che/docs/setup/configuration/index.html#workspace-address-resolution-strategy.
An example of configuration is: che.docker.server_evaluation_strategy.custom.template=<serverName>.<machineName>.<workspaceId>.<wildcardNipDomain>:<chePort>
The code that implemented it is outdated since we reworked that area completely. But you can implement something like it.
You could ping us if you have any questions about it. BTW @benoitf was the author of that code

… simplify HTTPS setup Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

…om configuratioN

…__DOMAIN_IPLESS Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

hkolvenbach · 2018-03-12T14:17:51Z

dockerfiles/init/modules/compose/templates/docker-compose.yml.erb

@@ -98,7 +98,7 @@ services:
 <% end -%>
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock
-      - '<%= scope.lookupvar('che::che_instance') -%>/config/traefik/traefik.toml:/etc/traefik/traefik.toml'
+      - '<%= scope.lookupvar('che::che_instance') -%>/config/traefik:/etc/traefik'


this is needed to place https certifactes in the config/traefik folder. not strictly necessary for single-level-subdomains

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

hkolvenbach · 2018-03-12T14:39:55Z

@benoitf @garagatyi @mshaposhnik I updated the pull request. I didn't implement a full URL rewriter, but I think the single-level-subdomain approach will solve the needs at least for HTTPS.

I changed the new ENV variable CHE_SINGLEPORT_WILDCARD__DOMAIN_IPLESS and fixed the code to continue to support nip.io and xip.io. If someone could give me some hints how to best implement @benoitf comment #8983 (comment), i could give it a try, but my feeling is that it would require quite some rewriting effort.

mshaposhnik · 2018-03-12T14:50:45Z

...ipse/che/workspace/infrastructure/docker/provisioner/labels/SinglePortLabelsProvisioner.java

   */
  private String getServiceName(String host) {
    int idx =
        (externalIpOfContainers != null && host.contains(externalIpOfContainers))
            ? host.indexOf(externalIpOfContainers)
            : host.indexOf(internalIpOfContainers);
-    return host.substring(0, idx - 1).replaceAll("\\.", "-");
+    if (idx > 1) {
+      return host.substring(0, idx - 1).replaceAll("\\.", "-");


Please, review logic here again. We dont need to replace dots anymore. Only cut until first dot and throw out any optional IP and wildcard domain.

good point!

mshaposhnik · 2018-03-12T14:50:58Z

Other is ok for me.

skabashnyuk · 2018-03-12T14:53:31Z

pr for docs?

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

hkolvenbach · 2018-03-12T15:14:45Z

created the docs pr here: eclipse-che/che-docs#371

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

riuvshin · 2018-03-14T09:34:25Z

dockerfiles/init/manifests/che.pp

@@ -66,6 +67,7 @@
  $system_super_privileged_mode=getValue("SYSTEM_SUPER__PRIVILEGED__MODE", "false")

  $che_keycloak_admin_require_update_password=getValue("CHE_KEYCLOAK_ADMIN_REQUIRE_UPDATE_PASSWORD", "true")
+  $che_keycloak_auth__server__url=getValue("CHE_KEYCLOAK_AUTH__SERVER__URL","")


I do not see where $che_keycloak_auth__server__url is used, do we need that?

yes, i think this is a leftover

Signed-off-by: Hanno Kolvenbach <hannokolvenbach@gmx.de>

hkolvenbach · 2018-03-14T23:37:10Z

i needed to change the joiner from _ to - because it would generate invalid hostnames otherwise (this becomes a problem if you try to generate a certificate from letsencrypt for it, browsers seem to not care about _ in hostnames

hkolvenbach · 2018-03-14T23:56:51Z

new docs pr: https://github.com/eclipse/che-docs/pull/373/files

garagatyi · 2018-03-30T11:38:02Z

@hkolvenbach is there something that needs to be added to this PR?
@benoitf Are you OK with the PR?

I would want to proceed with the PR to avoid possible issues because of changes from other PRs.
@hkolvenbach can you it one more time?

hkolvenbach · 2018-04-03T09:01:10Z

@garagatyi from my perspective, the PR is ready to be merged. or is there anything else i should do?

garagatyi · 2018-04-03T10:51:40Z

@hkolvenbach I've merged PR. Thank you for contributing this functionality!

hkolvenbach requested review from benoitf, garagatyi and riuvshin as code owners March 2, 2018 09:04

benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Mar 2, 2018

benoitf requested a review from mshaposhnik March 2, 2018 09:14

mshaposhnik reviewed Mar 2, 2018

View reviewed changes

garagatyi reviewed Mar 3, 2018

View reviewed changes

hkolvenbach requested review from ashumilova, azatsarynnyy, dkuleshov, evidolob, skabashnyuk, tolusha and vparfonov as code owners March 12, 2018 14:15

Hanno Kolvenbach and others added 5 commits March 12, 2018 15:16

Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to…

9ebce21

… simplify HTTPS setup Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

reverted to http only for workspace agent

b07961a

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

use whole folder for traefik conf to inject SSL certificates and cust…

bebcda9

…om configuratioN

renamed CHE_SINGLEPORT_WILDCARD_CUSTOM_DNS to CHE_SINGLEPORT_WILDCARD…

ca031cf

…__DOMAIN_IPLESS Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

added CHE_SINGLEPORT_WILDCARD__DOMAIN_IPLESS to che.env

e38d42a

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

hkolvenbach force-pushed the kolvenbach.single-level-subdomains branch from 28caef1 to 07e8d49 Compare March 12, 2018 14:16

hkolvenbach commented Mar 12, 2018

View reviewed changes

removed unnecessary code

6688eec

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

hkolvenbach force-pushed the kolvenbach.single-level-subdomains branch from d95383c to 6688eec Compare March 12, 2018 14:35

hkolvenbach changed the title ~~WIP: Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to simplify HTTPS setup~~ Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to simplify HTTPS setup Mar 12, 2018

mshaposhnik reviewed Mar 12, 2018

View reviewed changes

changed logic to split at first dot

f7c1f60

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

fixed formatting

bf4c592

Signed-off-by: Hanno Kolvenbach <kolvenbach@silexica.com>

mshaposhnik approved these changes Mar 12, 2018

View reviewed changes

skabashnyuk approved these changes Mar 13, 2018

View reviewed changes

riuvshin reviewed Mar 14, 2018

View reviewed changes

hkolvenbach added 2 commits March 14, 2018 23:00

removed unnecessary variable

5db0bfd

change _ to - to adhere to valid hostnames

7ee2e79

Signed-off-by: Hanno Kolvenbach <hannokolvenbach@gmx.de>

garagatyi approved these changes Mar 16, 2018

View reviewed changes

benoitf added the kind/enhancement A feature request - must adhere to the feature request template. label Mar 16, 2018

riuvshin approved these changes Mar 16, 2018

View reviewed changes

benoitf approved these changes Mar 30, 2018

View reviewed changes

garagatyi merged commit 3d841d5 into eclipse-che:master Apr 3, 2018

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Apr 3, 2018

benoitf added this to the 6.4.0 milestone Apr 3, 2018

garagatyi mentioned this pull request Apr 3, 2018

Fix failing tests in docker infrastructure #9320

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to simplify HTTPS setup #8983

Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to simplify HTTPS setup #8983

hkolvenbach commented Mar 2, 2018 •

edited

hkolvenbach commented Mar 2, 2018

benoitf commented Mar 2, 2018

hkolvenbach commented Mar 2, 2018

benoitf commented Mar 2, 2018

mshaposhnik Mar 2, 2018 •

edited

hkolvenbach Mar 12, 2018

mshaposhnik Mar 2, 2018 •

edited

hkolvenbach Mar 2, 2018

benoitf Mar 2, 2018

hkolvenbach Mar 12, 2018

mshaposhnik Mar 2, 2018

hkolvenbach Mar 12, 2018

JamesDrummond commented Mar 2, 2018 •

edited

garagatyi left a comment

hkolvenbach commented Mar 6, 2018 •

edited

garagatyi commented Mar 6, 2018

benoitf commented Mar 7, 2018

garagatyi commented Mar 7, 2018

hkolvenbach Mar 12, 2018

hkolvenbach commented Mar 12, 2018

mshaposhnik Mar 12, 2018

hkolvenbach Mar 12, 2018

mshaposhnik commented Mar 12, 2018

skabashnyuk commented Mar 12, 2018

hkolvenbach commented Mar 12, 2018

riuvshin Mar 14, 2018

hkolvenbach Mar 14, 2018

hkolvenbach commented Mar 14, 2018

hkolvenbach commented Mar 14, 2018

garagatyi commented Mar 30, 2018

hkolvenbach commented Apr 3, 2018

garagatyi commented Apr 3, 2018

		@@ -94,10 +95,11 @@ public void provision(DockerEnvironment internalEnv, RuntimeIdentity identity)
		* exec-agent-http-dev-machine-workspaceao6k83hkdav975g5

Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to simplify HTTPS setup #8983

Enable single-level DNS domains (*.domain.tld) in SINGLE_PORT mode to simplify HTTPS setup #8983

Conversation

hkolvenbach commented Mar 2, 2018 • edited

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

hkolvenbach commented Mar 2, 2018

benoitf commented Mar 2, 2018

hkolvenbach commented Mar 2, 2018

benoitf commented Mar 2, 2018

mshaposhnik Mar 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshaposhnik Mar 2, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesDrummond commented Mar 2, 2018 • edited

garagatyi left a comment

Choose a reason for hiding this comment

hkolvenbach commented Mar 6, 2018 • edited

garagatyi commented Mar 6, 2018

benoitf commented Mar 7, 2018

garagatyi commented Mar 7, 2018

Choose a reason for hiding this comment

hkolvenbach commented Mar 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshaposhnik commented Mar 12, 2018

skabashnyuk commented Mar 12, 2018

hkolvenbach commented Mar 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hkolvenbach commented Mar 14, 2018

hkolvenbach commented Mar 14, 2018

garagatyi commented Mar 30, 2018

hkolvenbach commented Apr 3, 2018

garagatyi commented Apr 3, 2018

hkolvenbach commented Mar 2, 2018 •

edited

mshaposhnik Mar 2, 2018 •

edited

mshaposhnik Mar 2, 2018 •

edited

JamesDrummond commented Mar 2, 2018 •

edited

hkolvenbach commented Mar 6, 2018 •

edited