Allow wildcard for Evaluations API #13530

philrenaud · 2022-06-29T20:17:20Z

UI:

Fixes an issue where the namespace=<namespace> query parameter was not being observed by the Evaluations route
~~Remove persistence of the passed namespace (from jobs, volumes, etc.)~~ moved to Remove namespace cache #13679

API:

Adds a failing test that checks to try to get evals with a valid token on the * namespace
Fixes the API return to check for AllNamespacesSentinel before sending ErrPermissionDenied

github-actions · 2022-06-29T21:00:00Z

Ember Asset Size action

As of 75e70c0

Files that got Bigger 🚨:

File	raw	gzip
nomad-ui.js	+65 B	+26 B

Files that stayed the same size 🤷‍:

File	raw	gzip
vendor.js	0 B	0 B
nomad-ui.css	0 B	0 B
vendor.css	0 B	0 B

ChaiWithJai

If we need to make any edits client-side, I would suspect we'd need to check the formatting logic in generateFilterExpression. However, the tests assert on how user actions impact the request we're sending and the default request we send when we enter the view. I'd love to learn if we wrote those tests and the formatting logic incorrectly.

I highly recommend reading that filtering logic and the tests to see if we got something wrong. But, I believe @mikenomitch pointed the real issue out 12918 and we should ask @lgfa29 to prioritize this issue in the backlog.

Thankfully, @lgfa29 is also an expert web developer and can provide insights if I'm wrong here.

ChaiWithJai · 2022-06-29T21:41:30Z

ui/app/controllers/evaluations/index.js

