UI: Add forking based on service instance id existence (#5392)

* ui: Add forking based on service instance id existence

Proxies come in 2 flavours, 'normal' and sidecar. We know when a proxy
is a sidecar proxy based on whether a DestinationServiceID is set.

LocalServiceAddress and LocalServicePort are only relevant for sidecar
proxies.

This adds template logic to show different text depending on this
information.

Additionally adds test around connect proxies (#5418)

1. Adds page object for the instance detail page
2. Adds further scenario steps used in the tests
3. Adds acceptance testing around the instance detail page. Services
with proxies and the sidecar proxies and proxies themselves
4. Adds datacenter column for upstreams
5. Fixes bug routing bug for decision as to whether to request proxy
information or not

ui-v2/app/models/proxy.js

@@ -8,5 +8,5 @@ export default Model.extend({
   [SLUG_KEY]: attr('string'),
   ServiceName: attr('string'),
   ServiceID: attr('string'),
-  ServiceProxyDestination: attr('string'),
+  ServiceProxy: attr(),
 });

ui-v2/app/routes/dc/services/instance.js

@@ -15,9 +15,9 @@ export default Route.extend({
     }).then(function(model) {
       return hash({
         proxy:
-          get(service, 'Kind') !== 'connect-proxy'
+          get(model.item, 'Kind') === 'connect-proxy'
-            ? proxyRepo.findInstanceBySlug(params.id, params.name, dc)
+            ? null
-            : null,
+            : proxyRepo.findInstanceBySlug(params.id, params.name, dc),
         ...model,
       });
     });

ui-v2/app/services/repository/proxy.js

@@ -22,7 +22,11 @@ export default RepositoryService.extend({
   findInstanceBySlug: function(id, slug, dc, configuration) {
     return this.findAllBySlug(slug, dc, configuration).then(function(items) {
       if (get(items, 'length') > 0) {
-        const instance = items.findBy('ServiceProxyDestination', id);
+        let instance = items.findBy('ServiceProxy.DestinationServiceID', id);
+        if (instance) {
+          return instance;
+        }
+        instance = items.findBy('ServiceProxy.DestinationServiceName', slug);
         if (instance) {
           return instance;
         }

ui-v2/app/services/repository/service.js

@@ -35,7 +35,15 @@ export default RepositoryService.extend({
         });
         return service;
       }
-      // TODO: probably need to throw a 404 here?
+      // TODO: Add an store.error("404", "message") or similar
+      const e = new Error();
+      e.errors = [
+        {
+          status: '404',
+          title: 'Unable to find instance',
+        },
+      ];
+      throw e;
     });
   },
 });

ui-v2/app/styles/components/tabular-collection.scss

@@ -181,5 +181,5 @@ html.template-node.template-show main table.sessions tr {
   width: calc(100% / 5);
 }
 %upstreams-row > * {
-  width: calc(100% / 3);
+  width: calc(100% / 4);
 }

ui-v2/app/templates/components/healthcheck-list.hbs

@@ -1,4 +1,4 @@
-<ul data-test-node-healthchecks>
+<ul data-test-healthchecks>
 {{#each (sort-by (action 'sortChecksByImportance') items) as |check| }}
   {{healthcheck-output data-test-node-healthcheck=check.Name tagName='li' name=check.Name class=check.Status status=check.Status notes=check.Notes output=check.Output}}
 {{/each}}

ui-v2/app/templates/dc/services/-upstreams.hbs

@@ -4,19 +4,23 @@
     items=item.Proxy.Upstreams as |item index|
 }}
     {{#block-slot 'header'}}
-      <th>Destination Name</th>
+      <th>Upstream</th>
-      <th>Destination Type</th>
+      <th>Datacenter</th>
-      <th>Local Bind Port</th>
+      <th>Type</th>
+      <th>Local Bind Address</th>
     {{/block-slot}}
     {{#block-slot 'row'}}
-      <td data-test-destination-name>
+      <td>
-        <a>{{item.DestinationName}}</a>
+        <a data-test-destination-name>{{item.DestinationName}}</a>
+      </td>
+      <td data-test-destination-datacenter>
+        {{item.Datacenter}}
       </td>
       <td data-test-destination-type>
           {{item.DestinationType}}
       </td>
-      <td data-test-local-bind-port>
+      <td data-test-local-bind-address>
-          {{item.LocalBindPort}}
+        {{item.LocalBindAddress}}:{{item.LocalBindPort}}
       </td>
     {{/block-slot}}
 {{/tabular-collection}}

ui-v2/app/templates/dc/services/instance.hbs

@@ -27,19 +27,26 @@
       </dl>
 {{#if proxy}}
       <dl>
-        <dt>Sidecar Proxy</dt>
+        <dt data-test-proxy-type="{{if proxy.ServiceProxy.DestinationServiceID "sidecar-proxy" "proxy"}}">{{if proxy.ServiceProxy.DestinationServiceID "Sidecar " ""}}Proxy</dt>
         <dd><a href="{{href-to 'dc.services.instance' proxy.ServiceName proxy.ServiceID}}">{{proxy.ServiceID}}</a></dd>
       </dl>
 {{/if}}
 {{#if (eq item.Kind 'connect-proxy')}}
+  {{#if item.Proxy.DestinationServiceID}}
       <dl>
-        <dt>Dest. Service Instance</dt>
+        <dt data-test-proxy-destination="instance">Dest. Service Instance</dt>
         <dd><a href="{{href-to 'dc.services.instance' item.Proxy.DestinationServiceName item.Proxy.DestinationServiceID}}">{{item.Proxy.DestinationServiceID}}</a></dd>
       </dl>
       <dl>
         <dt>Local Service Address</dt>
         <dd>{{item.Proxy.LocalServiceAddress}}:{{item.Proxy.LocalServicePort}}</dd>
       </dl>
+  {{else}}
+      <dl>
+        <dt data-test-proxy-destination="service">Dest. Service</dt>
+        <dd><a href="{{href-to 'dc.services.show' item.Proxy.DestinationServiceName}}">{{item.Proxy.DestinationServiceName}}</a></dd>
+      </dl>
+  {{/if}}
 {{/if}}
     {{/block-slot}}
     {{#block-slot 'content'}}

ui-v2/tests/acceptance/dc/services/instances/error.feature

@@ -0,0 +1,15 @@
+@setupApplicationTest
+Feature: dc / services / instances / error: Visit Service Instance what doesn't exist
+  Scenario: No instance can be found in the API response
+    Given 1 datacenter model with the value "dc1"
+    And 1 service model
+    When I visit the instance page for yaml
+    ---
+      dc: dc1
+      service: service-0
+      id: id-that-doesnt-exist
+    ---
+    Then the url should be /dc1/services/service-0/id-that-doesnt-exist
+    And I see the text "404 (Unable to find instance)" in "[data-test-error]"
+
+

ui-v2/tests/acceptance/dc/services/instances/proxy.feature

@@ -0,0 +1,48 @@
+@setupApplicationTest
+Feature: dc / services / instances / proxy: Show Proxy Service Instance
+  Scenario: A Proxy Service instance
+    Given 1 datacenter model with the value "dc1"
+    And 1 service model from yaml
+    ---
+    - Service:
+        Kind: connect-proxy
+        Name: service-0-proxy
+        ID: service-0-proxy-with-id
+        Proxy:
+          DestinationServiceName: service-0
+          Upstreams:
+            - DestinationType: service
+              DestinationName: service-1
+              LocalBindAddress: 127.0.0.1
+              LocalBindPort: 1111
+            - DestinationType: prepared_query
+              DestinationName: service-group
+              LocalBindAddress: 127.0.0.1
+              LocalBindPort: 1112
+    ---
+    When I visit the instance page for yaml
+    ---
+      dc: dc1
+      service: service-0-proxy
+      id: service-0-proxy-with-id
+    ---
+    Then the url should be /dc1/services/service-0-proxy/service-0-proxy-with-id
+    And I see destination on the proxy like "service"
+
+    And I see serviceChecksIsSelected on the tabs
+
+    When I click upstreams on the tabs
+    And I see upstreamsIsSelected on the tabs
+    And I see 2 of the upstreams object
+    And I see name on the upstreams like yaml
+    ---
+    - service-1
+    - service-group
+    ---
+    And I see type on the upstreams like yaml
+    ---
+    - service
+    - prepared_query
+    ---
+
+

ui-v2/tests/acceptance/dc/services/instances/show.feature

@@ -0,0 +1,68 @@
+@setupApplicationTest
+Feature: dc / services / instances / show: Show Service Instance
+  Scenario: A Service instance has no Proxy
+    Given 1 datacenter model with the value "dc1"
+    And 1 service model from yaml
+    ---
+    - Service:
+        ID: service-0-with-id
+        Tags: ['Tag1', 'Tag2']
+        Meta:
+          external-source: nomad
+      Checks:
+        - Name: Service check
+          ServiceID: service-0
+          Output: Output of check
+          Status: passing
+        - Name: Service check
+          ServiceID: service-0
+          Output: Output of check
+          Status: warning
+        - Name: Service check
+          ServiceID: service-0
+          Output: Output of check
+          Status: critical
+        - Name: Node check
+          ServiceID: ""
+          Output: Output of check
+          Status: passing
+        - Name: Node check
+          ServiceID: ""
+          Output: Output of check
+          Status: warning
+        - Name: Node check
+          ServiceID: ""
+          Output: Output of check
+          Status: critical
+    ---
+    And 1 proxy model from yaml
+    ---
+    - ServiceProxy:
+        DestinationServiceName: service-1
+        DestinationServiceID: ~
+    ---
+    When I visit the instance page for yaml
+    ---
+      dc: dc1
+      service: service-0
+      id: service-0-with-id
+    ---
+    Then the url should be /dc1/services/service-0/service-0-with-id
+    Then I don't see type on the proxy
+
+    Then I see externalSource like "nomad"
+
+    And I don't see upstreams on the tabs
+    And I see serviceChecksIsSelected on the tabs
+    And I see 3 of the serviceChecks object
+
+    When I click nodeChecks on the tabs
+    And I see nodeChecksIsSelected on the tabs
+    And I see 3 of the nodeChecks object
+
+    When I click tags on the tabs
+    And I see tagsIsSelected on the tabs
+
+    Then I see the text "Tag1" in "[data-test-tags] span:nth-child(1)"
+    Then I see the text "Tag2" in "[data-test-tags] span:nth-child(2)"
+

ui-v2/tests/acceptance/dc/services/instances/sidecar-proxy.feature

@@ -0,0 +1,29 @@
+@setupApplicationTest
+Feature: dc / services / instances / sidecar-proxy: Show Sidecar Proxy Service Instance
+  Scenario: A Sidecar Proxy Service instance
+    Given 1 datacenter model with the value "dc1"
+    And 1 service model from yaml
+    ---
+    - Service:
+        Kind: connect-proxy
+        Name: service-0-sidecar-proxy
+        ID: service-0-sidecar-proxy-with-id
+        Proxy:
+          DestinationServiceName: service-0
+          DestinationServiceID: service-0-with-id
+    ---
+    When I visit the instance page for yaml
+    ---
+      dc: dc1
+      service: service-0-sidecar-proxy
+      id: service-0-sidecar-proxy-with-id
+    ---
+    Then the url should be /dc1/services/service-0-sidecar-proxy/service-0-sidecar-proxy-with-id
+    And I see destination on the proxy like "instance"
+
+    And I see serviceChecksIsSelected on the tabs
+
+    When I click upstreams on the tabs
+    And I see upstreamsIsSelected on the tabs
+
+

ui-v2/tests/acceptance/dc/services/instances/with-proxy.feature

@@ -0,0 +1,20 @@
+@setupApplicationTest
+Feature: dc / services / instances / with-proxy: Show Service Instance with a proxy
+  Scenario: A Service instance has a Proxy (no DestinationServiceID)
+    Given 1 datacenter model with the value "dc1"
+    And 1 proxy model from yaml
+    ---
+    - ServiceProxy:
+        DestinationServiceID: ~
+    ---
+    When I visit the instance page for yaml
+    ---
+      dc: dc1
+      service: service-0
+      id: service-0-with-id
+    ---
+    Then the url should be /dc1/services/service-0/service-0-with-id
+    And I see type on the proxy like "proxy"
+
+    And I see serviceChecksIsSelected on the tabs
+    And I don't see upstreams on the tabs

ui-v2/tests/acceptance/dc/services/instances/with-sidecar.feature

@@ -0,0 +1,22 @@
+@setupApplicationTest
+Feature: dc / services / instances / with-sidecar: Show Service Instance with a Sidecar Proxy
+  Scenario: A Service instance has a Sidecar Proxy (a DestinationServiceID)
+    Given 1 datacenter model with the value "dc1"
+    And 1 proxy model from yaml
+    ---
+    - ServiceProxy:
+        DestinationServiceID: service-1
+    ---
+    When I visit the instance page for yaml
+    ---
+      dc: dc1
+      service: service-0
+      id: service-0-with-id
+    ---
+    Then the url should be /dc1/services/service-0/service-0-with-id
+    And I see type on the proxy like "sidecar-proxy"
+
+    And I see serviceChecksIsSelected on the tabs
+    And I don't see upstreams on the tabs
+
+

ui-v2/tests/acceptance/steps/dc/services/instances/error-steps.js

@@ -0,0 +1,10 @@
+import steps from '../../../steps';
+
+// step definitions that are shared between features should be moved to the
+// tests/acceptance/steps/steps.js file
+
+export default function(assert) {
+  return steps(assert).then('I should find a file', function() {
+    assert.ok(true, this.step);
+  });
+}

ui-v2/tests/acceptance/steps/dc/services/instances/proxy-steps.js

@@ -0,0 +1,10 @@
+import steps from '../../../steps';
+
+// step definitions that are shared between features should be moved to the
+// tests/acceptance/steps/steps.js file
+
+export default function(assert) {
+  return steps(assert).then('I should find a file', function() {
+    assert.ok(true, this.step);
+  });
+}

ui-v2/tests/acceptance/steps/dc/services/instances/show-steps.js

@@ -0,0 +1,10 @@
+import steps from '../../../steps';
+
+// step definitions that are shared between features should be moved to the
+// tests/acceptance/steps/steps.js file
+
+export default function(assert) {
+  return steps(assert).then('I should find a file', function() {
+    assert.ok(true, this.step);
+  });
+}

ui-v2/tests/acceptance/steps/dc/services/instances/sidecar-proxy-steps.js

@@ -0,0 +1,10 @@
+import steps from '../../../steps';
+
+// step definitions that are shared between features should be moved to the
+// tests/acceptance/steps/steps.js file
+
+export default function(assert) {
+  return steps(assert).then('I should find a file', function() {
+    assert.ok(true, this.step);
+  });
+}

ui-v2/tests/acceptance/steps/dc/services/instances/with-proxy-steps.js

@@ -0,0 +1,10 @@
+import steps from '../../../steps';
+
+// step definitions that are shared between features should be moved to the
+// tests/acceptance/steps/steps.js file
+
+export default function(assert) {
+  return steps(assert).then('I should find a file', function() {
+    assert.ok(true, this.step);
+  });
+}

ui-v2/tests/acceptance/steps/dc/services/instances/with-sidecar-steps.js

@@ -0,0 +1,10 @@
+import steps from '../../../steps';
+
+// step definitions that are shared between features should be moved to the
+// tests/acceptance/steps/steps.js file
+
+export default function(assert) {
+  return steps(assert).then('I should find a file', function() {
+    assert.ok(true, this.step);
+  });
+}

ui-v2/tests/helpers/type-to-url.js

@@ -7,6 +7,9 @@ export default function(type) {
     case 'service':
       requests = ['/v1/internal/ui/services', '/v1/health/service/'];
       break;
+    case 'proxy':
+      requests = ['/v1/catalog/connect'];
+      break;
     case 'node':
       requests = ['/v1/internal/ui/nodes', '/v1/internal/ui/node/'];
       break;

ui-v2/tests/pages.js

@@ -19,6 +19,7 @@ import dcs from 'consul-ui/tests/pages/dc';
 import settings from 'consul-ui/tests/pages/settings';
 import services from 'consul-ui/tests/pages/dc/services/index';
 import service from 'consul-ui/tests/pages/dc/services/show';
+import instance from 'consul-ui/tests/pages/dc/services/instance';
 import nodes from 'consul-ui/tests/pages/dc/nodes/index';
 import node from 'consul-ui/tests/pages/dc/nodes/show';
 import kvs from 'consul-ui/tests/pages/dc/kv/index';
@@ -41,6 +42,7 @@ export default {
   dcs: create(dcs(visitable, clickable, attribute, collection)),
   services: create(services(visitable, clickable, attribute, collection, page, catalogFilter)),
   service: create(service(visitable, attribute, collection, text, catalogFilter)),
+  instance: create(instance(visitable, attribute, collection, text, radiogroup)),
   nodes: create(nodes(visitable, clickable, attribute, collection, catalogFilter)),
   node: create(node(visitable, deletable, clickable, attribute, collection, radiogroup)),
   kvs: create(kvs(visitable, deletable, creatable, clickable, attribute, collection)),

ui-v2/tests/pages/dc/services/instance.js

@@ -0,0 +1,19 @@
+export default function(visitable, attribute, collection, text, radiogroup) {
+  return {
+    visit: visitable('/:dc/services/:service/:id'),
+    externalSource: attribute('data-test-external-source', 'h1 span'),
+    tabs: radiogroup('tab', ['service-checks', 'node-checks', 'upstreams', 'tags']),
+    serviceChecks: collection('#service-checks [data-test-healthchecks] li', {}),
+    nodeChecks: collection('#node-checks [data-test-healthchecks] li', {}),
+    upstreams: collection('#upstreams [data-test-tabular-row]', {
+      name: text('[data-test-destination-name]'),
+      datacenter: text('[data-test-destination-datacenter]'),
+      type: text('[data-test-destination-type]'),
+      address: text('[data-test-local-bind-address]'),
+    }),
+    proxy: {
+      type: attribute('data-test-proxy-type', '[data-test-proxy-type]'),
+      destination: attribute('data-test-proxy-destination', '[data-test-proxy-destination]'),
+    },
+  };
+}

ui-v2/tests/steps/assertions/dom.js

@@ -6,6 +6,12 @@ export default function(scenario, assert, find, currentURL) {
         `Expected to see "${text}" in "${selector}"`
       );
     })
+    .then(['I see the exact text "$text" in "$selector"'], function(text, selector) {
+      assert.ok(
+        find(selector).textContent.trim() === text,
+        `Expected to see the exact "${text}" in "${selector}"`
+      );
+    })
     // TODO: Think of better language
     // TODO: These should be mergeable
     .then(['"$selector" has the "$class" class'], function(selector, cls) {

ui-v2/tests/steps/assertions/model.js

@@ -20,10 +20,7 @@ export default function(scenario, assert, currentPage, pluralize) {
         }, 100);
       });
     })
-    .then(['I see $num $model', 'I see $num $model model', 'I see $num $model models'], function(
+    .then(['I see $num $model model[s]?'], function(num, model) {
-      num,
-      model
-    ) {
       const len = currentPage()[pluralize(model)].filter(function(item) {
         return item.isVisible;
       }).length;

ui-v2/tests/steps/assertions/page.js

@@ -56,6 +56,13 @@ export default function(scenario, assert, currentPage) {
       }
       assert.ok(_component[property], `Expected to see ${property} on ${component}`);
     })
+    .then(['I see $num of the $component object'], function(num, component) {
+      assert.equal(
+        currentPage()[component].length,
+        num,
+        `Expected to see ${num} items in the ${component} object`
+      );
+    })
     .then(["I don't see $property on the $component"], function(property, component) {
       // Collection
       var obj;
@@ -64,9 +71,9 @@ export default function(scenario, assert, currentPage) {
       } else {
         obj = currentPage()[component];
       }
-      const func = obj[property].bind(obj);
       assert.throws(
         function() {
+          const func = obj[property].bind(obj);
           func();
         },
         function(e) {
@@ -89,11 +96,20 @@ export default function(scenario, assert, currentPage) {
     .then(['I see $property'], function(property) {
       assert.ok(currentPage()[property], `Expected to see ${property}`);
     })
-    .then(['I see $property like "$value"'], function(property, value) {
+    .then(['I see $property on the $component like "$value"'], function(
+      property,
+      component,
+      value
+    ) {
+      const target = currentPage()[component][property];
       assert.equal(
-        currentPage()[property],
+        target,
         value,
-        `Expected to see ${property}, was ${currentPage()[property]}`
+        `Expected to see ${property} on ${component} as ${value}, was ${target}`
       );
+    })
+    .then(['I see $property like "$value"'], function(property, value) {
+      const target = currentPage()[property];
+      assert.equal(target, value, `Expected to see ${property} as ${value}, was ${target}`);
     });
 }

ui-v2/yarn.lock

@@ -690,9 +690,9 @@
     js-yaml "^3.10.0"
 
 "@hashicorp/consul-api-double@^2.0.1":
-  version "2.1.0"
+  version "2.2.0"
-  resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-2.1.0.tgz#511e6a48842ad31133e2070f3b2307568539b10e"
+  resolved "https://registry.yarnpkg.com/@hashicorp/consul-api-double/-/consul-api-double-2.2.0.tgz#b72b086dd4c88485f83b6f2fe30c5cb45b8d0e6a"
-  integrity sha512-cyW7TiKQylrWzVUORT1e6m4SU8tQ1V5BYEKW2th7QwHP8OFazn/+om9hud/9X5YtjEuSPIQCmFIvhEVwZgLVpQ==
+  integrity sha512-9U+pqdBJn/ZruUYg7J2A3k9FS44FZQsMGBfKQSxaBRU50dgkpGLtOyOOFUqgi/RCAwn4GUOzgXgHRRs5TAeStg==
 
 "@hashicorp/ember-cli-api-double@^1.3.0":
   version "1.7.0"