refactor(assets): API cleanups #2910
Conversation
BREAKING CHANGE: `AssetProps.packaging` has been removed and is now automatically discovered based on the file type. * **assets:** `ZipDirectoryAsset` has been removed, use `Asset`. * **assets:** `FileAsset` has been removed, use `Asset`. * **lambda:** `Code.directory` and `Code.file` have been removed. Use `Code.asset`.
There was a problem hiding this comment.
We're basically blocking all opportunities to extend Assets by bringing you own implementation... Conversely, this means Docker images can no longer be assets... I'm not sure this is a big deal or not, but somehow I'd really like those based off of the same mechanisms...
| export class ZipDirectoryAsset extends Asset { | ||
| constructor(scope: cdk.Construct, id: string, props: ZipDirectoryAssetProps) { | ||
| super(scope, id, { packaging: AssetPackaging.ZipDirectory, ...props }); | ||
| if (fs.statSync(assetPath).isDirectory()) { |
There was a problem hiding this comment.
Would like for this to feature & adhere to symlink control behavior...
There was a problem hiding this comment.
Not sure what you mean. stat follows symlinks, so if the path is a symlink to a directory or file, it should just work.
There was a problem hiding this comment.
What I mean is I might want to prohibit symlinking to /etc/passwd for example... Or taking files from /lib64 (which I have encountered before and can cause a mountain of problems).
Basically, I want to control Symlink traversal here... so that I can choose where files may come from. And if I say "no symlink out of app root", then if the asset lives out of there you should fail on the spot.
A good point about semantics... Docker images are still assets from the cloud assembly perspective, but you are right that calling this class |
|
I'm favorable to renaming here if we want to tighten up the semantics. We still will need the staging behavior to be shareable... Anything that copies directories to the staging area needs to have the same inclusion/exclusion/symlink behavior... |
- assets: core asset types - aws-s3-assets: file assets uploaded to s3 - aws-ecr-assets: docker image assets uploaded to ecr - assets-docker => deprecated
|
@RomainMuller @rix0rrr please take another look. I've reorganized the assets modules to:
|
|
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
Reorganize asset-related modules as follows:
BREAKING CHANGE:
AssetProps.packaginghas been removed and is now automatically discovered based on the file type.ZipDirectoryAssethas been removed, useaws-s3-assets.Asset.FileAssethas been removed, useaws-s3-assets.Asset.Code.directoryandCode.filehave been removed. UseCode.asset.Pull Request Checklist
designfolderBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.