Skip to content

Update usage and README - #10

Open
virgilwashere wants to merge 3 commits into
whereisaaron:masterfrom
virgilwashere:linting
Open

Update usage and README#10
virgilwashere wants to merge 3 commits into
whereisaaron:masterfrom
virgilwashere:linting

Conversation

@virgilwashere

@virgilwashere virgilwashere commented Jul 2, 2019

Copy link
Copy Markdown

📝 Update usage and README

  • ready for 📛 when you do an official release👍

⚙️ Add unofficial strict mode

🚨 shellcheck linting:

  • use printf instead of echo
  • use $HOME instead of ~ for robustness
  • quoting in cfg_parser
  • if cmd instead of $? checking

🚸 improve usage output

  • Use profile and $HOME in Usage output
    • ie if someone adds --help to the command line after specifying a --profile xxx already

✨ Enable short options

  • 🌘 but not added to Usage or README

shellcheck linting:

- use `printf` instead of `echo`
- use `$HOME` instead of `~`

Use profile and `$HOME` in Usage output
  - ie if someone adds `--help` to the command line after specifying
a `--profile xxx` already
- but not added to Usage or README

Add unofficial strict mode

shellcheck linting continued
- quoting in `cfg_parser`
- if cmd intead of `$?` checking
@virgilwashere
virgilwashere force-pushed the linting branch 2 times, most recently from fe74a31 to 6312865 Compare July 14, 2019 19:05
Support official AWSCLI environment variables AWS_DEFAULT_PROFILE and
AWS_SHARED_CREDENTIALS_FILE.

Use integers and arithmatic for show* variables.

Add short options to README.md
@whereisaaron

Copy link
Copy Markdown
Owner

Hi @virgilwashere thanks for the PR! I am happy with the make-over, though since so much code is touched it calls for some thorough testing. Have you be using and testing all the options?

The AWS_SESSION_EXPIRATION I wasn't planning to add as discussed in #7 as it not an AWS thing, it appears to be an ad hoc add on for some scripts, so far only raised by that one user. I left #7 open just to see anyone else chimed in. Do you use that environment variable yourself?

The short options I could take or leave. I don't think we are so short on bytes that we need them as well as the (more legible) long options. But neither do that hurt if some people are fond of them.

@virgilwashere

Copy link
Copy Markdown
Author

Hi @whereisaaron.

Hi @virgilwashere thanks for the PR! I am happy with the make-over, though since so much code is touched it calls for some thorough testing. Have you be using and testing all the options?

Yes, I'm actively using the script with this PR, and also the changes from my version branch incorporated.

I've manually tested just about all of the changes... I'm even contemplating writing some Bats tests to submit to the repo too.

The short options I could take or leave

They are useful when interactively setting up my .bashrc environment.

AWS_SESSION_EXPIRATION is added by my SAML authentication program, and it's nice to have a reference for credential expiration.

This is how I'm using get-aws-profile.sh...

export AWS_DEFAULT_PROFILE='role-name@account-name'

print-aws-profile() {
  [[ -z "$AWS_PROFILE" || -n "$1" ]] && AWS_PROFILE="${1:-$AWS_DEFAULT_PROFILE}"
  AWS_REGION="${AWS_REGION:-$AWS_DEFAULT_REGION}"
  get-aws-profile.sh --profile="$AWS_PROFILE"
  printf 'export AWS_REGION=%s\n' "$AWS_REGION"
}

aws-profile() { eval $(print-aws-profile); }

aws_auth0_saml() {
  [[ -v AWS_PROFILE ]] || AWS_PROFILE="${AWS_DEFAULT_PROFILE}"
  printf 'Starting Auth0 SAML authentication for profile %s\n' "$AWS_PROFILE" >&2
  saml-login -c "${AWS_PROFILE}" aws-assume-role "$@"
  aws-profile
}

Virgil

@whereisaaron

Copy link
Copy Markdown
Owner

Thanks @virgilwashere, well if you and @tomisaacson both use it and you're the ones contributing, let's add AWS_SESSION_EXPIRATION. But I'd like to note in the README and --help that it is an custom auth0 env var despite the AWS_ prefix. I don't want to confuse people into hunting the AWS documentation looking for it! If there a good reference to this env var in the auth0 documentation we can link to?

There are some other tools that use an env var of the same name, but they don't always use compatible time formats and don't store them in the the AWS credentials file. Does anything but auth0 do this?

@tomisaacson which tools did you use it with?

@whereisaaron

Copy link
Copy Markdown
Owner

The short options I could take or leave. I don't think we are so short on bytes that we need them as well as the (more legible) long options. But neither do that hurt if some people are fond of them.

They are useful when interactively setting up my .bashrc environment.

I'd say "that's what bash completion is for," except we don't provide completion for this tool, so we can add the short options. 😄

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