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

Add support for AWS_DEFAULT_PROFILE env variable when resolving profile #8354

Merged
merged 1 commit into from Oct 7, 2020
Merged

Add support for AWS_DEFAULT_PROFILE env variable when resolving profile #8354

merged 1 commit into from Oct 7, 2020

Conversation

marekpiotrowski
Copy link
Contributor

Closes: #7676

By default, aws-cli does not take [default] profile from credentials file if AWS_DEFAULT_PROFILE environment variable is defined. Serverless should behave in exactly same manner when resolving profiles - currently [default] is hard-coded. Fallback mechanism to [default] is still there if AWS_DEFAULT_PROFILE isn't defined.

Important remark 1: each test in getCredentials suite seems to carry some state, that's why I had to reset newAwsProvider.options['aws-profile'], below is an interesting log before introducing my changes at all:

> serverless@2.4.0 test /home/mpiotrowski/Projects/oss/fork-serverless/serverless
> mocha "lib/plugins/aws/provider/*.test.js" -g "newAwsProvider.options"



  AwsProvider
    #getCredentials()
newAwsProvider.options['aws-profile'] =  undefined
      ✓ [newAwsProvider.options 1]should get credentials when profile is provied via --aws-profile option even if profile is defined in serverless.yml
newAwsProvider.options['aws-profile'] =  notDefault
      ✓ [newAwsProvider.options 2]should get credentials when profile is provied via process.env.AWS_PROFILE even if profile is defined in serverless.yml


  2 passing (100ms)

logs correspond to the following:

    it('[newAwsProvider.options 1]should get credentials when profile is provied via --aws-profile option even if profile is defined in serverless.yml', () => {
      // eslint-disable-line max-len
      console.log("newAwsProvider.options['aws-profile'] = ", newAwsProvider.options['aws-profile'])
      process.env.AWS_PROFILE = 'notDefaultTemporary';
      newAwsProvider.options['aws-profile'] = 'notDefault';

      serverless.service.provider.profile = 'notDefaultTemporary2';

      const credentials = newAwsProvider.getCredentials();
      expect(credentials.credentials.profile).to.equal('notDefault');
    });

    it('[newAwsProvider.options 2]should get credentials when profile is provied via process.env.AWS_PROFILE even if profile is defined in serverless.yml', () => {
      // eslint-disable-line max-len
      console.log("newAwsProvider.options['aws-profile'] = ", newAwsProvider.options['aws-profile'])
      process.env.AWS_PROFILE = 'notDefault';

      serverless.service.provider.profile = 'notDefaultTemporary';

      const credentials = newAwsProvider.getCredentials();
      expect(credentials.credentials.profile).to.equal('notDefault');
    });

even though newAwsProvider is instantiated before each test. Maybe it'd be worth investigating, not sure.

Important remark 2: (okay, less important) there's a typo in few tests names ("provied"). Not sure if I should fix it in the same pull request?

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@d2fb696). Click here to learn what that means.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #8354   +/-   ##
=========================================
  Coverage          ?   88.08%           
=========================================
  Files             ?      249           
  Lines             ?     9309           
  Branches          ?        0           
=========================================
  Hits              ?     8200           
  Misses            ?     1109           
  Partials          ?        0           
Impacted Files Coverage Δ
lib/plugins/aws/provider/awsProvider.js 93.06% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2fb696...f2e06af. Read the comment docs.

Copy link
Contributor

@medikoo medikoo left a comment

Choose a reason for hiding this comment

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

Thank you @marekpiotrowski !

@medikoo medikoo merged commit 261c16f into serverless:master Oct 7, 2020
@OJFord
Copy link

OJFord commented Nov 16, 2021

@medikoo @marekpiotrowski This seems to have been removed, the docs only mention AWS_PROFILE, and AWS_DEFAULT_PROFILE is ignored?

@medikoo
Copy link
Contributor

medikoo commented Nov 16, 2021

@OJFord I assume docs were never updated with info that AWS_DEFAULT_PROFILE is supported. PR's welcome

@OJFord
Copy link

OJFord commented Nov 16, 2021

It's not a docs issue, it's not working. AWS_PROFILE="$AWS_DEFAULT_PROFILE" and I get further, but now it seems it's not respecting AWS_CONFIG_FILE either, so it says profile not configured (but has it's name now).

@marekpiotrowski
Copy link
Contributor Author

