Secure Variables: Build a path tree and traverse it at /variables/*path (#13202)

* Recursive trie-building with variable paths

* tree structure applied to new path routes and a new util class

* Breadcrumbs for SV paths and prompt when nothing exists at a path

* Lint and test cleanup

* Pre-review cleanup

* lintfix

* Abstracted pathtree each-ins into a new component class

* Path tree component styles

* Types added and PR feedback addressed

* Path tree to variable paths

* Slightly simpler path QP mods

* More pr feedback handling

* Trim moved into a function on variable model

* Traversal and compaction tests for PathTree

* Trim Path tests

* Variable-paths component tests

* Lint fixup for tests
This commit is contained in:
Phil Renaud 2022-06-06 21:42:23 -04:00 committed by Tim Gross
parent d5a214484c
commit da4cb6422e
23 changed files with 672 additions and 94 deletions

View File

@ -35,8 +35,7 @@ export default class SecureVariableFormComponent extends Component {
@action
async save(e) {
e.preventDefault();
this.args.model.id = this.args.model.path;
this.args.model.setAndTrimPath();
const transitionTarget = this.args.model.isNew
? 'variables'

View File

@ -0,0 +1,36 @@
<ListTable class="path-tree" @source={{@branch}} as |t|>
<t.head>
<th>
Path
</th>
</t.head>
<tbody>
{{#each this.folders as |folder|}}
<tr data-test-folder-row {{on "click" (fn this.handleFolderClick folder.data.absolutePath)}}>
<td>
<span>
<FlightIcon @name="folder" />
<LinkTo @route="variables.path" @model={{folder.data.absolutePath}}>
{{trim-path folder.name}}
</LinkTo>
</span>
</td>
</tr>
{{/each}}
{{#each this.files as |file|}}
<tr data-test-file-row {{on "click" (fn this.handleFileClick file.absoluteFilePath)}}>
<td>
<FlightIcon @name="file-text" />
<LinkTo
@route="variables.variable"
@model={{file.absoluteFilePath}}
>
{{file.name}}
</LinkTo>
</td>
</tr>
{{/each}}
</tbody>
</ListTable>

View File

@ -0,0 +1,31 @@
// @ts-check
import Component from '@glimmer/component';
import { action } from '@ember/object';
import { inject as service } from '@ember/service';
export default class VariablePathsComponent extends Component {
@service router;
/**
* @returns {Array<Object.<string, Object>>}
*/
get folders() {
return Object.entries(this.args.branch.children).map(([name, data]) => {
return { name, data };
});
}
get files() {
return this.args.branch.files;
}
@action
async handleFolderClick(path) {
this.router.transitionTo('variables.path', path);
}
@action
async handleFileClick(path) {
this.router.transitionTo('variables.variable', path);
}
}

View File

@ -1,2 +1,4 @@
import Controller from '@ember/controller';
export default class VariablesNewController extends Controller {}
export default class VariablesNewController extends Controller {
queryParams = ['path'];
}

View File

@ -0,0 +1,15 @@
import Controller from '@ember/controller';
export default class VariablesPathController extends Controller {
get breadcrumbs() {
let crumbs = [];
this.model.absolutePath.split('/').reduce((m, n) => {
crumbs.push({
label: n,
args: [`variables.path`, m + n],
});
return m + n + '/';
}, []);
return crumbs;
}
}

View File

