pipeline-aws-plugin icon indicating copy to clipboard operation
pipeline-aws-plugin copied to clipboard

[metadata] Unable to set content-disposition metadata

Open thegecko opened this issue 5 years ago • 13 comments

Description

In S3, it's possible to specify the filename which appears to the user when they download a file from a bucket.

This is done using the content-disposition metadata, e.g.:

Content-Disposition: attachment; filename=test.exe

I assumed this would be possible when setting the metadata during an s3Upload command, however the metadata ends up being prepended with x-amz-meta-:

x-amz-meta-Content-Disposition: attachment; filename=test.exe

Steps to Reproduce

Use these commands:

String metadata1 = "Content-Disposition: attachment; filename=test.exe"
s3Upload(bucket: "${BUCKET}", path: "${PATH}", file: "${FILE}", metadatas: [metadata1])

Expected behavior: [What you expected to happen] metadata set to Content-Disposition: attachment; filename=test.exe

Actual behavior: [What actually happened] metadata set to x-amz-meta-Content-Disposition: attachment; filename=test.exe

Environment

Plugin-Version: 1.36 Master/Slave Setup: Yes

Additional notes

This seems to be due to the fact that the Java SDK adds x-amz-meta- to any user-defined metadata as outlined here:

https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html

The SDK does allow setting the content-disposition:

https://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/model/ObjectMetadata.html#setContentDisposition-java.lang.String-

However, this isn't exposed in this plugin:

https://github.com/jenkinsci/pipeline-aws-plugin/blob/master/src/main/java/de/taimos/pipeline/aws/S3UploadStep.java#L378

IMO, the way to fix this would be to support the missing metadata set functions:

  • setContentDisposition(String disposition)
  • setContentLanguage(String contentLanguage)
  • setContentLength(long contentLength)
  • setContentMD5(String md5Base64)
  • setHttpExpiresDate(Date httpExpiresDate)
  • setRequesterCharged(boolean isRequesterCharged)

thegecko avatar Apr 09 '19 10:04 thegecko

Relates to #40

thegecko avatar Apr 09 '19 10:04 thegecko

Sounds like a good idea. Could you submit a pull request for this feature?

hoegertn avatar Apr 17 '19 08:04 hoegertn

Unfortunately I'm not a Java developer with a setup to modify and build any changes for this nor the time to invest in setting this up and learning the systems.

I'm happy to take a stab at adding this missing functionality, but it would be entirely untested.

thegecko avatar Apr 24 '19 09:04 thegecko

@hoegertn I have made changes to support this metadata (content-disposition) but I am unable to push my changes, it gives me permission denied error. Can you provide me with the required permissions? Thanks.

fatal: unable to access 'https://github.com/jenkinsci/pipeline-aws-plugin.git/': The requested URL returned error: 403 github account: [email protected]

nehagupta29 avatar Jul 28 '20 11:07 nehagupta29

Feel free to create a PullRequest with your changes: https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork

hoegertn avatar Jul 28 '20 11:07 hoegertn

Thanks @hoegertn. I made the changes and also added required test file but when I test on jenkins with this script, I see the below error repeatedly. The build is all successful though and I don't see any other errors. Can you please help here?

withAWS(region: 'us-west-2') { s3Upload acl: 'PublicRead', bucket: 'my-bucket', file: '/file_name.pdf', path: 'docs/file_name.pdf', contentType: 'application/pdf', contentDisposition:'attachment' }

Buckets and paths are all correct.

Started by user anonymous [Pipeline] withAWS Setting AWS region us-west-2 [Pipeline] { [Pipeline] s3Upload Required context class hudson.FilePath is missing Perhaps you forgot to surround the code with a step that provides this, such as: node [Pipeline] } [Pipeline] // withAWS [Pipeline] End of Pipeline org.jenkinsci.plugins.workflow.steps.MissingContextVariableException: Required context class hudson.FilePath is missing at org.jenkinsci.plugins.workflow.steps.StepDescriptor.checkContextAvailability(StepDescriptor.java:260) at org.jenkinsci.plugins.workflow.cps.DSL.invokeStep(DSL.java:206) at org.jenkinsci.plugins.workflow.cps.DSL.invokeMethod(DSL.java:153) at org.jenkinsci.plugins.workflow.cps.CpsScript.invokeMethod(CpsScript.java:108) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498)

nehagupta29 avatar Jul 29 '20 05:07 nehagupta29

@hoegertn Below is the PR I have raised as a fix for this issue. Can you please share by when I can get a review on this and get it merged? Thanks. https://github.com/jenkinsci/pipeline-aws-plugin/pull/241

nehagupta29 avatar Jul 30 '20 05:07 nehagupta29

@hoegertn Thanks for merging. Can you please tell me by when you will be able to release a version with this change as I don't see these changes available? Last release version I see is March 19. Thanks. https://github.com/jenkinsci/pipeline-aws-plugin/releases

nehagupta29 avatar Jul 31 '20 06:07 nehagupta29

I will cut a release this weekend

hoegertn avatar Jul 31 '20 06:07 hoegertn

Thanks for confirming @hoegertn

nehagupta29 avatar Jul 31 '20 06:07 nehagupta29

@hoegertn did you get a chance to cut the release?

nehagupta29 avatar Aug 03 '20 18:08 nehagupta29

I just cut the release. Sorry for the delay.

hoegertn avatar Aug 03 '20 22:08 hoegertn

Thanks @hoegertn

nehagupta29 avatar Aug 04 '20 02:08 nehagupta29