Add support for wildcard subdomains #853

spadgett · 2016-11-09T20:19:16Z

Add web console support for creating wildcard routes. Show wildcard routes as *.my.subdomain on the overview and routes pages. Warn when the router has not admitted the wildcard policy.

@jwforres @knobunc @rettori

spadgett · 2016-11-09T20:27:54Z

jwforres · 2016-11-09T20:44:50Z

app/scripts/constants.js


+  // true indicates that none of the routers support wildcard subdomains and
+  // removes the option from the route creation form.
+  DISABLE_WILDCARD_SUBDOMAINS: false,


@knobunc is it better to assume most routers will support this feature by default, or to assume most will not have it?

We default it to false, but I don't know what admins will eventually do. I vote that we keep it false for now.

I vote that we keep it false for now.

Note that the setting disables web console support for wildcards. So leaving it false will enable the feature. Just confirming that's what you mean :)

;^) - btw, called ROUTER_ALLOW_WILDCARD_ROUTES (defaults to false) on the router side to keep you on your toes ... double negatives.

@spadgett still need to switch the defaulting behavior here to match the router, I would probably do ALLOW_WILDCARD_ROUTES=false to match the setting in the router, but i would also be ok with DISABLE_WILDCARD_ROUTES=true for consistency with the other flags in the console. We will need to make sure in the docs that wherever the docs are for configuring the feature in the router that it also says how to configure the feature in the console. @ramr @knobunc do you know if the ansible installer is going to set the router flag?

also make sure this flag is only controlling the creation of these routes. No matter what if we see a wildcard route we should display it correctly

jwforres · 2016-11-09T20:48:42Z

app/scripts/services/applicationGenerator.js

