Skip to content
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.

Sign up for GitHub

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

Jump to bottom

(core): newStyleStackSynthesis=true causes files upload to s3 without file extension #12740

Closed
kokachev opened this issue Jan 28, 2021 · 2 comments · Fixed by #12765
Closed

(core): newStyleStackSynthesis=true causes files upload to s3 without file extension #12740

kokachev opened this issue Jan 28, 2021 · 2 comments · Fixed by #12765
Assignees
@rix0rrr @iliapolo
Labels
@aws-cdk/aws-s3-assets @aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@kokachev
Copy link
Contributor

New style synthesis uploads files to s3 without file extension. This causes AWS Glue script compilation failures and makes it impossible to use Glue with custom JARs and newStyleStackSynthesis: true.

Reproduction Steps

  1. Set "@aws-cdk/core:newStyleStackSynthesis": "true"
  2. Create s3.assets.Asset from local jar file
  3. Deploy stack

What did you expect to happen?

Uploaded file should have "jar" extension

What actually happened?

Filename does not have an extension

Environment

  • CDK CLI Version : 1.86.0
  • Framework Version: 1.86.0
  • Node.js Version: v15.7.0
  • OS :Mac Os
  • Language (Version): Java 8

Other

We use CDK to upload custom JAR files for AWS Glue jobs. Glue Java compiler requires custom jars to have "jar" extension, otherwise script compilation fails. Because, of this it's impossible for us to use CDK pipelines.

When core:newStyleStackSynthesis is set to false, files are uploaded with extensions and everything is working as expected.

This is 🐛 Bug Report
@kokachev kokachev added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 28, 2021
@kokachev kokachev changed the title (s3-asset): newStyleStackSynthesis upload files to s3 without file extension (s3-asset): newStyleStackSynthesis=true causes files upload to s3 without file extension Jan 28, 2021
@NGL321 NGL321 changed the title (s3-asset): newStyleStackSynthesis=true causes files upload to s3 without file extension (aws-s3-assets) newStyleStackSynthesis=true causes files upload to s3 without file extension Jan 28, 2021
kokachev added a commit to kokachev/aws-cdk that referenced this issue Jan 29, 2021
@kokachev
fix(core): append file extension to s3 asset key in new style synthes…
71f5c5a 
…izer

- This change fixes the issue with new style synthesizer, when s3 files assets are uploaded without file extension. This was breaking integration with some services, in particular Glue, which requires jar files to have "jar" extension in order to compile.
- Legacy synthesizer was uploading files with extensions, so this changes effectively aligns old and new synthesizers.
fixes aws#12740
@kokachev kokachev mentioned this issue Jan 29, 2021
Merged
@iliapolo iliapolo changed the title (aws-s3-assets) newStyleStackSynthesis=true causes files upload to s3 without file extension (core): newStyleStackSynthesis=true causes files upload to s3 without file extension Feb 2, 2021
@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Feb 2, 2021
@iliapolo iliapolo added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 2, 2021
@iliapolo
Copy link
Contributor

iliapolo commented Feb 2, 2021

@rix0rrr Can you take a look? Could this have been intentional?

@mergify mergify bot closed this as completed in #12765 Feb 4, 2021
mergify bot pushed a commit that referenced this issue Feb 4, 2021
@kokachev
fix(core): append file extension to s3 asset key in new style synthes…
77b9d39 
…izer (#12765)

- This change fixes the issue with new style synthesizer, when s3 files assets are uploaded without file extension. This was breaking integration with some services, in particular Glue, which requires jar files to have "jar" extension in order to compile.
- Legacy synthesizer was creating s3 key with extension, so this change effectively aligns old and new synthesizers.

fixes #12740


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Feb 4, 2021

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

NovakGu pushed a commit to NovakGu/aws-cdk that referenced this issue Feb 18, 2021
@kokachev @NovakGu
fix(core): append file extension to s3 asset key in new style synthes…
f2b2aa2 
…izer (aws#12765)

- This change fixes the issue with new style synthesizer, when s3 files assets are uploaded without file extension. This was breaking integration with some services, in particular Glue, which requires jar files to have "jar" extension in order to compile.
- Legacy synthesizer was creating s3 key with extension, so this change effectively aligns old and new synthesizers.

fixes aws#12740


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rix0rrr rix0rrr

@iliapolo iliapolo

Labels
@aws-cdk/aws-s3-assets @aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(core): append file extension to s3 asset key in new style synthesizer kokachev/aws-cdk
3 participants
@rix0rrr @iliapolo @kokachev