From 79a932bebb57f9759589bf1384f7e7dc8356fbe5 Mon Sep 17 00:00:00 2001 From: Zhihe Li Date: Fri, 19 Apr 2024 16:18:32 -0400 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Add=20aliases=20to=20lis?= =?UTF-8?q?t=20view=20(#2256)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-13487 --- .../handlers/client-daemon-search.js | 5 +- addons/api/addon/adapters/application.js | 6 +- .../addon/handlers/client-daemon-handler.js | 16 +- addons/api/addon/models/target.js | 6 + .../api/app/handlers/client-daemon-handler.js | 2 +- .../scopes/scope/projects/targets/index.js | 148 ++++++++++-------- .../scopes/scope/projects/targets/index.hbs | 6 +- .../projects/sessions/index-test.js | 1 + .../acceptance/projects/targets/index-test.js | 51 ++++-- .../projects/targets/target-test.js | 2 +- ui/desktop/tests/acceptance/scopes-test.js | 8 +- 11 files changed, 162 insertions(+), 89 deletions(-) diff --git a/addons/api/addon-test-support/handlers/client-daemon-search.js b/addons/api/addon-test-support/handlers/client-daemon-search.js index c49f76d7c..b7e771088 100644 --- a/addons/api/addon-test-support/handlers/client-daemon-search.js +++ b/addons/api/addon-test-support/handlers/client-daemon-search.js @@ -5,6 +5,8 @@ import sinon from 'sinon'; import { typeOf } from '@ember/utils'; +import { resourceNames } from 'api/handlers/client-daemon-handler'; +import { singularize } from 'ember-inflector'; /** * This test helper can be used to help setup your sinon stubs in your tests. @@ -99,7 +101,8 @@ export default function setupStubs(hooks) { resource = type.resource; } - sinon.assert.match(ipcCall.args[1], { resource }); + const mappedResourceName = resourceNames[singularize(resource)]; + sinon.assert.match(ipcCall.args[1], { resource: mappedResourceName }); }); sinon.restore(); diff --git a/addons/api/addon/adapters/application.js b/addons/api/addon/adapters/application.js index d69a5f043..6b0f4053b 100644 --- a/addons/api/addon/adapters/application.js +++ b/addons/api/addon/adapters/application.js @@ -283,8 +283,8 @@ export default class ApplicationAdapter extends RESTAdapter.extend( const baseError = { isInvalid: true, status, - code: payload.kind, - detail: payload.message, + code: payload?.kind, + detail: payload?.message, source: { pointer: '/data' }, }; // Normalize field-specific errors, if any. @@ -319,6 +319,6 @@ export default class ApplicationAdapter extends RESTAdapter.extend( * @return {String} detailed error message */ generatedDetailedMessage(status, headers, payload /*, requestData*/) { - return payload.message; + return payload?.message; } } diff --git a/addons/api/addon/handlers/client-daemon-handler.js b/addons/api/addon/handlers/client-daemon-handler.js index 2fe61c995..a0ba12723 100644 --- a/addons/api/addon/handlers/client-daemon-handler.js +++ b/addons/api/addon/handlers/client-daemon-handler.js @@ -5,7 +5,6 @@ import { inject as service } from '@ember/service'; import { getOwner, setOwner } from '@ember/application'; -import { pluralize } from 'ember-inflector'; import { generateMQLExpression } from '../utils/mql-query'; import { paginateResults } from '../utils/paginate-results'; @@ -13,11 +12,22 @@ import { paginateResults } from '../utils/paginate-results'; * Not all types are yet supported by the client daemon so we'll * just whitelist the ones we need to start with. Keys are the resource * types and the values are the fields that are searchable by the client. - * @type {{session: string[], target: string[]}} + * @type {{session: string[], alias: string[], target: string[]}} */ const supportedTypes = { target: ['id', 'name', 'description', 'address', 'scope_id'], session: ['id', 'type', 'status', 'endpoint', 'scope_id', 'target_id'], + alias: ['id', 'type', 'value', 'destination_id'], +}; + +/** + * Map the model resource name to the client daemon resource name + * @type {{session: string, alias: string, target: string}} + */ +export const resourceNames = { + target: 'targets', + session: 'sessions', + alias: 'resolvable-aliases', }; const fetchControllerData = async (context, next) => { @@ -88,7 +98,7 @@ export default class ClientDaemonHandler { query: searchQuery, auth_token_id, token, - resource: pluralize(type), + resource: resourceNames[type], }; let clientDaemonResults = {}; diff --git a/addons/api/addon/models/target.js b/addons/api/addon/models/target.js index 0068d8497..c9e697ccf 100644 --- a/addons/api/addon/models/target.js +++ b/addons/api/addon/models/target.js @@ -135,6 +135,12 @@ export default class TargetModel extends GeneratedTargetModel { return Boolean(pendingOrActiveSessions.length); } + get associatedAliases() { + return this.store + .peekAll('alias') + .filter((alias) => alias?.destination_id === this.id); + } + // =methods /** diff --git a/addons/api/app/handlers/client-daemon-handler.js b/addons/api/app/handlers/client-daemon-handler.js index 07804ca85..d03463abe 100644 --- a/addons/api/app/handlers/client-daemon-handler.js +++ b/addons/api/app/handlers/client-daemon-handler.js @@ -3,4 +3,4 @@ * SPDX-License-Identifier: MPL-2.0 */ -export { default } from 'api/handlers/client-daemon-handler'; +export { default, resourceNames } from 'api/handlers/client-daemon-handler'; diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets/index.js b/ui/desktop/app/routes/scopes/scope/projects/targets/index.js index 4bed8bdcf..5b5a8bf18 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets/index.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets/index.js @@ -82,16 +82,12 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { */ async model({ search, scopes, availableSessions, types, page, pageSize }) { const orgScope = this.modelFor('scopes.scope'); + const projects = this.modelFor('scopes.scope.projects'); + // orgFilter used to narrow down resources to only those under // the current org scope if org is not global const orgFilter = `"/item/scope/parent_scope_id" == "${orgScope.id}"`; - if (!this.allTargets) { - await this.getAllTargets(orgScope, orgFilter); - } - - const projects = this.modelFor('scopes.scope.projects'); - const filters = { scope_id: [], id: { values: [] }, type: [] }; scopes.forEach((scope) => { filters.scope_id.push({ equals: scope }); @@ -100,6 +96,62 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { filters.type.push({ equals: type }); }); + const aliasPromise = this.store.query('alias', { + recursive: true, + scope_id: 'global', + force_refresh: true, + }); + const allTargetsPromise = !this.allTargets + ? this.makeAllTargetsQuery(orgScope, orgFilter) + : Promise.resolve(); + + const sessions = await this.makeSessionQuery(orgScope, scopes, orgFilter); + this.addActiveSessionFilters(filters, availableSessions, sessions); + + const query = { + recursive: true, + scope_id: orgScope.id, + query: { search, filters }, + page, + pageSize, + force_refresh: true, + }; + if (orgScope.isOrg && scopes.length === 0) { + query.filter = orgFilter; + } + let targets = await this.store.query('target', query); + const totalItems = targets.meta?.totalItems; + // Filter out targets to which users do not have the connect ability + targets = targets.filter((target) => + this.can.can('connect target', target), + ); + + const allTargets = await allTargetsPromise; + if (!this.allTargets) { + // Filter out targets to which users do not have the connect ability + this.allTargets = allTargets.filter((target) => + target.authorized_actions.includes('authorize-session'), + ); + } + + try { + await aliasPromise; + } catch { + // TODO: Log this error + // Separately await and catch the error here so we can continue loading + // the page in case the controller doesn't support aliases yet + } + + return { + targets, + projects, + allTargets: this.allTargets, + totalItems, + isClientDaemonRunning: await this.ipc.invoke('isClientDaemonRunning'), + }; + } + + async makeSessionQuery(orgScope, scopes, orgFilter) { // Retrieve all sessions so that the session and activeSessions getters // in the target model always retrieve the most up-to-date sessions. const sessionQuery = { @@ -119,55 +171,7 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { if (orgScope.isOrg && scopes.length === 0) { sessionQuery.filter = orgFilter; } - const sessions = await this.store.query('session', sessionQuery); - - this.addActiveSessionFilters(filters, availableSessions, sessions); - - const query = { - recursive: true, - scope_id: orgScope.id, - query: { search, filters }, - page, - pageSize, - force_refresh: true, - }; - if (orgScope.isOrg && scopes.length === 0) { - query.filter = orgFilter; - } - let targets = await this.store.query('target', query); - const totalItems = targets.meta?.totalItems; - - // Filter out targets to which users do not have the connect ability - targets = targets.filter((target) => - this.can.can('connect target', target), - ); - - return { - targets, - projects, - allTargets: this.allTargets, - totalItems, - isClientDaemonRunning: await this.ipc.invoke('isClientDaemonRunning'), - }; - } - - resetController(controller, isExiting, transition) { - const fromScope = transition.from.find( - (routeInfo) => routeInfo.name === 'scopes.scope', - ).params.scope_id; - const toScope = transition.to.find( - (routeInfo) => routeInfo.name === 'scopes.scope', - ).params.scope_id; - - // Reset the query params when changing scope context - if (fromScope !== toScope) { - controller.setProperties({ - scopes: [], - availableSessions: [], - types: [], - search: '', - }); - } + return this.store.query('session', sessionQuery); } /** @@ -177,7 +181,7 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { * @param orgFilter * @returns {Promise} */ - async getAllTargets(orgScope, orgFilter) { + async makeAllTargetsQuery(orgScope, orgFilter) { // Query all targets for defining filtering values if entering route for first time const query = { scope_id: orgScope.id, @@ -188,12 +192,7 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { query.filter = orgFilter; } const options = { pushToStore: false }; - const allTargets = await this.store.query('target', query, options); - - // Filter out targets to which users do not have the connect ability - this.allTargets = allTargets.filter((target) => - target.authorized_actions.includes('authorize-session'), - ); + return this.store.query('target', query, options); } /** @@ -236,11 +235,34 @@ export default class ScopesScopeProjectsTargetsIndexRoute extends Route { }); }; + resetController(controller, isExiting, transition) { + const fromScope = transition.from.find( + (routeInfo) => routeInfo.name === 'scopes.scope', + ).params.scope_id; + const toScope = transition.to.find( + (routeInfo) => routeInfo.name === 'scopes.scope', + ).params.scope_id; + + // Reset the query params when changing scope context + if (fromScope !== toScope) { + controller.setProperties({ + scopes: [], + availableSessions: [], + types: [], + search: '', + }); + } + } + @action async refreshAll() { const orgScope = this.modelFor('scopes.scope'); const orgFilter = `"/item/scope/parent_scope_id" == "${orgScope.id}"`; - await this.getAllTargets(orgScope, orgFilter); + const allTargets = await this.makeAllTargetsQuery(orgScope, orgFilter); + // Filter out targets to which users do not have the connect ability + this.allTargets = allTargets.filter((target) => + target.authorized_actions.includes('authorize-session'), + ); // Prime the store by searching for only orgs in case there are new org scopes; // otherwise we won't be able to correctly peek the org scopes for their display name in the UI. diff --git a/ui/desktop/app/templates/scopes/scope/projects/targets/index.hbs b/ui/desktop/app/templates/scopes/scope/projects/targets/index.hbs index 3d7461695..66c07cb11 100644 --- a/ui/desktop/app/templates/scopes/scope/projects/targets/index.hbs +++ b/ui/desktop/app/templates/scopes/scope/projects/targets/index.hbs @@ -75,7 +75,7 @@ @model={{@model.targets}} @columns={{array (hash label=(t 'form.name.label')) - (hash label=(t 'form.id.label')) + (hash label=(t 'resources.alias.title_plural')) (hash label=(t 'resources.target.titles.active-sessions')) (hash label=(t 'form.type.label')) (hash label=(t 'resources.project.title')) @@ -104,8 +104,8 @@ {{/if}} - - + + {{truncate-list 'actions.more' B.data.associatedAliases}} {{#if B.data.isActive}} diff --git a/ui/desktop/tests/acceptance/projects/sessions/index-test.js b/ui/desktop/tests/acceptance/projects/sessions/index-test.js index 4bf02149b..df2d59fef 100644 --- a/ui/desktop/tests/acceptance/projects/sessions/index-test.js +++ b/ui/desktop/tests/acceptance/projects/sessions/index-test.js @@ -365,6 +365,7 @@ module('Acceptance | projects | sessions | index', function (hooks) { 'sessions', 'sessions', 'targets', + 'aliases', 'targets', 'sessions', 'targets', diff --git a/ui/desktop/tests/acceptance/projects/targets/index-test.js b/ui/desktop/tests/acceptance/projects/targets/index-test.js index 2095c16b7..800736830 100644 --- a/ui/desktop/tests/acceptance/projects/targets/index-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/index-test.js @@ -138,6 +138,10 @@ module('Acceptance | projects | targets | index', function (hooks) { scope: instances.scopes.project2, address: 'localhost', }); + instances.alias = this.server.create('alias', { + scope: instances.scopes.global, + destination_id: instances.target.id, + }); // Generate route URLs for resources urls.scopes.global = `/scopes/${instances.scopes.global.id}`; @@ -157,7 +161,7 @@ module('Acceptance | projects | targets | index', function (hooks) { setDefaultClusterUrl(this); this.ipcStub.withArgs('isClientDaemonRunning').returns(true); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); }); test('visiting index while unauthenticated redirects to global authenticate method', async function (assert) { @@ -197,7 +201,7 @@ module('Acceptance | projects | targets | index', function (hooks) { test('visiting targets list view with no targets', async function (assert) { this.server.db.targets.remove(); this.server.db.sessions.remove(); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); await visit(urls.projects); @@ -209,7 +213,7 @@ module('Acceptance | projects | targets | index', function (hooks) { test('user cannot navigate to a target without proper authorization', async function (assert) { instances.target.authorized_actions = instances.target.authorized_actions.filter((item) => item !== 'read'); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); await visit(urls.projects); @@ -238,7 +242,7 @@ module('Acceptance | projects | targets | index', function (hooks) { instances.target.authorized_actions.filter( (item) => item !== 'authorize-session', ); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); await visit(urls.projects); @@ -271,7 +275,7 @@ module('Acceptance | projects | targets | index', function (hooks) { test('user can connect without target read permissions', async function (assert) { instances.target.authorized_actions = instances.target.authorized_actions.filter((item) => item !== 'read'); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); this.ipcStub.withArgs('cliExists').returns(true); this.ipcStub.withArgs('connect').returns({ session_id: instances.session.id, @@ -297,7 +301,7 @@ module('Acceptance | projects | targets | index', function (hooks) { instances.target.authorized_actions.filter((item) => item !== 'read'); this.ipcStub.withArgs('cliExists').returns(true); this.ipcStub.withArgs('connect').rejects(); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); const confirmService = this.owner.lookup('service:confirm'); confirmService.enabled = true; await visit(urls.projects); @@ -338,9 +342,11 @@ module('Acceptance | projects | targets | index', function (hooks) { test('user can cancel a session from inside target sessions flyout', async function (assert) { this.stubClientDaemonSearch( + 'aliases', 'targets', 'sessions', 'targets', + 'aliases', { resource: 'sessions', func: () => [], @@ -375,7 +381,7 @@ module('Acceptance | projects | targets | index', function (hooks) { test('user cannot cancel a session from inside target sessions flyout without permissions', async function (assert) { instances.session.authorized_actions = instances.session.authorized_actions.filter((item) => item !== 'cancel'); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); await visit(urls.projects); await click(`[href="${urls.targets}"]`); @@ -413,7 +419,7 @@ module('Acceptance | projects | targets | index', function (hooks) { test('user can navigate to session details from sessions table in flyout without permissions', async function (assert) { instances.session.authorized_actions = instances.session.authorized_actions.filter((item) => item !== 'read'); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); await visit(urls.projects); await click(`[href="${urls.targets}"]`); @@ -434,10 +440,18 @@ module('Acceptance | projects | targets | index', function (hooks) { }); test('user can change org scope and only targets for that org will be displayed', async function (assert) { - this.stubClientDaemonSearch('targets', 'sessions', 'targets', 'sessions', { - resource: 'targets', - func: () => [instances.target], - }); + this.stubClientDaemonSearch( + 'aliases', + 'targets', + 'sessions', + 'targets', + 'aliases', + 'sessions', + { + resource: 'targets', + func: () => [instances.target], + }, + ); await visit(urls.scopes.global); @@ -470,4 +484,17 @@ module('Acceptance | projects | targets | index', function (hooks) { assert.strictEqual(currentURL(), urls.targets); assert.dom('tbody tr').exists({ count: targetsCount }); }); + + test('target includes aliases', async function (assert) { + await visit(urls.projects); + await click(`[href="${urls.targets}"]`); + + assert.strictEqual(currentURL(), urls.targets); + assert + .dom(`[data-test-target-aliases="${instances.target.id}"]`) + .hasText(instances.alias.value); + assert + .dom(`[data-test-target-aliases="${instances.target2.id}"]`) + .hasNoText(); + }); }); diff --git a/ui/desktop/tests/acceptance/projects/targets/target-test.js b/ui/desktop/tests/acceptance/projects/targets/target-test.js index 11f566d63..5d19729c6 100644 --- a/ui/desktop/tests/acceptance/projects/targets/target-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/target-test.js @@ -124,7 +124,7 @@ module('Acceptance | projects | targets | target', function (hooks) { setDefaultClusterUrl(this); this.ipcStub.withArgs('isClientDaemonRunning').returns(true); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); }); test('user can connect to a target with an address', async function (assert) { diff --git a/ui/desktop/tests/acceptance/scopes-test.js b/ui/desktop/tests/acceptance/scopes-test.js index 33b236fcf..fcece1527 100644 --- a/ui/desktop/tests/acceptance/scopes-test.js +++ b/ui/desktop/tests/acceptance/scopes-test.js @@ -143,7 +143,7 @@ module('Acceptance | scopes', function (hooks) { setDefaultClusterUrl(this); this.ipcStub.withArgs('isClientDaemonRunning').returns(true); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); }); test('visiting index', async function (assert) { @@ -195,13 +195,17 @@ module('Acceptance | scopes', function (hooks) { test('can navigate among org scopes via header navigation', async function (assert) { assert.expect(3); this.stubClientDaemonSearch( + 'aliases', 'targets', 'sessions', 'targets', + 'aliases', 'sessions', 'targets', + 'aliases', 'sessions', 'targets', + 'aliases', 'sessions', 'targets', ); @@ -242,7 +246,7 @@ module('Acceptance | scopes', function (hooks) { assert.expect(1); this.server.db.targets.remove(); this.server.db.sessions.remove(); - this.stubClientDaemonSearch('targets', 'sessions', 'targets'); + this.stubClientDaemonSearch('aliases', 'targets', 'sessions', 'targets'); await visit(urls.targets);