@ -1,10 +1,18 @@
import Controller from '@ember/controller';
export default class VariablesVariableController extends Controller {
get breadcrumb() {
return {
label: this.model.path,
args: [`variables.variable`, this.model.path],
};
get breadcrumbs() {
let crumbs = [];
this.model.path.split('/').reduce((m, n) => {
crumbs.push({
label: n,
args:
m + n === this.model.path // If the last crumb, link to the var itself
? [`variables.variable`, m + n]
: [`variables.path`, m + n],
});
return m + n + '/';
}, []);
return crumbs;
}
}

View File

@ -0,0 +1,19 @@
// @ts-check
import Helper from '@ember/component/helper';
/**
* Trims any number of slashes from the beginning and end of a string.
* @param {Array<string>} params
* @returns {string}
*/
export function trimPath([path = '']) {
if (path.startsWith('/')) {
path = trimPath([path.slice(1)]);
}
if (path.endsWith('/')) {
path = trimPath([path.slice(0, -1)]);
}
return path;
}
export default Helper.helper(trimPath);

View File

@ -1,25 +1,40 @@
// @ts-check
import Model from '@ember-data/model';
import { attr } from '@ember-data/model';
import classic from 'ember-classic-decorator';
// eslint-disable-next-line no-unused-vars
import MutableArray from '@ember/array/mutable';
import { trimPath } from '../helpers/trim-path';
/**
* @typedef SecureVariable
* @typedef KeyValue
* @type {object}
* @property {string} key
* @property {string} value
*/
/**
* @typedef SecureVariable
* @type {object}
*/
/**
* A Secure Variable has a path, namespace, and an array of key-value pairs within the client.
* On the server, these key-value pairs are serialized into object structure.
* @class
* @extends Model
*/
@classic
export default class VariableModel extends Model {
/**
* Can be any arbitrary string, but behaves best when used as a slash-delimited file path.
*
* @type {string}
*/
@attr('string') path;
@attr('string') namespace;
/**
* @type {MutableArray<SecureVariable>}
* @type {MutableArray<KeyValue>}
*/
@attr({
defaultValue() {
@ -27,4 +42,23 @@ export default class VariableModel extends Model {
},
})
keyValues;
/** @type {number} */
@attr('number') createIndex;
/** @type {number} */
@attr('number') modifyIndex;
/** @type {string} */
@attr('string') createTime;
/** @type {string} */
@attr('string') modifyTime;
/** @type {string} */
@attr('string') namespace;
/**
* Removes starting and trailing slashes, and sets the ID property
*/
setAndTrimPath() {
this.path = trimPath([this.path]);
this.id = this.path;
}
}

View File

@ -90,5 +90,9 @@ Router.map(function () {
this.route('edit');
}
);
this.route('path', {
path: '/path/*absolutePath',
});
});
});

View File

@ -1,21 +1,28 @@
import Route from '@ember/routing/route';
import { inject as service } from '@ember/service';
import WithForbiddenState from 'nomad-ui/mixins/with-forbidden-state';
import RSVP from 'rsvp';
import notifyForbidden from 'nomad-ui/utils/notify-forbidden';
import notifyError from 'nomad-ui/utils/notify-error';
import PathTree from 'nomad-ui/utils/path-tree';
export default class VariablesRoute extends Route.extend(WithForbiddenState) {
@service can;
@service router;
@service store;
beforeModel() {
if (this.can.cannot('list variables')) {
this.router.transitionTo('/jobs');
}
}
model() {
return RSVP.hash({
variables: this.store.findAll('variable'),
}).catch(notifyForbidden(this));
async model() {
try {
const variables = await this.store.findAll('variable');
return {
variables,
pathTree: new PathTree(variables),
};
} catch (e) {
notifyError(this)(e);
}
}
}

View File

@ -1,10 +1,12 @@
import Route from '@ember/routing/route';
export default class VariablesNewRoute extends Route {
model() {
return this.store.createRecord('variable');
model(params) {
return this.store.createRecord('variable', { path: params.path });
}
resetController(controller, isExiting) {
// If the user navigates away from /new, clear the path
controller.set('path', null);
if (isExiting) {
// If user didn't save, delete the freshly created model
if (controller.model.isNew) {

View File

@ -0,0 +1,13 @@
import Route from '@ember/routing/route';
export default class VariablesPathRoute extends Route {
model({ absolutePath }) {
const treeAtPath =
this.modelFor('variables').pathTree.findPath(absolutePath);
if (treeAtPath) {
return { treeAtPath, absolutePath };
} else {
return { absolutePath };
}
}
}

View File

@ -7,6 +7,6 @@ export default class VariablesVariableRoute extends Route.extend(
WithModelErrorHandling
) {
model(params) {
return this.store.findRecord('variable', params.path);
return this.store.findRecord('variable', decodeURIComponent(params.path));
}
}

View File

@ -23,8 +23,18 @@
border-color: $grey-blue;
}
}
// .add-more:focus {
// background-color: $grey-lighter;
// }
}
table.path-tree {
tr {
cursor: pointer;
a {
color: #0a0a0a;
text-decoration: none;
}
svg {
margin-bottom: -2px;
margin-right: 10px;
}
}
}

View File

@ -1,74 +1,51 @@
{{page-title "Secure Variables"}}
<section class="section">
{{#if this.isForbidden}}
<ForbiddenMessage />
{{else}}
<div class="toolbar">
<div class="toolbar-item">
{{#if this.variables.length}}
<SearchBox
@searchTerm={{mut this.searchTerm}}
@onChange={{action this.resetPagination}}
@placeholder="Search variables..."
/>
{{/if}}
</div>
<div class="toolbar-item is-right-aligned is-mobile-full-width">
<div class="button-bar">
{{#if (can "create variable" namespace=this.qpNamespace)}}
<LinkTo
@route="variables.new"
@query={{hash namespace=this.qpNamespace}}
data-test-run-job
class="button is-primary"
>
Create Secure Variable
</LinkTo>
{{else}}
<button
data-test-run-job
class="button is-primary is-disabled tooltip is-right-aligned"
aria-label="You dont have sufficient permissions"
disabled
type="button"
>
Create Secure Variable
</button>
{{/if}}
<div class="toolbar">
<div class="toolbar-item">
{{#if this.variables.length}}
<SearchBox
@searchTerm={{mut this.searchTerm}}
@onChange={{action this.resetPagination}}
@placeholder="Search variables..."
/>
{{/if}}
</div>
<div class="toolbar-item is-right-aligned is-mobile-full-width">
<div class="button-bar">
{{#if (can "create variable" namespace=this.qpNamespace)}}
<LinkTo
@route="variables.new"
@query={{hash namespace=this.qpNamespace}}
class="button is-primary"
>
Create Secure Variable
</LinkTo>
{{else}}
<button
class="button is-primary is-disabled tooltip is-right-aligned"
aria-label="You dont have sufficient permissions"
disabled
type="button"
>
Create Secure Variable
</button>
{{/if}}
</div>
</div>
</div>
{{#if @model.variables.length}}
<ListTable data-test-eval-table @source={{@model.variables}} as |t|>
<t.head>
<th>
Path
</th>
<th>
Namespace
</th>
</t.head>
<t.body as |row|>
<tr {{on "click" (fn this.goToVariable row.model)}}>
<td>
{{row.model.path}}
</td>
<td>
{{row.model.namespace}}
</td>
</tr>
</t.body>
</ListTable>
{{else}}
<div class="empty-message">
<h3 data-test-empty-volumes-list-headline class="empty-message-headline">
No Secure Variables
</h3>
<p class="empty-message-body">
Get started by <LinkTo @route="variables.new">creating a new secure variable</LinkTo>
</p>
</div>
{{/if}}
</div>
{{#if @model.variables.length}}
<VariablePaths
@branch={{this.model.pathTree.paths.root}}
/>
{{else}}
<div class="empty-message">
<h3 data-test-empty-volumes-list-headline class="empty-message-headline">
No Secure Variables
</h3>
<p class="empty-message-body">
Get started by <LinkTo @route="variables.new">creating a new secure variable</LinkTo>
</p>
</div>
{{/if}}
</section>

View File

@ -1,5 +1,5 @@
{{page-title "New Secure Variable"}}
<Breadcrumb @crumb={{hash label="New" args=(array "variables.new")}} />
<section class="section">
<SecureVariableForm @model={{this.model}} />
<SecureVariableForm @model={{this.model}} @path={{this.path}} />
</section>

View File

@ -0,0 +1,46 @@
{{page-title "Secure Variables: " this.model.absolutePath}}
{{#each this.breadcrumbs as |crumb|}}
<Breadcrumb @crumb={{crumb}} />
{{/each}}
<section class="section">
<div class="toolbar">
<div class="toolbar-item is-right-aligned is-mobile-full-width">
<div class="button-bar">
{{!-- TODO: make sure qpNamespace persists to here --}}
{{#if (can "create variable" namespace=this.qpNamespace)}}
<LinkTo
@route="variables.new"
@query={{hash namespace=this.qpNamespace path=(concat this.model.absolutePath "/")}}
class="button is-primary"
>
Create Secure Variable
</LinkTo>
{{else}}
<button
class="button is-primary is-disabled tooltip is-right-aligned"
aria-label="You dont have sufficient permissions"
disabled
type="button"
>
Create Secure Variable
</button>
{{/if}}
</div>
</div>
</div>
{{#if this.model.treeAtPath}}
<VariablePaths
@branch={{this.model.treeAtPath}}
/>
{{else}}
<div class="empty-message">
<h3 data-test-empty-volumes-list-headline class="empty-message-headline">
Path /{{this.model.absolutePath}} contains no variables
</h3>
<p class="empty-message-body">
To get started, <LinkTo @route="variables.new" @query={{hash path=(concat this.model.absolutePath "/")}}>create a new secure variable here</LinkTo>, or <LinkTo @route="variables">go back to the Secure Variables root directory</LinkTo>.
</p>
</div>
{{/if}}
</section>

View File

@ -1,5 +1,7 @@
{{page-title "Secure Variables: " this.model.path}}
<Breadcrumb @crumb={{this.breadcrumb}} />
{{#each this.breadcrumbs as |crumb|}}
<Breadcrumb @crumb={{crumb}} />
{{/each}}
<section class="section">
{{outlet}}
</section>

123
ui/app/utils/path-tree.js Normal file
View File

@ -0,0 +1,123 @@
// @ts-check
// eslint-disable-next-line no-unused-vars
import VariableModel from '../models/variable';
// eslint-disable-next-line no-unused-vars
import MutableArray from '@ember/array/mutable';
import { trimPath } from '../helpers/trim-path';
//#region Types
/**
* @typedef {Object} VariablePathObject
* @property {string} path - the folder path containing our "file", relative to parent
* @property {string} name - the secure variable "file" name
* @property {string} [absoluteFilePath] - the folder path containing our "file", absolute
* @property {string} [absolutePath] - the folder path containing our "file", absolute
*/
/**
* @typedef {Object.<string, Object>} NestedPathTreeNode
*/
//#endregion Types
/**
* Turns a file path into an object with file and path properties.
* @param {string} path - the file path
* @return {VariablePathObject}
*/
function PATH_TO_OBJECT(path) {
const split = path.split('/');
const [name, ...folderPath] = [split.pop(), ...split];
return {
name,
absoluteFilePath: path,
path: folderPath.join('/'),
};
}
/**
* Compacts an object of path:file key-values so any same-common-ancestor paths are collapsed into a single path.
* @param {NestedPathTreeNode} vars
* @returns {void}}
*/
function COMPACT_EMPTY_DIRS(vars) {
Object.keys(vars).map((pathString) => {
const encompasser = Object.keys(vars).find(
(ps) => ps !== pathString && pathString.startsWith(ps)
);
if (encompasser) {
vars[encompasser].children[pathString.replace(encompasser, '')] =
vars[pathString];
delete vars[pathString];
COMPACT_EMPTY_DIRS(vars[encompasser].children);
}
});
}
/**
* @returns {NestedPathTreeNode}
*/
export default class PathTree {
/**
* @param {MutableArray<VariableModel>} variables
*/
constructor(variables) {
this.variables = variables;
this.paths = this.generatePaths();
}
/**
* Takes our variables array and groups them by common path
* @returns {NestedPathTreeNode}
*/
generatePaths = () => {
const paths = this.variables
.map((variable) => trimPath([variable.path]))
.map(PATH_TO_OBJECT)
.reduce(
(acc, cur) => {
const { name, absoluteFilePath } = cur;
if (cur.path) {
acc.root.children[cur.path]
? acc.root.children[cur.path].files.push({
name,
absoluteFilePath,
})
: (acc.root.children[cur.path] = {
files: [{ name, absoluteFilePath }],
children: {},
});
acc.root.children[cur.path].absolutePath = cur.path;
} else {
acc.root.files
? acc.root.files.push({ name, absoluteFilePath })
: (acc.root.files = [{ name, absoluteFilePath }]);
}
return acc;
},
{ root: { files: [], children: {}, absolutePath: '' } }
);
COMPACT_EMPTY_DIRS(paths.root.children);
return paths;
};
/**
* Search for the named absolutePath within our tree using recursion
* @param {string} name
* @param {Object} root
*/
findPath = (name, root = this.paths.root) => {
if (root.absolutePath === name) {
return root;
}
if (root.children) {
return Object.keys(root.children).reduce((acc, cur) => {
if (!acc) {
return this.findPath(name, root.children[cur]);
}
return acc;
}, null);
}
};
}

View File

@ -51,6 +51,23 @@ function smallCluster(server) {
server.create('allocFile', 'dir', { depth: 2 });
server.createList('csi-plugin', 2);
server.createList('variable', 3);
[
'a/b/c/foo0',
'a/b/c/bar1',
'a/b/c/d/e/foo2',
'a/b/c/d/e/bar3',
'a/b/c/d/e/f/foo4',
'a/b/c/d/e/f/g/foo5',
'a/b/c/x/y/z/foo6',
'a/b/c/x/y/z/bar7',
'a/b/c/x/y/z/baz8',
'w/x/y/foo9',
'w/x/y/z/foo10',
'w/x/y/z/bar11',
'just some arbitrary file',
'another arbitrary file',
'another arbitrary file again',
].forEach((path) => server.create('variable', { path }));
// #region evaluations

View File

@ -0,0 +1,106 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import { componentA11yAudit } from 'nomad-ui/tests/helpers/a11y-audit';
import pathTree from 'nomad-ui/utils/path-tree';
const PATHSTRINGS = [
{ path: '/foo/bar/baz' },
{ path: '/foo/bar/bay' },
{ path: '/foo/bar/bax' },
{ path: '/a/b' },
{ path: '/a/b/c' },
{ path: '/a/b/canary' },
{ path: '/a/b/canine' },
{ path: '/a/b/chipmunk' },
{ path: '/a/b/c/d' },
{ path: '/a/b/c/dalmation/index' },
{ path: '/a/b/c/doberman/index' },
{ path: '/a/b/c/dachshund/index' },
{ path: '/a/b/c/dachshund/poppy' },
];
const tree = new pathTree(PATHSTRINGS);
module('Integration | Component | variable-paths', function (hooks) {
setupRenderingTest(hooks);
test('it renders without data', async function (assert) {
assert.expect(2);
this.set('emptyRoot', { children: {}, files: [] });
await render(hbs`<VariablePaths @branch={{this.emptyRoot}} />`);
assert.dom('tbody tr').exists({ count: 0 });
await componentA11yAudit(this.element, assert);
});
test('it renders with data', async function (assert) {
assert.expect(2);
this.set('tree', tree);
await render(hbs`<VariablePaths @branch={{this.tree.paths.root}} />`);
assert.dom('tbody tr').exists({ count: 2 }, 'There are two rows');
await componentA11yAudit(this.element, assert);
});
test('it allows for traversal: Folders', async function (assert) {
assert.expect(3);
this.set('tree', tree);
await render(hbs`<VariablePaths @branch={{this.tree.paths.root}} />`);
assert
.dom('tbody tr:first-child td:first-child a')
.hasAttribute(
'href',
'/ui/variables/path/foo/bar',
'Correctly links a folder'
);
assert
.dom('tbody tr:first-child svg')
.hasAttribute(
'data-test-icon',
'folder',
'Correctly renders the folder icon'
);
await componentA11yAudit(this.element, assert);
});
test('it allows for traversal: Files', async function (assert) {
assert.expect(5);
this.set('tree', tree.findPath('foo/bar'));
await render(hbs`<VariablePaths @branch={{this.tree}} />`);
assert
.dom('tbody tr:first-child td:first-child a')
.hasAttribute(
'href',
'/ui/variables/foo/bar/baz',
'Correctly links the first file'
);
assert
.dom('tbody tr:nth-child(2) td:first-child a')
.hasAttribute(
'href',
'/ui/variables/foo/bar/bay',
'Correctly links the second file'
);
assert
.dom('tbody tr:nth-child(3) td:first-child a')
.hasAttribute(
'href',
'/ui/variables/foo/bar/bax',
'Correctly links the third file'
);
assert
.dom('tbody tr:first-child svg')
.hasAttribute(
'data-test-icon',
'file-text',
'Correctly renders the file icon'
);
await componentA11yAudit(this.element, assert);
});
});

View File

@ -0,0 +1,29 @@
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { render } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
module('Integration | Helper | trim-path', function (hooks) {
setupRenderingTest(hooks);
test('it doesnt mess with internal slashes', async function (assert) {
this.set('inputValue', 'a/b/c/d');
await render(hbs`{{trim-path this.inputValue}}`);
assert.dom(this.element).hasText('a/b/c/d');
});
test('it will remove a prefix slash', async function (assert) {
this.set('inputValue', '/a/b/c/d');
await render(hbs`{{trim-path this.inputValue}}`);
assert.dom(this.element).hasText('a/b/c/d');
});
test('it will remove a suffix slash', async function (assert) {
this.set('inputValue', 'a/b/c/d/');
await render(hbs`{{trim-path this.inputValue}}`);
assert.dom(this.element).hasText('a/b/c/d');
});
test('it will remove both at once', async function (assert) {
this.set('inputValue', '/a/b/c/d/');
await render(hbs`{{trim-path this.inputValue}}`);
assert.dom(this.element).hasText('a/b/c/d');
});
});

View File

@ -0,0 +1,98 @@
import pathTree from 'nomad-ui/utils/path-tree';
import { module, test } from 'qunit';
const PATHSTRINGS = [
{ path: '/foo/bar/baz' },
{ path: '/foo/bar/bay' },
{ path: '/foo/bar/bax' },
{ path: '/a/b' },
{ path: '/a/b/c' },
{ path: '/a/b/canary' },
{ path: '/a/b/canine' },
{ path: '/a/b/chipmunk' },
{ path: '/a/b/c/d' },
{ path: '/a/b/c/dalmation/index' },
{ path: '/a/b/c/doberman/index' },
{ path: '/a/b/c/dachshund/index' },
{ path: '/a/b/c/dachshund/poppy' },
];
module('Unit | Utility | path-tree', function () {
test('it converts path strings to a Variable Path Object ', function (assert) {
const tree = new pathTree(PATHSTRINGS);
assert.ok(
'root' in tree.paths,
'Tree has a paths object that begins with a root'
);
assert.ok('children' in tree.paths.root, 'Root has children');
assert.equal(
Object.keys(tree.paths.root.children).length,
2,
'Root has 2 children (a[...] and foo[...])'
);
});
test('it compacts empty folders correctly', function (assert) {
const tree = new pathTree(PATHSTRINGS);
assert.ok(
'a' in tree.paths.root.children,
'root.a is uncompacted since it contains a file (b)'
);
assert.notOk(
'foo' in tree.paths.root.children,
'root.foo does not exist since it contains no files'
);
assert.ok(
'foo/bar' in tree.paths.root.children,
'root.foo/bar is compacted since the only child from foo is bar'
);
assert.equal(
tree.paths.root.children['foo/bar'].files.length,
3,
'A compacted directory contains all terminal files'
);
});
test('it allows for node-based search and traversal', function (assert) {
const tree = new pathTree(PATHSTRINGS);
assert.deepEqual(
tree.paths.root,
tree.findPath(''),
'Returns tree root on default findPath'
);
assert.notOk(
tree.findPath('foo'),
'No path found at the first part of a concatenated folder'
); // TODO: but maybe we want this to work eventually, so if this test fails because you add mid-tree traversal? Great!
assert.ok(
tree.findPath('foo/bar'),
'Finds a path at the concatenated folder path'
);
assert.ok(
tree.findPath('a/b'),
'Finds a path at the concatenated folder path with multiple subdirectories'
);
assert.equal(
Object.keys(tree.findPath('a/b/c').children).length,
3,
'Multiple subdirectories are listed at a found compacted path with many child paths'
);
assert.equal(
Object.keys(tree.findPath('a/b').files).length,
4,
'Multiple files are listed at a found non-terminal compacted path with many secure variables'
);
assert.equal(
Object.keys(tree.findPath('a/b/c/doberman').files).length,
1,
'One file listed at a found compacted path with a single secure variable'
);
assert.equal(
Object.keys(tree.findPath('a/b/c/dachshund').files).length,
2,
'Multiple files listed at a found terminal compacted path with many secure variables'
);
});
});