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: chroot namespace listener #23942

Merged
merged 9 commits into from
Nov 2, 2023
Merged

Conversation

hashishaw
Copy link
Contributor

@hashishaw hashishaw commented Nov 1, 2023

This PR consists of updates to make the UI usable when chroot namespace listener is set.

When accessing the UI via a listener where chroot_namespace is set, but the defined namespace does not exist, the UI will show an error:

Error shown when chroot namespace does not exist in Vault

Otherwise, the UI should work normally*

  • there is still a known issue where the UI in a chrooted listener incorrectly hides navigation items when not logged in with a root token. This is due to the resultant-acl endpoint not stripping out the chrooted namespace from the paths. This will be addressed separately from this PR.

@hashishaw hashishaw added ui bug Used to indicate a potential bug backport/1.15.x labels Nov 1, 2023
@hashishaw hashishaw added this to the 1.15.2 milestone Nov 1, 2023
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Nov 1, 2023
@hashishaw hashishaw marked this pull request as ready for review November 2, 2023 15:29
standby: attr('boolean'),
isActive: equal('standby', false),
clusterName: attr('string'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

clusterName isn't used anywhere, so removed it from the model. The other values are also available on seal-status which will be available in a chroot scenario, so I moved those below to that section

@@ -44,7 +44,7 @@ export default class VersionService extends Service {
@task
*getVersion() {
if (this.version) return;
const response = yield this.store.adapterFor('cluster').health();
const response = yield this.store.adapterFor('cluster').sealStatus();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Getting version from this endpoint instead since it's available in non-root namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will probably be updated again to a UI-specific endpoint in the near future, but this will do for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the error was happening in the cluster route, we needed to add this highest-level error template to show the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are subtle bugs introduced when you use {{@model}} instead of {{this.model}} in route templates -- https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-model-argument-in-route-templates.md
This route isn't problematic since it doesn't use route params, but in terms of best practices we should make route templates consistently use this.model
Shoutout to @fivetanley for sharing this knowledge (shamelessly stole your language)

? `Vault v${versionName.slice(0, versionName.indexOf('+'))} root`
: `Vault v${versionName}`;

assert.dom(SELECTORS.cardHeader('Vault version')).hasText(versionText);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this test because the root namespace badge shouldn't show in community version

Copy link

github-actions bot commented Nov 2, 2023

Build Results:
All builds succeeded! ✅

Copy link
Contributor

@zofskeez zofskeez left a comment

Choose a reason for hiding this comment

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

I don't have anything to comment on other than great work 👏 . I pulled the branch down and everything looks to be working as expected 🎉

@hashishaw hashishaw merged commit 3eb205a into main Nov 2, 2023
70 of 71 checks passed
@hashishaw hashishaw deleted the ui/VAULT-20264/chroot-namespace-listener branch November 2, 2023 17:55
hashishaw added a commit that referenced this pull request Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants