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.

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

Improve/converge AWS SDK credentials handling #8221

Closed
dougmoscrop opened this issue Sep 11, 2020 · 4 comments
Closed

Improve/converge AWS SDK credentials handling #8221

dougmoscrop opened this issue Sep 11, 2020 · 4 comments
Assignees
Labels

Comments

@dougmoscrop
Copy link
Contributor

Use case description

There are a few use cases that aren't quite met by the way the AWS SDK is configured internally in serverless, which I think creates some confusion (possibly related: #7676), but also some tech debt in the sense that, as far as I can tell, serverless is sort of manually configuring the SDK to behave very, very close to how it does normally, but in a way that prevents some use cases.. ah, I probably didn't do justice with that explanation, so I will post a link to my changes and comment on those below.

So, generically the use case is, "work more like the AWS SDK works everywhere else", but, very specifically, this enables serverless to work with a credentials_process - which is pretty neat (see https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html if you want the guts) -- basically, there are ways that you can set up credentials/config in ~/.aws that would work for everything but serverless and I'm proposing that you can be more SDK-compatible and have code that is easier to maintain! 😃 In our case, we have a command line tool that bridges our Okta SSO to AWS STS, and with these changes, everything should work as it does (at least that is my intention) but we can take advantage of some more powerful configurations in ~/.aws/credentials to improve our developer experience.

Proposed solution

I have already started a draft, but updating the tests is a bit of a chore so before I do that I wanted to talk about the approach.

https://github.com/dougmoscrop/serverless/commit/49cd19aa4b1954aa0441fbebeca8e9291bf635fe

Anyway, curious how people feel about this, both in terms of the idea/goal, as well as any alternative implementation details.

@jack1902
Copy link

for those who might find it useful, i have always done this for handling creds around serverless and using yarn for package management

serverless_direct
#!/usr/bin/env bash
## deploy.sh <serverless-command>

set -eou pipefail

AWS_ROLE_ARN="YOUR_IAM_ROLE_USED_TO_DEPLOY"
AWS_REGION="YOUR_REGION"

echo "--- Installing deps"
yarn --frozen-lockfile

echo "--- Authenticating"
AWS_STS=($(aws sts assume-role --role-arn="$AWS_ROLE_ARN" --role-session-name=deploy-script --query 'Credentials.[AccessKeyId,SecretAccessKey,SessionToken]' --output=text))

echo "+++ Calling Serverless with: ${@}"
AWS_ACCESS_KEY_ID="${AWS_STS[0]}" \
    AWS_SECRET_ACCESS_KEY="${AWS_STS[1]}" \
    AWS_SESSION_TOKEN="${AWS_STS[2]}" \
    AWS_REGION=$AWS_REGION \
    ./node_modules/.bin/serverless \
        ${@}
serverless_via_docker
#!/usr/bin/env bash
## deploy.sh <serverless-command>

set -eou pipefail

AWS_ROLE_ARN="YOUR_IAM_ROLE_USED_TO_DEPLOY"
AWS_REGION="YOUR_REGION"

echo "--- :docker: Building container"
docker build . -t serverless

echo "--- :aws-iam: Authenticating"
AWS_STS=(
  $(aws sts assume-role \
    --role-arn="$AWS_ROLE_ARN" \
    --role-session-name=deploy-script \
    --query "Credentials.[AccessKeyId,SecretAccessKey,SessionToken]" \
    --output=text
  )
)

echo "+++ :serverless: Calling Serverless with: ${@}"
docker run --rm --tty \
  -e AWS_ACCESS_KEY_ID="${AWS_STS[0]}" \
  -e AWS_SECRET_ACCESS_KEY="${AWS_STS[1]}" \
  -e AWS_SESSION_TOKEN="${AWS_STS[2]}" \
  -e AWS_REGION=$AWS_REGION \
  serverless \
  ${@}

@dougmoscrop
Copy link
Contributor Author

Yeah in our case, we map every ${service}-${stage} combination to an explicit AWS profile as we have dozens of accounts, and that profile can be set up to either delegate to another profile (which we do - as we may put dev/test stages in one account, but prod in a separate, and so then we have a profile for the account itself), which can then use the credential process to auto-generate STS tokens

@medikoo
Copy link
Contributor

medikoo commented Sep 14, 2020

@dougmoscrop great thanks for openning that!

So, generically the use case is, "work more like the AWS SDK works everywhere else",

It's definitely the goal. If Serverless doesn't resolve credentials as AWS SDK does, then it is a bug (could be a design bug)

very specifically, this enables serverless to work with a credentials_process

There's a dedicated issue: #4838 (and there's was a PR: #6430 but it involved a bit more work, and contributor didn't manage to complete it)

but updating the tests is a bit of a chore

We have a lot of issues with some of existing unit tests, they seem to heavily rely on characteristics of internal implementation (sometimes those internals are reshaped in tests) which makes any refactors painful.

To recover from that, all new tests we introduce are based on runServerless. Some tests were also refactored (e.g. Service.test.js)

I believe that in this case, it'll be good to first refactor all affected tests so they stand on runServerless, and having that in, propose a new refactored way of handling credentials resolution

I wanted to talk about the approach.

At first glance it looks very promising to me. Moving it out to dedicated file, makes big sense.

@dougmoscrop we'd love to have that issue finally fixed, so we definitely very welcome the PR (Although as you pointed, it might be first good to discuss internals. I'd say that refactoring related tests, would be a first good step)

Do you think we can move that discussion to #4838 and close this as duplicate? Ideally would be, if we keep related discussions in one place.

@dougmoscrop
Copy link
Contributor Author

yes absolutely, I just technically might say this doesn't only enable credentials_process, but I have no preference or strong feelings here, close away :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants