fix(cli): template created during import should be written to assets folder #29830

nburtsev · 2024-04-15T10:45:09Z

Issue # (if applicable)

I'm reopening as the original was closed due to inactivity.

Reason for this change

When the template is synthesized, if it is larger than 50kb and cannot be inlined in the CFN request, CLI writes it to the filesystem to be uploaded to S3. Unlike regular deployments import overrides the template, when it does so the code responsible for writing the large template to the filesystem writes to the project root.

This code reproduces the issue:

import * as cdk from "aws-cdk-lib";
import { Construct } from "constructs";
import * as sqs from "aws-cdk-lib/aws-sqs";
import * as iam from "aws-cdk-lib/aws-iam";

export class AwsCdkImportBugStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    for (let i = 0; i < 70; i++) {
      const q = new sqs.Queue(this, `AwsCdkImportBugQueue${i}`, {
        visibilityTimeout: cdk.Duration.seconds(300),
        enforceSSL: true,
      });
    }

    // new sqs.Queue(this, `AwsCdkImportBugQueue`, {
    //   visibilityTimeout: cdk.Duration.seconds(300),
    //   enforceSSL: true,
    //   removalPolicy: cdk.RemovalPolicy.RETAIN,
    // });
  }
}

Deploy the stack, uncomment the queue, and try to import. This error happens:

$ npx cdk import                                  
AwsCdkImportBugStack
AwsCdkImportBugStack/AwsCdkImportBugQueue/Resource (AWS::SQS::Queue): enter QueueUrl (empty to skip): https://sqs.eu-central-1.amazonaws.com/<cut>/AwsCdkBugStack-keep186A2ECA-IznVNwytuY1b
AwsCdkImportBugStack/AwsCdkImportBugQueue/Policy/Resource (AWS::SQS::QueuePolicy): enter Id (empty to skip): 
Skipping import of AwsCdkImportBugStack/AwsCdkImportBugQueue/Policy/Resource
AwsCdkImportBugStack: importing resources into stack...
[0%] start: Publishing 06a2c6415fd2982e3285e9d615e5f9b99d1d795a9064479fbe6b88455ac62f6c:current
[100%] fail: ENOENT: no such file or directory, open '/home/nburtsev/projects/aws-cdk-import-bug/cdk.out/AwsCdkImportBugStack.template.json-06a2c6415fd2982e3285e9d615e5f9b99d1d795a9064479fbe6b88455ac62f6c.yaml'

 ❌  AwsCdkImportBugStack failed: Error: Failed to publish one or more assets. See the error messages above for more information.
    at publishAssets (/home/nburtsev/projects/aws-cdk-import-bug/node_modules/aws-cdk/lib/index.js:420:84876)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async deployStack (/home/nburtsev/projects/aws-cdk-import-bug/node_modules/aws-cdk/lib/index.js:430:391)
    at async ResourceImporter.importResources (/home/nburtsev/projects/aws-cdk-import-bug/node_modules/aws-cdk/lib/index.js:433:171694)
    at async ResourceImporter.importResourcesFromMap (/home/nburtsev/projects/aws-cdk-import-bug/node_modules/aws-cdk/lib/index.js:433:171357)
    at async CdkToolkit.import (/home/nburtsev/projects/aws-cdk-import-bug/node_modules/aws-cdk/lib/index.js:433:205058)
    at async exec4 (/home/nburtsev/projects/aws-cdk-import-bug/node_modules/aws-cdk/lib/index.js:488:54378)

Failed to publish one or more assets. See the error messages above for more information.

The file is created, it's just in the project root not in cdk.out:

~/projects/aws-cdk-import-bug $ ls -la         
total 284
drwxr-xr-x   7 nburtsev users   4096 Mar 22 10:48 .
drwxr-xr-x   8 nburtsev users   4096 Mar 22 10:11 ..
-rw-r--r--   1 nburtsev users  64131 Mar 22 10:48 AwsCdkImportBugStack.template.json-06a2c6415fd2982e3285e9d615e5f9b99d1d795a9064479fbe6b88455ac62f6c.yaml
drwxr-xr-x   2 nburtsev users   4096 Mar 22 10:12 bin
-rw-r--r--   1 nburtsev users   3185 Mar 22 10:12 cdk.json
drwxr-xr-x   2 nburtsev users   4096 Mar 22 10:48 cdk.out
<cut>

Description of changes

I've just added a path.join similar to how it is done in regular template generation.

Description of how you validated changes

I've added a test case but I believe tests are broken globally as per @TheRealAmazonKendra

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

aws-cdk-automation · 2024-05-07T00:09:20Z

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

…plate-path

bergjaak · 2024-05-09T16:32:16Z

Thank you so much for this! I was able to get the integration test to run successfully, so I will test this change some more and push some of my changes to this branch :)

…plate-path

bergjaak · 2024-05-10T01:59:46Z

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

@@ -1,6 +1,7 @@
 import { promises as fs, existsSync } from 'fs';
 import * as os from 'os';
 import * as path from 'path';
+import * as CFN from '@aws-sdk/client-cloudformation';


I noticed that fixture has its own CFN client, so I should use that instead of this, probably.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

comcalvi · 2024-05-10T16:18:58Z

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

+    // Write a resource mapping file based on the ID from step one, then run an import
+    const mappingFile = path.join(fixture.integTestDir, 'outputs', `${randomPrefix}Mapping.json`);
+    const fullStackName = fixture.fullStackName('importable-stack');
+    const queueUrl = outputs[fullStackName].QueueUrl;
+    const queueLogicalId = outputs[fullStackName].QueueLogicalId;
+    queues[queueLogicalId] = { QueueUrl: queueUrl };