-        route.spec.host = input.routing.host;
+      var host = input.routing.host;
+      if (route.spec.wildcardPolicy === 'Subdomain') {
+        host = 'www.' + input.routing.subdomain;


@knobunc are you ok with 'www' as the default we inject here under the covers

I think you need to let them set it, but then let them set the wildcard flag separately.

I realized that we will expose those names when a router doesn't support wildcards, so we really do have to expose the full names to the end user :-(

jwforres · 2016-11-09T20:51:53Z

app/scripts/services/routes.js

+    // For host "foo.example.com" return "example.com"
+    var getSubdomain = function(route) {
+      var hostname = _.get(route, 'spec.host', '');
+      return hostname.replace(/^[a-z0-9]([-a-z0-9]*[a-z0-9])./, '');


shouldn't that be a \. and not just a . ?

Yes! thanks

jwforres · 2016-11-09T20:52:27Z

app/styles/_core.less

  right: -3px;
 }

-.create-route-icon,


guessing these werent being used anywhere?

guessing these werent being used anywhere?

Right

knobunc · 2016-11-10T15:54:08Z

@ramr It would be helpful if you took a look over the UI for this change. Thanks

spadgett · 2016-11-10T18:23:38Z

Updated based on feedback from design review. /cc @rhamilto @ajacobs21e

If the user types *.example.com, we'll use www.example.com for host and set wildcardPolicy: 'Subdomain'.

@knobunc I understand this isn't ideal if the router doesn't support wildcards. We were wondering if the entire route shouldn't be rejected if the wildcard policy can't be honored. It seems odd that the route is still admitted without the wildcards.

ramr · 2016-11-10T19:03:34Z

@spadgett if the router runs without wildcard support (ROUTER_ALLOW_WILDCARD_ROUTES=false or unset), then the route will get an error - the status would reflect that accordingly. This is what returns the error back:
https://github.com/openshift/origin/blob/master/pkg/cmd/infra/router/router.go#L140

ramr · 2016-11-10T19:05:56Z

@spadgett @knobunc will review this in depth sometime later today.

spadgett · 2016-11-10T19:08:58Z

@ramr This is the YAML I'm seeing. The route is admitted, but wildcard policy doesn't match?

apiVersion: v1
kind: Route
metadata:
  name: myroute
  namespace: demo
  selfLink: /oapi/v1/namespaces/demo/routes/myroute
  uid: e76ac1c9-a778-11e6-b359-080027242396
  resourceVersion: '47863'
  creationTimestamp: '2016-11-10T19:07:20Z'
  labels:
    app: node-ex
spec:
  host: www.mydomain.com
  to:
    kind: Service
    name: node-ex
    weight: 100
  port:
    targetPort: 8080-tcp
  wildcardPolicy: Subdomain
status:
  ingress:
    - host: www.mydomain.com
      routerName: router
      conditions:
        - type: Admitted
          status: 'True'
          lastTransitionTime: '2016-11-10T19:07:20Z'
      wildcardPolicy: None

ramr · 2016-11-10T21:47:08Z

@spadgett looks like you have an older router image. The old router would admit the route in (since it doesn't understand wildcards). There was a discussion on this on: openshift/origin#11201

Anyway, the crux of that discussion is older routers would still admit in the route since they don't understand wildcards. That also allows routes to be backward compatible. For newer routers, since the feature is opt-in if you create a wildcard route and if the policy is not supported - it would reject it.

Updated tried updating the link - how do you add a link containing hash/pound signs in markdown?

ramr · 2016-11-11T01:24:36Z

app/scripts/services/applicationGenerator.js

+      if (host) {
+        if (host.startsWith('*.')) {
+          route.spec.wildcardPolicy = 'Subdomain';
+          route.spec.host = 'www' + host.substring(1);


Just a thought, might be better to use some name other than `www``. We do allow for multiple routes (wildcard + non)to coexist in a single namespace ... and www is more than likely to be a name used there for non-wildcard hosts.

@ramr are you thinking something more like "generated" or "placeholder"

or maybe even "wildcard"

I like wildcard!

ramr · 2016-11-11T01:30:01Z

app/scripts/services/routes.js

        }
+
+        if (wildcardPolicy === 'Subdomain' && ingress.wildcardPolicy !== wildcardPolicy) {
+          warnings.push('Router "' + ingress.routerName + '" does not support wildcard subdomains. Your route will only be available at host ' + ingress.host + '.');


This will work for older routers (pre 1.4.0) which doesn't support wildcard subdomains.
On newer/the latest router (1.4.0) with wildcards disabled (ROUTER_ALLOW_WILDCARD_ROUTES=false or not set), status will be something like:

status: ingress: - conditions: - lastTransitionTime: 2016-11-11T00:51:05Z message: wildcard routes are not allowed reason: RouteNotAdmitted status: "False" type: Admitted host: wild.open.header.test routerName: router wildcardPolicy: Subdomain

Thanks, pulled the latest router image, and I see it now. It looks like we handle the case for both older and newer router images OK.

ramr · 2016-11-11T01:37:33Z

test/spec/services/applicationGeneratorSpec.js

            name: "theServiceName"
          },
          host: "www.example.com",
+          wildcardPolicy: "None",


Might want to also add a test case for wildcardPolicy: "" - though am not sure if the UI would ever see that.

This is an object the web console is generating, so the value will always be set.

ramr · 2016-11-11T01:40:25Z

test/spec/services/routesSpec.js

+        wildcardPolicy: 'None'
+      }];
+      var warnings = RoutesService.getRouteWarnings(route, serviceTemplate);
+      expect(warnings).toEqual(['Router "foo" does not support wildcard subdomains. Your route will only be available at host www.example.com.']);


The error message may be a bit different so you may want to check w/ older/newer router images.

This particular error message is from the web console :)

ajacobs21e · 2016-11-11T19:05:35Z

@spadgett did yesterday's technical discussion change the UI design?

Does changing the second sentence to "Generated if not specified" keep that whole section on one line?

spadgett · 2016-11-11T21:11:58Z

did yesterday's technical discussion change the UI design?

@ajacobs21e No, my current working code is

#853 (comment)

which I think is what we agreed to during the design review?

Does changing the second sentence to "Generated if not specified" keep that whole section on one line?

Depends entirely on screen width. At the widest widths, it doesn't wrap the text even without those changes. At narrower widths, it will wrap the the updated text. We generally use complete sentences in the help text, which is how I interpret the Patternfly guidelines.

https://www.patternfly.org/styles/terminology-and-wording

jwforres · 2016-11-14T14:56:06Z

app/scripts/constants.js


+  // true indicates that none of the routers support wildcard subdomains and
+  // removes the option from the route creation form.
+  DISABLE_WILDCARD_SUBDOMAINS: false,


@spadgett still need to switch the defaulting behavior here to match the router, I would probably do ALLOW_WILDCARD_ROUTES=false to match the setting in the router, but i would also be ok with DISABLE_WILDCARD_ROUTES=true for consistency with the other flags in the console. We will need to make sure in the docs that wherever the docs are for configuring the feature in the router that it also says how to configure the feature in the console. @ramr @knobunc do you know if the ansible installer is going to set the router flag?

also make sure this flag is only controlling the creation of these routes. No matter what if we see a wildcard route we should display it correctly

jwforres · 2016-11-14T15:02:51Z

app/scripts/services/routes.js

          warnings.push(message);
        }
+
+        if (wildcardPolicy === 'Subdomain' && ingress.wildcardPolicy !== wildcardPolicy) {


Add a comment here that this will only show up for older routers that don't reject the route based on wildcard policy.

spadgett · 2016-11-14T20:49:38Z

@jwforres thanks, updated

jwforres · 2016-11-14T21:00:23Z

@spadgett looks like your spec test failed in travis https://travis-ci.org/openshift/origin-web-console/builds/175844167#L1782

spadgett · 2016-11-14T21:02:36Z

looks like your spec test failed in travis

Oops, forgot to update the test for www -> wildcard prefix change

* Allow users to type "*.example.com" for route hostnames to create a wildcard route * Show wildcard routes as "*.example.com" on the overview and elsewhere

spadgett · 2016-11-14T21:02:55Z

Fixed the test.

ajacobs21e · 2016-11-16T16:24:37Z

thanks @spadgett. I'm happy with the UI.

jwforres · 2016-11-17T14:21:15Z

[merge]

openshift-bot · 2016-11-17T14:23:27Z

Evaluated for origin web console merge up to 83ca258

openshift-bot · 2016-11-17T14:23:27Z

[Test]ing while waiting on the merge queue

openshift-bot · 2016-11-17T14:32:21Z

Evaluated for origin web console test up to 83ca258

openshift-bot · 2016-11-17T14:48:42Z

Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/687/) (Base Commit: 3d8c136)

openshift-bot · 2016-11-17T17:39:24Z

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/708/) (Base Commit: 28c791a)

See openshift/origin-web-console#853

spadgett force-pushed the route-wildcards-b branch 2 times, most recently from cba203a to a53a570 Compare November 9, 2016 20:24

jwforres reviewed Nov 9, 2016

View reviewed changes

spadgett force-pushed the route-wildcards-b branch from a53a570 to abd2ed6 Compare November 9, 2016 21:14

spadgett force-pushed the route-wildcards-b branch from b22e2bf to 8c9c697 Compare November 10, 2016 18:51

spadgett force-pushed the route-wildcards-b branch from 8c9c697 to 5401bad Compare November 10, 2016 19:12

ramr reviewed Nov 11, 2016

View reviewed changes

jwforres reviewed Nov 14, 2016

View reviewed changes

spadgett force-pushed the route-wildcards-b branch 4 times, most recently from af66bdf to f76fd34 Compare November 14, 2016 20:47

spadgett changed the title ~~[WIP] Add support for wildcard subdomains~~ Add support for wildcard subdomains Nov 14, 2016

spadgett force-pushed the route-wildcards-b branch from f76fd34 to 81bd340 Compare November 14, 2016 20:49

spadgett force-pushed the route-wildcards-b branch from 81bd340 to 83ca258 Compare November 14, 2016 21:02

Add support for wildcard routes

83ca258

* Allow users to type "*.example.com" for route hostnames to create a wildcard route * Show wildcard routes as "*.example.com" on the overview and elsewhere

jwforres approved these changes Nov 14, 2016

View reviewed changes

jwforres added the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2016

openshift-bot merged commit 26e4d1f into openshift:master Nov 17, 2016

spadgett deleted the route-wildcards-b branch November 17, 2016 17:59

spadgett added a commit to spadgett/openshift-docs that referenced this pull request Jan 3, 2017

Document enabling wildcard routes in the web console

126952d

See openshift/origin-web-console#853

spadgett mentioned this pull request Jan 3, 2017

Document enabling wildcard routes in the web console openshift/openshift-docs#3439

Merged

Add support for wildcard subdomains #853

Add support for wildcard subdomains #853

Uh oh!

Conversation

spadgett commented Nov 9, 2016

Uh oh!

spadgett commented Nov 9, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knobunc commented Nov 10, 2016

Uh oh!

spadgett commented Nov 10, 2016

Uh oh!

ramr commented Nov 10, 2016

Uh oh!

ramr commented Nov 10, 2016

Uh oh!

spadgett commented Nov 10, 2016

Uh oh!

ramr commented Nov 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajacobs21e commented Nov 11, 2016

Uh oh!

spadgett commented Nov 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spadgett commented Nov 14, 2016

Uh oh!

jwforres commented Nov 14, 2016

Uh oh!

spadgett commented Nov 14, 2016

Uh oh!

spadgett commented Nov 14, 2016

Uh oh!

ajacobs21e commented Nov 16, 2016

ramr commented Nov 10, 2016 •

edited

Loading

openshift-bot commented Nov 17, 2016 •

edited

Loading