From e51c18c7bc044ecf5f560a20279b6aaa8f7b8850 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 29 Oct 2024 13:58:03 +0000 Subject: [PATCH 1/5] ui: show region in header gutter when only one region exists This PR adds a plain text label of the region to the header when there is only one region present. Before, nothing was showin in this case, and a dropdown was shown on federated clusters. The use case here is for operators of multiple non-federated Nomad clusters, when all the UI's involved otherwise look identical. --- ui/app/styles/core/navbar.scss | 12 +++++++++ .../templates/components/region-switcher.hbs | 4 +++ .../unit/components/region-switcher-test.js | 25 +++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 ui/tests/unit/components/region-switcher-test.js diff --git a/ui/app/styles/core/navbar.scss b/ui/app/styles/core/navbar.scss index 9e9d08be16d..2906aa1ed45 100644 --- a/ui/app/styles/core/navbar.scss +++ b/ui/app/styles/core/navbar.scss @@ -142,6 +142,18 @@ $secondaryNavbarHeight: 4.5rem; display: flex; align-items: center; + &.is-region { + display: block; + padding: 0 1rem; + font-size: 1em; + color: rgba($primary-invert, 0.9); + + > span { + padding-right: 0.4em; + font-weight: 500; + } + } + &.is-gutter { width: $gutter-width; display: block; diff --git a/ui/app/templates/components/region-switcher.hbs b/ui/app/templates/components/region-switcher.hbs index 5c3eafb1e42..b7c0d9ced99 100644 --- a/ui/app/templates/components/region-switcher.hbs +++ b/ui/app/templates/components/region-switcher.hbs @@ -18,4 +18,8 @@ Region: {{region}} +{{else}} +
+ Region: {{this.system.activeRegion}} +
{{/if}} diff --git a/ui/tests/unit/components/region-switcher-test.js b/ui/tests/unit/components/region-switcher-test.js new file mode 100644 index 00000000000..15a67b847ee --- /dev/null +++ b/ui/tests/unit/components/region-switcher-test.js @@ -0,0 +1,25 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render } from '@ember/test-helpers'; +import { hbs } from 'ember-cli-htmlbars'; + +module('Unit | Component | region-switcher', function (hooks) { + setupRenderingTest(hooks); + + test('displays single region', async function (assert) { + const system = { + shouldShowRegions: false, + }; + + this.set('system', system); + + await render(hbs``); + + assert.dom('.is-region').exists(); + }); +}); From e9ee2ae66c473cf04c76de7095b8a817f85608b9 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Fri, 1 Nov 2024 16:48:59 -0400 Subject: [PATCH 2/5] [ui] Signing in with a token explicitly sets the region dropdown activeRegion (#24347) * Signing in with a token explicitly sets the region dropdown activeREgion * Test and Select a Region default text * Account for 403 on mocked agent members req --- ui/app/controllers/settings/tokens.js | 10 +++++++- ui/app/services/system.js | 2 +- .../templates/components/region-switcher.hbs | 7 ++++-- ui/mirage/config.js | 7 +++++- ui/tests/acceptance/regions-test.js | 23 +++++++++++++++++++ ui/tests/unit/adapters/job-test.js | 3 +++ 6 files changed, 47 insertions(+), 5 deletions(-) diff --git a/ui/app/controllers/settings/tokens.js b/ui/app/controllers/settings/tokens.js index 74e2728f3d2..074086001e5 100644 --- a/ui/app/controllers/settings/tokens.js +++ b/ui/app/controllers/settings/tokens.js @@ -23,7 +23,7 @@ export default class Tokens extends Controller { @service token; @service store; @service router; - + @service system; queryParams = ['code', 'state', 'jwtAuthMethod']; @tracked secret = this.token.secret; @@ -164,6 +164,14 @@ export default class Tokens extends Controller { // Refetch the token and associated policies this.token.get('fetchSelfTokenAndPolicies').perform().catch(); + if (!this.system.activeRegion) { + this.system.get('defaultRegion').then((res) => { + if (res.region) { + this.system.set('activeRegion', res.region); + } + }); + } + this.signInStatus = 'success'; this.token.set('tokenNotFound', false); }, diff --git a/ui/app/services/system.js b/ui/app/services/system.js index d1408a3c4f2..090d7477994 100644 --- a/ui/app/services/system.js +++ b/ui/app/services/system.js @@ -57,7 +57,7 @@ export default class SystemService extends Service { }); } - @computed + @computed('token.selfToken') get defaultRegion() { const token = this.token; return PromiseObject.create({ diff --git a/ui/app/templates/components/region-switcher.hbs b/ui/app/templates/components/region-switcher.hbs index b7c0d9ced99..091b0cf58ff 100644 --- a/ui/app/templates/components/region-switcher.hbs +++ b/ui/app/templates/components/region-switcher.hbs @@ -12,10 +12,13 @@ @tagName="div" @triggerClass={{this.decoration}} @options={{this.sortedRegions}} - @selected={{this.system.activeRegion}} + @selected={{or this.system.activeRegion 'Select a Region'}} @searchEnabled={{false}} @onChange={{action this.gotoRegion}} as |region|> - Region: {{region}} + {{#if this.system.activeRegion}} + Region: + {{/if}} + {{region}} {{else}} diff --git a/ui/mirage/config.js b/ui/mirage/config.js index 2067aa081a6..0e780ae612f 100644 --- a/ui/mirage/config.js +++ b/ui/mirage/config.js @@ -708,7 +708,12 @@ export default function () { return this.serialize(volume); }); - this.get('/agent/members', function ({ agents, regions }) { + this.get('/agent/members', function ({ agents, regions }, req) { + const tokenPresent = req.requestHeaders['X-Nomad-Token']; + if (!tokenPresent) { + return new Response(403, {}, 'Forbidden'); + } + const firstRegion = regions.first(); return { ServerRegion: firstRegion ? firstRegion.id : null, diff --git a/ui/tests/acceptance/regions-test.js b/ui/tests/acceptance/regions-test.js index 48ccdda3bb4..14b90619e18 100644 --- a/ui/tests/acceptance/regions-test.js +++ b/ui/tests/acceptance/regions-test.js @@ -15,6 +15,7 @@ import JobsList from 'nomad-ui/tests/pages/jobs/list'; import ClientsList from 'nomad-ui/tests/pages/clients/list'; import Layout from 'nomad-ui/tests/pages/layout'; import Allocation from 'nomad-ui/tests/pages/allocations/detail'; +import Tokens from 'nomad-ui/tests/pages/settings/tokens'; module('Acceptance | regions (only one)', function (hooks) { setupApplicationTest(hooks); @@ -218,4 +219,26 @@ module('Acceptance | regions (many)', function (hooks) { } }); }); + + test('Signing in sets the active region', async function (assert) { + window.localStorage.clear(); + let managementToken = server.create('token'); + await Tokens.visit(); + assert.equal( + Layout.navbar.regionSwitcher.text, + 'Select a Region', + 'Region picker says "Select a Region" before signing in' + ); + await Tokens.secret(managementToken.secretId).submit(); + assert.equal( + window.localStorage.nomadActiveRegion, + 'global', + 'Region is set in localStorage after signing in' + ); + assert.equal( + Layout.navbar.regionSwitcher.text, + 'Region: global', + 'Region picker says "Region: global" after signing in' + ); + }); }); diff --git a/ui/tests/unit/adapters/job-test.js b/ui/tests/unit/adapters/job-test.js index c7d57b51f31..d6437ac0e21 100644 --- a/ui/tests/unit/adapters/job-test.js +++ b/ui/tests/unit/adapters/job-test.js @@ -468,6 +468,9 @@ module('Unit | Adapter | Job', function (hooks) { }); test('when the region is set to the default region, requests are made without the region query param', async function (assert) { + const secret = 'here is the secret'; + this.subject().set('token.secret', secret); + await this.initializeUI({ region: 'region-1' }); const { pretender } = this.server; From c7e980585933eb14da807c610533dbc260f267ea Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Fri, 1 Nov 2024 16:58:51 -0400 Subject: [PATCH 3/5] Dont show the region if it isnt set in agent config --- .changelog/24320.txt | 3 +++ ui/app/services/system.js | 6 ++++++ ui/app/templates/components/region-switcher.hbs | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 .changelog/24320.txt diff --git a/.changelog/24320.txt b/.changelog/24320.txt new file mode 100644 index 00000000000..9152496a96f --- /dev/null +++ b/.changelog/24320.txt @@ -0,0 +1,3 @@ +```release-note:improvement +ui: Show region in header when only one region exists, and set it immediately upon logging in with a token +``` diff --git a/ui/app/services/system.js b/ui/app/services/system.js index 090d7477994..f88163b4bb0 100644 --- a/ui/app/services/system.js +++ b/ui/app/services/system.js @@ -110,6 +110,12 @@ export default class SystemService extends Service { return this.get('regions.length') > 1; } + get hasNonDefaultRegion() { + return this.get('regions') + .toArray() + .some((region) => region !== 'global'); + } + @computed('activeRegion', 'defaultRegion.region', 'shouldShowRegions') get shouldIncludeRegion() { return ( diff --git a/ui/app/templates/components/region-switcher.hbs b/ui/app/templates/components/region-switcher.hbs index 091b0cf58ff..1d592cbdc8a 100644 --- a/ui/app/templates/components/region-switcher.hbs +++ b/ui/app/templates/components/region-switcher.hbs @@ -21,7 +21,7 @@ {{region}} -{{else}} +{{else if this.system.hasNonDefaultRegion}}
Region: {{this.system.activeRegion}}
From 9f4abffba2cf1025fde45c8a64ff0baa529cd3cc Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Fri, 1 Nov 2024 17:01:41 -0400 Subject: [PATCH 4/5] Small padding css change --- ui/app/styles/core/navbar.scss | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/app/styles/core/navbar.scss b/ui/app/styles/core/navbar.scss index 2906aa1ed45..cff29ac5b37 100644 --- a/ui/app/styles/core/navbar.scss +++ b/ui/app/styles/core/navbar.scss @@ -144,12 +144,11 @@ $secondaryNavbarHeight: 4.5rem; &.is-region { display: block; - padding: 0 1rem; + padding: 0; font-size: 1em; color: rgba($primary-invert, 0.9); > span { - padding-right: 0.4em; font-weight: 500; } } From 22dc7db6119b061efe8ed4a856a8e7809876b8e6 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Fri, 1 Nov 2024 21:57:10 -0400 Subject: [PATCH 5/5] unit test condition moved to stubbable acceptance test --- ui/app/styles/core/navbar.scss | 2 +- .../templates/components/region-switcher.hbs | 2 +- ui/tests/acceptance/regions-test.js | 6 +++-- ui/tests/pages/layout.js | 5 ++++ .../unit/components/region-switcher-test.js | 25 ------------------- 5 files changed, 11 insertions(+), 29 deletions(-) delete mode 100644 ui/tests/unit/components/region-switcher-test.js diff --git a/ui/app/styles/core/navbar.scss b/ui/app/styles/core/navbar.scss index cff29ac5b37..a26dee23ef4 100644 --- a/ui/app/styles/core/navbar.scss +++ b/ui/app/styles/core/navbar.scss @@ -142,7 +142,7 @@ $secondaryNavbarHeight: 4.5rem; display: flex; align-items: center; - &.is-region { + &.single-region { display: block; padding: 0; font-size: 1em; diff --git a/ui/app/templates/components/region-switcher.hbs b/ui/app/templates/components/region-switcher.hbs index 1d592cbdc8a..9d9817295b8 100644 --- a/ui/app/templates/components/region-switcher.hbs +++ b/ui/app/templates/components/region-switcher.hbs @@ -22,7 +22,7 @@ {{else if this.system.hasNonDefaultRegion}} -
+
Region: {{this.system.activeRegion}}
{{/if}} diff --git a/ui/tests/acceptance/regions-test.js b/ui/tests/acceptance/regions-test.js index 14b90619e18..a17529fd3b9 100644 --- a/ui/tests/acceptance/regions-test.js +++ b/ui/tests/acceptance/regions-test.js @@ -36,21 +36,23 @@ module('Acceptance | regions (only one)', function (hooks) { await a11yAudit(assert); }); - test('when there is only one region, the region switcher is not shown in the nav bar and the region is not in the page title', async function (assert) { + test('when there is only one region, and it is the default one, the region switcher is not shown in the nav bar and the region is not in the page title', async function (assert) { server.create('region', { id: 'global' }); await JobsList.visit(); assert.notOk(Layout.navbar.regionSwitcher.isPresent, 'No region switcher'); + assert.notOk(Layout.navbar.singleRegion.isPresent, 'No single region'); assert.ok(document.title.includes('Jobs')); }); - test('when the only region is not named "global", the region switcher still is not shown', async function (assert) { + test('when the only region is not named "global", the region switcher still is not shown, but the single region name is', async function (assert) { server.create('region', { id: 'some-region' }); await JobsList.visit(); assert.notOk(Layout.navbar.regionSwitcher.isPresent, 'No region switcher'); + assert.ok(Layout.navbar.singleRegion.isPresent, 'Single region'); }); test('pages do not include the region query param', async function (assert) { diff --git a/ui/tests/pages/layout.js b/ui/tests/pages/layout.js index 3fbca6063f8..d521bd382a0 100644 --- a/ui/tests/pages/layout.js +++ b/ui/tests/pages/layout.js @@ -28,6 +28,11 @@ export default create({ }), }, + singleRegion: { + scope: '.single-region', + text: text(), + }, + search: { scope: '[data-test-search-parent]', diff --git a/ui/tests/unit/components/region-switcher-test.js b/ui/tests/unit/components/region-switcher-test.js deleted file mode 100644 index 15a67b847ee..00000000000 --- a/ui/tests/unit/components/region-switcher-test.js +++ /dev/null @@ -1,25 +0,0 @@ -/** - * Copyright (c) HashiCorp, Inc. - * SPDX-License-Identifier: BUSL-1.1 - */ - -import { module, test } from 'qunit'; -import { setupRenderingTest } from 'ember-qunit'; -import { render } from '@ember/test-helpers'; -import { hbs } from 'ember-cli-htmlbars'; - -module('Unit | Component | region-switcher', function (hooks) { - setupRenderingTest(hooks); - - test('displays single region', async function (assert) { - const system = { - shouldShowRegions: false, - }; - - this.set('system', system); - - await render(hbs``); - - assert.dom('.is-region').exists(); - }); -});