fix(s3-assets): cannot publish a file without extension #30597

sakurai-ryo · 2024-06-20T12:43:05Z

Issue # (if applicable)

Reason for this change

Publishing a file with no extension using the Asset class with BundlingOutput.SINGLE_FILE and AssetHashType.SOURCE(default), as shown below, will result in an error fail: EISDIR: illegal operation on a directory, read, and publishing will fail.

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

      const asset = new s3assets.Asset(this, 'asset', {
          path: path.join(__dirname, '../assets/main'),
          bundling: {
              image: DockerImage.fromRegistry('golang:1.21'),
              entrypoint: ["bash", "-c"],
              command: ["echo 123 > /asset-output/main"], // a file without extension
              outputType: BundlingOutput.SINGLE_FILE,
          },
      });
      new CfnOutput(this, 'AssetHash', { value: asset.assetHash });
  }
}

This is because the path in *.asset.json is different from the actual file path.
The *.asset.json expects the file to be in asset.bead5b2c0d128650228f146d2326d5f3cbfb36738a9383fc6a09b1e9278803f0, but when I check the cdk.out directory, I see that asset.bead5b2c0d128650228f146d2326d5f3cbfb36738a9383fc6a09b1e9278803f0 is a directory, not a file.

{
  "version": "36.0.0",
  "files": {
    "bead5b2c0d128650228f146d2326d5f3cbfb36738a9383fc6a09b1e9278803f0": {
      "source": {
        "path": "asset.bead5b2c0d128650228f146d2326d5f3cbfb36738a9383fc6a09b1e9278803f0",
        "packaging": "file"
      },
      "destinations": {
        "current_account-us-east-1": {
          "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-us-east-1",
          "objectKey": "bead5b2c0d128650228f146d2326d5f3cbfb36738a9383fc6a09b1e9278803f0.bead5b2c0d128650228f146d2326d5f3cbfb36738a9383fc6a09b1e9278803f0",
          "region": "us-east-1",
          "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-us-east-1"
        }
      }
    }
}

cdk.out
├── SampleStack.assets.json
├── SampleStack.template.json
├── asset.bead5b2c0d128650228f146d2326d5f3cbfb36738a9383fc6a09b1e9278803f0
│   └── main
├── cdk.out
├── manifest.json
└── tree.json

If I change it to a file with an extension, as shown below, I see that the file with the extension is staged under cdk.out dir, and the asset is published successfully.

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

      const asset = new s3assets.Asset(this, 'asset', {
          path: path.join(__dirname, '../assets/main'),
          bundling: {
              image: DockerImage.fromRegistry('golang:1.21'),
              entrypoint: ["bash", "-c"],
              command: ["echo 123 > /asset-output/main.bin"], // a file with an extension
              outputType: BundlingOutput.SINGLE_FILE,
          },
      });
      new CfnOutput(this, 'AssetHash', { value: asset.assetHash });
  }
}

cdk.out
├── SampleStack.assets.json
├── SampleStack.template.json
├── asset.dc5ce447844d7490834e46df016edc7f671b4fae19ab55b6c78973dcb5af98f8
│   └── main.bin
├── asset.dc5ce447844d7490834e46df016edc7f671b4fae19ab55b6c78973dcb5af98f8.bin # !! staged file here !!
├── cdk.out
├── manifest.json
└── tree.json

{
  "version": "36.0.0",
  "files": {
    "dc5ce447844d7490834e46df016edc7f671b4fae19ab55b6c78973dcb5af98f8": {
      "source": {
        "path": "asset.dc5ce447844d7490834e46df016edc7f671b4fae19ab55b6c78973dcb5af98f8.bin",
        "packaging": "file"
      },
      "destinations": {
        "current_account-us-east-1": {
          "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-us-east-1",
          "objectKey": "dc5ce447844d7490834e46df016edc7f671b4fae19ab55b6c78973dcb5af98f8.bin",
          "region": "us-east-1",
          "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-us-east-1"
        }
      }
    }
}

Files without extensions must be staged correctly in order to be published correctly.

Description of changes

The directory to write the bundling output is usually cdk.out/asset.{asset hash}.
If the extension exists, it will be renamed from cdk.out/asset.{asset hash}/{asset file name} to cdk.out/{asset hash}.{asset file extension}.

aws-cdk/packages/aws-cdk-lib/core/lib/asset-staging.ts

Line 392 in c826d8f

fs.renameSync(sourcePath, targetPath);

If the extension does not exist, the file name cdk.out/asset.{asset hash} (without extension) will be the same as the directory where bundling output is written.
Therefore, the file is already considered staged and will not be staged correctly.

aws-cdk/packages/aws-cdk-lib/core/lib/asset-staging.ts

Line 383 in c826d8f

if (isAlreadyStaged) {

Therefore, in such cases, I fix to change the file name by adding a suffix such as noext after the file name so that the file is correctly renamed.

Description of how you validated changes

unit tests and integ test.

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

sakurai-ryo · 2024-07-30T11:40:07Z

@Mergifyio update

mergify · 2024-07-30T11:40:21Z

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/github-merit-badger.yml without workflows permission

rix0rrr

Thanks for the deep dive!

I wonder if we can simplify this a bit, but apart from that looks good!

rix0rrr · 2024-09-04T12:01:21Z

packages/aws-cdk-lib/core/lib/asset-staging.ts

+  private renderStagedPath(sourcePath: string, targetPath: string): string {
+    // Add a suffix to the asset file name
+    // because when a file without extension is specified, the source directory name is the same as the staged asset file name.
+    if (this.hashType === AssetHashType.SOURCE && path.dirname(sourcePath) === targetPath) {


Does this even on the hashType? It seems that if dirname(sourcePath) === targetPath we always have a problem?

Changed the conditional to this.hashType !== AssetHashType.OUTPUT and added the reason to the comment.
9f5adb5

rix0rrr · 2024-09-04T12:01:56Z

packages/aws-cdk-lib/core/lib/asset-staging.ts

+    // Add a suffix to the asset file name
+    // because when a file without extension is specified, the source directory name is the same as the staged asset file name.
+    if (this.hashType === AssetHashType.SOURCE && path.dirname(sourcePath) === targetPath) {
+      targetPath = targetPath + 'noext';


Suggested change

targetPath = targetPath + 'noext';

targetPath = targetPath + '_noext';

Changed suffix to _noext.
ecee169

rix0rrr · 2024-09-04T12:03:20Z

packages/aws-cdk-lib/core/lib/stack-synthesizers/asset-manifest-builder.ts

+    // If the fileName begins with 'asset.', remove it here
+    // because when a file without extension is added to the manifest, the path.extension() will return an file hash.
+    // ex) path.extension('asset.dc5ce447844') => 'dc5ce447844'


This comment seems to not match what the code is doing? Or does it?

Deleted comment.
I forgot to delete it.
69889ec

…x-s3-assets-no-extension

sakurai-ryo · 2024-10-04T15:41:58Z

@rix0rrr
Thanks for your review, and sorry for the late response.
I've addressed all of your comments.

rix0rrr

<3

mergify · 2024-10-25T13:54:13Z

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).

mergify · 2024-10-25T13:58:02Z

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).

Pull request has been modified.

mergify · 2024-10-29T10:34:23Z

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-10-29T11:05:41Z

AWS CodeBuild CI Report

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

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

mergify · 2024-10-29T11:06:09Z

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).

github-actions · 2024-10-29T11:06:23Z

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.

fix: upload assets with no extension

9449634

aws-cdk-automation requested a review from a team June 20, 2024 12:43

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p1 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels Jun 20, 2024

delete func

98a0371

sakurai-ryo changed the title ~~fix(s3-assets): cannot publish files without extension~~ fix(s3-assets): cannot publish a file without extension Jun 20, 2024

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

This was referenced Jun 24, 2024

Weekly PR metrics report #30637

Closed

Monthly PR metrics report #30715

Closed

rix0rrr reviewed Sep 4, 2024

View reviewed changes

rix0rrr self-assigned this Sep 4, 2024

iliapolo added @aws-cdk/core Related to core CDK functionality @aws-cdk/aws-s3-assets labels Oct 1, 2024

sakurai-ryo added 4 commits October 4, 2024 21:56

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into fi…

5c443ad

…x-s3-assets-no-extension

remove comment

69889ec

change suffix

ecee169

update renderStagedPath

9f5adb5

rix0rrr previously approved these changes Oct 25, 2024

View reviewed changes

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

Merge branch 'main' into fix-s3-assets-no-extension

ca501e7

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

rix0rrr approved these changes Oct 29, 2024

View reviewed changes

Merge branch 'main' into fix-s3-assets-no-extension

12bb8a2

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

mergify bot merged commit ccab485 into aws:main Oct 29, 2024
15 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(s3-assets): cannot publish a file without extension #30597

fix(s3-assets): cannot publish a file without extension #30597

sakurai-ryo commented Jun 20, 2024 •

edited

Loading

sakurai-ryo commented Jul 30, 2024

mergify bot commented Jul 30, 2024

rix0rrr left a comment

rix0rrr Sep 4, 2024

sakurai-ryo Oct 4, 2024

rix0rrr Sep 4, 2024

sakurai-ryo Oct 4, 2024

rix0rrr Sep 4, 2024

sakurai-ryo Oct 4, 2024

sakurai-ryo commented Oct 4, 2024

rix0rrr left a comment

mergify bot commented Oct 25, 2024

mergify bot commented Oct 25, 2024

mergify bot commented Oct 29, 2024

aws-cdk-automation commented Oct 29, 2024

mergify bot commented Oct 29, 2024

github-actions bot commented Oct 29, 2024

	targetPath = targetPath + 'noext';
	targetPath = targetPath + '_noext';

fix(s3-assets): cannot publish a file without extension #30597

fix(s3-assets): cannot publish a file without extension #30597

Conversation

sakurai-ryo commented Jun 20, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

sakurai-ryo commented Jul 30, 2024

mergify bot commented Jul 30, 2024

❌ Mergify doesn't have permission to update

rix0rrr left a comment

Choose a reason for hiding this comment

rix0rrr Sep 4, 2024

Choose a reason for hiding this comment

sakurai-ryo Oct 4, 2024

Choose a reason for hiding this comment

rix0rrr Sep 4, 2024

Choose a reason for hiding this comment

sakurai-ryo Oct 4, 2024

Choose a reason for hiding this comment

rix0rrr Sep 4, 2024

Choose a reason for hiding this comment

sakurai-ryo Oct 4, 2024

Choose a reason for hiding this comment

sakurai-ryo commented Oct 4, 2024

rix0rrr left a comment

Choose a reason for hiding this comment

mergify bot commented Oct 25, 2024

mergify bot commented Oct 25, 2024

mergify bot commented Oct 29, 2024

aws-cdk-automation commented Oct 29, 2024

AWS CodeBuild CI Report

mergify bot commented Oct 29, 2024

github-actions bot commented Oct 29, 2024

sakurai-ryo commented Jun 20, 2024 •

edited

Loading