-
Notifications
You must be signed in to change notification settings - Fork 4.3k
fix(core): platform
is ignored during asset bundling
#33865
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
Conversation
…BindMount and AssetBundlingVolumeCopy
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.
(This review is outdated)
|
Added exemption from integration test because indeed this change does not cause a snapshot diff. Modifications to existing tests are sufficient. |
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
@ejschaefer I'm happy to merge this but can you please explain how this change fixes #33522? Seems unrelated because |
This PR has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled. |
commenting to keep PR open @iliapolo - I believe the DockerImageCode.fromImageAsset code path uses the same AssetBundling path to build locally before pushing to ECR. I will confirm shorly |
Also came across this, where platform is not passed to the cli command. |
@iliapolo - finally had a free moment to recheck #33522. You're correct, that issue is unrelated to this change. I updated the PR description to remove the reference. |
platform
is ignored during asset bundling
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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #30239
Reason for this change
The
BundlingOptions
platform
property is not being used inAssetBundlingBindMount
orAssetBundlingVolumeCopy
and is not passed as a command line option to Docker.Fix makes the platform property work as described in
BundlingOptions
documentation: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.BundlingOptions.html#platform.Description of changes
added
platform: this.options.platform
toAssetBundlingBindMount.run
andAssetBundlingVolumeCopy.run
inpackages/aws-cdk-lib/core/lib/asset-staging.ts
Describe any new or updated permissions being added
N/A
Description of how you validated changes
Added platform property to unit and integration tests
packages/aws-cdk-lib/core/test/private/asset-staging.test.ts
packages/@aws-cdk-testing/framework-integ/test/aws-s3-assets/test/integ.assets.bundling.docker-opts.ts
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license