aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
3.06k stars 573 forks source link

making `credential-provider-node` only a peer-dep in `client-sts` is a breaking change #5818

Open alex-at-cascade opened 7 months ago

alex-at-cascade commented 7 months ago

Checkboxes for prior research

Describe the bug

In https://github.com/aws/aws-sdk-js-v3/commit/d27301d48f3e75fdaccabf58f779f0b33a70664e#diff-c00e767676da286301fe37f850b4c4cdaa0bf86930bcb97a454342605f95938e the credential-provider-node dep in client-sts/package.json was changed to a "peer dependency", which breaks a script that only imports the @aws-sdk/client-sts package.

SDK version number

@aws-sdk/client-sts@3.513.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.18.2

Reproduction Steps

try to call getSessionToken without having "@aws-sdk/credential-provider-node" as an explicit dependency

Observed Behavior

Error: Cannot find module '@aws-sdk/credential-provider-node'

Expected Behavior

success

Possible Solution

revert it to a true dependency

Additional Information/Context

No response

kuhe commented 7 months ago

The node credential chain has to be a peer of the STS and SSO-OIDC clients because these clients may be used in the credential chain, for example if resolving to assume-role credentials. To break the cyclical dependency, this was weighed against other solutions like including a redundant bundle of the STS and other clients in the credential provider code.

Because we receive many requests for performance improvement, we decided to make the peerDependency change instead of increasing the size of the SDK with the other solution. In modern NPM, peerDependencies are installed automatically. For other package management configurations in which this is not the case, install the peerDependency explicitly as requested by the package manager.

alex-at-cascade commented 7 months ago

it was still a breaking change however - seems like that should have been a major version jump

kuhe commented 7 months ago

Yes, it should. Due to some wording in the existing maintenance policy, we are not releasing changes in adherence to semver.

But as this repeatedly causes problems for users like yourself, I will see if we can detach the product iteration version "v3" from the version string major component (e.g. 3.xxx.y). I can't guarantee that this will happen, or that it will be decided quickly.

I will make a new issue to track that: https://github.com/aws/aws-sdk-js-v3/issues/5821

abhishek-parative commented 7 months ago

@kuhe What is the workaround that allows for STS to be used in package.json? As of this writing, it is impossible to install @aws-sdk/client-sts as a dependency in npm

kuhe commented 7 months ago

Install both @aws-sdk/client-sts and @aws-sdk/credential-provider-node in your package.json, if STS is the only client you are using.

kb-ucs commented 6 months ago

This is in fact breaking change and requires new major version IMO. With yarn, adding @aws-sdk/client-sts and/or @aws-sdk/credential-provider-node doesn't seem to resolve the missing dependency warning. I think that the problem is that @aws-sdk/token-providers depends on @aws-sdk/client-sso-oidc, which itself depends on @aws-sdk/client-sts (which requires @aws-sdk/credential-provider-node). So while @aws-sdk/client-sso-oidc declares @aws-sdk/credential-provider-node as a peerDependency, looking at the graph from @aws-sdk/token-providers perspective, @aws-sdk/credential-provider-node is not provided.

I've used yarn's packageExtensions feature (docs) to "patch" the dependency graph:

  "@aws-sdk/token-providers@>=3.501.0":
    peerDependencies:
      "@aws-sdk/credential-provider-node": "*"
  "@aws-sdk/credential-provider-sso@>=3.501.0":
    peerDependencies:
      "@aws-sdk/credential-provider-node": "*"
  "@aws-sdk/credential-provider-ini@>=3.501.0":
    peerDependencies:
      "@aws-sdk/credential-provider-node": "*"
  "@aws-sdk/credential-provider-web-identity@>=3.501.0":
    peerDependencies:
      "@aws-sdk/credential-provider-node": "*"

which helped resolve the warnings on my side (note that you might need another version).

kb-ucs commented 6 months ago

I've created a PR that aims to resolve this issue: https://github.com/aws/aws-sdk-js-v3/pull/5959