[ui] Job Variables page #17964

philrenaud · 2023-07-17T20:59:51Z

A new route to:

Show a user the variables that a job and its groups and tasks have automatic access to (by naming convention)
Inform a user about how variables and automatic access work
Give a user a quick way to create/edit variables with appropriately pathed names.

A few new conventions added to facilitate good UX here:

We want to link to /variables/variable/edit if the variable exists, but /variables/new?path=$varpath if it does not. A new helper and component handle this for us.
We want to show the tasks/task groups they could be adding variables for here, but not too many of them. We show up to 2 examples of each and slightly change our copy depending on number of tasks/groups.

philrenaud · 2023-07-17T21:01:08Z

ui/app/components/variable-form.hbs

@@ -72,7 +72,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")}}


Side-effect: noticed this had stopped working in namespace-less environments.

PS: hey here's that "this variable path already exists" warning I mentioned a comment ago!

github-actions · 2023-07-17T21:20:15Z

Ember Test Audit comparison

	main	`cc9640c`	change
passes	1498	1505	+7
failures	1	1	0
flaky	0	0	0
duration	000ms	000ms	-000ms

philrenaud · 2023-07-21T18:09:02Z

ui/app/abilities/job.js

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


This seems out of place in a PR like this, right?

When we save a variable, there are a few things that we want to update and depend on that change. One of those is job.pathLinkedVariable, which looks against a job's variables array (more on that later!) and finds the one that's named nomad/jobs/$jobname.

So now, in our saveVariable() function elsewhere, we can say "Oh great you saved! Do you have access to read jobs so that this might matter for them? Yes? Okay, update them."

This provides the ability to check that they can read jobs.

philrenaud · 2023-07-21T18:10:10Z

ui/app/components/editable-variable-link.hbs

+  {{/with}}
+{{else}}
+  @path
+{{/if}}


A little component that either links to variables/new?path=YourNewVar or variables/YourExistingVar/edit, depending on whether it exists or not.

All this just to save the user a click! We already inform them, when creating a new variable whose path already exists, that that variable already exists and give them a link to it. But this is nicer UX IMO.

philrenaud · 2023-07-21T18:10:38Z

ui/app/components/variable-form.hbs

@@ -72,7 +72,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")}}


PS: hey here's that "this variable path already exists" warning I mentioned a comment ago!

philrenaud · 2023-07-21T18:12:52Z

ui/app/helpers/editable-variable-link.js

+ * Copyright (c) HashiCorp, Inc.
+ * SPDX-License-Identifier: MPL-2.0
+ */
+


The backing logic for the editable-variable-link.hbs component. Could have set this up as a component script, rather than a helper, but I thought we might want to use this in a different context sometime.

philrenaud · 2023-07-21T18:16:23Z

ui/app/models/job.js

+  async getPathLinkedVariable() {
+    await this.variables;
+    if (this.parent.get('id')) {
+      return await this.variables?.findBy(
+        'path',
+        `nomad/jobs/${JSON.parse(this.parent.get('id'))[0]}`
+      );
+    } else {
+      return await this.variables?.findBy('path', `nomad/jobs/${this.plainId}`);
+    }
+  }


This is, IMO, the most notable change I'm making in this PR. See the pathLinkedVariable() getter above this for how we've traditionally done it.

For the past few months, looking at a variable in the Web UI has been in a context where we'd have just gotten variables loaded. The operation, though, isn't immediate or promise-friendly. Frankly if this gets merged I intend to do away with pathLinkedVariable() in favour of getPathLinkedVariable() and make the whole operation more awaitable.

This lets us wait for the value of the job's (or group's, or task's, as you'll see in a moment) related variable without making any assumptions about what's already been loaded (see for example _fetchParentJob in the task model)

philrenaud · 2023-07-21T18:21:14Z

ui/app/templates/jobs/job/variables.hbs

