Skip to content

Switch jessevdk/go-flags for spf13/cobra #66

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

Merged
merged 6 commits into from
Jun 30, 2020
Merged

Conversation

chrisgilmerproj
Copy link

@chrisgilmerproj chrisgilmerproj commented Jun 29, 2020

The primary change here is to use spf13/cobra as the CLI backend for this utility, similar to other tools Truss maintains.

This is a breaking change and requires a new release - but a release will not be cut after this PR is merged, more changes are needed.

Additionally:

  • Sets up subcommands for: completion, help, version and puts the original functionality under setup subcommand
  • Modifies the --role flag to be --iam-role for clarity
  • Modifies the --profile flag to be --aws-profile for clarity
  • Puts the environment variable AWS_VAULT_KEYCHAIN_NAME into a flag named --aws-vault-keychain-name.
  • Uses a new logger for all output instead of the default log. This will help disambiguate the output from aws-vault golang libs (and other libs) and the logging for this code.
  • Some small copy changes to make the output of the command more readable

Addresses these Pivotal stories:

@chrisgilmerproj chrisgilmerproj self-assigned this Jun 29, 2020
@chrisgilmerproj chrisgilmerproj requested a review from a team June 29, 2020 23:06
@chrisgilmerproj chrisgilmerproj marked this pull request as ready for review June 29, 2020 23:06
@chrisgilmerproj
Copy link
Author

For testing I used this script to set up a new AWS user. It proved to do exactly what was expected. Try these commands:

go run ./cmd/ setup --iam-user cgilmer-delete-me --iam-role engineer --aws-profile delete-id --aws-account-id REDACTED

or

make bin/setup-new-aws-user
bin/setup-new-aws-user help
bin/setup-new-aws-user version
bin/setup-new-aws-user setup --iam-user cgilmer-delete-me --iam-role engineer --aws-profile delete-id --aws-account-id REDACTED

Copy link
Contributor

@eeeady eeeady left a comment

Choose a reason for hiding this comment

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

Generally looks good to me. Have an inline comment on something I found using the docs.

I might ask that you geta 2nd review since my golang is probably not the best.

@chrisgilmerproj chrisgilmerproj merged commit bf9e57b into master Jun 30, 2020
@chrisgilmerproj chrisgilmerproj deleted the cg_new_cli_lib branch June 30, 2020 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants