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

aws-lambda-nodejs/lib/bundlingts.ts: Missing condition when checking for v2 runtime #26966

Closed
Minh-Duc-Ng opened this issue Sep 1, 2023 · 3 comments · Fixed by #27014
Closed
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@Minh-Duc-Ng
Copy link

Describe the bug

Hi,

I have been receiving weird warnings from aws cdk about nodejs runtime since upgrading to v2.93.

When creating new NodejsFunction, I make sure to pass runtime: Runtime.NODEJS_16_X as one of the props. However, I still get this warning: "If you are relying on AWS SDK v2 to be present in the Lambda environment already, please explicitly configure a NodeJS runtime of Node 16 or lower".

My company requires all cdk deployment to be executed with strict-mode so we can't ignore this warning.

Expected Behavior

No warning raised by cdk because I am specifying Nodejs 16 or lower as instructed

Current Behavior

A warning is raised whenever I create a NodejsFunction with runtime: Runtime.NODEJS_16_X

Reproduction Steps

Run cdk synth to create following resource:

const lambda = new NodejsFunction(this, 'id', {
      entry: join(__dirname, "..", "lambdas", "lambda.ts"),
      handler: "handler",
      timeout: Duration.minutes(3),
      functionName: lambdaFnName,
      runtime: Runtime.NODEJS_16_X,
      memorySize: 512,
      depsLockFilePath: join(__dirname, "..", "..", "..", "..", "pnpm-lock.yaml"),
      role: lambdaExecutionRole,
      logRetention: RetentionDays.TWO_WEEKS,
    });

Possible Solution

I believe this is the code section that cause the warning:

https://github.com/aws/aws-cdk/blob/d6b71adb41cb2cccf30e2301f872806e12ad4a87/packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts#L127C5-L143C1

// Modules to externalize when using a constant known version of the runtime.
    // Mark aws-sdk as external by default (available in the runtime)
    const isV2Runtime = isSdkV2Runtime(props.runtime);
    const versionedExternals = isV2Runtime ? ['aws-sdk'] : ['@aws-sdk/*'];
    // Don't automatically externalize any dependencies when using a `latest` runtime which may
    // update versions in the future.
    const defaultExternals = props.runtime?.isVariable ? [] : versionedExternals;
    const externals = props.externalModules ?? defaultExternals;

    // Warn users if they are trying to rely on global versions of the SDK that aren't available in
    // their environment.
    if (isV2Runtime && externals.some((pkgName) => pkgName.startsWith('@aws-sdk/'))) {
      cdk.Annotations.of(scope).addWarningV2('@aws-cdk/aws-lambda-nodejs:sdkV3NotInRuntime', 'If you are relying on AWS SDK v3 to be present in the Lambda environment already, please explicitly configure a NodeJS runtime of Node 18 or higher.');
    } else if (externals.includes('aws-sdk')) {
      cdk.Annotations.of(scope).addWarningV2('@aws-cdk/aws-lambda-nodejs:sdkV2NotInRuntime', 'If you are relying on AWS SDK v2 to be present in the Lambda environment already, please explicitly configure a NodeJS runtime of Node 16 or lower.');
    }

If someone pass runtime: Runtime.NODEJS_16_X and no externalModules provided like my example above
-> isV2Runtime is True -> versionedExternals = ['aws-sdk'] -> defaultExternals = ['aws-sdk'] -> externals = ['aws-sdk']
-> the condition if (externals.includes('aws-sdk')) on line 140 is true
-> warning message displayed

Possible fix: adding more condition to line 140: if (!isV2Runtime && externals.includes('aws-sdk'))

Additional Information/Context

No response

CDK CLI Version

2.93.0

Framework Version

No response

Node.js Version

16

OS

Ubuntu

Language

Typescript

Language Version

No response

Other information

No response

@Minh-Duc-Ng Minh-Duc-Ng added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 1, 2023
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Sep 1, 2023
@plumdog
Copy link
Contributor

plumdog commented Sep 5, 2023

I also encountered this issue and agree with the investigation above that the logic around generating the warning is invalid.

Have opened #27014 to attempt to fix and write some tests.

@mergify mergify bot closed this as completed in #27014 Sep 5, 2023
mergify bot pushed a commit that referenced this issue Sep 5, 2023
Fixes to remove an incorrectly generated warning when using Node JS runtime <= 16 with `NodejsFunction`

Closes #26966 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@Minh-Duc-Ng
Copy link
Author

Hi @plumdog, thank you for looking into this issue. I realize my proposed solutions might not be correct in some cases, such that the user specify Runtime.NODEJS_18_X in NodejsFunction, and then follow the warning instruction to add a 'aws-sdk' as external modules -> the warning is still going to be there.

Honestly, I am not sure about the wording of this warning. Is it allowed for someone to use Nodejs 18 and request 'aws-sdk' as external modules like my example above? If not, maybe the warning should tell the dev to upgrade their code to use sdk v3.

mikewrighton pushed a commit that referenced this issue Sep 14, 2023
Fixes to remove an incorrectly generated warning when using Node JS runtime <= 16 with `NodejsFunction`

Closes #26966 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants