Credential Provider Chain Resolution

[RLovelett]

This is a question about expected behavior.

Let's say I have the following code (Node 7.5.0 because of async/await).

(async function () {
  const govcloudCredential = new CredentialProviderChain([
    () => { return new EnvironmentCredentials('GC_AWS'); },
    () => { return new EnvironmentCredentials('GC_AMAZON'); },
    () => { return new SharedIniFileCredentials({ profile: 'govcloud' }); },
    () => { return new SharedIniFileCredentials({ profile: 'default' }); },
    () => { return new EC2MetadataCredentials(); }
  ]);

  const s3 = new S3({
    region: 'us-gov-west-1',
    credentials: undefined,
    credentialProvider: govcloudCredential
  });

  try {
    const foo = await s3.listObjectsV2({
      Bucket: 'bucket'
    }).promise();
    console.log(foo.KeyCount);
  } catch (error) {
    console.error(error);
  }
})()

And ~/.aws/credentials file:

[default]
aws_access_key_id = valid-for-commercial
aws_secret_access_key = valid-for-commercial

[govcloud]
aws_access_key_id = valid-for-gov-cloud
aws_secret_access_key = valid-for-gov-cloud

[commercial]
aws_access_key_id = valid-for-commercial
aws_secret_access_key = valid-for-commercial

I would expect that the resulting S3 instance would pull from the explicit provider chain. Such that if I ran the code it would pull from the govcloud section and work fine. However, it pulls from default and thus prevents my call from succeeding. Just to be sure I checked by removing the default group. Then the call actually pulls from govcloud.

This behavior, to me, seems counter-intuitive. Can someone help my intuition?

Update

(async function () {
  const govcloudCredential = new CredentialProviderChain([
    () => { return new EnvironmentCredentials('GC_AWS'); },
    () => { return new EnvironmentCredentials('GC_AMAZON'); },
    () => { return new SharedIniFileCredentials({ profile: 'govcloud' }); },
    () => { return new SharedIniFileCredentials({ profile: 'default' }); },
    () => { return new EC2MetadataCredentials(); }
  ]);

  const creds = new Promise<Credentials>((resolve, reject) => {
    govcloudCredential.resolve((error, creds) => {
      if (error) { reject(error); }
      else { resolve(creds); }
    });
  });

  const s3 = new S3({
    region: 'us-gov-west-1',
    credentials: (await creds)
  });

  try {
    const foo = await s3.listObjectsV2({
      Bucket: 'refman-bundler-dev'
    }).promise();
    console.log(foo.KeyCount);
  } catch (error) {
    console.error(error);
  }
})()

This second example code does work as I expect but notice now I have explicitly called resolve and I am awaiting the credentials resolving before creating my S3 object. This does feel like the way the documentation indicates this should work.

[RLovelett]

Related to #1276?

[chrisradek]

@RLovelett
I agree, the behavior you are seeing is not what I'd expect. You have found a valid work-around by resolving the credential provider chain. Another way would be to set credentials to null instead of undefined.

The long explanation is that when you pass configuration into a service client's constructor, behind the scenes the global config is copied, then service-bound params on the global config are merged on top of that, followed by the configuration passed to the constructor. This is done so that users don't have to specify every configuration option for every service client.

However, during this merge, if a field in the configuration is set to undefined, a default value is chosen instead.
https://github.com/aws/aws-sdk-js/blob/master/lib/config.js#L429

Setting the field to null gets around this.

We should document that null and undefined have different behavior when setting configuration. We'll also make sure not to have this behavior in future major versions.

[RLovelett]

I like the null idea; it feels the closest to what I'd like to achieve. Though unfortunately when I went to test that out in TypeScript credentials cannot be nulled. So it looks like I'll be submitting a PR for that shortly. 🤣

Update

@chrisradek I think something like this needs to be made in the TypeScript definition. Thoughts?

diff --git a/lib/config.d.ts b/lib/config.d.ts
index e22134d..9312931 100644
--- a/lib/config.d.ts
+++ b/lib/config.d.ts
@@ -171,7 +171,7 @@ export abstract class ConfigurationOptions {
     /**
      * The AWS credentials to sign requests with.
      */
-    credentials?: Credentials|CredentialsOptions
+    credentials?: Credentials|CredentialsOptions|null
     /**
      * The provider chain used to resolve credentials if no static credentials property is set.
      */

Update 2

I actually think this is necessary because of --strictNullChecks. I assume that does not change anything. I just thought I'd point it out.

[chrisradek]

@RLovelett
You're right, a union with null is the best option here.

[ajredniwja]

Closing this issue since the feature was implemented.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.