SDK Clients without Credential Providers

jd-carroll commented 1 month ago

Describe the feature

The SDK clients are capable of auto (magically) finding the appropriate credentials when you instatiate them. This capability exists even if you provide your own credential provider to the SDK Client.

While this is great, for ESM this functionality provides a lot of (potentially) unnecessary overhead.

For example, here are the included @aws-sdk modules in a simple lambda which utilizes @aws-sdk/client-sts

The issue is that the following modules are bundled, but will never be executed:

client-sso-oidc
client-sso
credential-provider-ini
credential-provider-http
credential-provider-sso
credential-provider-node (sort of)
credential-provider-process
credential-provider-web-identity

The only credential provider that will be utilized in credential-provider-env.

That's 128kb of wasted / unnecessary code!

It would be great if, in addition to STSClient, there was an additional STSClientRaw where you were required to provide credentials. That way, the SDK Client would not need to depend on credential-provider-node and we could avoid all of these unnecessary dependencies.

Use Case

Minimize bundled code in ESM formatted code.

Proposed Solution

In addition to standard (existing) SDK Clients, provide an additional SDK Client "Raw" where there is no dependency on credential-provider-node.

(please come up with a better name than "raw")

Other Information

No response

Acknowledgements

[ ] I may be able to implement this feature request
[ ] This feature might incur a breaking change

SDK version used

latest

Environment details (OS name and version, etc.)

na

RanVaknin commented 1 month ago

Hi @jd-carroll ,

Are you consuming the SDK through ES modules (i.e., using import instead of require)? Modern bundlers can be configured to include only the code that is actually used in your application, even if other modules are imported.

What is your bundler configuration?

Thanks, Ran

kuhe commented 1 month ago

Mark those modules are external while bundling. Then they will sit in a split chunk that should never be imported if you are indeed providing credentials.

That reduces the size of the parsed and executed code. If you actually have a space limitation in the Lambda with regard to the 128kb, you can mark the external modules as empty, which varies depending on the bundler, so that they are split into a chunk and that chunk is deleted or empty.

jd-carroll commented 1 month ago

@kuhe - Right, that is currently the only way to accomplish this.

