Skip to content

Commit

Permalink
[ui] Job Variables page (#17964)
Browse files Browse the repository at this point in the history
* Bones of a component that has job variable awareness

* Got vars listed woo

* Variables as its own subnav and some pathLinkedVariable perf fixes

* Automatic Access to Variables alerter

* Helper and component to conditionally render the right link

* A bit of cleanup post-template stuff

* testfix for looping right-arrow keynav bc we have a new subnav section

* A very roundabout way of ensuring that, if a job exists when saving a variable with a pathLinkedEntity of that job, its saved right through to the job itself

* hacky but an async version of pathLinkedVariable

* model-driven and async fetcher driven with cleanup

* Only run the update-job func if jobname is detected in var path

* Test cases begun

* Management token for variables to appear in tests

* Its a management token so it gets to see the clients tab under system jobs

* Pre-review cleanup

* More tests

* Number of requests test and small fix to groups-by-way-or-resource-arrays elsewhere

* Variable intro text tests

* Variable name re-use

* Simplifying our wording a bit

* parse json vs plainId

* Addressed PR feedback, including de-waterfalling
  • Loading branch information
philrenaud authored Jul 31, 2023
1 parent 4fb5bf9 commit 18dd9e7
Show file tree
Hide file tree
Showing 22 changed files with 698 additions and 14 deletions.
3 changes: 3 additions & 0 deletions .changelog/17964.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: adds a new Variables page to all job pages
```
21 changes: 21 additions & 0 deletions ui/app/abilities/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ export default class Job extends AbstractAbility {
)
canScale;

@or(
'bypassAuthorization',
'selfTokenIsManagement',
'specificNamespaceSupportsReading',
'policiesSupportReading'
)
canRead;

// TODO: A person can also see all jobs if their token grants read access to all namespaces,
// but given the complexity of namespaces and policy precedence, there isn't a good quick way
// to confirm this.
Expand Down Expand Up @@ -59,11 +67,24 @@ export default class Job extends AbstractAbility {
);
}

@computed('token.selfTokenPolicies.[]')
get policiesSupportReading() {
return this.policyNamespacesIncludePermissions(
this.token.selfTokenPolicies,
['read-job']
);
}

@computed('[email protected]')
get specificNamespaceSupportsRunning() {
return this.namespaceIncludesCapability('submit-job');
}

@computed('[email protected]')
get specificNamespaceSupportsReading() {
return this.namespaceIncludesCapability('read-job');
}

@computed('[email protected]')
get policiesSupportScaling() {
return this.namespaceIncludesCapability('scale-job');
Expand Down
17 changes: 17 additions & 0 deletions ui/app/components/editable-variable-link.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{{!
Copyright (c) HashiCorp, Inc.
SPDX-License-Identifier: MPL-2.0
~}}

{{!-- Either link to a new variable with a pre-filled path, or the existing variable in edit mode, depending if it exists --}}
{{#if (can "write variable")}}
{{#with (editable-variable-link @path existingPaths=@existingPaths namespace=@namespace) as |link|}}
{{#if link.model}}
<LinkTo @route={{link.route}} @model={{link.model}} @query={{link.query}}>{{@path}}</LinkTo>
{{else}}
<LinkTo @route={{link.route}} @query={{link.query}}>{{@path}}</LinkTo>
{{/if}}
{{/with}}
{{else}}
@path
{{/if}}
4 changes: 2 additions & 2 deletions ui/app/components/variable-form.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
@job={{@model.pathLinkedEntities.job}}
@group={{@model.pathLinkedEntities.group}}
@task={{@model.pathLinkedEntities.task}}
@namespace={{this.variableNamespace}}
@namespace={{or this.variableNamespace "default"}}
/>
{{/if}}

Expand Down Expand Up @@ -73,7 +73,7 @@
Please choose a different path, or
<LinkTo
@route="variables.variable.edit"
@model={{this.duplicatePathWarning.path}}
@model={{concat this.duplicatePathWarning.path "@" (or this.variableNamespace "default")}}
>
edit the existing variable
</LinkTo>
Expand Down
30 changes: 30 additions & 0 deletions ui/app/components/variable-form.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export default class VariableFormComponent extends Component {
@service notifications;
@service router;
@service store;
@service can;

@tracked variableNamespace = null;
@tracked namespaceOptions = null;
Expand Down Expand Up @@ -255,6 +256,15 @@ export default class VariableFormComponent extends Component {
message: `${this.path} successfully saved`,
color: 'success',
});

if (
this.can.can('read job', null, {
namespace: this.variableNamespace || 'default',
})
) {
this.updateJobVariables(this.args.model.pathLinkedEntities.job);
}

this.removeExitHandler();
this.router.transitionTo('variables.variable', this.args.model.id);
} catch (error) {
Expand All @@ -275,6 +285,26 @@ export default class VariableFormComponent extends Component {
}
}

/**
* A job, its task groups, and tasks, all have a getter called pathLinkedVariable.
* These are dependent on a variables list that may already be established. If a variable
* is added or removed, this function will update job.variables[] list to reflect the change.
* and force an update to the job's pathLinkedVariable getter.
*/
async updateJobVariables(jobName) {
if (!jobName) {
return;
}
const fullJobId = JSON.stringify([
jobName,
this.variableNamespace || 'default',
]);
let job = await this.store.findRecord('job', fullJobId, { reload: true });
if (job) {
job.variables.pushObject(this.args.model);
}
}

//#region JSON Editing

view = this.args.view;
Expand Down
2 changes: 1 addition & 1 deletion ui/app/components/variable-paths.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

{{#each this.files as |file|}}
<tr
data-test-file-row
data-test-file-row="{{file.name}}"
{{on "click" (fn this.handleFileClick file)}}
class={{if (can "read variable" path=file.absoluteFilePath namespace=file.variable.namespace) "" "inaccessible"}}
{{keyboard-shortcut
Expand Down
58 changes: 58 additions & 0 deletions ui/app/controllers/jobs/job/variables.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* Copyright (c) HashiCorp, Inc.
* SPDX-License-Identifier: MPL-2.0
*/

// @ts-check

import Controller from '@ember/controller';
import { alias } from '@ember/object/computed';
// eslint-disable-next-line no-unused-vars
import VariableModel from '../../../models/variable';
// eslint-disable-next-line no-unused-vars
import JobModel from '../../../models/job';
// eslint-disable-next-line no-unused-vars
import MutableArray from '@ember/array/mutable';

export default class JobsJobVariablesController extends Controller {
/** @type {JobModel} */
@alias('model.job') job;

/** @type {MutableArray<VariableModel>} */
@alias('model.variables') variables;

get firstFewTaskGroupNames() {
return this.job.taskGroups.slice(0, 2).mapBy('name');
}

get firstFewTaskNames() {
return this.job.taskGroups
.map((tg) => tg.tasks.map((task) => `${tg.name}/${task.name}`))
.flat()
.slice(0, 2);
}

/**
* Structures the flattened variables in a "path tree" like we use in the main variables routes
* @returns {import("../../../utils/path-tree").VariableFolder}
*/
get jobRelevantVariables() {
/**
* @type {import("../../../utils/path-tree").VariableFile[]}
*/
let variableFiles = this.variables.map((v) => {
return {
name: v.path,
path: v.path,
absoluteFilePath: v.path,
variable: v,
};
});

return {
files: variableFiles,
children: {},
absolutePath: '',
};
}
}
47 changes: 47 additions & 0 deletions ui/app/helpers/editable-variable-link.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* Copyright (c) HashiCorp, Inc.
* SPDX-License-Identifier: MPL-2.0
*/

// @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';

/**
* @typedef LinkToParams
* @property {string} route
* @property {string} model
* @property {Object} query
*/

import Helper from '@ember/component/helper';

/**
* Either generates a link to edit an existing variable, or else create a new one with a pre-filled path, depending on whether a variable with the given path already exists.
* Returns an object with route, model, and query; all strings.
* @param {Array<string>} positional
* @param {{ existingPaths: MutableArray<VariableModel>, namespace: string }} named
* @returns {LinkToParams}
*/
export function editableVariableLink(
[path],
{ existingPaths, namespace = 'default' }
) {
if (existingPaths.findBy('path', path)) {
return {
route: 'variables.variable.edit',
model: `${path}@${namespace}`,
query: {},
};
} else {
return {
route: 'variables.new',
model: '',
query: { path },
};
}
}

export default Helper.helper(editableVariableLink);
15 changes: 14 additions & 1 deletion ui/app/models/job.js
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ export default class Job extends Model {
// that will be submitted to the create job endpoint, another prop is necessary.
@attr('string') _newDefinitionJSON;

@computed('variables', 'parent', 'plainId')
@computed('variables.[]', 'parent', 'plainId')
get pathLinkedVariable() {
if (this.parent.get('id')) {
return this.variables?.findBy(
Expand All @@ -366,4 +366,17 @@ export default class Job extends Model {
return this.variables?.findBy('path', `nomad/jobs/${this.plainId}`);
}
}

// TODO: This async fetcher seems like a better fit for most of our use-cases than the above getter (which cannot do async/await)
async getPathLinkedVariable() {
await this.variables;
if (this.parent.get('id')) {
return this.variables?.findBy(
'path',
`nomad/jobs/${JSON.parse(this.parent.get('id'))[0]}`
);
} else {
return this.variables?.findBy('path', `nomad/jobs/${this.plainId}`);
}
}
}
18 changes: 17 additions & 1 deletion ui/app/models/task-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default class TaskGroup extends Fragment {
if (this.job.parent.get('id')) {
return this.job.variables?.findBy(
'path',
`nomad/jobs/${JSON.parse(this.job.parent.get('id'))[0]}/${this.name}`
`nomad/jobs/${this.job.parent.get('plainId')}/${this.name}`
);
} else {
return this.job.variables?.findBy(
Expand All @@ -38,6 +38,22 @@ export default class TaskGroup extends Fragment {
}
}

// TODO: This async fetcher seems like a better fit for most of our use-cases than the above getter (which cannot do async/await)
async getPathLinkedVariable() {
await this.job.variables;
if (this.job.parent.get('id')) {
return await this.job.variables?.findBy(
'path',
`nomad/jobs/${this.job.parent.get('plainId')}/${this.name}`
);
} else {
return await this.job.variables?.findBy(
'path',
`nomad/jobs/${this.job.plainId}/${this.name}`
);
}
}

@fragmentArray('task') tasks;

@fragmentArray('service-fragment') services;
Expand Down
29 changes: 26 additions & 3 deletions ui/app/models/task.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@ export default class Task extends Fragment {
@fragmentArray('volume-mount', { defaultValue: () => [] }) volumeMounts;

async _fetchParentJob() {
let job = await this.store.findRecord('job', this.taskGroup.job.id, {
reload: true,
});
let job = this.store.peekRecord('job', this.taskGroup.job.id);
if (!job) {
job = await this.store.findRecord('job', this.taskGroup.job.id, {
reload: true,
});
}
this._job = job;
}

Expand All @@ -79,4 +82,24 @@ export default class Task extends Fragment {
);
}
}

// TODO: This async fetcher seems like a better fit for most of our use-cases than the above getter (which cannot do async/await)
async getPathLinkedVariable() {
if (!this._job) {
await this._fetchParentJob();
}
await this._job.variables;
let jobID = this._job.plainId;
// not getting plainID because we dont know the resolution status of the task's job's parent yet
let parentID = this._job.belongsTo('parent').id()
? JSON.parse(this._job.belongsTo('parent').id())[0]
: null;
if (parentID) {
jobID = parentID;
}
return await this._job.variables?.findBy(
'path',
`nomad/jobs/${jobID}/${this.taskGroup.name}/${this.name}`
);
}
}
1 change: 1 addition & 0 deletions ui/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ Router.map(function () {
this.route('services', function () {
this.route('service', { path: '/:name' });
});
this.route('variables');
});
});

Expand Down
Loading

0 comments on commit 18dd9e7

Please sign in to comment.