-
Notifications
You must be signed in to change notification settings - Fork 47
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
Feature: Interactive Role Finder #87
Merged
patricksanders
merged 24 commits into
Netflix:master
from
nsiow:feat/interactive_role_finder
Aug 28, 2021
Merged
Feature: Interactive Role Finder #87
patricksanders
merged 24 commits into
Netflix:master
from
nsiow:feat/interactive_role_finder
Aug 28, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ep into feat/interactive_role_finder
patricksanders
approved these changes
Aug 28, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR implements an interactive role finder in the terminal, built on top of the existing
promptui
dependency. The goal of this feature is to allow the user to browse available roles and search for roles in the instance where they know roughly what the desired role is, but do not have the entire role ARN on hand.Dependencies
#86
Implementation
For
weep
commands that expect a role argument (export
+file
), the required role argument is now optional. If the role is not provided, an interactive prompt will be displayed with all roles available to the user. This prompt uses a fuzzy-search that allows users to use approximate text searches to filter the role list in realtime.The prompt is disabled in two cases:
weep
is being run from a source other than an interactive terminal (where prompting for input is not helpful)WEEP_DISABLE_INTERACTIVE_PROMPTS=1
in their shell environmentIn the event that a role is not supplied and the prompt is not generated,
weep
will fail with an error as it did previously.Notes to Reviewers
At the moment, prompting is enabled by default as I believe it will be the most configuration for new and existing weep users. If we would like to keep behavior more consistent with the current implementation, we could have prompts disabled by default and have some options such as (a) adding a separate command-line flag to prompt (b) adding a separate environment variable that must be set by the user to enable prompts
This feature was omitted from
weep serve
since it already has a valid zero-argument form. Open to ideas on how we might be able to intelligently determine if the user wants this zero-argument form vs would like to be prompted, but nothing I thought of felt intuitiveThis feature was omitted from
weep open
since it has many other valid types of input other than IAM roles, and so we would not be sure what to prompt in this caseCode for this feature was kept out of
prompt.go
since it uses the ConsoleMe API functionality directly and introduces a circular dependency between those modules andutil
. I put it in ahelpers.go
file incmd/
instead since it only would be used in the context of the CLI, but open to other ideasDemo