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

core: @aws-sdk dependencies are not being upgraded by the Yarn Upgrade workflow #29824

Open
nmussy opened this issue Apr 13, 2024 · 5 comments
Open
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@nmussy
Copy link
Contributor

nmussy commented Apr 13, 2024

Describe the bug

AWS SDK v3 dependencies (@aws-sdk/*) are currently not being upgraded.

Expected Behavior

Their minor versions should be getting bumped during the weekly upgrade workflow

Current Behavior

The @aws-sdk/* package versions are frozen

Reproduction Steps

Here is the latest PR:

https://github.com/aws/aws-cdk/pull/29703/files#diff-e5ad1bb1515606fb9c8d948c8b920d225ac47584f4cb69124700710c22556f1a

The packages are still on v3.421.0, which was published 7 months ago

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.137.0

Framework Version

No response

Node.js Version

N/A

OS

N/A

Language

TypeScript

Language Version

N/A

Other information

No response

@nmussy nmussy added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 13, 2024
@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Apr 13, 2024
@khushail khushail added p1 effort/medium Medium work item – several days of effort labels Apr 15, 2024
@khushail
Copy link
Contributor

khushail commented Apr 15, 2024

Thanks for reporting this @nmussy. I would bring this to team's attention.

@khushail khushail added p2 and removed needs-triage This issue or PR still needs to be triaged. p1 labels Apr 15, 2024
@colifran
Copy link
Contributor

@nmussy thanks for raising this concern. This was something we did intentionally - #28277. These packages are used in custom resources which run on Lambda, without having the SDK bundled. This means it's a fixed version of the SDK that the Lambda is providing. When using SDK V3 packages in test that are not the same version as the one in the Lambda, we run the risk of relying on newer APIs that don't exist in the Lambda runtime yet.

Credit to @mrgrain for the explanation.

@nmussy
Copy link
Contributor Author

nmussy commented Apr 17, 2024

Hey Colin, thanks for getting back to me!

For the record, my current issue is that the SDK is so far behind that I'm missing newer EC2 instance type properties for #29507 (mediaAcceleratorInfo, neuronInfo and processorInfo.manufacturer)

Did #28277 address a specific issue, or was it a precaution? I feel like integration tests should already cover custom resources and the associated Lambda runtime SDK version. It's especially painful because we're holding back all of our internal constructs, which can run the latest SDK version, just for the sake of asset code.

Looking back at other issues, I'm seeing some overlap with #24090, which should have been fixed with #29207. The way I see it, there are two somewhat straightforward solutions here. We can either create a different package, with its own dependencies, for bundled Lambda asset code, or start using #29207's bundleAwsSDK: true everywhere we feel it's necessary.


In the meantime, we should already be able to upgrade the SDK dependency up to v3.515.0, which is 5 months more recent than v3.421.0. The current versions of the SDK are no longer listed in the Lambda runtimes docs, but I ran the following test:

const app = new App();

const stack = new Stack(app, "LambdaSdkVersionStack");
const httpApi = new HttpApi(stack, "HttpApi", {
  createDefaultStage: false,
});

const stage = new HttpStage(stack, "Stage", {
  httpApi,
  stageName: "dev",
  autoDeploy: true,
});

const runtimes = [
  Runtime.NODEJS_18_X,
  Runtime.NODEJS_20_X,
];

for (const runtime of runtimes) {
  const runtimeName = runtime.name.replace(".", "_");

  httpApi.addRoutes({
    path: `/${runtimeName}`,
    integration: new HttpLambdaIntegration(
      `Integration_${runtimeName}`,
      new LambdaFunction(stack, `Lambda_${runtimeName}`, {
        runtime,
        handler: "index.handler",
        code: AssetCode.fromInline(`exports.handler = async () => (
        { statusCode: 200, body: require('@aws-sdk/client-s3/package.json').version } 
      );`),
      })
    ),
  });

  new CfnOutput(stack, runtimeName, { value: `${stage.url}/${runtimeName}` });
}
$ curl https://<id>.execute-api.us-east-1.amazonaws.com/dev/nodejs18_x
3.515.0                                                                                                                           
$ curl https://<id>.execute-api.us-east-1.amazonaws.com/dev/nodejs20_x
3.515.0

I'm not seeing a version difference between the few regions I tested (us-east-1, eu-west-3, ap-northeast-1, sa-east-1), but the CN/Gov partitions might not be using the same runtime images, since I assume Lambda is pulling them from a regional source?

@mrgrain
Copy link
Contributor

mrgrain commented Apr 17, 2024

Great effort with your PR @nmussy 👏

What I'm not getting from it though, is how are you actually needing the SDK there? I'm guessing it's used in some build step that generates the JSON file?

@nmussy
Copy link
Contributor Author

nmussy commented Apr 17, 2024

I'm only using the SDK as a secondary typing verification here, as the intersection of the actual JSON file type and the expected output. It's just a failsafe in case there is any bad/unexpected data inside the JSON file, which I generate externally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests

4 participants