(aws-lambda): fromDockerBuild output is located incorrectly

[phuctm97]

In v1.92.0, when fromDockerBuild is used, it is expected that contents /asset is copied, but instead the whole /asset is copied to output path, this makes Lambda handler reference index.handler failed, it has to be asset/index.handler.

Reproduction Steps

Create a index.js.

exports.handler = () => {
  console.info("Test");
}

A Dockerfile:

FROM amazon/aws-sam-cli-build-image-nodejs14.x
COPY index.js /asset/

Use lambda.Code.fromDockerBuild:

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

    new lambda.Function(this, "ExampleFunction", {
      runtime: lambda.Runtime.NODEJS_14_X,
      handler: "index.handler",
      code: lambda.Code.fromDockerBuild(ROOT_DIR),
    });
  }
}

What did you expect to happen?

The Lamda code contains only index.js.

What actually happened?

The Lambda code contains asset/index.js.

Environment

• CDK CLI Version : 1.92.0
• Framework Version: 1.92.0
• Node.js Version: 14
• OS : macOS Catalina
• Language (Version): TypeScript 4.2.3

Other

This is caused by expected behavior of docker cp.

Suggestion: automatically append /. to imagePath if it doesn't.

---

This is 🐛 Bug Report

[nija-at]

@jogold - can you take a look at this?

[jogold]

@phuctm97 are you taking this? I see a commit on your fork that references this issue.

I was thinking that something like this would do it:

public static fromDockerBuild(path: string, options: DockerBuildAssetOptions = {}): AssetCode {
  const imagePath = options.imagePath ?? '/asset';
  const assetPath = cdk.DockerImage
    .fromBuild(path, options)
    // ensure imagePath ends with /. to copy the **content** at this path
    .cp(imagePath.replace(/(\/|\/\.)?$/, '/.'), options.outputPath);
  return new AssetCode(assetPath);
}

I would personally not change the cp() method in core, I think it must remain general purpose.

(cc @eladb)

[bigkraig]

@jogold i tested similar logic by changing my imagePath to /asset/. and got the right behavior

[phuctm97]

@phuctm97 are you taking this? I see a commit on your fork that references this issue.

I was thinking that something like this would do it:

public static fromDockerBuild(path: string, options: DockerBuildAssetOptions = {}): AssetCode {
  const imagePath = options.imagePath ?? '/asset';
  const assetPath = cdk.DockerImage
    .fromBuild(path, options)
    // ensure imagePath ends with /. to copy the **content** at this path
    .cp(imagePath.replace(/(\/|\/\.)?$/, '/.'), options.outputPath);
  return new AssetCode(assetPath);
}

I would personally not change the cp() method in core, I think it must remain general purpose.

(cc @eladb)

Awesome, this is a great idea!

By the way, I wanted to make a PR for this, but yarn build doesn't seem to work on Gitpod because it lacks Docker access, if the contributing guide has instruction for that, it would be very helpful!

[phuctm97]

@jogold i tested similar logic by changing my imagePath to /asset/. and got the right behavior

If you use /asset/., it will work with the current version too. The expected behavior is either /asset, /asset/, or /asset/. should work similarly.

[jogold]

@phuctm97 are you taking this? I see a commit on your fork that references this issue.
I was thinking that something like this would do it:

public static fromDockerBuild(path: string, options: DockerBuildAssetOptions = {}): AssetCode {
  const imagePath = options.imagePath ?? '/asset';
  const assetPath = cdk.DockerImage
    .fromBuild(path, options)
    // ensure imagePath ends with /. to copy the **content** at this path
    .cp(imagePath.replace(/(\/|\/\.)?$/, '/.'), options.outputPath);
  return new AssetCode(assetPath);
}

I would personally not change the cp() method in core, I think it must remain general purpose.
(cc @eladb)

Awesome, this is a great idea!

By the way, I wanted to make a PR for this, but yarn build doesn't seem to work on Gitpod because it lacks Docker access, if the contributing guide has instruction for that, it would be very helpful!

@phuctm97 Gitpod now supports Docker (https://www.gitpod.io/docs/feature-preview/#root-access and gitpod-io/gitpod#52 (comment)) but it doesn't work with the docker image/config of the repo https://github.com/aws/aws-cdk/blob/master/.gitpod.yml

[github-actions]

⚠️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.