The way this comment is scoped makes it look like it's supposed to apply to the immediate block, which isn't the case. Can we move the comment outside the try so it's clear that it applies to the entire block?

It would also be helpful if we numbered the steps, like

Do something

Do something else

Pull request has been modified.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

aws-cdk-automation · 2024-05-10T18:05:03Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

mergify · 2024-05-10T18:13:28Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-05-10T18:42:26Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: f5cc882
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-05-10T18:42:49Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

nburtsev and others added 8 commits March 15, 2024 19:02

fix: import should write overrideTemplate to assembly.outdir

c332aff

Merge branch 'main' into main

8d73b83

Merge branch 'main' into fix/cdk-import-template-path

b91e06a

fix: update cli import test to ensure template tiss larger than 50kb

8b5e5cb

Merge branch 'main' into fix/cdk-import-template-path

16f3529

Merge branch 'main' into fix/cdk-import-template-path

b798c93

Merge branch 'main' into fix/cdk-import-template-path

3f3f730

Merge branch 'main' into fix/cdk-import-template-path

de6d0da

aws-cdk-automation requested a review from a team April 15, 2024 10:45

github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Apr 15, 2024

nburtsev requested a deployment to test-pipeline April 15, 2024 10:46 — with GitHub Actions Waiting

nburtsev changed the title ~~Fix/cdk import template path~~ fix(cli): template created during import should be written to assets folder Apr 15, 2024

aws-cdk-automation previously requested changes Apr 15, 2024

View reviewed changes

Merge branch 'main' into fix/cdk-import-template-path

de09c72

nburtsev requested a deployment to test-pipeline April 15, 2024 11:35 — with GitHub Actions Waiting

Merge branch 'main' into fix/cdk-import-template-path

72af4a2

nburtsev requested a deployment to test-pipeline April 16, 2024 15:16 — with GitHub Actions Waiting

github-actions bot mentioned this pull request Apr 22, 2024

Weekly PR metrics report #29922

Closed

github-actions bot mentioned this pull request May 1, 2024

Monthly PR metrics report #30026

Closed

bergjaak and others added 2 commits May 8, 2024 14:09

Merge branch 'main' of github.com:aws/aws-cdk into fix/cdk-import-tem…

8578e9f

…plate-path

Merge branch 'main' into fix/cdk-import-template-path

84a9409

bergjaak requested a deployment to test-pipeline May 8, 2024 18:23 — with GitHub Actions Waiting

bergjaak added 2 commits May 8, 2024 16:12

in progress

75a5dcb

made test actually work

c520676

Merge branch 'main' of github.com:aws/aws-cdk into fix/cdk-import-tem…

ca4607e

…plate-path

bergjaak added 2 commits May 9, 2024 21:29

Merge branch 'main' of github.com:aws/aws-cdk into fix/cdk-import-tem…

0261c0a

…plate-path

fix failing tests

bd0eb88

bergjaak temporarily deployed to test-pipeline May 10, 2024 01:59 — with GitHub Actions Inactive

bergjaak reviewed May 10, 2024

View reviewed changes

use cfn fixture

fa109b3

bergjaak temporarily deployed to test-pipeline May 10, 2024 02:24 — with GitHub Actions Inactive

fix test again

75918ac

bergjaak temporarily deployed to test-pipeline May 10, 2024 12:56 — with GitHub Actions Inactive

remove duplicate line

7e3b24e

bergjaak requested a deployment to test-pipeline May 10, 2024 15:26 — with GitHub Actions Waiting

bergjaak added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label May 10, 2024

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 10, 2024

comcalvi previously requested changes May 10, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 10, 2024

improve test comments

2cb9614

bergjaak temporarily deployed to test-pipeline May 10, 2024 17:14 — with GitHub Actions Inactive

comcalvi approved these changes May 10, 2024

View reviewed changes

comcalvi removed the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label May 10, 2024

aws-cdk-automation previously requested changes May 10, 2024

View reviewed changes

bergjaak added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label May 10, 2024

Merge branch 'main' into fix/cdk-import-template-path

f5cc882

mergify bot requested a deployment to test-pipeline May 10, 2024 18:14 Waiting

mergify bot merged commit a96cf55 into aws:main May 10, 2024
8 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): template created during import should be written to assets folder #29830

fix(cli): template created during import should be written to assets folder #29830

nburtsev commented Apr 15, 2024 •

edited

aws-cdk-automation left a comment

aws-cdk-automation commented May 7, 2024

bergjaak commented May 9, 2024

bergjaak May 10, 2024

comcalvi May 10, 2024

aws-cdk-automation left a comment

aws-cdk-automation commented May 10, 2024

mergify bot commented May 10, 2024

aws-cdk-automation commented May 10, 2024

mergify bot commented May 10, 2024

fix(cli): template created during import should be written to assets folder #29830

fix(cli): template created during import should be written to assets folder #29830

Conversation

nburtsev commented Apr 15, 2024 • edited

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented May 7, 2024

bergjaak commented May 9, 2024

bergjaak May 10, 2024

Choose a reason for hiding this comment

comcalvi May 10, 2024

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented May 10, 2024

mergify bot commented May 10, 2024

aws-cdk-automation commented May 10, 2024

AWS CodeBuild CI Report

mergify bot commented May 10, 2024

nburtsev commented Apr 15, 2024 •

edited