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 namespace is used for node API requests #9410

Merged
merged 5 commits into from
Jan 4, 2021

Conversation

johncowen
Copy link
Contributor

Nodes themselves are not namespaced, so we'd originally assumed we did not need to pass through the ns query parameter when listing or viewing nodes.

As it turns out the API endpoints we use to list and view nodes (and related things) return things that are namespaced, therefore any API requests for nodes do require a the ns query parameter to be passed through to the request.

This PR adds the necessary ns query param to all things Node, apart from the querying for the leader which only returns node related information.

Additionally here we decided to show 0 Services text in the node listing if there are nodes with no service instances within the namespace you are viewing, as this is clearer than showing nothing at all. We also cleaned up/standardized the text we use to in the empty state for service instances.

Thanks @cpu601 for the report!

@johncowen johncowen added type/bug Feature does not function as expected theme/ui Anything related to the UI backport/1.9 labels Dec 16, 2020
@johncowen johncowen requested a review from kaxcode December 16, 2020 11:59
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 8c0473a into master Jan 4, 2021
@johncowen johncowen deleted the ui/bugfix/node-related-nspaces branch January 4, 2021 16:42
@hashicorp-ci
Copy link
Contributor

🍒 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/302870.

hashicorp-ci pushed a commit that referenced this pull request Jan 4, 2021
Nodes themselves are not namespaced, so we'd originally assumed we did not need to pass through the ns query parameter when listing or viewing nodes.

As it turns out the API endpoints we use to list and view nodes (and related things) return things that are namespaced, therefore any API requests for nodes do require a the ns query parameter to be passed through to the request.

This PR adds the necessary ns query param to all things Node, apart from the querying for the leader which only returns node related information.

Additionally here we decided to show 0 Services text in the node listing if there are nodes with no service instances within the namespace you are viewing, as this is clearer than showing nothing at all. We also cleaned up/standardized the text we use to in the empty state for service instances.
@hashicorp-ci
Copy link
Contributor

🍒✅ Cherry pick of commit 8c0473a onto release/1.9.x succeeded!

@hashicorp-ci
Copy link
Contributor

🍒❌ Cherry pick of commit 8c0473a onto release/1.8.x failed! Build Log

@johncowen
Copy link
Contributor Author

1.8 and 1.7 are very different (not only the folder change but also the move from ES5 to ES6 i.e. native classes etc), so I'm gonna merge it manually and PR separately.

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 type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants