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.

Sign up for GitHub

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

Jump to bottom

feat: create ssh type target in admin-ui #1027

Merged
merged 20 commits into from
Mar 31, 2022
Merged

feat: create ssh type target in admin-ui #1027

merged 20 commits into from
Mar 31, 2022

Conversation

DhariniJeeva
Copy link
Collaborator

@DhariniJeeva DhariniJeeva commented Mar 17, 2022 •
edited
Loading

🎟️ Jira ticket

Description

🧑‍💻

TCP

SSH

This pr updates existing target forms to support both TCP and SSH types.

Added a feature flag check to display and create ssh types.

  • If the feature flag[ssh-target] is set to false, then the users are allowed to view and create only the TCP target type.

@vercel
Copy link

vercel bot commented Mar 17, 2022 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

boundary-ui – ./ui/admin

🔍 Inspect: https://vercel.com/hashicorp/boundary-ui/A2GAY484iWr3aRb6y9uMDMAUrazA
✅ Preview: https://boundary-ui-git-icu-3644-create-ssh-target-in-admin-hashicorp.vercel.app

boundary-ui-storybook – ./addons/rose

🔍 Inspect: https://vercel.com/hashicorp/boundary-ui-storybook/E56dZWr3U47Ts3C2Wjr7aUc3ASQ9
✅ Preview: https://boundary-ui-storybook-git-icu-3644-create-ssh-a0c990-hashicorp.vercel.app

boundary-ui-desktop – ./ui/desktop

🔍 Inspect: https://vercel.com/hashicorp/boundary-ui-desktop/CFWCBZU2gwaknL54Ppw8FsQE1cE6
✅ Preview: https://boundary-ui-desktop-git-icu-3644-create-ssh-ta-178f34-hashicorp.vercel.app

@DhariniJeeva DhariniJeeva changed the title Icu 3644 create ssh target in admin create ssh type target in admin Mar 17, 2022
@DhariniJeeva DhariniJeeva changed the title create ssh type target in admin create ssh type target in admin-ui Mar 17, 2022
@DhariniJeeva DhariniJeeva changed the title create ssh type target in admin-ui feat: create ssh type target in admin-ui Mar 17, 2022
@DhariniJeeva DhariniJeeva marked this pull request as ready for review March 17, 2022 21:38
@DhariniJeeva DhariniJeeva marked this pull request as draft March 17, 2022 21:39
@DhariniJeeva DhariniJeeva marked this pull request as ready for review March 17, 2022 21:41
@DhariniJeeva DhariniJeeva marked this pull request as draft March 17, 2022 21:41
@DhariniJeeva
Copy link
Collaborator Author

Overall looks great. I believe @calcaide mentioned something similar to this before but I would love to see if we can refactor the form/target/[tcp|ssh]/index.hbs form. From a glance, much of it looks like duplication that we could potentially pull out into a form/target/base.hbs maybe.

@gsusmi and I discussed this in the past when we were working on dynamic host-catalogs and couldn't get the form data to be shared from the root index.hbs hence followed this existing pattern of auth-methods but completely open to discussions on how we can make this better.

cameronperera
cameronperera approved these changes Mar 29, 2022
View reviewed changes
Copy link
Collaborator

@cameronperera cameronperera left a comment

Choose a reason for hiding this comment

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

LGTM!

calcaide
calcaide requested changes Mar 29, 2022
View reviewed changes
Copy link
Collaborator

@calcaide calcaide left a comment

Choose a reason for hiding this comment

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

I left some comments.

@calcaide
Copy link
Collaborator

Overall looks great. I believe @calcaide mentioned something similar to this before but I would love to see if we can refactor the form/target/[tcp|ssh]/index.hbs form. From a glance, much of it looks like duplication that we could potentially pull out into a form/target/base.hbs maybe.

@cameronperera I add this comment regarding the code duplication between these two: https://github.com/hashicorp/boundary-ui/pull/1027/files#r837785763

Base automatically changed from create-ssh-target-route to main March 31, 2022 16:50
calcaide
calcaide approved these changes Mar 31, 2022
View reviewed changes
Copy link
Collaborator

@calcaide calcaide left a comment

Choose a reason for hiding this comment

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

LGTM

@DhariniJeeva DhariniJeeva merged commit 9fae8f2 into main Mar 31, 2022
@DhariniJeeva DhariniJeeva deleted the ICU-3644-create-ssh-target-in-admin branch March 31, 2022 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cameronperera cameronperera cameronperera approved these changes

@calcaide calcaide calcaide approved these changes

Assignees
No one assigned
Labels
addons admin ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DhariniJeeva @calcaide @cameronperera