2
\$\begingroup\$

Background

Our team has a utility called envmgr to set aws iam roles. The credentials are only good for 4 hours or so. As such, I have a number of scripts that call the command $(envmgr -e ENVIRONMENT -r ROLE) to refresh the credentials and avoid a time-out while in the middle of a process.

The problem is that there is a lot of duplication in our codebase because we have separate scripts in separate parts of our architecture that do functionally the same things, but under a different environment. So I've written a script that sets a global AWS_ENVIRONMENT variable, so the first time the script is called, it'll make sure you properly set the environment, but still subsequently call the original envmgr... command without necessarily having to re-take an input from the user.

Potential Improvements

  • Is there a way to limit choices? currently the script requires the user to type "production" or "systems" or it will keep prompting them. Is there a way where they are given options that will auto fill?
  • Maybe there could be a check to see remaining time on credentials? And only refresh at a certain point? This is likely unnecessary as setting AWS credentials has minimal overhead.
  • For our purposes, I don't want the environment to change, so I'm ok to only take a single input and then automatically use that input for other scripts that get ran in the process. Some may want the option to assume different environments and will want a way to set environment variables automatically.

Script

#!/usr/bin/env bash -e

export AWS_ROLE='event-platform-developer'

# If our AWS_ENVIRONMENT variable has been not ben set, then we need to ask the user for input
if [ -z ${AWS_ENVIRONMENT+x} ]; then

    read -p "Which environment are you trying to access? (production or systems): " response
    response=$(echo "$response" | tr '[:upper:]' '[:lower:]')

    while [ $response != 'production' ] && [ $response != 'systems' ]
    do
        read -p "Which environment are you trying to access? (production or systems): " response
        response=$(echo "$response" | tr '[:upper:]' '[:lower:]')
    done

    AWS_ENVIRONMENT=$response
    echo -e "Setting your AWS credentials as: envmgr -e $AWS_ENVIRONMENT --role $AWS_ROLE"
    eval $(envmgr -e $AWS_ENVIRONMENT --role $AWS_ROLE)

    unset response

# If our AWS_ENVIORNMENT variable has been set previously, then we can skip taking a response, but
# we still want to update our credentials
else 
    echo -e  "Your AWS enviornment was previously set to: $AWS_ENVIRONMENT\n"
    echo -e "Refreshing your AWS credentials as: envmgr -e $AWS_ENVIRONMENT --role $AWS_ROLE"
    eval $(envmgr -e $AWS_ENVIRONMENT --role $AWS_ROLE)

fi 
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

good

  • Nice indentation
  • Good variable names
  • Excellent sh-bang line (env lets bash be anywhere and -e for exiting on errors is good too.)

suggestions

  • Put a comment at the top of the code explaining what it is trying to do
  • Use double square brackets instead of single square brackets to avoid surprises.
  • Your eval $(envmgr... lines are a bit confusing. Firstly, you don't need eval to substitute variables or have it run the command. The only point to eval would be if envmgr outputs variables that you want to effect your current script. Since there's no code below this those variables wouldn't effect anything anyway. Secondly, you are repeating yourself and this line could move below the fi.
  • Put quotes around your variable substitutions to protect against something containing a space causing it to get split into multiple arguments.
  • Your conditional for the while has extra brackets in the middle. It should be one long [[ .... && .... ]].
  • The unset response seems superfluous. This variable will go away when the script exits, which is right after you unset this variable.
  • try shellcheck

rewrite (untested)

#!/usr/bin/env bash -e

export AWS_ROLE='event-platform-developer'

# If our AWS_ENVIRONMENT variable has been not ben set, then we need to ask the user for input
if [[ -z "${AWS_ENVIRONMENT+x}" ]]; then

    read -p "Which environment are you trying to access? (production or systems): " response
    response=$(echo "$response" | tr '[:upper:]' '[:lower:]')

    while [[ "$response" != 'production'  && "$response" != 'systems' ]]; do
        read -p "Which environment are you trying to access? (production or systems): " response
        response=$(echo "$response" | tr '[:upper:]' '[:lower:]')
    done

    AWS_ENVIRONMENT="$response"
    echo -e "Setting your AWS credentials as: envmgr -e $AWS_ENVIRONMENT --role $AWS_ROLE"

# If our AWS_ENVIORNMENT variable has been set previously, then we can
# skip taking a response, but we still want to update our credentials
else 
    echo -e  "Your AWS enviornment was previously set to: $AWS_ENVIRONMENT\n"
    echo -e "Refreshing your AWS credentials as: envmgr -e $AWS_ENVIRONMENT --role $AWS_ROLE"

fi 

envmgr -e "$AWS_ENVIRONMENT" --role "$AWS_ROLE"

answers to questions

Is there a way to limit choices? currently the script requires the user to type "production" or "systems" or it will keep prompting them. Is there a way where they are given options that will auto fill?

First, I'd put the two lines that prompt and read the response into a function so you code follows the Don't Repeat Yourself principal.

Then, I would use a case statement to see if the choices fit one of the acceptable ones. You could add a case for empty responses that set the response to one of your valid choices. Then the default/fall-through case would print a warning about the response not fitting one of the available responses.

Sorry for not including these ideas in the rewrite.

Maybe there could be a check to see remaining time on credentials? And only refresh at a certain point? This is likely unnecessary as setting AWS credentials has minimal overhead.

You can use if aws sts get-caller-identity > /dev/null to see if your credentials are still valid. It will hit the else case when you need to reauthenticate. This if does not need square brackets since we are testing the return code of the aws command. Otherwise, it operates like the if you've already used.

I have not found a way to see how long you have left on your current credentials.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ I've updated my answer to include a first cut and rewriting the code and answers to your questions. \$\endgroup\$
    – chicks
    Jul 21, 2022 at 1:50
  • \$\begingroup\$ Thanks @chicks this is very thorough and helpful feedback \$\endgroup\$
    – Hofbr
    Jul 21, 2022 at 21:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.