@@ -35,7 +35,7 @@ export default class EvaluationsController extends Controller {
    'currentEval',
    'pageSize',
    'status',
-    'qpNamespace',
+    { qpNamespace: 'namespace' },


blocking: I don't think this resolves the issue. This sets the alias of the query parameter in the URL. But, this shouldn't impact formatting the request in the route.

If we need to make any edits, I would suspect we'd need to check the formatting logic in generateFilterExpression. However, the tests assert on how user actions impact the request we're sending and the default request we send when we enter the view. I'd love to learn if we wrote those tests and the formatting logic incorrectly.

This impacts the initial query that our /evaluations route makes; it is making the (currently bugged, hopefully fixed by this PR) call to * under all circumstances currently.

That's overeager for what a user might be looking for, though, and inconsistent with how other routes that are namespace-aware (volumes, jobs, which only fetch according to your namespace on load) operate.

That's your call to make. We can totally adopt that pattern, but that's an undocumented behavior we'd have to load into our mental models. We specifically designed this to set namespace to * when entering this view. I could see the mental baggage of keeping this type of knowledge in our heads with an Ember Routing API change.

It's good UX to have. So totally your call, I'll support whatever you decide. I'll update my review on this PR to approved and let you handle this how you think is best.

github-actions · 2022-07-04T19:55:18Z

Ember Test Audit comparison

	main	`75e70c0`	change
passes	1288	1288	0
failures	2	2	0
flaky	0	0	0
duration	000ms	000ms	-000ms

lgfa29 · 2022-07-05T00:26:08Z

If we need to make any edits client-side, I would suspect we'd need to check the formatting logic in generateFilterExpression. However, the tests assert on how user actions impact the request we're sending and the default request we send when we enter the view. I'd love to learn if we wrote those tests and the formatting logic incorrectly.

I highly recommend reading that filtering logic and the tests to see if we got something wrong. But, I believe @mikenomitch pointed the real issue out 12918 and we should ask @lgfa29 to prioritize this issue in the backlog.

Thankfully, @lgfa29 is also an expert web developer and can provide insights if I'm wrong here.

#12918 could have contributed to this error if the UI requests a specific eval using * as the namespace, but I'm not sure if it does. If you still run into problems you can rebase onto #13551 or wait for it be merged and rebase main.

tgross

LGTM. I left some comments on the API bits in #13552 but nothing blocking.

lgfa29 · 2022-07-05T14:20:38Z

LGTM. I left some comments on the API bits in #13552 but nothing blocking.

I will port the changes from #13552 into here as well.

Apply the same verification process as in job, allocs and scaling policy list endpoints to handle the eval list when using an ACL token with limited namespace support but querying using the `*` wildcard namespace.

Evals have a unique UUID as ID, but when querying them the Nomad API still expects a namespace query param, otherwise it assumes `default`.

Apply the same verification process as in job, allocs and scaling policy list endpoints to handle the eval list when using an ACL token with limited namespace support but querying using the `*` wildcard namespace.

github-actions · 2022-11-09T02:38:16Z

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.

philrenaud linked an issue Jun 29, 2022 that may be closed by this pull request

Evaluations UI Permissions Issue with Namespaces #13142

Closed

philrenaud changed the title ~~Failing test and TODO for wildcard~~ Allow wildcard for Evaluations API Jun 29, 2022

vercel bot deployed to Preview – nomad-storybook-and-ui June 29, 2022 20:20 View deployment

philrenaud requested review from ChaiWithJai and tgross June 29, 2022 20:54

vercel bot deployed to Preview – nomad-storybook-and-ui June 29, 2022 20:56 View deployment

ChaiWithJai suggested changes Jun 29, 2022

View reviewed changes

philrenaud assigned philrenaud and lgfa29 Jun 30, 2022

ChaiWithJai approved these changes Jun 30, 2022

View reviewed changes

lgfa29 mentioned this pull request Jul 4, 2022

Backport #13530 to 1.2.x #13552

Merged

lgfa29 added a commit that referenced this pull request Jul 4, 2022

changelog: add entry for #13530

39f7c54

lgfa29 added the backport/1.3.x backport to 1.3.x release line label Jul 4, 2022

vercel bot deployed to Preview – nomad-storybook-and-ui July 4, 2022 19:14 View deployment

lgfa29 added this to the 1.3.2 milestone Jul 4, 2022

tgross approved these changes Jul 5, 2022

View reviewed changes

lgfa29 mentioned this pull request Jul 6, 2022

api: refactor ACL check for namespace wildcard #13606

Merged

lgfa29 force-pushed the 13142-evaluations-ui-permissions-issue-with-namespaces branch from 39f7c54 to 9f4095d Compare July 6, 2022 00:35

lgfa29 changed the base branch from main to refactor-wildcard-ns-acl-check July 6, 2022 00:35

lgfa29 force-pushed the 13142-evaluations-ui-permissions-issue-with-namespaces branch from 9f4095d to 81ab4d9 Compare July 6, 2022 00:36

lgfa29 force-pushed the 13142-evaluations-ui-permissions-issue-with-namespaces branch from 81ab4d9 to c5d251a Compare July 6, 2022 00:37

vercel bot deployed to Preview – nomad-storybook-and-ui July 6, 2022 00:42 View deployment

Base automatically changed from refactor-wildcard-ns-acl-check to main July 6, 2022 19:22

lgfa29 added 2 commits July 6, 2022 15:27

eval: fix list when using ACLs and * namespace

1cd08f3

Apply the same verification process as in job, allocs and scaling policy list endpoints to handle the eval list when using an ACL token with limited namespace support but querying using the `*` wildcard namespace.

changelog: add entry for #13530

fe3a7f6

lgfa29 force-pushed the 13142-evaluations-ui-permissions-issue-with-namespaces branch from c5d251a to fe3a7f6 Compare July 6, 2022 19:30

vercel bot deployed to Preview – nomad-storybook-and-ui July 6, 2022 19:33 View deployment

lgfa29 force-pushed the 13142-evaluations-ui-permissions-issue-with-namespaces branch from 599df8c to 45642bd Compare July 8, 2022 01:23

vercel bot deployed to Preview – nomad-storybook-and-ui July 8, 2022 01:27 View deployment

ui: set namespace when querying eval

75e70c0

Evals have a unique UUID as ID, but when querying them the Nomad API still expects a namespace query param, otherwise it assumes `default`.

lgfa29 force-pushed the 13142-evaluations-ui-permissions-issue-with-namespaces branch from 45642bd to 75e70c0 Compare July 8, 2022 14:19

vercel bot deployed to Preview – nomad-storybook-and-ui July 8, 2022 14:23 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 8, 2022 14:40 View deployment

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

vercel bot deployed to Preview – nomad-storybook-and-ui July 8, 2022 17:38 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 8, 2022 17:50 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 8, 2022 18:14 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 9, 2022 03:45 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui July 9, 2022 04:16 View deployment

tgross mentioned this pull request Jul 9, 2022

Deployment Status Command Does Not Respect -namespace Wildcard #13660

Closed

philrenaud marked this pull request as ready for review July 11, 2022 13:35

vercel bot deployed to Preview – nomad-storybook-and-ui July 11, 2022 16:32 View deployment

philrenaud force-pushed the 13142-evaluations-ui-permissions-issue-with-namespaces branch from 7f8abfd to 75e70c0 Compare July 11, 2022 17:01

lgfa29 approved these changes Jul 11, 2022

View reviewed changes

lgfa29 merged commit a7bd071 into main Jul 11, 2022

lgfa29 deleted the 13142-evaluations-ui-permissions-issue-with-namespaces branch July 11, 2022 20:42

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

Backport of Allow wildcard for Evaluations API into release/1.3.x #13697

Merged

4 tasks

philrenaud mentioned this pull request Jul 12, 2022

Evaluations UI Permissions Issue with Namespaces #13142

Closed

github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow wildcard for Evaluations API #13530

Allow wildcard for Evaluations API #13530

philrenaud commented Jun 29, 2022 •

edited

Loading

github-actions bot commented Jun 29, 2022 •

edited

Loading

ChaiWithJai left a comment

ChaiWithJai Jun 29, 2022

philrenaud Jun 30, 2022

ChaiWithJai Jun 30, 2022

github-actions bot commented Jul 4, 2022 •

edited

Loading

lgfa29 commented Jul 5, 2022

tgross left a comment

lgfa29 commented Jul 5, 2022

github-actions bot commented Nov 9, 2022

Allow wildcard for Evaluations API #13530

Allow wildcard for Evaluations API #13530

Conversation

philrenaud commented Jun 29, 2022 • edited Loading

UI:

API:

github-actions bot commented Jun 29, 2022 • edited Loading

Ember Asset Size action

ChaiWithJai left a comment

Choose a reason for hiding this comment

ChaiWithJai Jun 29, 2022

Choose a reason for hiding this comment

philrenaud Jun 30, 2022

Choose a reason for hiding this comment

ChaiWithJai Jun 30, 2022

Choose a reason for hiding this comment

github-actions bot commented Jul 4, 2022 • edited Loading

Ember Test Audit comparison

lgfa29 commented Jul 5, 2022

tgross left a comment

Choose a reason for hiding this comment

lgfa29 commented Jul 5, 2022

github-actions bot commented Nov 9, 2022

philrenaud commented Jun 29, 2022 •

edited

Loading

github-actions bot commented Jun 29, 2022 •

edited

Loading

github-actions bot commented Jul 4, 2022 •

edited

Loading