From 221ab1f7bd932a531bf38f7b79f49108374bcd84 Mon Sep 17 00:00:00 2001 From: Cameron Perera Date: Wed, 22 Nov 2023 12:57:37 -0600 Subject: [PATCH] Icu 11748 add free search for targets (#1995) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: 💡 fix how we access socket_address from status call ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * feat: 🎸 add free search capability to targets list ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * test: 💍 update test asserts to look for hds appl state title ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * refactor: 💡 remove filtering for yes, no, & narrow-results ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * refactor: 💡 add aria-label to dropdown search input ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * refactor: 💡 add to handleInput signature comment ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * refactor: 💡 move buildQuery to client daemon handler ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * refactor: 💡 Add new parser for MQL expressions ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * refactor: 💡 use queryParams for searching ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * fix: 🐛 Fix some MQL expressions not being properly filtered ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * style: 💄 update handleSearchInput comment signature ✅ Closes: https://hashicorp.atlassian.net/browse/ICU-11748 * Update addons/api/addon/utils/mqlQuery.js Co-authored-by: Lisbet Alvarez <107949262+lisbet-alvarez@users.noreply.github.com> --------- Co-authored-by: Zhihe Li Co-authored-by: Lisbet Alvarez <107949262+lisbet-alvarez@users.noreply.github.com> --- .../addon/handlers/client-daemon-handler.js | 41 ++- addons/api/addon/utils/mqlQuery.js | 128 +++++++ addons/core/translations/actions/en-us.yaml | 4 + addons/core/translations/resources/en-us.yaml | 3 + .../scopes/scope/projects/targets/index.js | 37 +++ .../routes/scopes/scope/projects/targets.js | 93 +----- .../scopes/scope/projects/targets/index.js | 49 ++- ui/desktop/app/styles/app.scss | 15 + .../scopes/scope/projects/targets/index.hbs | 313 ++++++++---------- ui/desktop/electron-app/src/ipc/handlers.js | 6 +- .../src/services/client-daemon-manager.js | 2 +- .../acceptance/projects/targets/index-test.js | 5 +- .../projects/targets/target-test.js | 2 + ui/desktop/tests/acceptance/scopes-test.js | 9 +- 14 files changed, 419 insertions(+), 288 deletions(-) create mode 100644 addons/api/addon/utils/mqlQuery.js diff --git a/addons/api/addon/handlers/client-daemon-handler.js b/addons/api/addon/handlers/client-daemon-handler.js index 2aec52fe4..43ff35de6 100644 --- a/addons/api/addon/handlers/client-daemon-handler.js +++ b/addons/api/addon/handlers/client-daemon-handler.js @@ -1,11 +1,16 @@ import { pluralize } from 'ember-inflector'; +import { generateMQLExpression } from '../utils/mqlQuery'; /** * Not all types are yet supported by the client daemon so we'll - * just whitelist the ones we need to start with. - * @type {string[]} + * 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[]}} */ -const supportedTypes = ['target', 'session']; +const supportedTypes = { + target: ['id', 'name', 'description', 'address', 'scope_id'], + session: ['id', 'type', 'status', 'endpoint', 'scope_id', 'target_id'], +}; /** * Handler to sit in front of the API request layer @@ -18,20 +23,36 @@ const ClientDaemonHandler = { const { store, data } = context.request; const { type, query } = data; const isClientDaemonRunning = await this.ipc.invoke( - 'isClientDaemonRunning' + 'isClientDaemonRunning', ); - if (!supportedTypes.includes(type) || !isClientDaemonRunning) { + if ( + !Object.keys(supportedTypes).includes(type) || + !isClientDaemonRunning + ) { return next(context.request); } // TODO: Remove usages of recursive from callers and scope_id since // all calls to daemon are recursive, scope_id can be part of search query instead // eslint-disable-next-line no-unused-vars - let { recursive, scope_id, ...modifiedQuery } = query; + let { recursive, scope_id, ...remainingQuery } = query; + let searchQuery = ''; + if (remainingQuery.query) { + const { search, filters } = remainingQuery.query; + + searchQuery = generateMQLExpression({ + search: { + text: search, + fields: supportedTypes[type], + }, + filters, + }); + } const auth_token_id = this.session.data?.authenticated?.id; - modifiedQuery = { - ...modifiedQuery, + remainingQuery = { + ...remainingQuery, + query: searchQuery, auth_token_id, resource: pluralize(type), }; @@ -40,7 +61,7 @@ const ClientDaemonHandler = { // e.g. { targets: [...] } or { sessions: [..] } // So this just unwraps to the array, or undefined const [results] = Object.values( - await this.ipc.invoke('searchClientDaemon', modifiedQuery) + await this.ipc.invoke('searchClientDaemon', remainingQuery), ); const payload = { items: results ?? [] }; @@ -51,7 +72,7 @@ const ClientDaemonHandler = { schema, payload, null, - 'query' + 'query', ); // TODO: Add some pagination diff --git a/addons/api/addon/utils/mqlQuery.js b/addons/api/addon/utils/mqlQuery.js new file mode 100644 index 000000000..39e5535ae --- /dev/null +++ b/addons/api/addon/utils/mqlQuery.js @@ -0,0 +1,128 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: MPL-2.0 + */ + +/** + * Takes a POJO representing a filter query and converts it to an + * MQL expression string which is understood by the client daemon. + * + * @example + * const filter = generateMQLFilterExpression({ + * name: { contains: 'search text' }, + * scope: [{ equals: 'scope1'}, { equals: 'scope2' }], + * }); + * + * // (name % "search text") and (scope = "scope1" or scope = "scope2") + */ +export function generateMQLFilterExpression(filterObj) { + if (!filterObj) { + return null; + } + + const expressions = Object.entries(filterObj) + .map(([key, filterObjValue]) => { + const filterObjValueArray = Array.isArray(filterObjValue) + ? filterObjValue + : [filterObjValue]; + + const orClauses = or( + filterObjValueArray.map((filterObjValue) => { + if (!filterObjValue) { + return null; + } + + const [operation, value] = Object.entries(filterObjValue)[0]; + + switch (operation) { + case 'contains': + return contains(key, value); + case 'gt': + return gt(key, value); + case 'gte': + return gte(key, value); + case 'lt': + return lt(key, value); + case 'lte': + return lte(key, value); + default: + return equals(key, value); + } + }), + ); + + return parenthetical(orClauses); + }) + .filter((item) => item); + return and(expressions); +} + +/** + * Takes a POJO representing a search query and converts it to an + * MQL expression string which is understood by the client daemon. + * + * @example + * const search = generateMQLSearchExpression({ + * text: 'search', + * fields: ['id', 'name', 'description'] + * }); + * + * // (id % 'search' or name % 'search' or description % 'search') + */ +export function generateMQLSearchExpression(searchObj) { + if (!searchObj) { + return null; + } + + const { text, fields } = searchObj; + + return parenthetical( + or(fields.map((field) => contains(field, text)).filter((item) => item)), + ); +} + +/** + * Generates the MQL expression when a user passes in both search and filters. + */ +export function generateMQLExpression(obj) { + const { search, filters } = obj; + + const expressions = [ + generateMQLSearchExpression(search), + generateMQLFilterExpression(filters), + ].filter((item) => item); + + return and(expressions); +} + +const isEmpty = (input) => + input === null || input === undefined || input === ''; + +// Escape any double quotes or backslashes +const sanitize = (input) => input?.replace(/(["\\])/g, '\\$1'); + +// Comparison Operators +const equals = (key, value) => + !isEmpty(value) ? `${key} = "${sanitize(value)}"` : null; + +const contains = (key, value) => + !isEmpty(value) ? `${key} % "${sanitize(value)}"` : null; + +const gt = (key, value) => + !isEmpty(value) ? `${key} > "${sanitize(value)}"` : null; + +const gte = (key, value) => + !isEmpty(value) ? `${key} >= "${sanitize(value)}"` : null; + +const lt = (key, value) => + !isEmpty(value) ? `${key} < "${sanitize(value)}"` : null; + +const lte = (key, value) => + !isEmpty(value) ? `${key} <= "${sanitize(value)}"` : null; + +// Logical Operators +const and = (clauses) => clauses.join(' and '); + +const or = (clauses) => clauses.join(' or '); + +const parenthetical = (expression) => (expression ? `(${expression})` : null); diff --git a/addons/core/translations/actions/en-us.yaml b/addons/core/translations/actions/en-us.yaml index f6f081e8f..c9fff6205 100644 --- a/addons/core/translations/actions/en-us.yaml +++ b/addons/core/translations/actions/en-us.yaml @@ -52,3 +52,7 @@ toggle-color-theme: Toggle Color Theme toggle-feature-edition: Toggle Feature Edition toggle-licensed-features: Toggle Licensed Features more: ', +{remainingItems} more' +search: Search +yes: Yes +no: No +narrow-results: Narrow results diff --git a/addons/core/translations/resources/en-us.yaml b/addons/core/translations/resources/en-us.yaml index 419a35089..9a7956824 100644 --- a/addons/core/translations/resources/en-us.yaml +++ b/addons/core/translations/resources/en-us.yaml @@ -564,6 +564,9 @@ target: connect-more-info: title: Connect for more info description: Additional information for this target will be provided here after your session is established. + no-search-results: + title: No results found + description: Sorry, we weren't able to find any targets for "{query}". titles: new: New Target actions: diff --git a/ui/desktop/app/controllers/scopes/scope/projects/targets/index.js b/ui/desktop/app/controllers/scopes/scope/projects/targets/index.js index 4773cc68d..90bec2a01 100644 --- a/ui/desktop/app/controllers/scopes/scope/projects/targets/index.js +++ b/ui/desktop/app/controllers/scopes/scope/projects/targets/index.js @@ -7,6 +7,7 @@ import Controller from '@ember/controller'; import { inject as service } from '@ember/service'; import { action } from '@ember/object'; import { loading } from 'ember-loading'; +import { tracked } from '@glimmer/tracking'; export default class ScopesScopeProjectsTargetsIndexController extends Controller { // =services @@ -17,8 +18,31 @@ export default class ScopesScopeProjectsTargetsIndexController extends Controlle @service session; @service store; + // =attributes + + queryParams = ['search']; + + @tracked search; + @tracked searchItems = ['Project 1', 'Project 2', 'Project 3']; + // =methods + /** + * Returns true if model is empty but we have a search term + * @returns {boolean} + */ + get noResults() { + return this.model.length === 0 && this.search; + } + + /** + * Returns true if model is empty and we have no search term + * @returns {boolean} + */ + get noTargets() { + return this.model.length === 0 && !this.search; + } + /** * Quick connect method used to call main connect method and handle * connection errors unique to this route @@ -103,4 +127,17 @@ export default class ScopesScopeProjectsTargetsIndexController extends Controlle session_id, ); } + + /** + * Handles input on each keystroke and the search queryParam + * @param {object} event + */ + @action + handleSearchInput(event) { + const { value } = event.target; + this.search = value; + this.router.replaceWith({ + queryParams: { ...this.queryParams, search: this.search }, + }); + } } diff --git a/ui/desktop/app/routes/scopes/scope/projects/targets.js b/ui/desktop/app/routes/scopes/scope/projects/targets.js index 3711b0207..59d085bc8 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets.js @@ -4,96 +4,5 @@ */ import Route from '@ember/routing/route'; -import { inject as service } from '@ember/service'; -import { action } from '@ember/object'; -import { resourceFilter } from 'core/decorators/resource-filter'; -export default class ScopesScopeProjectsTargetsRoute extends Route { - // =services - - @service can; - @service clusterUrl; - @service resourceFilterStore; - @service router; - @service session; - @service store; - - // =attributes - - @resourceFilter({ - allowed: (route) => route.modelFor('scopes.scope.projects'), - serialize: ({ id }) => id, - findBySerialized: ({ id }, value) => id === value, - }) - project; - - // =methods - - /** - * If arriving here unauthenticated, redirect to index for further processing. - */ - beforeModel() { - if (!this.session.isAuthenticated) this.router.transitionTo('index'); - } - - /** - * Loads all targets under current scope. - * - * NOTE: previously, targets were filtered with API filter queries. - * In an effort to offload processing from the controller, targets - * are now filtered on the client by projects and authorized_actions. - * - * @return {Promise{[TargetModel]}} - */ - async model() { - const { id: scope_id } = this.modelFor('scopes.scope'); - const queryOptions = { scope_id, recursive: true }; - - // Recursively query all targets within the current scope - let targets = await this.store.query('target', queryOptions); - - // Filter out targets to which users do not have the connect ability - targets = targets.filter((t) => this.can.can('connect target', t)); - - // If project filters are selected, filter targets by the selected projects - if (this.project?.length) { - const projectIDs = this.project.map((p) => p?.id); - targets = targets.filter((t) => projectIDs.includes(t?.scopeID)); - } - - return targets; - } - - // =actions - - @action - acknowledge(session) { - session.acknowledged = true; - } - - /** - * Sets the specified resource filter field to the specified value. - * @param {string} field - * @param value - */ - @action - filterBy(field, value) { - this[field] = value; - } - - /** - * Clears and filter selections. - */ - @action - clearAllFilters() { - this.project = []; - } - - /** - * refreshes target data. - */ - @action - async refreshTargets() { - return this.refresh(); - } -} +export default class ScopesScopeProjectsTargetsRoute extends Route {} 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 787a10cbb..5de35665f 100644 --- a/ui/desktop/app/routes/scopes/scope/projects/targets/index.js +++ b/ui/desktop/app/routes/scopes/scope/projects/targets/index.js @@ -4,5 +4,52 @@ */ import Route from '@ember/routing/route'; +import { inject as service } from '@ember/service'; -export default class ScopesScopeProjectsTargetsIndexRoute extends Route {} +export default class ScopesScopeProjectsTargetsIndexRoute extends Route { + // =services + + @service can; + @service clusterUrl; + @service resourceFilterStore; + @service router; + @service session; + @service store; + + // =attributes + + queryParams = { + search: { + refreshModel: true, + }, + }; + + // =methods + + /** + * If arriving here unauthenticated, redirect to index for further processing. + */ + beforeModel() { + if (!this.session.isAuthenticated) this.router.transitionTo('index'); + } + + /** + * Loads all targets under current scope. + * + * @return {Promise{[TargetModel]}} + */ + async model({ search = '' }) { + // TODO: Filter targets by scope we're in manually + // const { id: scope_id } = this.modelFor('scopes.scope'); + const queryOptions = { query: { search } }; + + let targets = await this.store.query('target', queryOptions); + + // Filter out targets to which users do not have the connect ability + targets = targets.filter((target) => + this.can.can('connect target', target), + ); + + return targets; + } +} diff --git a/ui/desktop/app/styles/app.scss b/ui/desktop/app/styles/app.scss index ae411bc9e..9f676b770 100644 --- a/ui/desktop/app/styles/app.scss +++ b/ui/desktop/app/styles/app.scss @@ -684,3 +684,18 @@ $chevron-down: url("data:image/svg+xml;utf-8, - - {{#if - (or - @model - (has-resource-filter-selections - 'scopes.scope.projects.targets' 'project' - ) - ) - }} - - - - {{#let - (resource-filter 'scopes.scope.projects.targets' 'project') - as |filter| - }} - - - - {{/let}} - - {{#if - (has-resource-filter-selections - 'scopes.scope.projects.targets' 'project' - ) - }} - {{t 'actions.clear-all-filters'}} - - {{/if}} - - - - - - - - - {{/if}} - - {{#if @model}} - - - <:body as |B|> - - - - {{#if (can 'read model' B.data)}} - + + {{#if (or @model this.search)}} +
+ + + + + + + + {{#each this.searchItems as |item|}} + + {{item}} + + {{/each}} + + + + {{t 'actions.yes'}} + {{t 'actions.no'}} + + + + {{t 'resources.target.types.tcp'}} + {{t 'resources.target.types.ssh'}} + + +
+ {{#if @model}} + + <:body as |B|> + + + + {{#if (can 'read model' B.data)}} + + {{B.data.displayName}} + + {{else}} {{B.data.displayName}} - - {{else}} - {{B.data.displayName}} - {{/if}} - {{#if B.data.description}} -

{{B.data.description}}

- {{/if}} - - - - - {{B.data.id}} - - - - {{#if B.data.type}} - - {{/if}} - - -
+ {{/if}} + {{#if B.data.description}} +

{{B.data.description}}

+ {{/if}} + + + - {{B.data.project.id}} + {{B.data.id}} -
- - - - {{#if (can 'connect target' B.data)}} - {{#if (can 'read target' B.data)}} - - {{else}} - + + {{#if B.data.type}} + {{/if}} - {{/if}} - - - - - - {{else}} - - - - - {{t 'resources.target.messages.none.description'}} - - - - + + +
+ + {{B.data.project.id}} + +
+ + + + {{#if (can 'connect target' B.data)}} + {{#if (can 'read target' B.data)}} + + {{else}} + + {{/if}} + {{/if}} + + + + + {{/if}} + {{#if this.noResults}} + + + + + {{/if}} + {{/if}} + {{#if this.noTargets}} + + + + {{/if}} diff --git a/ui/desktop/electron-app/src/ipc/handlers.js b/ui/desktop/electron-app/src/ipc/handlers.js index 175554e10..60f61a27e 100644 --- a/ui/desktop/electron-app/src/ipc/handlers.js +++ b/ui/desktop/electron-app/src/ipc/handlers.js @@ -123,7 +123,7 @@ handle('checkCommand', async (command) => which(command, { nothrow: true })); * Adds the user's token to the client daemon. */ handle('addTokenToClientDaemon', async (data) => - clientDaemonManager.addToken(data) + clientDaemonManager.addToken(data), ); /** @@ -139,7 +139,7 @@ handle('checkOS', () => ({ * Call the client daemon's search endpoint to retrieve cached results. */ handle('searchClientDaemon', async (request) => - clientDaemonManager.search(request) + clientDaemonManager.search(request), ); /** @@ -147,7 +147,7 @@ handle('searchClientDaemon', async (request) => * socket path as a proxy for whether the daemon is running. */ handle('isClientDaemonRunning', async (request) => - Boolean(clientDaemonManager.socketPath) + Boolean(clientDaemonManager.socketPath), ); /** diff --git a/ui/desktop/electron-app/src/services/client-daemon-manager.js b/ui/desktop/electron-app/src/services/client-daemon-manager.js index f3457e4d9..54fcb6d77 100644 --- a/ui/desktop/electron-app/src/services/client-daemon-manager.js +++ b/ui/desktop/electron-app/src/services/client-daemon-manager.js @@ -24,7 +24,7 @@ class ClientDaemonManager { const data = spawnSync(daemonStatusCommand); const status = jsonify(data); - this.#socketPath = status?.socket_address; + this.#socketPath = status?.item?.socket_address; return status; } diff --git a/ui/desktop/tests/acceptance/projects/targets/index-test.js b/ui/desktop/tests/acceptance/projects/targets/index-test.js index 1d64e499d..0c8383abb 100644 --- a/ui/desktop/tests/acceptance/projects/targets/index-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/index-test.js @@ -29,7 +29,6 @@ module('Acceptance | projects | targets', function (hooks) { let getTargetCount; - const ROSE_APP_STATE_TITLE = '.rose-message-title'; const APP_STATE_TITLE = '.hds-application-state__title'; const TARGET_DETAILS_ROUTE_NAME = 'scopes.scope.projects.targets.target.index'; @@ -84,6 +83,8 @@ module('Acceptance | projects | targets', function (hooks) { hooks.beforeEach(function () { authenticateSession(); + // bypass mirage config that expects recursive to be passed in as queryParam + this.server.get('/targets', ({ targets }) => targets.all()); // Generate scopes instances.scopes.global = this.server.create('scope', { id: 'global' }); @@ -190,7 +191,7 @@ module('Acceptance | projects | targets', function (hooks) { await click(`[href="${urls.targets}"]`); - assert.dom(ROSE_APP_STATE_TITLE).hasText('No Targets Available'); + assert.dom(APP_STATE_TITLE).hasText('No Targets Available'); }); test('user cannot navigate to a target without proper authorization', async function (assert) { diff --git a/ui/desktop/tests/acceptance/projects/targets/target-test.js b/ui/desktop/tests/acceptance/projects/targets/target-test.js index ebdfb26b3..4592b9e53 100644 --- a/ui/desktop/tests/acceptance/projects/targets/target-test.js +++ b/ui/desktop/tests/acceptance/projects/targets/target-test.js @@ -68,6 +68,8 @@ module('Acceptance | projects | targets | target', function (hooks) { hooks.beforeEach(function () { authenticateSession(); + // bypass mirage config that expects recursive to be passed in as queryParam + this.server.get('/targets', ({ targets }) => targets.all()); // Generate scopes instances.scopes.global = this.server.create('scope', { id: 'global' }); diff --git a/ui/desktop/tests/acceptance/scopes-test.js b/ui/desktop/tests/acceptance/scopes-test.js index a459b9341..099b44085 100644 --- a/ui/desktop/tests/acceptance/scopes-test.js +++ b/ui/desktop/tests/acceptance/scopes-test.js @@ -22,6 +22,8 @@ module('Acceptance | scopes', function (hooks) { setupApplicationTest(hooks); setupMirage(hooks); + const APP_STATE_TITLE = '.hds-application-state__title'; + const instances = { scopes: { global: null, @@ -72,6 +74,8 @@ module('Acceptance | scopes', function (hooks) { hooks.beforeEach(function () { authenticateSession(); + // bypass mirage config that expects recursive to be passed in as queryParam + this.server.get('/targets', ({ targets }) => targets.all()); // create scopes instances.scopes.global = this.server.create('scope', { id: 'global' }); @@ -228,10 +232,7 @@ module('Acceptance | scopes', function (hooks) { await visit(urls.targets); - assert.ok( - find('.rose-message-title').textContent.trim(), - 'No Targets Available', - ); + assert.ok(find(APP_STATE_TITLE).textContent.trim(), 'No Targets Available'); }); test.skip('connecting to a target', async function (assert) {