You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2020/01/28 16:18:19 UTC

[GitHub] [hadoop] mukund-thakur opened a new pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

mukund-thakur opened a new pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823
 
 
   Debugging details:
   When a put operation is performed without -d parameter is used, it happens in two steps. First is copying to a temporary file and then copying the temp file to the right location. The is issue is durning the copy step as an extra header "x-amz-server-side-encryption: aws:kms" is set. 
   When -d parameter is used, the file is uploaded directly and in that api call this kms header is not set. 
   
   Solution: Removing the kms header from the ClonedObjectMetatData.
   
   Testing: Tested by compiling the complete hadoop distribution and invoking the hadoop shell command locally. 
   Bucket used : https://mthakur-data.s3.ap-south-1.amazonaws.com/file2
   
   Please suggest how to write testcases for this. Thanks

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-579379966
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  35m 58s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | -1 :x: |  test4tests  |   0m  0s |  The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  20m 18s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 26s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 37s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 35s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 59s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 19s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m 29s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m 15s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 31s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  94m 29s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 6d792b0d042a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 0825153 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/1/testReport/ |
   | Max. process+thread count | 453 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/1/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-587567591
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m  0s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m  3s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 59s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 28s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 8 new + 10 unchanged - 3 fixed = 18 total (was 13)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 57s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 24s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate ASF License warnings.  |
   |  |   |  58m 16s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 077010daea5d 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / a562942 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/testReport/ |
   | Max. process+thread count | 440 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r377842035
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractTestS3AEncryption.java
 ##########
 @@ -107,10 +106,15 @@ public void testEncryptionOverRename() throws Throwable {
     validateEncrytionSecrets(secrets);
     writeDataset(fs, src, data, data.length, 1024 * 1024, true);
     ContractTestUtils.verifyFileContents(fs, src, data);
-    Path dest = path(src.getName() + "-copy");
-    fs.rename(src, dest);
-    ContractTestUtils.verifyFileContents(fs, dest, data);
-    assertEncrypted(dest);
+    Path targetDir = path("target");
 
 Review comment:
   bq. The reason dest file has to be created is to enforce rename to consider targetDir as a directory else it considers it as file.
   
   mkdir(targetDir) should have done that. Or is it not because of that funny "rename into empty dir" problem with rename() which everyone hates (historical mistake, BTW)
   
   If somehow that doesn't work and you want to create a file, ContractTestUtils.touch() will do that; add a comment above about why its needed
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582575991
 
 
   @davidarcher -so if we add an extra header, that will do it? OK.
   
   That's an interesting thought. It means that on rename() whatever key an object had is preserved, but I don't see any problems with that...AFAIK people don't deliberately rename files to change the encryption options

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-581025186
 
 
   >  My solution is something like -
   > Just override the test method in the derived test class and put @ignore annotation.
   
   yes, do that. it'll stop the test runs doing needless work
   
   one thing to consider (actually, we may want to do it in the base), is verify that on directory rename the keys propagate properly too. I know, it's "obvious" it will, but with object stores, we have to not retain our filesystem preconceptions.  
   
   Maybe: in testEncryptionOverRename , rename the file into a directory. Then followup with a rename of the directory and verification of the file again

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] davidarcher edited a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
davidarcher edited a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-581898688
 
 
   Hi @mukund-thakur and thank you for offering up this PR. I've run into this bug and while removing the SSE header when copying will work (assuming the bucket has that KMS set as the default encryption on the bucket), another option could be to copy both the SSE header `x-amz-server-side-encryption` as well as the `x-amz-server-side-encryption-aws-kms-key-id` header that specifies the specific key to use. That would ensure that files don't revert to using the generic `aws/s3` KMS key and might be less likely to cause issues in edge cases. Just my $0.02... :)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-585208625
 
 
   see #1843 for a patch on top of this, to help propagate settings better; let's keep discussion in this patch & mukund can cherrypick that new commit while we get this right.
   
   I think we should have a consistent policy here of
   
   1. if the client has any encryption settings, including explicit AES256, KMS+default key, KMS+custom key, then they will set the encryption options on the copy.
   2. else the encryption settings of the source file are retained.
   
   This is nice and memorable. It needs to apply for all s3a encryption settings; this patch now only does it for SSE-KMS.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384724258
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
 ##########
 @@ -3394,6 +3396,42 @@ private CopyResult copyFile(String srcKey, String dstKey, long size,
         });
   }
 
+  /**
+   * Propagate encryption parameters from source file if set else use the
+   * current file system encryption settings.
+   * @param srcom
+   * @param copyObjectRequest
+   */
+  private void propagateEncryptionParams(ObjectMetadata srcom,
+                                         CopyObjectRequest copyObjectRequest) {
+    Optional<SSEAwsKeyManagementParams> kmsParams = Optional.empty();
+    String sourceKMSId = srcom.getSSEAwsKmsKeyId();
+    if (isNotEmpty(sourceKMSId)) {
+      // source KMS ID is propagated
+      LOG.debug("Propagating SSE-KMS settings from source {}",
+          sourceKMSId);
+      kmsParams = Optional.of(new SSEAwsKeyManagementParams(sourceKMSId));
+    }
+    kmsParams.ifPresent(
 
 Review comment:
   pull that up into the if() clause and you can avoid doing the optional work, just
   setSSE...(new SS3AwsKMP(sourceKMSId)).
   
   now, troublespot, and its one I'm curious about. What if there's SSE-C set, as it is also being set on the request? FWIW, I think things will break trying to read the file by setting the SSE-C key will inevitably break too.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-592163392
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 40s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 7 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m  8s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  trunk passed  |
   | -1 :x: |  shadedclient  |  16m 22s |  branch has errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   1m  5s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  1s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 20s |  hadoop-tools/hadoop-aws: The patch generated 18 new + 17 unchanged - 3 fixed = 35 total (was 20)  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  shadedclient  |  15m 37s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 19s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  61m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 0ed51e100dcb 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 10461e0 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   | whitespace | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/artifact/out/whitespace-eol.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/testReport/ |
   | Max. process+thread count | 455 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-592577196
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 35s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 8 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m 23s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  14m 55s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   1m  0s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 3 new + 17 unchanged - 3 fixed = 20 total (was 20)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 57s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  3s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate ASF License warnings.  |
   |  |   |  58m 42s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/9/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 2dd49a081a50 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / fccfb02 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/9/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/9/testReport/ |
   | Max. process+thread count | 420 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/9/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384734793
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3ATestBase.java
 ##########
 @@ -159,4 +168,63 @@ protected void assertStatusCode(AWSServiceIOException e, int code)
       throw e;
     }
   }
+
 
 Review comment:
   I think we should actually have this in a new class, say, EncryptionTestUtils; we can add more as and when needed. I'd have suggested S3ATestUtils, but that's too much source of cherrypick/merge pain these days, which is why I'm trying to split work up

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] davidarcher edited a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
davidarcher edited a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582994184
 
 
   > So what is proposed is
   > 
   > If we have local SSE-KMS settings, use that
   > fall back to that on the source object
   > 
   > What if we have local SSE-S3 enabled? would we want to switch for a simple rule: "if you have encryption set, renamed files pick it up; otherwise files encrypted with SSE-KMS keep their encryption"
   
   I'll defer to you for expected semantics here as I'm very new to this code base but my understanding is that this is the existing behavior for Copy operations for files encrypted with other SSE types.
   
   > As @mukund-thakur's patch currently has
   > 
   > "files encrypted with SSE-KMS retain that encryption setting when renamed, irrespective of your client settings"
   > 
   > I think I prefer the one in mukund's patch as its simpler to explain. Encryption with SSE-KMS is "sticky"
   
   Isn't it "files encrypted with SSE-KMS or SSE-S3 reset to the bucket default encryption when renamed, irrespective of your client settings" though?
   
   It seems a bit inconsistent that for SSE-C, renames don't change encryption settings (unless explicitly specified in the client settings) but for SSE-KMS & SSE-S3, with this change, renames reset to whatever is specified as default encryption on the bucket (unless explicitly specified in the client settings).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-579379966
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  35m 58s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | -1 :x: |  test4tests  |   0m  0s |  The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  20m 18s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 26s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 37s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 35s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 59s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 19s |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  14m 29s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m 15s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 31s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  94m 29s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/1/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 6d792b0d042a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 0825153 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/1/testReport/ |
   | Max. process+thread count | 453 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/1/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-587626465
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 5 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m 22s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m  9s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   1m  0s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 28s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 15 new + 17 unchanged - 3 fixed = 32 total (was 20)  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  shadedclient  |  13m 39s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 19s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  58m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 5fb73514b598 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / a562942 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   | whitespace | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/artifact/out/whitespace-eol.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/testReport/ |
   | Max. process+thread count | 449 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-581467919
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  18m 13s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 29s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 58s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 57s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 0 new + 13 unchanged - 1 fixed = 13 total (was 14)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 20s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  55m 56s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 48283c2c2842 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 1e3a0b0 |
   | Default Java | 1.8.0_242 |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/4/testReport/ |
   | Max. process+thread count | 454 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/4/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384733189
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3ATestBase.java
 ##########
 @@ -159,4 +168,63 @@ protected void assertStatusCode(AWSServiceIOException e, int code)
       throw e;
     }
   }
+
+  /**
+   * Assert that a path is encrypted with right encryption settings.
+   * @param path file path.
+   * @param algorithm encryption algorithm.
+   * @param kmsKeyArn
+   * @throws IOException
+   */
+  protected void assertEncrypted(final Path path,
+                                 final S3AEncryptionMethods algorithm,
+                                 final String kmsKeyArn)
+          throws IOException {
+    ObjectMetadata md = getFileSystem().getObjectMetadata(path);
+    String details = String.format(
+            "file %s with encryption algorthm %s and key %s",
 
 Review comment:
   Typo

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r385857403
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
 ##########
 @@ -3394,6 +3396,42 @@ private CopyResult copyFile(String srcKey, String dstKey, long size,
         });
   }
 
+  /**
+   * Propagate encryption parameters from source file if set else use the
+   * current file system encryption settings.
+   * @param srcom
+   * @param copyObjectRequest
+   */
+  private void propagateEncryptionParams(ObjectMetadata srcom,
+                                         CopyObjectRequest copyObjectRequest) {
+    Optional<SSEAwsKeyManagementParams> kmsParams = Optional.empty();
+    String sourceKMSId = srcom.getSSEAwsKmsKeyId();
+    if (isNotEmpty(sourceKMSId)) {
+      // source KMS ID is propagated
+      LOG.debug("Propagating SSE-KMS settings from source {}",
+          sourceKMSId);
+      kmsParams = Optional.of(new SSEAwsKeyManagementParams(sourceKMSId));
+    }
+    kmsParams.ifPresent(
 
 Review comment:
   > pull that up into the if() clause and you can avoid doing the optional work, just
   > setSSE...(new SS3AwsKMP(sourceKMSId)).
   
   Done
   > now, troublespot, and its one I'm curious about. What if there's SSE-C set, as it is also being set on the request? FWIW, I think things will break trying to read the file by setting the SSE-C key will inevitably break too.
   
   If SSE-C is used, sseKmsKey won't be present in the sourceObjectMeta. So that shouldn't be a problem. I debugged this test ITestS3AEncryptionSSEC#testRenameFile and it works fine. Please let me know if I am missing something.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-592163392
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 40s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 7 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m  8s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  trunk passed  |
   | -1 :x: |  shadedclient  |  16m 22s |  branch has errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   1m  5s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m  1s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 35s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 20s |  hadoop-tools/hadoop-aws: The patch generated 18 new + 17 unchanged - 3 fixed = 35 total (was 20)  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  shadedclient  |  15m 37s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 19s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  61m 46s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 0ed51e100dcb 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 10461e0 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   | whitespace | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/artifact/out/whitespace-eol.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/testReport/ |
   | Max. process+thread count | 455 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/8/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-581439423
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  30m 35s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  18m 14s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 30s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 58s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 26s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 18s |  hadoop-tools/hadoop-aws: The patch generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14)  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 20s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 31s |  The patch does not generate ASF License warnings.  |
   |  |   |  85m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 69568f12de59 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 1e3a0b0 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/testReport/ |
   | Max. process+thread count | 411 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-580732492
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  18m 16s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 48s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   1m  1s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14)  |
   | +1 :green_heart: |  mvnsite  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 11s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 22s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate ASF License warnings.  |
   |  |   |  56m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 48dc3bcd243b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / bf8686f |
   | Default Java | 1.8.0_232 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/testReport/ |
   | Max. process+thread count | 450 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r377571443
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractTestS3AEncryption.java
 ##########
 @@ -107,10 +106,15 @@ public void testEncryptionOverRename() throws Throwable {
     validateEncrytionSecrets(secrets);
     writeDataset(fs, src, data, data.length, 1024 * 1024, true);
     ContractTestUtils.verifyFileContents(fs, src, data);
-    Path dest = path(src.getName() + "-copy");
-    fs.rename(src, dest);
-    ContractTestUtils.verifyFileContents(fs, dest, data);
-    assertEncrypted(dest);
+    Path targetDir = path("target");
 
 Review comment:
   I am looking at this, trying to understand what it is doing.
   
   Before: we created a file src, renamed it to dest and verified the contents were unchanged; dest encrypted.
   
   After: 
   1. src is created as a dataset
   1. new path targetDir created
   1. file `dest` is created in a target/src+"-another" with a different dataset; contents verified
   1. rename(src, targetDir) to create the file targetDir/src
   1. which is verified
   1. dest file is completely ignored
   
   So why the change here? I don't see why we need the new test file, and the only change is now that you're renaming into a subdirectory  which already exists rather than a path of the destination file.
   
   I need some clarification here.
   * why the change
   * before the encryption settings were changed in copy, how did this new test fail?
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran merged pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran merged pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-580732492
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  18m 16s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 48s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   1m  1s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14)  |
   | +1 :green_heart: |  mvnsite  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 11s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 22s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate ASF License warnings.  |
   |  |   |  56m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 48dc3bcd243b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / bf8686f |
   | Default Java | 1.8.0_232 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/testReport/ |
   | Max. process+thread count | 450 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/2/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384734793
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3ATestBase.java
 ##########
 @@ -159,4 +168,63 @@ protected void assertStatusCode(AWSServiceIOException e, int code)
       throw e;
     }
   }
+
 
 Review comment:
   add this to org.apache.hadoop.fs.s3a.test.ExtraAssertions

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-593810970
 
 
   Thanks @steveloughran.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] davidarcher edited a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
davidarcher edited a comment on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582994184
 
 
   > So what is proposed is
   > 
   > If we have local SSE-KMS settings, use that
   > fall back to that on the source object
   > 
   > What if we have local SSE-S3 enabled? would we want to switch for a simple rule: "if you have encryption set, renamed files pick it up; otherwise files encrypted with SSE-KMS keep their encryption"
   
   I'll defer to you for expected semantics here as I'm very new to this code base but my understanding is that this is the existing behavior for Copy operations for files encrypted with other SSE types.
   
   > As @mukund-thakur's patch currently has
   > 
   > "files encrypted with SSE-KMS retain that encryption setting when renamed, irrespective of your client settings"
   > 
   > I think I prefer the one in mukund's patch as its simpler to explain. Encryption with SSE-KMS is "sticky"
   
   Isn't it "files encrypted with SSE-KMS reset to the bucket default encryption when renamed, irrespective of your client settings" though?
   
   It seems a bit inconsistent that for other SSE types, renames don't change encryption settings (unless explicitly specified in the client settings) but for SSE-KMS, with this change, renames reset to whatever is specified as default encryption on the bucket (unless explicitly specified in the client settings).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#discussion_r374114951
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionWithDefaultS3Settings.java
 ##########
 @@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.io.IOException;
+
+import com.amazonaws.services.s3.model.ObjectMetadata;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import static org.apache.hadoop.fs.contract.ContractTestUtils.skip;
+import static org.apache.hadoop.fs.s3a.Constants.SERVER_SIDE_ENCRYPTION_KEY;
+import static org.apache.hadoop.fs.s3a.S3AEncryptionMethods.SSE_KMS;
+
+/**
+ * Concrete class that extends {@link AbstractTestS3AEncryption}
+ * and tests already configured bucket level encryption using s3 console.
+ * This requires the SERVER_SIDE_ENCRYPTION_KEY
+ * to be set in auth-keys.xml for it to run. The value should match with the
+ * kms key set for the bucket.
+ */
+public class ITestS3AEncryptionWithDefaultS3Settings extends
+        AbstractTestS3AEncryption {
+
+  @Override
+  protected Configuration getConfiguration() {
+    // get the KMS key for this test.
+    Configuration c = new Configuration();
+    String kmsKey = c.get(SERVER_SIDE_ENCRYPTION_KEY);
+    if (StringUtils.isBlank(kmsKey)) {
+      skip(SERVER_SIDE_ENCRYPTION_KEY + " is not set for " +
+              SSE_KMS.getMethod());
+    }
+    return super.createConfiguration();
+  }
+
+  /**
+   * Setting this to NONE as we don't want to overwrite
+   * already configured encryption settings.
+   * @return
+   */
+  @Override
+  protected S3AEncryptionMethods getSSEAlgorithm() {
+    return S3AEncryptionMethods.NONE;
+  }
+
+  @Override
+  protected void assertEncrypted(Path path) throws IOException {
+    Configuration c = new Configuration();
+    String kmsKey = c.get(SERVER_SIDE_ENCRYPTION_KEY);
+    ObjectMetadata objectMetadata = getFileSystem().getObjectMetadata(path);
+    assertEquals("SSE KMS key id should match", kmsKey, objectMetadata.getSSEAwsKmsKeyId());
 
 Review comment:
   see checkstyle -line needs splitting. We like under/near 80 chars for better side-by-side review

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-587626465
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 5 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m 22s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 36s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m  9s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   1m  0s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 28s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 15 new + 17 unchanged - 3 fixed = 32 total (was 20)  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  shadedclient  |  13m 39s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 19s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  58m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 5fb73514b598 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / a562942 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   | whitespace | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/artifact/out/whitespace-eol.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/testReport/ |
   | Max. process+thread count | 449 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/6/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-581439423
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |  30m 35s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  18m 14s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 28s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 30s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 58s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 56s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 26s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 18s |  hadoop-tools/hadoop-aws: The patch generated 1 new + 13 unchanged - 1 fixed = 14 total (was 14)  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 20s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 31s |  The patch does not generate ASF License warnings.  |
   |  |   |  85m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 69568f12de59 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 1e3a0b0 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/testReport/ |
   | Max. process+thread count | 411 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/3/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-587567591
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  19m  0s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 34s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m  3s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 59s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 58s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 28s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 28s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 8 new + 10 unchanged - 3 fixed = 18 total (was 13)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 57s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 24s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate ASF License warnings.  |
   |  |   |  58m 16s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 077010daea5d 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / a562942 |
   | Default Java | 1.8.0_242 |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/testReport/ |
   | Max. process+thread count | 440 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/5/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] bgaborg commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
bgaborg commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-579683383
 
 
   Please describe the steps of how to set up an env to test this: what to configure on AWS and how.
   Also, describe the steps to test it manually.
   Please add this as a jira comment, so if we search back we will know.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384725607
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
 ##########
 @@ -3394,6 +3396,42 @@ private CopyResult copyFile(String srcKey, String dstKey, long size,
         });
   }
 