iirc (and if I read my comment correctly) AWS_DEFAULT_PROFILE allows you to change the default profile name from within ~/.aws/credentials to use if nothing is explicitly configured. So you clearly have to unset AWS_PROFILE if you're planning to use it. I haven't been using serverless for quite a while now, will take a look though.

@marekpiotrowski
Copy link
Contributor Author

I'd say it works fine:

(base) mpiotrowski@mpiotrowski-VirtualBox:~/aws-node-project$ serverless deploy
Serverless: Packaging service...
Serverless: Excluding development dependencies...
 
 Serverless Error ----------------------------------------
 
  AWS provider credentials not found. Learn how to set up AWS provider credentials in our docs here: <http://slss.io/aws-creds-setup>.
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com
 
  Your Environment Information ---------------------------
     Operating System:          linux
     Node Version:              16.13.0
     Framework Version:         2.66.1
     Plugin Version:            5.5.1
     SDK Version:               4.3.0
     Components Version:        3.18.0
 
(base) mpiotrowski@mpiotrowski-VirtualBox:~/aws-node-project$ export AWS_DEFAULT_PROFILE=mp_test
(base) mpiotrowski@mpiotrowski-VirtualBox:~/aws-node-project$ serverless deploy
Serverless: Packaging service...
Serverless: Excluding development dependencies...
 
 Serverless Error ----------------------------------------
 
  The security token included in the request is invalid.
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com
 
  Your Environment Information ---------------------------
     Operating System:          linux
     Node Version:              16.13.0
     Framework Version:         2.66.1
     Plugin Version:            5.5.1
     SDK Version:               4.3.0
     Components Version:        3.18.0

obviously, I do not have valid AWS credentials, but it resolved the profile correctly.

@marekpiotrowski
Copy link
Contributor Author

same after installing from sources, even added a printout of the credentials which are selected

(base) mpiotrowski@mpiotrowski-VirtualBox:~/serverless/aws-node-project$ ../bin/serverless.js deploy
{
  credentials: SharedIniFileCredentials {
    expired: false,
    expireTime: null,
    refreshCallbacks: [],
    accessKeyId: 'AKIAIOSFODNN7EXAMPLE',
    sessionToken: undefined,
    filename: undefined,
    profile: 'mp_test',
    disableAssumeRole: false,
    preferStaticCredentials: false,
    tokenCodeFn: [Function (anonymous)],
    httpOptions: null
  }
}
Serverless: Packaging service...
Serverless: Excluding development dependencies...
 
 Serverless Error ----------------------------------------
 
  The security token included in the request is invalid.
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com
 
  Your Environment Information ---------------------------
     Operating System:          linux
     Node Version:              16.13.0
     Framework Version:         2.66.1
     Plugin Version:            5.5.1
     SDK Version:               4.3.0
     Components Version:        3.18.0
 
(base) mpiotrowski@mpiotrowski-VirtualBox:~/serverless/aws-node-project$ unset AWS_DEFAULT_PROFILE
(base) mpiotrowski@mpiotrowski-VirtualBox:~/serverless/aws-node-project$ ../bin/serverless.js deploy
{
  credentials: SharedIniFileCredentials {
    expired: false,
    expireTime: null,
    refreshCallbacks: [],
    accessKeyId: 'whateva',
    sessionToken: undefined,
    filename: undefined,
    profile: 'default',
    disableAssumeRole: false,
    preferStaticCredentials: false,
    tokenCodeFn: [Function (anonymous)],
    httpOptions: null
  }
}
Serverless: Packaging service...
Serverless: Excluding development dependencies...
 
 Serverless Error ----------------------------------------
 
  The security token included in the request is invalid.
 
  Get Support --------------------------------------------
     Docs:          docs.serverless.com
     Bugs:          github.com/serverless/serverless/issues
     Issues:        forum.serverless.com
 
  Your Environment Information ---------------------------
     Operating System:          linux
     Node Version:              16.13.0
     Framework Version:         2.66.1
     Plugin Version:            5.5.1
     SDK Version:               4.3.0
     Components Version:        3.18.0

@OJFord
Copy link

OJFord commented Nov 16, 2021

So you clearly have to unset AWS_PROFILE if you're planning to use it.

I meant that setting AWS_PROFILE to the same value as it works as expected (well, progressing anyway..), not giving the error I had with it unset and only AWS_DEFAULT_PROFILE set.

@mentos1386 mentos1386 mentioned this pull request Oct 14, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS_DEFAULT_PROFILE seems not supported by profile resolver
4 participants