diff --git a/.changelog/9410.txt b/.changelog/9410.txt new file mode 100644 index 000000000..ff73f8c17 --- /dev/null +++ b/.changelog/9410.txt @@ -0,0 +1,3 @@ +```release-note:bug +ui: ensure namespace is used for node API requests +``` diff --git a/ui/packages/consul-ui/app/adapters/node.js b/ui/packages/consul-ui/app/adapters/node.js index a4e3672c5..33f039938 100644 --- a/ui/packages/consul-ui/app/adapters/node.js +++ b/ui/packages/consul-ui/app/adapters/node.js @@ -2,17 +2,27 @@ import Adapter from './application'; // TODO: Update to use this.formatDatacenter() +// Node and Namespaces are a little strange in that Nodes don't belong in a +// namespace whereas things that belong to a Node do (Health Checks and +// Service Instances). So even though Nodes themselves don't require a +// namespace filter, you sill needs to pass the namespace through to API +// requests in order to get the correct information for the things that belong +// to the node. + export default class NodeAdapter extends Adapter { - requestForQuery(request, { dc, index, id, uri }) { + requestForQuery(request, { dc, ns, index, id, uri }) { return request` GET /v1/internal/ui/nodes?${{ dc }} X-Request-ID: ${uri} - ${{ index }} + ${{ + ...this.formatNspace(ns), + index, + }} `; } - requestForQueryRecord(request, { dc, index, id, uri }) { + requestForQueryRecord(request, { dc, ns, index, id, uri }) { if (typeof id === 'undefined') { throw new Error('You must specify an id'); } @@ -20,7 +30,10 @@ export default class NodeAdapter extends Adapter { GET /v1/internal/ui/node/${id}?${{ dc }} X-Request-ID: ${uri} - ${{ index }} + ${{ + ...this.formatNspace(ns), + index, + }} `; } diff --git a/ui/packages/consul-ui/app/components/consul/node/list/index.hbs b/ui/packages/consul-ui/app/components/consul/node/list/index.hbs index a7a9f871b..c568346d8 100644 --- a/ui/packages/consul-ui/app/components/consul/node/list/index.hbs +++ b/ui/packages/consul-ui/app/components/consul/node/list/index.hbs @@ -29,11 +29,9 @@ as |item index|> {{#if (eq item.Address @leader.Address)}} Leader {{/if}} - {{#if (gt item.Services.length 0)}} - {{item.Services.length}} {{pluralize item.Services.length 'Service' without-count=true}} + {{format-number item.Services.length}} {{pluralize item.Services.length 'Service' without-count=true}} - {{/if}}
Address diff --git a/ui/packages/consul-ui/app/routes/dc/nodes/index.js b/ui/packages/consul-ui/app/routes/dc/nodes/index.js index 932ca87d8..fea305f73 100644 --- a/ui/packages/consul-ui/app/routes/dc/nodes/index.js +++ b/ui/packages/consul-ui/app/routes/dc/nodes/index.js @@ -20,7 +20,7 @@ export default class IndexRoute extends Route { model(params) { const dc = this.modelFor('dc').dc.Name; - const nspace = '*'; + const nspace = this.modelFor('nspace').nspace.substr(1); return hash({ items: this.data.source(uri => uri`/${nspace}/${dc}/nodes`), leader: this.data.source(uri => uri`/${nspace}/${dc}/leader`), diff --git a/ui/packages/consul-ui/app/templates/dc/nodes/show/services.hbs b/ui/packages/consul-ui/app/templates/dc/nodes/show/services.hbs index 52a8f3933..3f2c003f8 100644 --- a/ui/packages/consul-ui/app/templates/dc/nodes/show/services.hbs +++ b/ui/packages/consul-ui/app/templates/dc/nodes/show/services.hbs @@ -35,7 +35,7 @@ @sort={{sort}} @filters={{filters}} @search={{search}} - @items={{reject-by 'Service.Kind' 'connect-proxy' items}} + @items={{items}} as |collection|>

- There are no services. + This node has no service instances{{#if (gt items.length 0)}} matching that search{{/if}}.

diff --git a/ui/packages/consul-ui/tests/acceptance/page-navigation.feature b/ui/packages/consul-ui/tests/acceptance/page-navigation.feature index 614d841a1..eef74e1da 100644 --- a/ui/packages/consul-ui/tests/acceptance/page-navigation.feature +++ b/ui/packages/consul-ui/tests/acceptance/page-navigation.feature @@ -23,7 +23,7 @@ Feature: page-navigation Where: ------------------------------------------------------------------------------------- | Link | URL | Endpoint | - | nodes | /dc-1/nodes | /v1/internal/ui/nodes?dc=dc-1 | + | nodes | /dc-1/nodes | /v1/internal/ui/nodes?dc=dc-1&ns=@namespace | | kvs | /dc-1/kv | /v1/kv/?keys&dc=dc-1&separator=%2F&ns=@namespace | | acls | /dc-1/acls/tokens | /v1/acl/tokens?dc=dc-1&ns=@namespace | # | settings | /settings | /v1/catalog/datacenters | @@ -39,11 +39,10 @@ Feature: page-navigation And I click "[data-test-back]" Then the url should be [Back] Where: - ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- - | Item | Model | URL | Endpoint | Back | - | kv | kvs | /dc-1/kv/0-key-value/edit | /v1/session/info/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=dc-1&ns=@namespace | /dc-1/kv | - # | acl | acls | /dc-1/acls/anonymous | /v1/acl/info/anonymous?dc=dc-1 | /dc-1/acls | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + ------------------------------------------------------------------------------------------------------------------------------------- + | Item | Model | URL | Endpoint | Back | + | kv | kvs | /dc-1/kv/0-key-value/edit | /v1/session/info/ee52203d-989f-4f7a-ab5a-2bef004164ca?dc=dc-1&ns=@namespace | /dc-1/kv | + ------------------------------------------------------------------------------------------------------------------------------------- Scenario: The node detail page calls the correct API endpoints When I visit the node page for yaml --- @@ -55,7 +54,7 @@ Feature: page-navigation --- - /v1/catalog/datacenters - /v1/namespaces - - /v1/internal/ui/node/node-0?dc=dc-1 + - /v1/internal/ui/node/node-0?dc=dc-1&ns=@namespace - /v1/coordinate/nodes?dc=dc-1 --- Scenario: The kv detail page calls the correct API endpoints diff --git a/ui/packages/consul-ui/tests/integration/adapters/node-test.js b/ui/packages/consul-ui/tests/integration/adapters/node-test.js index bc9bb45f4..b718e984f 100644 --- a/ui/packages/consul-ui/tests/integration/adapters/node-test.js +++ b/ui/packages/consul-ui/tests/integration/adapters/node-test.js @@ -1,28 +1,42 @@ import { module, test } from 'qunit'; import { setupTest } from 'ember-qunit'; +import { env } from '../../../env'; +const shouldHaveNspace = function(nspace) { + return typeof nspace !== 'undefined' && env('CONSUL_NSPACES_ENABLED'); +}; module('Integration | Adapter | node', function(hooks) { setupTest(hooks); const dc = 'dc-1'; const id = 'node-name'; - test('requestForQuery returns the correct url', function(assert) { - const adapter = this.owner.lookup('adapter:node'); - const client = this.owner.lookup('service:client/http'); - const expected = `GET /v1/internal/ui/nodes?dc=${dc}`; - const actual = adapter.requestForQuery(client.requestParams.bind(client), { - dc: dc, + const undefinedNspace = 'default'; + [undefinedNspace, 'team-1', undefined].forEach(nspace => { + test(`requestForQuery returns the correct url when nspace is ${nspace}`, function(assert) { + const adapter = this.owner.lookup('adapter:node'); + const client = this.owner.lookup('service:client/http'); + const expected = `GET /v1/internal/ui/nodes?dc=${dc}${ + shouldHaveNspace(nspace) ? `&ns=${nspace}` : `` + }`; + const actual = adapter.requestForQuery(client.requestParams.bind(client), { + dc: dc, + ns: nspace, + }); + assert.equal(`${actual.method} ${actual.url}`, expected); }); - assert.equal(`${actual.method} ${actual.url}`, expected); - }); - test('requestForQueryRecord returns the correct url', function(assert) { - const adapter = this.owner.lookup('adapter:node'); - const client = this.owner.lookup('service:client/http'); - const expected = `GET /v1/internal/ui/node/${id}?dc=${dc}`; - const actual = adapter.requestForQueryRecord(client.requestParams.bind(client), { - dc: dc, - id: id, + test(`requestForQueryRecord returns the correct url when the nspace is ${nspace}`, function(assert) { + const adapter = this.owner.lookup('adapter:node'); + const client = this.owner.lookup('service:client/http'); + const expected = `GET /v1/internal/ui/node/${id}?dc=${dc}${ + shouldHaveNspace(nspace) ? `&ns=${nspace}` : `` + }`; + const actual = adapter.requestForQueryRecord(client.requestParams.bind(client), { + dc: dc, + id: id, + ns: nspace, + }); + assert.equal(`${actual.method} ${actual.url}`, expected); }); - assert.equal(`${actual.method} ${actual.url}`, expected); }); + // the following don't require nspacing test("requestForQueryRecord throws if you don't specify an id", function(assert) { const adapter = this.owner.lookup('adapter:node'); const client = this.owner.lookup('service:client/http');