Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: [BUGFIX] Ensure we use the ns query param name when requesting permissions #10608

Merged
merged 3 commits into from
Jul 15, 2021

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Jul 13, 2021

Previously when namespaces were enabled, we weren't requesting permission for the actively selected namespace, and instead always checking the permissions for the default namespace.

This PR ensures we request permissions for the actively selected namespace.

Notes:

All of the UI/API reading interaction uses a GET request with?ns= to specify the namespace, we have one API request which requires a POST with the request body containing the Namespace. This is so we can request various pieces of information required for the UI using a method we can customise for exactly what we need (there are more details in the comments in the changeset here). Unfortunately we were passing through a nspace parameter instead of a ns parameter which is the param name we are expecting deeper down, pretty much a typo.

An initial fix (6f00c81) meant converting this from nspace to ns inside the repository itself. But then I realized that the entire 'repository' interface as of not too long ago now uses/expects ns not nspace, so in a follow up commit (5c86827) I undid the first approach and made the fix consistent with the rest of the interface.

@johncowen johncowen added the theme/ui Anything related to the UI label Jul 13, 2021
@vercel vercel bot temporarily deployed to Preview – consul July 14, 2021 11:34 Inactive
@johncowen johncowen added this to the 1.10.1 milestone Jul 14, 2021
@johncowen johncowen changed the title ui: Ensure we use the ns query param name when requesting permissions ui: [BUGFIX] Ensure we use the ns query param name when requesting permissions Jul 14, 2021
@vercel vercel bot temporarily deployed to Preview – consul July 14, 2021 11:54 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging July 14, 2021 11:54 Inactive
@johncowen johncowen requested a review from kaxcode July 14, 2021 12:05
@johncowen johncowen marked this pull request as ready for review July 14, 2021 12:05
Copy link
Contributor

@kaxcode kaxcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johncowen johncowen merged commit 0762da3 into main Jul 15, 2021
@johncowen johncowen deleted the ui/bugfix/acl-perms-ns branch July 15, 2021 11:19
@hc-github-team-consul-core
Copy link
Collaborator

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/410079.

@hc-github-team-consul-core
Copy link
Collaborator

🍒✅ Cherry pick of commit 0762da3 onto release/1.10.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Jul 15, 2021
…rmissions (#10608)

Previously when namespaces were enabled, we weren't requesting permission for the actively selected namespace, and instead always checking the permissions for the default namespace.

This commit ensures we request permissions for the actively selected namespace.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants