diff --git a/.changelog/10503.txt b/.changelog/10503.txt new file mode 100644 index 000000000..6614be2cd --- /dev/null +++ b/.changelog/10503.txt @@ -0,0 +1,4 @@ +```release-note:bug +ui: Use the token's namespace instead of the default namespace when not +specifying a namespace in the URL +``` diff --git a/ui/packages/consul-ui/app/components/auth-form/index.hbs b/ui/packages/consul-ui/app/components/auth-form/index.hbs index 9b47405fd..66dddd505 100644 --- a/ui/packages/consul-ui/app/components/auth-form/index.hbs +++ b/ui/packages/consul-ui/app/components/auth-form/index.hbs @@ -75,8 +75,9 @@ Contact your administrator for login credentials. {{#if (env 'CONSUL_SSO_ENABLED')}} + {{!-- This `or` can be completely removed post 1.10 as 1.10 has optional params with default values --}} {{yield}} {{#if isOpen}} {{/if}} diff --git a/ui/packages/consul-ui/app/components/token-source/index.hbs b/ui/packages/consul-ui/app/components/token-source/index.hbs index 711f6704c..e83e5da07 100644 --- a/ui/packages/consul-ui/app/components/token-source/index.hbs +++ b/ui/packages/consul-ui/app/components/token-source/index.hbs @@ -1,6 +1,7 @@ - {{#let (concat '/' (or nspace 'default') '/' dc) as |path|}} + {{!-- This `or` can be completely removed post 1.10 as 1.10 has optional params with default values --}} + {{#let (concat '/' (or nspace '') '/' dc) as |path|}} 1 ? findActiveNspace(app.nspaces, nspace) : app.nspaces.firstObject; // When disabled nspaces is [], so nspace is undefined const permissions = await this.permissionsRepo.findAll({ diff --git a/ui/packages/consul-ui/app/routes/dc/kv/edit.js b/ui/packages/consul-ui/app/routes/dc/kv/edit.js index 34f0a106d..dd16f6961 100644 --- a/ui/packages/consul-ui/app/routes/dc/kv/edit.js +++ b/ui/packages/consul-ui/app/routes/dc/kv/edit.js @@ -21,7 +21,7 @@ export default class EditRoute extends Route { const nspace = this.optionalParams().nspace; return hash({ dc: dc, - nspace: nspace || 'default', + nspace: nspace, parent: typeof key !== 'undefined' ? this.repo.findBySlug({ diff --git a/ui/packages/consul-ui/app/routes/settings.js b/ui/packages/consul-ui/app/routes/settings.js index 6cdf65380..2b7f32f10 100644 --- a/ui/packages/consul-ui/app/routes/settings.js +++ b/ui/packages/consul-ui/app/routes/settings.js @@ -1,33 +1,50 @@ -import { inject as service } from '@ember/service'; import Route from 'consul-ui/routing/route'; -import { hash } from 'rsvp'; -import { get, set, action } from '@ember/object'; +import { inject as service } from '@ember/service'; +import { get, action } from '@ember/object'; export default class SettingsRoute extends Route { - @service('client/http') - client; + @service('client/http') client; - @service('settings') - repo; + @service('settings') repo; + @service('repository/dc') dcRepo; + @service('repository/permission') permissionsRepo; + @service('repository/nspace/disabled') nspacesRepo; - @service('repository/dc') - dcRepo; + async model(params) { + // reach up and grab things from the application route/controller + const app = this.controllerFor('application'); - @service('repository/nspace/disabled') - nspacesRepo; + // figure out if we have anything missing for menus etc and get them if + // so, otherwise just use what they already are + const [item, dc] = await Promise.all([ + this.repo.findAll(), + typeof app.dc === 'undefined' ? this.dcRepo.getActive() : app.dc, + ]); + const nspace = + typeof app.nspace === 'undefined' + ? await this.nspacesRepo.getActive(item.nspace) + : app.nspace; + const permissions = + typeof app.permissions === 'undefined' + ? await this.permissionsRepo.findAll({ + dc: dc.Name, + nspace: nspace.Name, + }) + : app.permissions; - model(params) { - const app = this.modelFor('application'); - return hash({ - item: this.repo.findAll(), - dc: this.dcRepo.getActive(undefined, app.dcs), - nspace: this.nspacesRepo.getActive(), - }).then(model => { - if (typeof get(model.item, 'client.blocking') === 'undefined') { - set(model, 'item.client', { blocking: true }); - } - return model; + // reset the things higher up in the application if they were already set + // this won't do anything + this.controllerFor('application').setProperties({ + dc: dc, + nspace: nspace, + token: item.token, + permissions: permissions, }); + + if (typeof get(item, 'client.blocking') === 'undefined') { + item.client = { blocking: true }; + } + return { item }; } setupController(controller, model) { diff --git a/ui/packages/consul-ui/app/serializers/application.js b/ui/packages/consul-ui/app/serializers/application.js index 9176a0842..ed9318968 100644 --- a/ui/packages/consul-ui/app/serializers/application.js +++ b/ui/packages/consul-ui/app/serializers/application.js @@ -11,7 +11,7 @@ import { FOREIGN_KEY as DATACENTER_KEY } from 'consul-ui/models/dc'; import { NSPACE_KEY } from 'consul-ui/models/nspace'; import createFingerprinter from 'consul-ui/utils/create-fingerprinter'; -const DEFAULT_NSPACE = 'default'; +const DEFAULT_NSPACE = ''; const map = function(obj, cb) { if (!Array.isArray(obj)) { diff --git a/ui/packages/consul-ui/app/services/repository/dc.js b/ui/packages/consul-ui/app/services/repository/dc.js index 6eec243ef..118211fe4 100644 --- a/ui/packages/consul-ui/app/services/repository/dc.js +++ b/ui/packages/consul-ui/app/services/repository/dc.js @@ -18,11 +18,11 @@ export default class DcService extends RepositoryService { return this.store.query(this.getModelName(), {}); } - async findBySlug(name, items) { + async findBySlug(name) { + const items = this.store.peekAll('dc'); if (name != null) { const item = await items.findBy('Name', name); if (typeof item !== 'undefined') { - await this.settings.persist({ dc: get(item, 'Name') }); return item; } } @@ -32,17 +32,13 @@ export default class DcService extends RepositoryService { } async getActive(name, items) { - return Promise.all([name || this.settings.findBySlug('dc'), items || this.findAll()]).then( - ([name, items]) => { - return this.findBySlug(name, items).catch(async e => { - const item = - items.findBy('Name', this.env.var('CONSUL_DATACENTER_LOCAL')) || - get(items, 'firstObject'); - await this.settings.persist({ dc: get(item, 'Name') }); - return item; - }); - } - ); + return Promise.all([name, items || this.findAll()]).then(([name, items]) => { + return this.findBySlug(name, items).catch(async e => { + return ( + items.findBy('Name', this.env.var('CONSUL_DATACENTER_LOCAL')) || get(items, 'firstObject') + ); + }); + }); } async clearActive() { diff --git a/ui/packages/consul-ui/app/services/repository/nspace/disabled.js b/ui/packages/consul-ui/app/services/repository/nspace/disabled.js index 79eff5c63..b76f27dd2 100644 --- a/ui/packages/consul-ui/app/services/repository/nspace/disabled.js +++ b/ui/packages/consul-ui/app/services/repository/nspace/disabled.js @@ -2,7 +2,6 @@ import RepositoryService from 'consul-ui/services/repository'; import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/nspace'; const modelName = 'nspace'; -const DEFAULT_NSPACE = 'default'; export default class NspaceDisabledService extends RepositoryService { getPrimaryKey() { return PRIMARY_KEY; @@ -22,7 +21,7 @@ export default class NspaceDisabledService extends RepositoryService { getActive() { return { - Name: DEFAULT_NSPACE, + Name: 'default', }; } diff --git a/ui/packages/consul-ui/app/services/repository/nspace/enabled.js b/ui/packages/consul-ui/app/services/repository/nspace/enabled.js index 0f289f802..b0290a208 100644 --- a/ui/packages/consul-ui/app/services/repository/nspace/enabled.js +++ b/ui/packages/consul-ui/app/services/repository/nspace/enabled.js @@ -1,7 +1,27 @@ import { inject as service } from '@ember/service'; import RepositoryService from 'consul-ui/services/repository'; import { PRIMARY_KEY, SLUG_KEY } from 'consul-ui/models/nspace'; +import { runInDebug } from '@ember/debug'; +const findActiveNspace = function(nspaces, nspace) { + let found = nspaces.find(function(item) { + return item.Name === nspace.Name; + }); + if (typeof found === 'undefined') { + runInDebug(_ => + console.info(`${nspace.Name} not found in [${nspaces.map(item => item.Name).join(', ')}]`) + ); + // if we can't find the nspace that was specified try default + found = nspaces.find(function(item) { + return item.Name === 'default'; + }); + // if there is no default just choose the first + if (typeof found === 'undefined') { + found = nspaces[0]; + } + } + return found; +}; const modelName = 'nspace'; export default class NspaceEnabledService extends RepositoryService { @service('router') router; @@ -46,20 +66,14 @@ export default class NspaceEnabledService extends RepositoryService { }); } - getActive(paramsNspace) { - return this.settings - .findBySlug('nspace') - .then(function(nspace) { - // If we can't figure out the nspace from the URL use - // the previously saved nspace and if thats not there - // then just use default - return paramsNspace || nspace || 'default'; - }) - .then(nspace => this.settings.persist({ nspace: nspace })) - .then(function(item) { - return { - Name: item.nspace, - }; - }); + async getActive(paramsNspace = '') { + const nspaces = this.store.peekAll('nspace').toArray(); + if (paramsNspace.length === 0) { + const token = await this.settings.findBySlug('token'); + paramsNspace = token.Namespace || 'default'; + } + // if there is only 1 namespace then use that, otherwise find the + // namespace object that corresponds to the active one + return nspaces.length === 1 ? nspaces[0] : findActiveNspace(nspaces, { Name: paramsNspace }); } } diff --git a/ui/packages/consul-ui/app/services/repository/oidc-provider.js b/ui/packages/consul-ui/app/services/repository/oidc-provider.js index 6c5286924..c89522665 100644 --- a/ui/packages/consul-ui/app/services/repository/oidc-provider.js +++ b/ui/packages/consul-ui/app/services/repository/oidc-provider.js @@ -7,8 +7,8 @@ import dataSource from 'consul-ui/decorators/data-source'; const modelName = 'oidc-provider'; const OAUTH_PROVIDER_NAME = 'oidc-with-url'; export default class OidcProviderService extends RepositoryService { - @service('torii') - manager; + @service('torii') manager; + @service('repository/settings') settings; init() { super.init(...arguments); @@ -25,8 +25,31 @@ export default class OidcProviderService extends RepositoryService { } @dataSource('/:ns/:dc/oidc/provider/:id') - async findBySlug() { - return super.findBySlug(...arguments); + async findBySlug(params) { + // This addition is mainly due to ember-data book-keeping This is one of + // the only places where Consul w/namespaces enabled doesn't return a + // response with a Namespace property, but in order to keep ember-data + // id's happy we need to fake one. Usually when we make a request to consul + // with an empty `ns=` Consul will use the namespace that is assigned to + // the token, and when we get the response we can pick that back off the + // responses `Namespace` property. As we don't receive a `Namespace` + // property here, we have to figure this out ourselves. Biut we also want + // to make this completely invisible to 'the application engineer/a + // template engineer'. This feels like the best place/way to do it as we + // are already in a asynchronous method, and we avoid adding extra 'just + // for us' parameters to the query object. There is a chance that as we + // are discovering the tokens default namespace on the frontend and + // assigning that to the ns query param, the token default namespace 'may' + // have changed by the time the request hits the backend. As this is + // extremely unlikely and in the scheme of things not a big deal, we + // decided that doing this here is ok and avoids doing this in a more + // complicated manner. + const token = (await this.settings.findBySlug('token')) || {}; + return super.findBySlug({ + ns: params.ns || token.Namespace || 'default', + dc: params.dc, + id: params.id, + }); } @dataSource('/:ns/:dc/oidc/authorize/:id/:code/:state') diff --git a/ui/packages/consul-ui/docs/significant-patterns.mdx b/ui/packages/consul-ui/docs/significant-patterns.mdx index 38f10456b..657da2d5e 100644 --- a/ui/packages/consul-ui/docs/significant-patterns.mdx +++ b/ui/packages/consul-ui/docs/significant-patterns.mdx @@ -1,16 +1,18 @@ # Significant patterns +## Ember specific + The Consul UI tries to follow typical conventional ember but differs significantly in several places: -## Routing +### Routing We use a declarative configuration format for our routing rather than Ember's DSL. The format is closely modeled on Ember's DSL and if you need to generate Ember's DSL from this for some reason you can use one of our Debug Utilities to do this. -## Routes +### Routes We use a specific BaseRoute as a parent Route for **all** our Routes. This contains project wide functionality for several important additions. If you use the normal `ember @@ -27,7 +29,7 @@ has no query parameters configured they are copied over from the Route. Preferably we don't use Controllers, but this doesn't mean you shouldn't if you need to. -## Routlets +### Routlets We use have a concept of 'routlets', the combination of a `` and `` components that we use within our route templates to achieve @@ -38,7 +40,7 @@ Every route template should be wrapped in a `` compon The `routeName` variable is made available in every single route template and it equal to the routeName of the current template e.g. `dc.services.index` -## DataSources +### DataSources In order to support our live updating long-polling blocking queries we use 'DataSources' which come in two flavours. A service backed @@ -79,7 +81,7 @@ And a component based approach for use within templates. See the relavant component/service documentation for more detail. -## Components +### Components You could group our components into two different groups: @@ -94,7 +96,7 @@ We currently use a mix of named/block slots and contextual components and its fair to say that we use more named/block slots, but both are fine depending on your use case. -## Significant Addons +### Significant Addons - `ember-data` - model layer - `ember-stargate` - wormhole/portal/put this DOM over there functionality @@ -105,4 +107,47 @@ your use case. Please see our package.json file dependencies. +## Consul specific +### Namespace support + +Whilst Consul namespaces are a OSS feature of Consul, we almost always build +things 'pretending' that namespaces are always enabled. Then there is code +within the data layer of the application that removes any namespace related +query parameters. + +Namespaces are a little more complex to think about than other things in +Consul as we have the idea of 'the default' namespace, and then additionally +an ACL token can have 'a default' namespace (not the same as 'the default' +namespace) which is a little like a namespace that is assigned to the token, +or where the token originated from. The Consul backend can then use the tokens +default/origin/assigned namespace to figure out whether and how to filter any +results by namespace before sending those results to the UI. + +When you are requesting data from the API you should always include a +namespace (if applicable to the API endpoint), but you should never default +the namespace value to 'the default' namespace - `default`. Conversly, when +receiving data from an API response, there are some places where we do default +a namespace to 'the default' namespace `default`. The easiest way to think +about this is: If its on the way out, don't add a default ever, if its on the +way in you may need to default any potentially empty Namespace parameters to +`default` (due to using Consul without namespaces enabled), there is a +NspaceAbility and/or an `env` feature flag to help you to discover this. + +We've made various decisions in the UI to make it hard to omit a namespace +when you shouldn't, for example using our DataSource component requires you to +include a defined namespace parameter (even if the value for that is `''`), +and the route parameter `nspace` value will always default to `''`. It also +reminds you to be continually thinking about namespaces whilst adding new +features. + +Lastly, we always refer to namespaces in code as the word `nspace` or +`nspaces`. Both `ns` and `namespace` have other purposes in Ember and Ember +Data (in various places one is significant but not the other and vice versa). +Always using the term `nspace` means we avoid any strange bugs or +clashes/overwrites with anything in Ember or Ember Data core. The only place +where we use `ns` or `Namespace` is to construct API requests/responses. +Sometimes when using this term in CSS or on plain javascript objects, this may +seem unnecessary but it just helps smooth over writing code around namespaces +as you don't have the split second decision of whether you write `ns` or +`namespace` - just use `nspace` and everything will be fine :) diff --git a/ui/packages/consul-ui/tests/integration/serializers/acl-test.js b/ui/packages/consul-ui/tests/integration/serializers/acl-test.js index 32eb056a4..0029ac61f 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/acl-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/acl-test.js @@ -49,7 +49,7 @@ module('Integration | Serializer | acl', function(hooks) { Datacenter: dc, [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: nspace, + [NSPACE.toLowerCase()]: '', }, // TODO: default isn't required here, once we've // refactored out our Serializer this can go diff --git a/ui/packages/consul-ui/tests/integration/serializers/kv-test.js b/ui/packages/consul-ui/tests/integration/serializers/kv-test.js index 7907a994b..82438ec8a 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/kv-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/kv-test.js @@ -57,7 +57,7 @@ module('Integration | Serializer | kv', function(hooks) { Datacenter: dc, [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: payload[0].Namespace || undefinedNspace, + [NSPACE.toLowerCase()]: nspace || '', }, Namespace: payload[0].Namespace || undefinedNspace, uid: `["${payload[0].Namespace || undefinedNspace}","${dc}","${id}"]`, diff --git a/ui/packages/consul-ui/tests/integration/serializers/node-test.js b/ui/packages/consul-ui/tests/integration/serializers/node-test.js index 4b40854cf..2ef5b8520 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/node-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/node-test.js @@ -80,7 +80,7 @@ module('Integration | Serializer | node', function(hooks) { Port: '8500', [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: nspace, + [NSPACE.toLowerCase()]: '', }, }; const actual = serializer.respondForQueryLeader( diff --git a/ui/packages/consul-ui/tests/integration/serializers/oidc-provider-test.js b/ui/packages/consul-ui/tests/integration/serializers/oidc-provider-test.js index 6a4925e13..f0f601e67 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/oidc-provider-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/oidc-provider-test.js @@ -44,18 +44,24 @@ module('Integration | Serializer | oidc-provider', function(hooks) { const dc = 'dc-1'; const id = 'slug'; const request = { - url: `/v1/acl/oidc/auth-url?dc=${dc}`, + url: `/v1/acl/oidc/auth-url?dc=${dc}${ + typeof nspace !== 'undefined' ? `&ns=${nspace || undefinedNspace}` : `` + }`, }; return get(request.url).then(function(payload) { + // The response here never has a Namespace property so its ok to just + // use the query parameter as the expected nspace value. See + // implementation of this method for info on why this is slightly + // different to other tests const expected = Object.assign({}, payload, { Name: id, Datacenter: dc, [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: payload.Namespace || undefinedNspace, + [NSPACE.toLowerCase()]: nspace || '', }, - Namespace: payload.Namespace || undefinedNspace, - uid: `["${payload.Namespace || undefinedNspace}","${dc}","${id}"]`, + Namespace: nspace || undefinedNspace, + uid: `["${nspace || undefinedNspace}","${dc}","${id}"]`, }); const actual = serializer.respondForQueryRecord( function(cb) { @@ -66,6 +72,7 @@ module('Integration | Serializer | oidc-provider', function(hooks) { { dc: dc, id: id, + ns: nspace, } ); assert.deepEqual(actual, expected); diff --git a/ui/packages/consul-ui/tests/integration/serializers/policy-test.js b/ui/packages/consul-ui/tests/integration/serializers/policy-test.js index 6a098574f..238cfd41c 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/policy-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/policy-test.js @@ -49,7 +49,7 @@ module('Integration | Serializer | policy', function(hooks) { Datacenter: dc, [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: payload.Namespace || undefinedNspace, + [NSPACE.toLowerCase()]: nspace || '', }, Namespace: payload.Namespace || undefinedNspace, uid: `["${payload.Namespace || undefinedNspace}","${dc}","${id}"]`, diff --git a/ui/packages/consul-ui/tests/integration/serializers/role-test.js b/ui/packages/consul-ui/tests/integration/serializers/role-test.js index 10da23561..bebf39a53 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/role-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/role-test.js @@ -53,7 +53,7 @@ module('Integration | Serializer | role', function(hooks) { Policies: createPolicies(payload), [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: payload.Namespace || undefinedNspace, + [NSPACE.toLowerCase()]: nspace || '', }, Namespace: payload.Namespace || undefinedNspace, uid: `["${payload.Namespace || undefinedNspace}","${dc}","${id}"]`, diff --git a/ui/packages/consul-ui/tests/integration/serializers/service-instance-test.js b/ui/packages/consul-ui/tests/integration/serializers/service-instance-test.js index e53b6ce3f..8ccab5e79 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/service-instance-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/service-instance-test.js @@ -28,7 +28,7 @@ module('Integration | Serializer | service-instance', function(hooks) { Datacenter: dc, [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: payload[0].Service.Namespace || undefinedNspace, + [NSPACE.toLowerCase()]: nspace || '', }, Namespace: payload[0].Service.Namespace || undefinedNspace, uid: `["${payload[0].Service.Namespace || undefinedNspace}","${dc}","${node}","${id}"]`, diff --git a/ui/packages/consul-ui/tests/integration/serializers/session-test.js b/ui/packages/consul-ui/tests/integration/serializers/session-test.js index e68f32344..e472088b9 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/session-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/session-test.js @@ -54,7 +54,7 @@ module('Integration | Serializer | session | response', function(hooks) { Datacenter: dc, [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: payload[0].Namespace || undefinedNspace, + [NSPACE.toLowerCase()]: nspace || '', }, Namespace: payload[0].Namespace || undefinedNspace, uid: `["${payload[0].Namespace || undefinedNspace}","${dc}","${id}"]`, diff --git a/ui/packages/consul-ui/tests/integration/serializers/token-test.js b/ui/packages/consul-ui/tests/integration/serializers/token-test.js index 11db9ff23..4485ff2a0 100644 --- a/ui/packages/consul-ui/tests/integration/serializers/token-test.js +++ b/ui/packages/consul-ui/tests/integration/serializers/token-test.js @@ -53,7 +53,7 @@ module('Integration | Serializer | token', function(hooks) { Datacenter: dc, [META]: { [DC.toLowerCase()]: dc, - [NSPACE.toLowerCase()]: payload.Namespace || undefinedNspace, + [NSPACE.toLowerCase()]: nspace || '', }, Namespace: payload.Namespace || undefinedNspace, uid: `["${payload.Namespace || undefinedNspace}","${dc}","${id}"]`, diff --git a/ui/packages/consul-ui/tests/unit/serializers/application-test.js b/ui/packages/consul-ui/tests/unit/serializers/application-test.js index 39eb5f084..c0bef1c6d 100644 --- a/ui/packages/consul-ui/tests/unit/serializers/application-test.js +++ b/ui/packages/consul-ui/tests/unit/serializers/application-test.js @@ -67,7 +67,7 @@ module('Unit | Serializer | application', function(hooks) { Name: 'name', [META]: { [DC.toLowerCase()]: 'dc-1', - [NSPACE.toLowerCase()]: 'default', + [NSPACE.toLowerCase()]: '', }, 'primary-key-name': 'name', };