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-cdk: cdk import always reports pending changes with lambda function w/alias #31677

Closed
1 task
perpil opened this issue Oct 6, 2024 · 3 comments
Closed
1 task
Assignees
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI

Comments

@perpil
Copy link
Contributor

perpil commented Oct 6, 2024

Describe the bug

When using a Lambda NodeJsFunction with an alias and a function url attached to it, cdk import always reports pending changes even when no changes exist.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

Running cdk import when there are no changes to your lambda function should report no pending changes that need to be resolved on existing resources.

Current Behavior

Running cdk deploy then cdk import always reports resource updates, when no resources are updated.

No resource updates or deletes are allowed on import operation. Make sure to resolve pending changes to existing resources, before attempting an import. Updated/deleted resources: ImportReproStack/ImportReproFunction/Resource, ImportReproFunctionCurrentVersionE3152290f48b63b389af562bcca81f410c827bd9, ImportReproStack/FunctionAlias/Resource (--force to override)

Reproduction Steps

Deploy this stack with cdk deploy
Run cdk import

The cdk template is located here

Possible Solution

Using cdk import --force works, but shouldn't be necessary.

Additional Information/Context

No response

CDK CLI Version

2.160.0 (build 7a8ae02)

Framework Version

2.160.0

Node.js Version

20.13.1

OS

MacOS Sonoma 14.6.1

Language

TypeScript

Language Version

No response

Other information

No response

@perpil perpil added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 6, 2024
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Oct 6, 2024
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 and removed needs-triage This issue or PR still needs to be triaged. labels Oct 7, 2024
@khushail
Copy link
Contributor

khushail commented Oct 7, 2024

Hi @perpil , thanks for reaching out.

The issue is reproducible. Even without using the alias, I am getting the error display-

Screenshot 2024-10-07 at 2 16 43 PM

Checking the code how cdk import is implemented , I see that in case of when no changes are detected, by default the error the thrown in this function which checks the diff between importableResources -

this ResourceImporter class -

export class ResourceImporter {

checks for the importable resrources and does the needful by invoking this function-

public async discoverImportableResources(allowNonAdditions = false): Promise<DiscoverImportableResourcesResult> {

  public async discoverImportableResources(allowNonAdditions = false): Promise<DiscoverImportableResourcesResult> {
    const currentTemplate = await this.currentTemplate();

    const diff = cfnDiff.fullDiff(currentTemplate, this.stack.template);

    // Ignore changes to CDKMetadata
    const resourceChanges = Object.entries(diff.resources.changes)
      .filter(([logicalId, _]) => logicalId !== 'CDKMetadata');

    // Split the changes into additions and non-additions. Imports only make sense
    // for newly-added resources.
    const nonAdditions = resourceChanges.filter(([_, dif]) => !dif.isAddition);
    const additions = resourceChanges.filter(([_, dif]) => dif.isAddition);

    if (nonAdditions.length) {
      const offendingResources = nonAdditions.map(([logId, _]) => this.describeResource(logId));

      if (allowNonAdditions) {
        warning(`Ignoring updated/deleted resources (--force): ${offendingResources.join(', ')}`);
      } else {
        throw new Error('No resource updates or deletes are allowed on import operation. Make sure to resolve pending changes ' +
          `to existing resources, before attempting an import. Updated/deleted resources: ${offendingResources.join(', ')} (--force to override)`);
      }
    }

so if there are changes which are to be ignored by setting--force, those will be ignored and warning message is displayed
otherwise we get the error message thrown.

Marking this as P2 as the workaround exists ( running command with --force) , which means it might not be immediately addressed by the team but would be on their radar.

@khushail khushail added effort/small Small work item – less than a day of effort package/tools Related to AWS CDK Tools or CLI and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 7, 2024
@jiayiwang7 jiayiwang7 added p1 and removed p2 labels Feb 7, 2025
mergify bot pushed a commit that referenced this issue Feb 10, 2025
…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*
@iankhou
Copy link
Contributor

iankhou commented Feb 17, 2025

Fixed in #33322

@iankhou iankhou closed this as completed Feb 17, 2025
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-lambda Related to AWS Lambda bug This issue is a bug. effort/small Small work item – less than a day of effort p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

4 participants