[ui] Global token CRUD in the web UI #23506

philrenaud · 2024-07-04T21:04:12Z

This PR allows for creation of global tokens, tokens of differing regions, and generally improves the handling of regions in the UI a bit better.

Resolves #23429

…etch time

github-actions · 2024-07-04T21:18:33Z

Ember Test Audit comparison

	main	`ffa45c2`	change
passes	1574	1578	+4
failures	0	0	0
flaky	0	0	0
duration	11m 28s 962ms	11m 48s 971ms	+20s 009ms

philrenaud · 2024-07-05T19:45:01Z

ui/app/components/token-editor.js

+      const adapterRegion = this.activeToken.global
+        ? this.system.get('defaultRegion.region')
+        : this.tokenRegion;


Note: even though the token is marked Global: true, I found that I still have to unset its region (or set it to the authoritative region), otherwise it gets created as global but within whatever region the user happens to be using at the time.

Once you're browsing the web UI with a region set, all requests get a queryParam of ?region=. Thus you could end up with a token that, yes, is global, but is stored in the context of an agent that isn't global/authoritative.

As such it seems like you can create a global, non-global token. Weird!

Thus you could end up with a token that, yes, is global, but is stored in the context of an agent that isn't global/authoritative.

Depending on what you mean by the "context" of an agent, that smells like a possible bug in the RPC handler. We have logic (ref acl_endpoint.go#L602-L611) to override the region flag if the token is global.

philrenaud · 2024-07-05T19:46:15Z

ui/app/controllers/jobs/index.js

@@ -240,7 +240,7 @@ export default class JobsIndexController extends Controller {
          console.log('error fetching job ids', e);
          this.notifyFetchError(e);
        }
-        if (this.jobs.length) {
+        if (this.jobs?.length) {


Fix that I should've included in #23427 — errors from outside of Nomad (which I was experiencing when trying to switch regions with an invalid token) didn't yield jobs so I'd get a "cannot get .length of undefined" error

philrenaud · 2024-07-05T20:42:59Z

ui/app/routes/clients.js

@@ -22,7 +22,8 @@ export default class ClientsRoute extends Route.extend(WithForbiddenState) {
  model() {
    return RSVP.hash({
      nodes: this.store.findAll('node'),
-      agents: this.store.findAll('agent'),


Note: I don't think this is a needed line for the clients index (anymore?).

As best as I can tell, non-authoritative-region management tokens do not have access to the server members list (cannot do nomad server members either)

This hadn't been a concern in the UI before, but the following is a situation where it would be problematic:

I am a management token for a non-authoritative region. I view the servers page and get a "not authorized" message. OK, that's fine, I'm not part of the global/authoritative region.

I switch to the clients tab, which should at least show me the clients within the region that holds my token. However, since the model for this page has a check for server members (agents in Ember), this page also gives me a "not authorized" error.

This removal makes it so non-authoritative region tokens with node list abilities can still view this page.

As best as I can tell, non-authoritative-region management tokens do not have access to the server members list (cannot do nomad server members either)

To close the loop on a Slack thread, the Status.Members RPC isn't forwarded between servers (much less regions), so a local management token can get the server members list but only when sending that request to its own region.

philrenaud · 2024-07-05T20:49:02Z

ui/app/components/region-switcher.js

+    this.system.set('activeRegion', region);
+    await this.get('token.fetchSelfTokenAndPolicies').perform().catch();
+
+    this.router.transitionTo({ queryParams: { region } });


Note: made the decision not to send you back to the jobs index when you switch regions anymore. This was a ~6 year old decision that made sense at the time, but we now have many more routes that may be context-shared between regions.

For example, if I sit on the /administration/policies page and switch between regions, I should now see my policies list simply update. Before, I'd have been jostled, have to click Administration, then have to click Policies again.

The drawback from this is that sometimes routes (like a particular task in a particular job) are NOT shared between regions. Switching a region on a page like this will put you on a (friendly) 404 page. This is a tradeoff that I think is worthwhile.

That's reasonable, for sure. Another thing to consider is that all the administrative routes are cross-region (because the authoritative region owns all these objects), whereas everything else is single region. But having two different behaviors depending on context might be more confusing than it's worth.

tgross

LGTM!

tgross · 2024-07-08T14:16:31Z

ui/app/routes/clients.js

@@ -22,7 +22,8 @@ export default class ClientsRoute extends Route.extend(WithForbiddenState) {
  model() {
    return RSVP.hash({
      nodes: this.store.findAll('node'),
-      agents: this.store.findAll('agent'),


As best as I can tell, non-authoritative-region management tokens do not have access to the server members list (cannot do nomad server members either)

To close the loop on a Slack thread, the Status.Members RPC isn't forwarded between servers (much less regions), so a local management token can get the server members list but only when sending that request to its own region.

tgross · 2024-07-08T14:21:03Z

ui/app/components/region-switcher.js

+    this.system.set('activeRegion', region);
+    await this.get('token.fetchSelfTokenAndPolicies').perform().catch();
+
+    this.router.transitionTo({ queryParams: { region } });


That's reasonable, for sure. Another thing to consider is that all the administrative routes are cross-region (because the authoritative region owns all these objects), whereas everything else is single region. But having two different behaviors depending on context might be more confusing than it's worth.

tgross · 2024-07-08T14:27:32Z

ui/app/components/token-editor.js

+      const adapterRegion = this.activeToken.global
+        ? this.system.get('defaultRegion.region')
+        : this.tokenRegion;


Thus you could end up with a token that, yes, is global, but is stored in the context of an agent that isn't global/authoritative.

Depending on what you mean by the "context" of an agent, that smells like a possible bug in the RPC handler. We have logic (ref acl_endpoint.go#L602-L611) to override the region flag if the token is global.

github-actions · 2024-12-30T02:23:27Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

First pass at global token creation and regional awareness at token f…

4d57157

…etch time

philrenaud self-assigned this Jul 4, 2024

philrenaud linked an issue Jul 4, 2024 that may be closed by this pull request

UI: allow creating global ACL tokens #23429

Closed

vercel bot deployed to Preview – nomad-ui July 4, 2024 21:05 View deployment

Reset and refetch token when you switch region but stay in place

bad82ea

vercel bot deployed to Preview – nomad-ui July 5, 2024 15:56 View deployment

Ugly and functional global token save

8a4d302

vercel bot deployed to Preview – nomad-ui July 5, 2024 17:53 View deployment

philrenaud commented Jul 5, 2024

View reviewed changes

vercel bot deployed to Preview – nomad-ui July 5, 2024 20:45 View deployment

philrenaud requested review from tgross and gulducat July 5, 2024 20:45

philrenaud marked this pull request as ready for review July 5, 2024 20:46

philrenaud commented Jul 5, 2024

View reviewed changes

philrenaud force-pushed the 23429-ui-allow-creating-global-acl-tokens branch from 136ba6a to 9a74139 Compare July 5, 2024 20:49

vercel bot deployed to Preview – nomad-ui July 5, 2024 20:50 View deployment

tgross approved these changes Jul 8, 2024

View reviewed changes

philrenaud added the backport/1.8.x backport to 1.8.x release line label Jul 8, 2024

Tests and log cleanup

ffa45c2

philrenaud force-pushed the 23429-ui-allow-creating-global-acl-tokens branch from 9a74139 to ffa45c2 Compare July 8, 2024 20:58

vercel bot deployed to Preview – nomad-ui July 8, 2024 20:59 View deployment

philrenaud merged commit 0324e78 into main Jul 11, 2024
15 checks passed

philrenaud deleted the 23429-ui-allow-creating-global-acl-tokens branch July 11, 2024 18:54

hc-github-team-nomad-core mentioned this pull request Jul 11, 2024

Backport of [ui] Global token CRUD in the web UI into release/1.8.x #23561

Merged

github-actions bot locked as resolved and limited conversation to collaborators Dec 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ui] Global token CRUD in the web UI #23506

[ui] Global token CRUD in the web UI #23506

philrenaud commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024 •

edited

Loading

philrenaud Jul 5, 2024

tgross Jul 8, 2024

philrenaud Jul 5, 2024

philrenaud Jul 5, 2024

tgross Jul 8, 2024

philrenaud Jul 5, 2024

tgross Jul 8, 2024

tgross left a comment

tgross Jul 8, 2024

tgross Jul 8, 2024

tgross Jul 8, 2024

github-actions bot commented Dec 30, 2024

[ui] Global token CRUD in the web UI #23506

[ui] Global token CRUD in the web UI #23506

Conversation

philrenaud commented Jul 4, 2024 • edited Loading

github-actions bot commented Jul 4, 2024 • 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

tgross left a 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

github-actions bot commented Dec 30, 2024

philrenaud commented Jul 4, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024 •

edited

Loading