+          {{#if (gt this.firstFewTaskGroupNames.length 1)}}
+            {{#each this.firstFewTaskGroupNames as |name|}}
+              <code><EditableVariableLink @path="nomad/jobs/{{this.model.job.name}}/{{name}}" @existingPaths={{this.jobRelevantVariables.files}} @namespace={{this.model.job.namespace.name}} /></code>,
+            {{/each}}
+            etc. to grant a specific task group access
+          {{else}}
+            <code>
+              <EditableVariableLink @path="nomad/jobs/{{this.model.job.name}}/{{object-at 0 this.firstFewTaskGroupNames}}" @existingPaths={{this.jobRelevantVariables.files}} @namespace={{this.model.job.namespace.name}} />
+            </code>
+            to grant your task group access
+          {{/if}}


Lots of template logic for a little benefit, but I think it reads much nicer this way.

We give a single example if you only have a single group/task, but multiple examples (with ", etc." language) when you have multiple groups/tasks.

ui/app/templates/jobs/job/variables.hbs

philrenaud · 2023-07-21T18:22:13Z

ui/mirage/config.js

+    let variable = variables.find(params.id);
+    if (!variable) {
+      return new Response(404, {}, {});
+    }


This emerged because I've started looking up the nomad/jobs variable in isolation when loading a given job/variables page now.

The HCP repo has helpers for these little common mirage things. They're quite handy; basically turns this into return notFound().

philrenaud · 2023-07-21T18:23:15Z

ui/mirage/scenarios/default.js

+  server.create('token', {
+    name: 'Novars Murphy',
+    id: 'n0-v4r5-4cc355',
+    type: 'client',
+  });


All our recent tokens are Irish and I can no longer remember why

philrenaud · 2023-07-21T18:24:53Z

ui/tests/acceptance/keyboard-test.js

+        `/jobs/${jobID}@default/clients`,
+        'Shift+ArrowRight takes you to the next tab (Clients)'
+      );
+


Side-effect-of-a-side-effect:

I wanted to make sure that keynav shift+right took you to the variables page, but you only see that if you have variable access

so I said okay let's make the token a management token so that they'd definitely see it

well the jobs created for this mock are all system jobs, which have client pages, which were otherwise unseen because before now the mocked token didn't have node read access, either, and now they do

ui/app/templates/jobs/job/variables.hbs

ui/app/routes/jobs/job/variables.js

ui/app/models/task-group.js

philrenaud · 2023-07-24T15:11:31Z

ui/tests/integration/components/job-status-panel-test.js

@@ -71,7 +71,7 @@ module(
        activeDeployment: true,
        groupTaskCount: ALLOCS_PER_GROUP,
        shallow: true,
-        resourceSpec: Array(NUMBER_OF_GROUPS).fill(['M: 257, C: 500']), // length of this array determines number of groups
+        resourceSpec: Array(NUMBER_OF_GROUPS).fill('M: 257, C: 500'), // length of this array determines number of groups


Side-effect: noticed that the .fill format I was using here would only work for shallow: true jobs, and just in case we want these to be un-shallow jobs in the future, fixed it.

… variable with a pathLinkedEntity of that job, its saved right through to the job itself

… jobs

…rays elsewhere

* 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

* 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 Co-authored-by: Phil Renaud <[email protected]>

philrenaud self-assigned this Jul 17, 2023

philrenaud linked an issue Jul 17, 2023 that may be closed by this pull request

Allow users to see all the variables from the job page including groups and tasks #17937

Closed

vercel bot deployed to Preview – nomad-storybook-and-ui July 17, 2023 21:00 View deployment

philrenaud commented Jul 17, 2023

View reviewed changes

vercel bot deployed to Preview – nomad-storybook-and-ui July 17, 2023 21:05 View deployment

philrenaud force-pushed the 17937-allow-users-to-see-all-the-variables-from-the-job-page-including-groups-and-tasks branch from 8c7c117 to 0c04bde Compare July 18, 2023 01:36

vercel bot deployed to Preview – nomad-storybook-and-ui July 18, 2023 01:38 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 18, 2023 02:20 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 19, 2023 20:20 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 20, 2023 14:35 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 20, 2023 15:16 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 13:44 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 14:43 View deployment

philrenaud force-pushed the 17937-allow-users-to-see-all-the-variables-from-the-job-page-including-groups-and-tasks branch from 41bc382 to 97afaf3 Compare July 21, 2023 15:13

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 15:16 View deployment

philrenaud force-pushed the 17937-allow-users-to-see-all-the-variables-from-the-job-page-including-groups-and-tasks branch from 97afaf3 to ef9afc9 Compare July 21, 2023 15:33

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 15:36 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 16:48 View deployment

philrenaud requested review from DingoEatingFuzz and tgross July 21, 2023 18:05

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 18:07 View deployment

philrenaud commented Jul 21, 2023

View reviewed changes

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 18:37 View deployment

tgross reviewed Jul 21, 2023

View reviewed changes

ui/app/templates/jobs/job/variables.hbs Outdated Show resolved Hide resolved

ui/app/templates/jobs/job/variables.hbs Outdated Show resolved Hide resolved

ui/app/routes/jobs/job/variables.js Outdated Show resolved Hide resolved

ui/app/models/task-group.js Outdated Show resolved Hide resolved

vercel bot deployed to Preview – nomad-storybook-and-ui July 21, 2023 20:30 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 24, 2023 14:28 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 24, 2023 15:05 View deployment

philrenaud marked this pull request as ready for review July 24, 2023 15:10

philrenaud commented Jul 24, 2023

View reviewed changes

philrenaud added 17 commits July 31, 2023 10:12

Helper and component to conditionally render the right link

eaead42

A bit of cleanup post-template stuff

8b56841

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

de9abc1

A very roundabout way of ensuring that, if a job exists when saving a…

b6af33f

… variable with a pathLinkedEntity of that job, its saved right through to the job itself

hacky but an async version of pathLinkedVariable

be20df1

model-driven and async fetcher driven with cleanup

ac4a73b

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

ff06092

Test cases begun

c88b4f4

Management token for variables to appear in tests

acecc86

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

2c3b5db

… jobs

Pre-review cleanup

dcb49e2

More tests

2d6bcd1

Number of requests test and small fix to groups-by-way-or-resource-ar…

d8e06a4

…rays elsewhere

Variable intro text tests

88d0e08

Variable name re-use

9d78ebd

Simplifying our wording a bit

c04f689

parse json vs plainId

cd1d0f8

philrenaud force-pushed the 17937-allow-users-to-see-all-the-variables-from-the-job-page-including-groups-and-tasks branch from 20ff07f to cd1d0f8 Compare July 31, 2023 14:12

vercel bot deployed to Preview – nomad-storybook-and-ui July 31, 2023 14:15 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 31, 2023 15:27 View deployment

Addressed PR feedback, including de-waterfalling

cc9640c

philrenaud force-pushed the 17937-allow-users-to-see-all-the-variables-from-the-job-page-including-groups-and-tasks branch from 5fbf537 to cc9640c Compare July 31, 2023 17:06

vercel bot deployed to Preview – nomad-storybook-and-ui July 31, 2023 17:09 View deployment

philrenaud added the backport/1.6.x backport to 1.6.x release line label Jul 31, 2023

philrenaud merged commit 18dd9e7 into main Jul 31, 2023

philrenaud deleted the 17937-allow-users-to-see-all-the-variables-from-the-job-page-including-groups-and-tasks branch July 31, 2023 19:04

hc-github-team-nomad-core mentioned this pull request Jul 31, 2023

Backport of [ui] Job Variables page into release/1.6.x #18106

Merged

philrenaud mentioned this pull request Jun 28, 2024

e2e: add SecureVariable UI tests #14180

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ui] Job Variables page #17964

[ui] Job Variables page #17964

philrenaud commented Jul 17, 2023 •

edited

Loading

philrenaud Jul 17, 2023

philrenaud Jul 21, 2023

github-actions bot commented Jul 17, 2023 •

edited

Loading

philrenaud Jul 21, 2023

philrenaud Jul 21, 2023

philrenaud Jul 21, 2023

philrenaud Jul 21, 2023

philrenaud Jul 21, 2023

philrenaud Jul 21, 2023

philrenaud Jul 21, 2023

DingoEatingFuzz Jul 28, 2023

philrenaud Jul 21, 2023

philrenaud Jul 21, 2023

philrenaud Jul 24, 2023

[ui] Job Variables page #17964

[ui] Job Variables page #17964

Conversation

philrenaud commented Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 17, 2023 • edited Loading

Ember Test Audit comparison

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philrenaud commented Jul 17, 2023 •

edited

Loading

github-actions bot commented Jul 17, 2023 •

edited

Loading