Skip to content

Commit

Permalink
fix: do not skip bundling for stacks during CDK Import, otherwise fai…
Browse files Browse the repository at this point in the history
…ls (#33322)

### Issues #31999 #31677 

Closes #31999 #31677 

### Reason for this change

`cdk import` reports changes in the stack when none are present, for NodeJS Lambda functions. This results in the command failing. The only way to work around this is to use the --force flag, which replaces stack resources. This is not ideal as it overwrites existing resources that may contain data, and is just generally unnecessary.

### Description of changes

I added `cdk import` to a configuration that determines which CLI commands bundle code. Before this change, `cdk import` skipped code bundling. This is fine for regular Lambda functions that don't rely on bundling. However, NodeJSFunction does.

The original implementation of skipping bundling for certain CLI commands was introduced via Issue #9540.

`cdk import` was introduced two years later in PR #17666.

### Describe any new or updated permissions being added

No permissions changes.


### Description of how you validated changes
I added an integration test in `packages/@aws-cdk-testing/framework-integ/test/aws-lambda-nodejs/test/integ.nodejs.build.images.ts`.

<img width="757" alt="Screenshot 2025-02-06 at 17 10 41" src="https://github.com/user-attachments/assets/f7b3939f-240c-4021-b0ad-fd7e423e7c09" />

The test ensures that NodeJSFunction Lambdas are always bundled. I will add a canary in a follow-up PR to ensure that new CLI commands are explicitly determined to either need or not need bundling.


I manually validated changes:
1. Created an S3 bucket: `iankhou-1738867338384`
2. Ran `cdk synth`, `cdk deploy` on a stack with a NodeJSFunction, and without an S3 bucket
3. Added code that initializes an S3 bucket and ran `cdk import`

Output and console:
<img width="745" alt="Screenshot 2025-02-06 at 13 45 52" src="https://github.com/user-attachments/assets/c5e4b9ef-ae84-46a4-9b37-a1f58532a00e" />
<img width="1015" alt="Screenshot 2025-02-06 at 13 45 08" src="https://github.com/user-attachments/assets/10298665-281c-4fed-a32b-5c4fbb917f92" />

### Checklist
- [X] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
iankhou authored Feb 10, 2025
1 parent ba485ef commit 5160796
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as path from 'path';
import { App, Stack, StackProps } from 'aws-cdk-lib';
import * as fs from 'fs';
import { App, Stack, StackProps, ValidationError } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import * as lambda from 'aws-cdk-lib/aws-lambda-nodejs';
import * as lambda from 'aws-cdk-lib/aws-lambda';
import * as lambdaNodeJs from 'aws-cdk-lib/aws-lambda-nodejs';
import { ExpectedResult, IntegTest } from '@aws-cdk/integ-tests-alpha';
import { IFunction, Runtime } from 'aws-cdk-lib/aws-lambda';

Expand All @@ -18,13 +20,13 @@ class TestStack extends Stack {
const uniqueRuntimes: Runtime[] = runtimes.filter((value, index, array) => array.findIndex(value1 => value1.runtimeEquals(value)) === index);

uniqueRuntimes.forEach((runtime) => {
this.lambdaFunctions.push(new lambda.NodejsFunction(this, `func-${runtime.name}`, {
this.lambdaFunctions.push(new lambdaNodeJs.NodejsFunction(this, `func-${runtime.name}`, {
entry: path.join(__dirname, 'integ-handlers/dependencies.ts'),
runtime: runtime,
bundling: {
minify: true,
sourceMap: true,
sourceMapMode: lambda.SourceMapMode.BOTH,
sourceMapMode: lambdaNodeJs.SourceMapMode.BOTH,
},
}));
});
Expand All @@ -49,3 +51,39 @@ stack.lambdaFunctions.forEach(func=> {
ExecutedVersion: '$LATEST',
}));
});

// Ensure that the code is bundled
const assembly = app.synth();

stack.lambdaFunctions.forEach((func) => {
const template = assembly.getStackArtifact(stack.artifactId).template;
const resourceName = stack.getLogicalId(func.node.defaultChild as lambda.CfnFunction);
const resource = template.Resources[resourceName];

if (!resource || resource.Type !== 'AWS::Lambda::Function') {
throw new ValidationError(`Could not find Lambda function resource for ${func.functionName}`, stack);
}

const s3Bucket = resource.Properties.Code.S3Bucket;
const s3Key = resource.Properties.Code.S3Key;

if (!s3Bucket || !s3Key) {
throw new ValidationError(`Could not find S3 location for function ${func.functionName}`, stack);
}

const assetId = s3Key.split('.')[0]; // S3Key format is <hash>.zip"
const assetDir = path.join(assembly.directory, `asset.${assetId}`);

try {
if (!fs.existsSync(assetDir) || !fs.statSync(assetDir).isDirectory()) {
throw new ValidationError(`Asset directory does not exist for function ${func.functionName}: ${assetDir}`, stack);
}

const indexPath = path.join(assetDir, 'index.js');
if (!fs.existsSync(indexPath)) {
throw new ValidationError(`index.js not found in asset directory for function ${func.functionName}`, stack);
}
} catch (error) {
throw error;
}
});
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/cli/user-configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const BUNDLING_COMMANDS = [
Command.SYNTH,
Command.SYNTHESIZE,
Command.WATCH,
Command.IMPORT,
];

export type Arguments = {
Expand Down

0 comments on commit 5160796

Please sign in to comment.