[
https://issues.apache.org/jira/browse/HADOOP-13131?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15302902#comment-15302902
]
Chris Nauroth commented on HADOOP-13131:
----------------------------------------
Patch v007 looks good. Thank you, Steve. I had a successful full test run in
parallel mode against US-west-2.
{{S3AFastOutputStream}} also supports server-side encryption. What would you
think of adding a test covering that?
Aside from that, I have some minor comments. Much of this is subjective, so
feel free to take it or leave it as you please.
Should {{S3AFileSystem#getObjectMetadata}} be annotated {{VisibleForTesting}}?
I think I see why there are 2 overloads of {{getObjectMetadata}}. It
simplifies HADOOP-13171, right? However, I'm not sure why
{{getObjectMetadata(String)}} is protected. Should that be private?
{code}
the S3 protocols to the extent that the amazon s3 SDK is capable of talking
{code}
Suggest capitalizing "Amazon S3".
{code}
@BeforeClass
public static void nameThread() {
Thread.currentThread().setName("JUnit");
}
{code}
A possible evolution of this could be to switch to a {{Before}} method and
change the thread name to "JUnit-" + {{methodName}}, so that the thread name is
specific to the test being run.
{code}
public TestS3AEncryption() {
}
{code}
No-op default constructor can be removed?
{code}
@Override
protected AbstractFSContract createContract(Configuration conf) {
S3ATestUtils.disableFilesystemCaching(conf);
conf.set(Constants.SERVER_SIDE_ENCRYPTION_ALGORITHM,
AES256);
return new S3AContract(conf);
}
{code}
Would it be better to do this config setup by overriding
{{AbstractFSContractTestBase#createConfiguration}}? Then, the subclasses
wouldn't need to instantiate {{S3AContract}} and could instead rely on
{{AbstractS3ATestBase#createContract}}.
{code}
protected S3AFileSystem getS3AFS() {
return (S3AFileSystem) getFileSystem();
}
{code}
Instead of adding a new method, I suggest overriding
{{AbstractFSContractTestBase#getFileSystem}}, with the method's return type
changed to {{S3AFileSystem}}, since Java allows covariant return types.
> Add tests to verify that S3A supports SSE-S3 encryption
> -------------------------------------------------------
>
> Key: HADOOP-13131
> URL: https://issues.apache.org/jira/browse/HADOOP-13131
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 2.7.2
> Reporter: Steve Loughran
> Assignee: Steve Loughran
> Priority: Minor
> Attachments: HADOOP-13131-001.patch, HADOOP-13131-002.patch,
> HADOOP-13131-003.patch, HADOOP-13131-004.patch, HADOOP-13131-005.patch,
> HADOOP-13131-branch-2-006.patch, HADOOP-13131-branch-2-007.patch
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Although S3A claims to support server-side S3 encryption (and does, if you
> set the option), we don't have any test to verify this. Of course, as the
> encryption is transparent, it's hard to test.
> Here's what I propose
> # a test which sets encryption = AES256; expects things to work as normal.
> # a test which sets encyption = DES and expects any operation creating a file
> or directory to fail with a 400 "bad request" error
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
