HADOOP-19587. SSE-C integration changes #7738
Open
+97
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
|
just created https://issues.apache.org/jira/browse/HADOOP-19587, can you update your PR title to "HADOOP-19587. SSE-C integration changes". Adding the JIRA issue |
ahmarsuhail
reviewed
Jun 17, 2025
|
|
||
| import org.apache.hadoop.fs.contract.s3a.S3AContract; |
There was a problem hiding this comment.
this test needs work, you shouldn't need to depend on anything from the contract package
There was a problem hiding this comment.
ahh my bad, ITestS3AEncryptionSSEC also does this, ignore my comment
ahmarsuhail
reviewed
Jun 17, 2025
| */ | ||
|
|
||
| @Test | ||
| public void testAnalyticsStreamOpenStreamInfoWhenSSECEncryptionEnabled() throws Exception { |
There was a problem hiding this comment.
this test is messy, I think because a lot of boiler plate that's needed here already exists in ITestS3AEncryptionSSEC.
How about instead of adding the SSE-C test here, add it in ITestS3AEncryptionSSEC?
All you have to do there is set AAL to enabled.
ahmarsuhail
reviewed
Jun 17, 2025
| @Test | ||
| public void testAnalyticsStreamOpenStreamInfoWhenSSECEncryptionEnabled() throws Exception { | ||
| Configuration confSSEC = this.createConfiguration(); | ||
| S3ATestUtils.disableFilesystemCaching(confSSEC); |
There was a problem hiding this comment.
We're also not testing completely. You're basically checking if the SSE-C key was set in openStreamInformation, which is fine, but not really necessary. What you want to test is the end behaviour.
Can you write and then read a file using SSE-C with AAL? So all you need to do is call writeThenReadFile() with AAL enabled.
If that read succeeds then things worked, and the SSE-C was passed correctly.
Then you also want to check what happens if you pass in an empty key to AAL, or an invalid key? it's similar to the test we wrote in AAL, but we should also add it here.
There was a problem hiding this comment.
There are tests similar to what you are suggesting in ITestS3AEncryptionSSEC, may be we can run those with AAL enabled always
rajdchak
changed the title
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of PR
This PR is for the integration of S3A with SSE_C support introduced in Analytics Accelerator library.
How was this patch tested?
Ran all the integrations tests in hadoop-aws on EC2 instance.
Setup:
Created personal bucket in us-west-2 region. Ran the integrations tests in hadoop-aws with the configs mainly enabling Analytics stream and SSEC encryption.
However unrelated to these changes whenever we enabled SSE_C there are multiple tests which fail with Bad Request exception as they are running on public buckets like
noaa-cors-pds. We should take an action item to disable those tests if SSE_C is enabled. Created an issue for this error https://issues.apache.org/jira/browse/HADOOP-19590 .For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?