@RanVaknin - Of course, including setting mainFields to ['module', 'main']. Let me know if you need help with the esbuild command to bundle the demo lambda function below (you'll also need to make sure you are minimizing the code)

Here is a simple example:

import { AssumeRoleCommand, STSClient } from '@aws-sdk/client-sts';

export const handler = async () => {
  const client = new STSClient({
    region: 'us-west-2',
    credentials: {
      accessKeyId: 'fake-access-key-id',
      secretAccessKey: 'fake-secret-access-key'
    }
  });
  await client.send(
    new AssumeRoleCommand({ RoleArn: 'arn:aws:iam::123456789012:role/demo', RoleSessionName: 'session1' })
  );
};

The bundled lambda function from esbuild looks like:

If we look at just @aws-sdk/*, that has:

There are a couple of problems here

Problem 1: Non "excludable" code paths

The first problem is that bundler has no way of knowing that the credential providers are unneccessary. This boils down into two parts:

credentialDefaultProvider is deprecated I believe the more appropriate use is credentials as I have in the example. You are also able to give a provider for credentials. However the issue is that the runtimeConfig for STSClient (and likely all clients) has:
```
export const getRuntimeConfig = (config) => {
...
return {
    ...config,
    ...
    credentialDefaultProvider: config?.credentialDefaultProvider ?? credentialDefaultProvider,
    ...
```

So if you do not set credentialDefaultProvider there is a hard import of the @aws-sdk/credential-provider-node

httpAuthSchemes always uses credentialDefaultProvider But even if you set the credentialDefaultProvider in the client config. The httpAuthSchemes always uses the credentialDefaultProvider from the import, i.e.:
```
import { defaultProvider as credentialDefaultProvider } from "@aws-sdk/credential-provider-node";
```

So you would also need to do something like:

  const client = new STSClient({
    region: 'us-west-2',
    credentials: {
      accessKeyId: 'fake-access-key-id',
      secretAccessKey: 'fake-secret-access-key'
    },
    credentialDefaultProvider: undefined,
    httpAuthSchemes: [
      {
        schemeId: 'aws.auth#sigv4',
        identityProvider: (ipc) =>
          ipc.getIdentityProvider('aws.auth#sigv4') ||
          (() => Promise.resolve({ expiration: new Date(Date.now() + 1000) })),
        signer: new AwsSdkSigV4Signer()
      },
      {
        schemeId: 'smithy.api#noAuth',
        identityProvider: (ipc) => ipc.getIdentityProvider('smithy.api#noAuth') || (async () => ({})),
        signer: new NoAuthSigner()
      }
    ]
  });

But even doing all that, doesn't seem to make any difference.

Only if I completely comment out the credentialDefaultProvider and httpAuthSchemes from the getRuntimeConfig will the @aws-sdk/credential-provider-node be treeshook from the bundle.

I tried a couple of different ways to add a default credentialDefaultProvider into the config while still allowing it to be tree shaken, but none seemed to work. My guess would be that it needs to be something more like:

    const combined = {
        ...clientSharedValues,
        ...config,
        runtime: "node",
        ...,
    };
    if (!combined.credentialDefaultProvider) {
      combined.credentialDefaultProvider = getCredentialDefaultProvider();
    }
    return result;

Where the import of @aws-sdk/credential-provider-node is moved one level deep into the getCredentialDefaultProvider method call. All that to say, the code flow is too complex for static analysis to identify that credentialDefaultProvider is not necessary.

Problem 2: Incorrect exports conditions

The other item you should notice is that not 100% of the code is ESM. And if we look at where the non-ESM code is coming from (by clicking on the percentages), some of it is coming from @aws-sdk

The issue is that the exports from @aws-sdk/core are setup incorrectly. For example, the "default" export from core looks like:

  "main": "./dist-cjs/index.js",
  "module": "./dist-es/index.js",
  "types": "./dist-types/index.d.ts",
  "exports": {
    ".": {
      "node": "./dist-cjs/index.js",
      "import": "./dist-es/index.js",
      "require": "./dist-cjs/index.js",
      "types": "./dist-types/index.d.ts"
    },

The problem is that bundlers will prefer "exports" over any top-level "mian" / "module" and when bundling for a lambda we are setting the platform to be node which forces the conditions ['node', 'default'] when evaluating the exports.

The result is that any ESM module importing core will only ever receive the cjs bundle.

The exports should be structured like:

  "main": "./dist-cjs/index.js",
  "module": "./dist-es/index.js",
  "types": "./dist-types/index.d.ts",
  "exports": {
    ".": {
      "node": {
        "import": "./dist-es/index.js",
        "require": "./dist-cjs/index.js"
      },
      "import": "./dist-es/index.js",
      "require": "./dist-cjs/index.js",
      "types": "./dist-types/index.d.ts"
    },

Or really, the node is probably unnecessary, so it should really just be:

  "main": "./dist-cjs/index.js",
  "module": "./dist-es/index.js",
  "types": "./dist-types/index.d.ts",
  "exports": {
    ".": {
      "import": "./dist-es/index.js",
      "require": "./dist-cjs/index.js",
      "types": "./dist-types/index.d.ts"
    },

The impact of including core as a CJS module is only minor, the real issue is @aws-sdk/credential-provider-node. Once these are resolved, it would be good to reach out to fast-xml-parser to see if they can release an ESM version of the module in addition to the CJS.

Going back to the first problem... I suspect that even the example solution I suggest may not be enough to indicate to the bundle that @aws-sdk/credential-provider-node can be treeshaken from the bundle (without significant code changes).

Which brings me back to my original suggestion in that there should be a new "Raw" client which has bare-bones configuration and requires the user to provide other necessary configuration details, such as credentials.

kuhe commented 1 month ago

Please make a pre-bundle build step that replaces the runtimeConfig.ts file with one that does not contain @aws-sdk/credential-provider-node if that is what you need.

The use-case is not common enough to provide an official variant from the AWS SDK.

Regarding exports:

  "exports": {
    ".": {
      "import": "./dist-es/index.js",
      "require": "./dist-cjs/index.js",
      "types": "./dist-types/index.d.ts"
    },

There is a reason the node exports overrides import.

We only offer our CJS distribution for Node.js because our ES distribution is not MJS compliant and It is only for bundlers. We have not analyzed what effect MJS compliance would have on web bundlers. The CJS distribution will likely remain the only or at least preferred Node.js offering for as long as our minimum Node.js version supports CJS-within-ESM but not ESM-within-CJS.

jd-carroll commented 1 month ago

Got it. That's unfortunate that we are required to exclude the unnecessary credential providers ourselves, because its a savings of >75kb minimized (so at scale would be pretty meaningful). But all good.

For the esbuild'ers, as @-kuhe mentioned above, the best/easist thing to do is drop all of those providers into the excludes of esbuild.

github-actions[bot] commented 3 weeks ago

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.

aws / aws-sdk-js-v3