thomasmichaelwallace / serverless-better-credentials

Better AWS credentials resolution plugin for serverless
MIT License
54 stars 9 forks source link

The simplest AWS_PROFILE doesn't even work #12

Closed pkit closed 1 year ago

pkit commented 1 year ago

Describe the bug

AWS_PROFILE=profile1 sls info
Running "serverless" from node_modules
✔ serverless-better-credentials: credentials resolved from config ini profile: AWS_DEFAULT_PROFILE (default)

To Reproduce Set a regular SSO profile:

[profile profile1]
sso_start_url = https://my-company-sso.awsapps.com/start
sso_region = us-east-1
sso_account_id = 12345678900
sso_role_name = AWSAdministratorAccess
region = us-east-1
output = json

And then: see above

Expected behavior AWS_PROFILE is taken into account, as AWS_PROFILE=profile1 aws sts get-caller-identity works fine

Desktop (please complete the following information):

thomasmichaelwallace commented 1 year ago

Hmm - that is interesting because it is working for me.

➜ DP_STAGE=development AWS_PROFILE=iamadmin-general  ../../node_modules/.bin/serverless info --verbose
✔ serverless-better-credentials: credentials resolved from config ini profile: AWS_PROFILE (iamadmin-general)

Something which I'm suspicious of - do you have AWS_SDK_LOAD_CONFIG set in your enviornment?

Perhaps you could share the output of:

AWS_SDK_LOAD_CONFIG=1 AWS_PROFILE=profile1 sls info --verbose
pkit commented 1 year ago

Nope:

$ echo $AWS_SDK_LOAD_CONFIG
pkit commented 1 year ago

AWS_SDK_LOAD_CONFIG=1 AWS_PROFILE=profile1 sls info --verbose correctly loads credentials.

thomasmichaelwallace commented 1 year ago

Cool - then that explains it then.

AWS_SDK_LOAD_CONFIG must be truthy if you're using a ~/.aws/config file instead of the credentials file with any javascript tooling. (docs)

It is actually mentioned on the project readme:

Take note that if you are using SSO with the approach AWS document (a shared .aws/config file) you'll also need to set the AWS_SDK_LOAD_CONFIG enviornment value to something truthy (e.g. AWS_SDK_LOAD_CONFIG=1), as described in the AWS SDK documentation.

So I'm going to close this on a 'the simplest of documentation doesn't even get read' 😉 technicality.

Jokes aside - though - happy to take a PR if you have a better way of formatting the readme to make it more obvious to people who haven't encountered that quirk of the AWS + javascript toolkits before; StackOverflow attests to you not being the only one.

pkit commented 1 year ago

Ooof, it was probably created just to prevent nodejs servers from reading stuff in the filesystem. We are talking about a cli tool, I cannot see how it shouldn't be always set to 1. But ok. Thanks.

P.S. profiles always exist in ~/.aws/config only.

pkit commented 1 year ago

Just FYI, the following patch to serverless solved all my problems

diff --git a/node_modules/serverless/bin/serverless.js b/node_modules/serverless/bin/serverless.js
index b3f91c3..4a4b6fb 100755
--- a/node_modules/serverless/bin/serverless.js
+++ b/node_modules/serverless/bin/serverless.js
@@ -16,6 +16,12 @@ const nodeVersionMinor = Number(process.version.split('.')[1]);
 const minimumSupportedVersionMajor = 12;
 const minimumSupportedVersionMinor = 13;

+if (process.env.AWS_SDK_LOAD_CONFIG != '1') {
+  const { spawn } = require('child_process');
+  const args = [];
+  args.push(...process.execArgv, ...process.argv.slice(1));
+  spawn(process.argv[0], args, { stdio: 'inherit', env: { ...process.env, AWS_SDK_LOAD_CONFIG: '1' } });
+} else {
 if (
   nodeVersionMajor < minimumSupportedVersionMajor ||
   (nodeVersionMajor === minimumSupportedVersionMajor &&
@@ -124,3 +130,4 @@ require('../lib/cli/triage')().then((cliName) => {
       throw new Error(`Unrecognized CLI name "${cliName}"`);
   }
 });
+}
thomasmichaelwallace commented 1 year ago

FWIW I have it set in my ~/.zshrc, as it only comes back to bite you when you run javascript scripts using the aws-sdk locally.

I get why they do it that way, although I appreciate it's confusing. There's a whole variety of fun flags &c. that exist surrounding credentials in AWS; scars from multiple design choices and workarounds and extensions throughout the ages.

For example - you can have named profiles in ~/.aws/credentials (https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-profiles.html); although most keys are technically only valid in the config file - but the javascript aws-sdk just treats them interchangeably. You can also specify where these files are using AWS_SHARED_CREDENTIALS_FILE; and they all have random (/badly documentet) precedents.

Even worse - you shouldn't prefix the name of a profile with profile if you want it to be read by the java aws-sdk, but if you don't you run in to problems other languages.

Like all good, old, evolving systems, it's a mess 😄 . My main aim with this plugin was just to make that mess consistent across the aws-sdk and serverless so people don't have to work around things twice.