-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
feat(core,s3-assets,lambda): custom asset bundling #7898
Conversation
Add `Code.fromDockerImage` and `Code.fromDockerAsset` that offer a generic way of working with Docker for Lambda code. `Code.fromDockerImage`: run a command in an existing Docker image `Code.fromDockerAsset`: build an image and then run a command in it Both `Code` classes take an `assetPath` prop that corresponds to the path of the asset directory that will contain the build output of the Docker container. This path is automatically mounted at `/asset` in the container. Using a combination of image and command, the container is then responsible for putting content at this location. Additional volumes can be mounted if needed. This will allow to refactor `aws-lambda-nodejs` and create other language specific modules.
@eladb see integ test |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These APIs implies that the code comes from a docker image ("code from docker image" and "code from docker asset"), which could be a pretty interesting feature but this is not what this is doing.
My mental model for this is basically this is a new option we add to the normal lambda.Code.fromAsset
:
code: lambda.Code.fromAsset('/path/to/source', {
bundle: {
image: lambda.BundleImage.fromBuild('/directory', { buildOpts: ... }),
// or
image: lambda.BundleImage.fromImage('python'),
command: [ 'foo', 'bar' ],
volumes: // ...
environment: // ...
}
});
Here's how the integ test would look like:
new lambda.Function(this, 'Function', {
code: lambda.Code.fromAsset(path.join(__dirname, 'python-lambda-handler'), {
bundle: {
image: lambda.BundleImage.fromImage('python:3.6'),
command: [ 'pip', 'install', '-r ./requirements.txt', '-t .' ],
}
})
});
@eladb addressed all your feedback, are you sure about the naming here? is |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like where this is going. @nija-at please review as well...
@jogold I think we should think about this as "custom code bundling". The docker stuff is just the mechanism (update PR title & description accordingly).
Also, I am bothered by the fact that the input and output are the same directory. Feels to me that they should be seprated to /src
and /dist
(or /bundle
) or /output
or /outdir
or something like that. If the bundling logic wants to copy all the sources they can do it but a general rule, I am not sure it makes total sense to bundle "in-place". What do you think?
* | ||
* @param image the image name | ||
*/ | ||
public static fromImage(image: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am thinking that maybe we should model this like ecs.ContainerImage
fromImage
=>fromRegistry
fromBuild
=>fromAsset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the question will be: do we want to also add fromEcrRepository
, but we can add that later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should model this like ecs.ContainerImage
done
do we want to also add
fromEcrRepository
for another PR I would say
const hashType = props.assetHash ? AssetHashType.CUSTOM : props.assetHashType ?? AssetHashType.SOURCE; | ||
this.assetHash = this.calculateHash(hashType, props.assetHash); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically you are missing a validation here (assetHash
is defined but assetHashType
is not CUSTOM).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's on L73-L75 above, will move it closer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just move the validation into calculateHash
and pass in props
.
} | ||
|
||
private calculateHash(hashType: AssetHashType, assetHash?: string): string { | ||
if (hashType === AssetHashType.SOURCE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use switch
?
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
@jogold Thanks for this - been waiting for it for a while and had some custom tooling to do exactly what this is doing. Quick note - I have tested it, the output files from the container is owned by root. bundling_options = core.BundlingOptions(image=aws_lambda.Runtime.PYTHON_3_7.bundling_docker_image,
command=[
'bash', '-c',
'pip install -r requirements.txt -t /asset-output && rsync -r . /asset-output',
])
self.source_code = aws_lambda.Code.from_asset('src/test', bundling=bundling_options)
handler = aws_lambda.Function(self, f"{self.id}-TestBundling",
function_name=f"{self.id}-TestBundling",
handler=f'handlers.test',
runtime=aws_lambda.Runtime.PYTHON_3_7,
code=self.source_code) Anything I am doing wrong or workaround for this? If not will create an issue for it. EDIT I can work around this in linux by passing in |
I think we should be able to pass |
New issue created - #8489 |
Works great; now to figure out what to do when |
You will need your ci/cd environment to support “docker in docker”. |
There are some other tricks.. When not using your own Dockerfile to create the bundle it relies in the But docker-in-docker means that the new container will look for the path in the host filesystem. It's possible to pass a new custom volume in the I made it work by:
|
Given that we have no way of controlling how aws-cdk/packages/cdk-assets/lib/private/archive.ts Lines 8 to 16 in dac9bb3
Is there a way to provide the output as a zipfile instead of a directory? ie, do the zipping in the docker build to My use case: I currently can't see any way to get CDK to zip up a directory that contains symlinks correctly (see #9251 ). If I could have full control over the zipping in a docker build, then that would at least be a workaround for this issue (and anyone else who wanted to control the zipping themselves). Currently the only way seems to be (please correct me) to do the zipping externally to the CDK process, and then specify the zipfile as the asset. |
For anyone else wanting to do this, I created my own construct (based largely on Usage: const layer = new lambda.LayerVersion(this, "GitLayer", {
code: new BundledZipCode({
bundling: {
image: cdk.BundlingDockerImage.fromRegistry("lambci/yumda:2"),
user: "root",
command: [
"bash",
"-c",
`yum install -y git &&
cd /lambda/opt &&
find . -exec touch -h -t 198001010000 {} + &&
zip -qXyr /asset-output/bundle.zip .`,
],
},
}),
}); (the And here's the construct: class BundledZipCode extends lambda.AssetCode {
constructor(options) {
const { bundling, ...assetCodeOptions } = options;
const stagingTmp = path.join(".", ".cdk.staging");
if (!fs.existsSync(stagingTmp)) {
fs.mkdirSync(stagingTmp);
}
const bundleDir = path.resolve(fs.mkdtempSync(path.join(stagingTmp, "asset-bundle-")));
fs.chmodSync(bundleDir, 0o777);
let user = bundling.user;
if (!user) {
const userInfo = os.userInfo();
user = userInfo.uid !== -1 ? `${userInfo.uid}:${userInfo.gid}` : "1000:1000";
}
const volumes = [
{
hostPath: bundleDir,
containerPath: cdk.AssetStaging.BUNDLING_OUTPUT_DIR,
},
...(bundling.volumes || []),
];
bundling.image._run({
command: bundling.command,
user,
volumes,
environment: bundling.environment,
});
super(path.join(bundleDir, "bundle.zip"), assetCodeOptions);
}
} |
Adds support for asset bundling by running a command inside a Docker container.
The asset path is mounted in the container at
/asset-input
and is set as the workingdirectory. The container is responsible for putting content at
/asset-output
. The contentat
/asset-output
will be zipped and used as the final asset.This allows to use Docker for Lambda code bundling.
It will also be possible to refactor
aws-lambda-nodejs
and create other languagespecific modules.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license