+  /**
+   * Propagate encryption parameters from source file if set else use the
+   * current file system encryption settings.
+   * @param srcom
+   * @param copyObjectRequest
+   */
+  private void propagateEncryptionParams(ObjectMetadata srcom,
+                                         CopyObjectRequest copyObjectRequest) {
+    Optional<SSEAwsKeyManagementParams> kmsParams = Optional.empty();
+    String sourceKMSId = srcom.getSSEAwsKmsKeyId();
+    if (isNotEmpty(sourceKMSId)) {
+      // source KMS ID is propagated
+      LOG.debug("Propagating SSE-KMS settings from source {}",
+          sourceKMSId);
+      kmsParams = Optional.of(new SSEAwsKeyManagementParams(sourceKMSId));
+    }
+    kmsParams.ifPresent(
+            copyObjectRequest::setSSEAwsKeyManagementParams);
+    switch(encryptionSecrets.getEncryptionMethod()) {
 
 Review comment:
   use  getServerSideEncryptionAlgorithm() as before

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-581898688
 
 
   Hi @mukund-thakur and thank you for offering up this PR. I've run into this bug and while removing the SSE header when copying will work (assuming the bucket has that KMS set as the default encryption on the bucket), another option could be to copy both the SSE header `x-amz-server-side-encryption` as well as the `x-amz-server-side-encryption-kms-key-id` header that specifies the specific key to use. That would ensure that files don't revert to using the generic `aws/s3` KMS key and might be less likely to cause issues in edge cases. Just my $0.02... :)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-588996092
 
 
   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 46s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 5 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | -1 :x: |  mvninstall  |   7m  4s |  root in trunk failed.  |
   | +1 :green_heart: |  compile  |   0m 44s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 26s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 37s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 54s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 57s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 55s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 34s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 29s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 29s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 2 new + 17 unchanged - 3 fixed = 19 total (was 20)  |
   | +1 :green_heart: |  mvnsite  |   0m 32s |  the patch passed  |
   | -1 :x: |  whitespace  |   0m  0s |  The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply  |
   | +1 :green_heart: |  shadedclient  |  14m 42s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  49m 44s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/7/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 773c98ea0241 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / ec75071 |
   | Default Java | 1.8.0_242 |
   | mvninstall | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/7/artifact/out/branch-mvninstall-root.txt |
   | checkstyle | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/7/artifact/out/diff-checkstyle-hadoop-tools_hadoop-aws.txt |
   | whitespace | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/7/artifact/out/whitespace-eol.txt |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/7/testReport/ |
   | Max. process+thread count | 455 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/7/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-579837188
 
 
   How to test?
   
   What are we trying to do? Our goal should be
   
   * the final file is encrypted
   
   Can you do a test case which replicates the problem, e.g. 
   
   * writes a file with SSE-KMS/SSE-S3 (ignore SSE-C)
   * with an FS instance which doesn't have any encryption settings, copy that file (i.e. rename it)
   * using getObjectMetadata, look at its encryption settings to verify it is in encrypted
   
   We also have to handle
   
   * unencrypted file in S3
   * renamed with an FS client which has SSE-S3/SSE-KMS set (and here, SSE-C)
   * final file MUST be encrypted with the given setting
   
   I still don't understand what's going wrong here though -we are trying to copy all the relevant keys
   
   Maybe we should be ignoring all of them and only explicitly setting those of our current instance. In which case: all encryption keys should be droped.
   
   How to test?
   
   What are we trying to do? Our goal should be
   
   * the final file is encrypted
   
   Can you do a test case which replicates the problem, e.g. 
   
   * writes a file with SSE-KMS/SSE-S3 (ignore SSE-C)
   * with an FS instance which doesn't have any encryption settings, copy that file (i.e. rename it)
   * using getObjectMetadata, look at its encryption settings to verify it is in encrypted
   
   We also have to handle
   
   * unencrypted file in S3
   * renamed with an FS client which has SSE-S3/SSE-KMS set (and here, SSE-C)
   * final file MUST be encrypted with the given setting
   
   I still don't understand what's going wrong here though -we are trying to copy all the relevant keys
   
   Maybe we should be ignoring all of them and only explicitly setting those of our current instance. In which case: all encryption keys should be droped.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582994184
 
 
   > So what is proposed is
   > 
   > If we have local SSE-KMS settings, use that
   > fall back to that on the source object
   > 
   > What if we have local SSE-S3 enabled? would we want to switch for a simple rule: "if you have encryption set, renamed files pick it up; otherwise files encrypted with SSE-KMS keep their encryption"
   
   I'll defer to you for expected semantics here as I'm very new to this code base but my understanding is that this is the existing behavior for Copy operations for other SSE types.
   
   > As @mukund-thakur's patch currently has
   > 
   > "files encrypted with SSE-KMS retain that encryption setting when renamed, irrespective of your client settings"
   > 
   > I think I prefer the one in mukund's patch as its simpler to explain. Encryption with SSE-KMS is "sticky"
   
   Isn't it "files encrypted with SSE-KMS reset to the bucket default encryption when renamed, irrespective of your client settings" though?
   
   It seems a bit inconsistent that for other SSE types, renames don't change encryption settings (unless explicitly specified in the client settings) but for SSE-KMS, with this change, renames reset to whatever is specified as default encryption on the bucket (unless explicitly specified in the client settings).
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
hadoop-yetus removed a comment on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-581467919
 
 
   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  5s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 2 new or modified test files.  |
   ||| _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  18m 13s |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  trunk passed  |
   | +1 :green_heart: |  checkstyle  |   0m 27s |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  13m 29s |  branch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  trunk passed  |
   | +0 :ok: |  spotbugs  |   0m 58s |  Used deprecated FindBugs config; considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   0m 57s |  trunk passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 33s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 19s |  hadoop-tools/hadoop-aws: The patch generated 0 new + 13 unchanged - 1 fixed = 13 total (was 14)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  shadedclient  |  13m 20s |  patch has no errors when building and testing our client artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  findbugs  |   1m  2s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 25s |  hadoop-aws in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate ASF License warnings.  |
   |  |   |  55m 56s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/4/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/1823 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient findbugs checkstyle |
   | uname | Linux 48283c2c2842 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/hadoop.sh |
   | git revision | trunk / 1e3a0b0 |
   | Default Java | 1.8.0_242 |
   |  Test Results | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/4/testReport/ |
   | Max. process+thread count | 454 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | https://builds.apache.org/job/hadoop-multibranch/job/PR-1823/4/console |
   | versions | git=2.7.4 maven=3.3.9 findbugs=3.1.0-RC1 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582749303
 
 
   > @davidarcher -so if we add an extra header, that will do it? OK.
   
   Yeah, it's mentioned in the [S3 docs](https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingKMSEncryption.html#require-sse-kms):
   
   > When you upload an object, you can specify the AWS KMS CMK using the `x-amz-server-side-encryption-aws-kms-key-id` header. If the header is not present in the request, Amazon S3 assumes the AWS managed CMK.
   
   The "AWS managed CMK" here means the generic `aws/s3` KMS key.
   
   For context, my use case is Apache Spark jobs writing to S3 buckets. The client has no encryption settings specified and each bucket is configured with its own KMS key set as the default encryption. We noticed that files were still showing up using the generic `aws/s3` key and determined it is because of this behavior of files being uploaded and then renamed to their final name (and switching to the generic `aws/s3` KMS key due to this bug).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384732906
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3ATestBase.java
 ##########
 @@ -159,4 +168,63 @@ protected void assertStatusCode(AWSServiceIOException e, int code)
       throw e;
     }
   }
+
+  /**
+   * Assert that a path is encrypted with right encryption settings.
+   * @param path file path.
+   * @param algorithm encryption algorithm.
+   * @param kmsKeyArn
 
 Review comment:
   add a javadoc entry for the argument; just delete the @throws IOE line altogether

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582781268
 
 
   @davidarcher  Yes this could be the other solution which even I thought initially. But looks like S3's ObjectMeta doesn't have any setter method to set this x-amz-server-side-encryption-aws-kms-key-id directly. I think this is done to ensure the transparency of default S3 encryption.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384727810
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
 ##########
 @@ -45,6 +45,7 @@
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+
 
 Review comment:
   cut. I know, we all hate imports. But S3A FS is big and even minor changes break other patches

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r385857403
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
 ##########
 @@ -3394,6 +3396,42 @@ private CopyResult copyFile(String srcKey, String dstKey, long size,
         });
   }
 
+  /**
+   * Propagate encryption parameters from source file if set else use the
+   * current file system encryption settings.
+   * @param srcom
+   * @param copyObjectRequest
+   */
+  private void propagateEncryptionParams(ObjectMetadata srcom,
+                                         CopyObjectRequest copyObjectRequest) {
+    Optional<SSEAwsKeyManagementParams> kmsParams = Optional.empty();
+    String sourceKMSId = srcom.getSSEAwsKmsKeyId();
+    if (isNotEmpty(sourceKMSId)) {
+      // source KMS ID is propagated
+      LOG.debug("Propagating SSE-KMS settings from source {}",
+          sourceKMSId);
+      kmsParams = Optional.of(new SSEAwsKeyManagementParams(sourceKMSId));
+    }
+    kmsParams.ifPresent(
 
 Review comment:
   > pull that up into the if() clause and you can avoid doing the optional work, just
   > setSSE...(new SS3AwsKMP(sourceKMSId)).
   Done
   > now, troublespot, and its one I'm curious about. What if there's SSE-C set, as it is also being set on the request? FWIW, I think things will break trying to read the file by setting the SSE-C key will inevitably break too.
   If SSE-C is used, sseKmsKey won't be present in the sourceObjectMeta. So that shouldn't be a problem. I debugged this test ITestS3AEncryptionSSEC#testRenameFile and it works fine. Please let me know if I am missing something.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#discussion_r374113961
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionWithDefaultS3Settings.java
 ##########
 @@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.io.IOException;
+
+import com.amazonaws.services.s3.model.ObjectMetadata;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.junit.Ignore;
+import org.junit.Test;
+
+import static org.apache.hadoop.fs.contract.ContractTestUtils.skip;
+import static org.apache.hadoop.fs.s3a.Constants.SERVER_SIDE_ENCRYPTION_KEY;
+import static org.apache.hadoop.fs.s3a.S3AEncryptionMethods.SSE_KMS;
+
+/**
+ * Concrete class that extends {@link AbstractTestS3AEncryption}
+ * and tests already configured bucket level encryption using s3 console.
+ * This requires the SERVER_SIDE_ENCRYPTION_KEY
+ * to be set in auth-keys.xml for it to run. The value should match with the
+ * kms key set for the bucket.
+ */
+public class ITestS3AEncryptionWithDefaultS3Settings extends
+        AbstractTestS3AEncryption {
+
+  @Override
+  protected Configuration getConfiguration() {
+    // get the KMS key for this test.
+    Configuration c = new Configuration();
+    String kmsKey = c.get(SERVER_SIDE_ENCRYPTION_KEY);
+    if (StringUtils.isBlank(kmsKey)) {
+      skip(SERVER_SIDE_ENCRYPTION_KEY + " is not set for " +
+              SSE_KMS.getMethod());
+    }
+    return super.createConfiguration();
+  }
+
+  /**
+   * Setting this to NONE as we don't want to overwrite
+   * already configured encryption settings.
+   * @return
+   */
+  @Override
+  protected S3AEncryptionMethods getSSEAlgorithm() {
+    return S3AEncryptionMethods.NONE;
+  }
+
+  @Override
+  protected void assertEncrypted(Path path) throws IOException {
+    Configuration c = new Configuration();
+    String kmsKey = c.get(SERVER_SIDE_ENCRYPTION_KEY);
+    ObjectMetadata objectMetadata = getFileSystem().getObjectMetadata(path);
+    assertEquals("SSE KMS key id should match", kmsKey, objectMetadata.getSSEAwsKmsKeyId());
+  }
+
+  @Override
+  @Ignore
+  @Test
+  public void testEncryptionSettingPropagation() throws Throwable {
+    super.testEncryptionSettingPropagation();
+  }
+
+  @Override
+  @Ignore
+  @Test
+  public void testEncryption() throws Throwable {
+    super.testEncryption();
 
 Review comment:
   not needed

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-580710341
 
 
   I was able to reproduce the test failure AbstractTestS3AEncryption#testEncryptionOverRename using the new overwritten test class. Before applying the fix , this testcase fails and post applying it passes. 
   
   Though I see a bit of problem in my test class, it runs the other two tests also testEncryption() and testEncryptionSettingPropagation() which is of not much significance here. I know a solution to ignore this but i am not sure if that is the right one. My solution is something like -
   Just override the test method in the derived test class and put @Ignore annotation. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
davidarcher commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582895340
 
 
   > @davidarcher Yes this could be the other solution which even I thought initially. But looks like S3's ObjectMeta doesn't have any setter method to set this x-amz-server-side-encryption-aws-kms-key-id directly. I think this is done to ensure the transparency of default S3 encryption.
   
   Thanks for looking into it @mukund-thakur -- From the [AWS example for using S3 KMS with Java](https://aws.amazon.com/blogs/developer/amazon-s3-encryption-with-aws-key-management-service/), it looks like it needs to be set directly on the request using the `withSSEAwsKeyManagementParams` or `setSSEAwsKeyManagementParams` method.
   
   We are already calling `setSSEAwsKeyManagementParams` [here](https://github.com/apache/hadoop/blob/db822aa905537ee266c8146633d973cad165a887/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java#L3434-L3449) in the `setOptionalCopyObjectRequestParameters` method but we only consider the KMS key id configured on the client -- maybe we can fallback there to looking at the KMS key id from the source object?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-591023245
 
 
   Don't worry about the test failures, we have a patch for that which I should see about merging in
   https://issues.apache.org/jira/browse/HADOOP-16319

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
steveloughran commented on issue #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-582929516
 
 
   So what is proposed is
   
   1. If we have local SSE-KMS settings, use that
   1. fall back to that on the source object
   
   What if we have local SSE-S3 enabled? would we want to switch for a simple rule: "if you have encryption set, renamed files pick it up; otherwise files encrypted with SSE-KMS keep their encryption" 
   
   As @mukund-thakur's patch currently has
   
   "files encrypted with SSE-KMS retain that encryption setting when renamed, irrespective of your client settings"
   
   I think I prefer the one in mukund's patch as its simpler to explain. Encryption with SSE-KMS is "sticky"

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r377693355
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractTestS3AEncryption.java
 ##########
 @@ -107,10 +106,15 @@ public void testEncryptionOverRename() throws Throwable {
     validateEncrytionSecrets(secrets);
     writeDataset(fs, src, data, data.length, 1024 * 1024, true);
     ContractTestUtils.verifyFileContents(fs, src, data);
-    Path dest = path(src.getName() + "-copy");
-    fs.rename(src, dest);
-    ContractTestUtils.verifyFileContents(fs, dest, data);
-    assertEncrypted(dest);
+    Path targetDir = path("target");
 
 Review comment:
   
   
   > 6. dest file is completely ignored
   > 
   The reason dest file has to be created is to enforce rename to consider targetDir as a directory else it considers it as file. 
   
   > I need some clarification here.
   > 
   > * why the change
   >
   This change was done to address one of your above comment.
   "Maybe: in testEncryptionOverRename , rename the file into a directory." 
   
   > * before the encryption settings were changed in copy, how did this new test fail?
   > 
   The encryption key of the destination file  targetDir/src was not matching with the configured kms key of the bucket rather it was equal to some default key generated by the S3 itself.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption key is not getting set properly during put operation.
URL: https://github.com/apache/hadoop/pull/1823#discussion_r374113472
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionWithDefaultS3Settings.java
 ##########
 @@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import java.io.IOException;
+
+import com.amazonaws.services.s3.model.ObjectMetadata;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.junit.Ignore;
 
 Review comment:
   should go into the same block as the amazon one

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-587532022
 
 
   Not sure why two tests are failing. Btw they are failing in trunk branch as well.
   
   > [ERROR] Tests run: 12, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 20.515 s <<< FAILURE! - in org.apache.hadoop.fs.s3a.ITestS3AMiscOperations
   [ERROR] testEmptyFileChecksums(org.apache.hadoop.fs.s3a.ITestS3AMiscOperations)  Time elapsed: 2.728 s  <<< FAILURE!
   java.lang.AssertionError: checksums of empty files expected:<etag: "1463cbcf1dbae282559d1fec36f131f2"> but was:<etag: "a92916168376ec017a3c98c92102dd7e">
   	at org.apache.hadoop.fs.s3a.ITestS3AMiscOperations.testEmptyFileChecksums(ITestS3AMiscOperations.java:165)
   [ERROR] testNonEmptyFileChecksumsUnencrypted(org.apache.hadoop.fs.s3a.ITestS3AMiscOperations)  Time elapsed: 2.031 s  <<< FAILURE!
   java.lang.AssertionError: checksums expected:<etag: "e52b1ed8f37dfca9a3bc8954875d049a"> but was:<etag: "4aaae27ccdbc799df38d35eed11f4c52">
   	at org.apache.hadoop.fs.s3a.ITestS3AMiscOperations.testNonEmptyFileChecksumsUnencrypted(ITestS3AMiscOperations.java:219)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384727376
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
 ##########
 @@ -3394,6 +3396,42 @@ private CopyResult copyFile(String srcKey, String dstKey, long size,
         });
   }
 
+  /**
 
 Review comment:
   1. can you move down to where setOptionalCopyObjectRequestParameters() was before; it will make it easier to cherry pick.  
   2. Restore the original name, with new args (and javadoc). Same reason -and because we may want add more options in future

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
steveloughran commented on a change in pull request #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#discussion_r384730866
 
 

 ##########
 File path: hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/AbstractS3ATestBase.java
 ##########
 @@ -32,8 +34,11 @@
 
 import java.io.IOException;
 
+import com.amazonaws.services.s3.model.ObjectMetadata;
 
 Review comment:
   lets stick this one above L22 in its own block

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


[GitHub] [hadoop] mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation

Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on issue #1823: HADOOP-16794 S3 Encryption keys not propagating correctly during copy operation
URL: https://github.com/apache/hadoop/pull/1823#issuecomment-592667330
 
 
   All review comments addressed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org