[
https://issues.apache.org/jira/browse/HADOOP-17871?focusedWorklogId=649910&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-649910
]
ASF GitHub Bot logged work on HADOOP-17871:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 13/Sep/21 10:30
Start Date: 13/Sep/21 10:30
Worklog Time Spent: 10m
Work Description: steveloughran commented on a change in pull request
#3412:
URL: https://github.com/apache/hadoop/pull/3412#discussion_r707201841
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -44,21 +44,21 @@ public void testEncryptionAlgorithmSetToDES() throws
Throwable {
assumeEnabled();
intercept(IOException.class, "Unknown Server Side algorithm DES", () -> {
- Configuration conf = super.createConfiguration();
- //DES is an invalid encryption algorithm
- conf.set(Constants.S3_ENCRYPTION_ALGORITHM, "DES");
- S3AContract contract = (S3AContract) createContract(conf);
- contract.init();
- //extract the test FS
- FileSystem fileSystem = contract.getTestFileSystem();
- assertNotNull("null filesystem", fileSystem);
- URI fsURI = fileSystem.getUri();
- LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
- assertEquals("wrong filesystem of " + fsURI,
- contract.getScheme(), fsURI.getScheme());
- fileSystem.initialize(fsURI, conf);
- throw new Exception("Do not reach here");
- });
+ Configuration conf = super.createConfiguration();
+ //DES is an invalid encryption algorithm
+ conf.set(Constants.S3_ENCRYPTION_ALGORITHM, "DES");
+ S3AContract contract = (S3AContract) createContract(conf);
+ contract.init();
+ //extract the test FS
+ FileSystem fileSystem = contract.getTestFileSystem();
+ assertNotNull("null filesystem", fileSystem);
+ URI fsURI = fileSystem.getUri();
+ LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+ assertEquals("wrong filesystem of " + fsURI,
+ contract.getScheme(), fsURI.getScheme());
+ fileSystem.initialize(fsURI, conf);
+ throw new Exception("Do not reach here");
Review comment:
can you just return `fileSystem`
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -44,21 +44,21 @@ public void testEncryptionAlgorithmSetToDES() throws
Throwable {
assumeEnabled();
intercept(IOException.class, "Unknown Server Side algorithm DES", () -> {
- Configuration conf = super.createConfiguration();
- //DES is an invalid encryption algorithm
- conf.set(Constants.S3_ENCRYPTION_ALGORITHM, "DES");
- S3AContract contract = (S3AContract) createContract(conf);
- contract.init();
- //extract the test FS
- FileSystem fileSystem = contract.getTestFileSystem();
- assertNotNull("null filesystem", fileSystem);
- URI fsURI = fileSystem.getUri();
- LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
- assertEquals("wrong filesystem of " + fsURI,
- contract.getScheme(), fsURI.getScheme());
- fileSystem.initialize(fsURI, conf);
- throw new Exception("Do not reach here");
- });
+ Configuration conf = super.createConfiguration();
Review comment:
nit; if we generate that error "unknown server side algorithm", should
we change to "unknown s3 encryption algorithm"
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -119,24 +119,24 @@ public void
testEncryptionAlgorithmSSES3WithEncryptionKey() throws
assumeEnabled();
intercept(IOException.class, S3AUtils.SSE_S3_WITH_KEY_ERROR, () -> {
- Configuration conf = super.createConfiguration();
- //SSE-S3 cannot be configured with an encryption key
- conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
- S3AEncryptionMethods.SSE_S3.getMethod());
- conf.set(Constants.S3_ENCRYPTION_KEY,
- "4niV/jPK5VFRHY+KNb6wtqYd4xXyMgdJ9XQJpcQUVbs=");
- S3AContract contract = (S3AContract) createContract(conf);
- contract.init();
- //extract the test FS
- FileSystem fileSystem = contract.getTestFileSystem();
- assertNotNull("null filesystem", fileSystem);
- URI fsURI = fileSystem.getUri();
- LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
- assertEquals("wrong filesystem of " + fsURI,
- contract.getScheme(), fsURI.getScheme());
- fileSystem.initialize(fsURI, conf);
- throw new Exception("Do not reach here");
- });
+ Configuration conf = super.createConfiguration();
+ //SSE-S3 cannot be configured with an encryption key
+ conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+ S3AEncryptionMethods.SSE_S3.getMethod());
+ conf.set(Constants.S3_ENCRYPTION_KEY,
+ "4niV/jPK5VFRHY+KNb6wtqYd4xXyMgdJ9XQJpcQUVbs=");
+ S3AContract contract = (S3AContract) createContract(conf);
+ contract.init();
+ //extract the test FS
+ FileSystem fileSystem = contract.getTestFileSystem();
+ assertNotNull("null filesystem", fileSystem);
+ URI fsURI = fileSystem.getUri();
+ LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+ assertEquals("wrong filesystem of " + fsURI,
+ contract.getScheme(), fsURI.getScheme());
+ fileSystem.initialize(fsURI, conf);
+ throw new Exception("Do not reach here");
Review comment:
same
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -69,47 +69,47 @@ public void
testEncryptionAlgorithmSSECWithNoEncryptionKey() throws
intercept(IllegalArgumentException.class, "The value of property " +
Constants.S3_ENCRYPTION_KEY + " must not be null", () -> {
- Configuration conf = super.createConfiguration();
- //SSE-C must be configured with an encryption key
- conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
- S3AEncryptionMethods.SSE_C.getMethod());
- conf.set(Constants.S3_ENCRYPTION_KEY, null);
- S3AContract contract = (S3AContract) createContract(conf);
- contract.init();
- //extract the test FS
- FileSystem fileSystem = contract.getTestFileSystem();
- assertNotNull("null filesystem", fileSystem);
- URI fsURI = fileSystem.getUri();
- LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
- assertEquals("wrong filesystem of " + fsURI,
- contract.getScheme(), fsURI.getScheme());
- fileSystem.initialize(fsURI, conf);
- throw new Exception("Do not reach here");
- });
+ Configuration conf = super.createConfiguration();
+ //SSE-C must be configured with an encryption key
+ conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+ S3AEncryptionMethods.SSE_C.getMethod());
+ conf.set(Constants.S3_ENCRYPTION_KEY, null);
+ S3AContract contract = (S3AContract) createContract(conf);
+ contract.init();
+ //extract the test FS
+ FileSystem fileSystem = contract.getTestFileSystem();
+ assertNotNull("null filesystem", fileSystem);
+ URI fsURI = fileSystem.getUri();
+ LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+ assertEquals("wrong filesystem of " + fsURI,
+ contract.getScheme(), fsURI.getScheme());
+ fileSystem.initialize(fsURI, conf);
+ throw new Exception("Do not reach here");
Review comment:
again, return fileSystem
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionAlgorithmValidation.java
##########
@@ -69,47 +69,47 @@ public void
testEncryptionAlgorithmSSECWithNoEncryptionKey() throws
intercept(IllegalArgumentException.class, "The value of property " +
Constants.S3_ENCRYPTION_KEY + " must not be null", () -> {
- Configuration conf = super.createConfiguration();
- //SSE-C must be configured with an encryption key
- conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
- S3AEncryptionMethods.SSE_C.getMethod());
- conf.set(Constants.S3_ENCRYPTION_KEY, null);
- S3AContract contract = (S3AContract) createContract(conf);
- contract.init();
- //extract the test FS
- FileSystem fileSystem = contract.getTestFileSystem();
- assertNotNull("null filesystem", fileSystem);
- URI fsURI = fileSystem.getUri();
- LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
- assertEquals("wrong filesystem of " + fsURI,
- contract.getScheme(), fsURI.getScheme());
- fileSystem.initialize(fsURI, conf);
- throw new Exception("Do not reach here");
- });
+ Configuration conf = super.createConfiguration();
+ //SSE-C must be configured with an encryption key
+ conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+ S3AEncryptionMethods.SSE_C.getMethod());
+ conf.set(Constants.S3_ENCRYPTION_KEY, null);
+ S3AContract contract = (S3AContract) createContract(conf);
+ contract.init();
+ //extract the test FS
+ FileSystem fileSystem = contract.getTestFileSystem();
+ assertNotNull("null filesystem", fileSystem);
+ URI fsURI = fileSystem.getUri();
+ LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+ assertEquals("wrong filesystem of " + fsURI,
+ contract.getScheme(), fsURI.getScheme());
+ fileSystem.initialize(fsURI, conf);
+ throw new Exception("Do not reach here");
+ });
}
@Test
public void testEncryptionAlgorithmSSECWithBlankEncryptionKey() throws
Throwable {
intercept(IOException.class, S3AUtils.SSE_C_NO_KEY_ERROR, () -> {
- Configuration conf = super.createConfiguration();
- //SSE-C must be configured with an encryption key
- conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
- S3AEncryptionMethods.SSE_C.getMethod());
- conf.set(Constants.S3_ENCRYPTION_KEY, "");
- S3AContract contract = (S3AContract) createContract(conf);
- contract.init();
- //extract the test FS
- FileSystem fileSystem = contract.getTestFileSystem();
- assertNotNull("null filesystem", fileSystem);
- URI fsURI = fileSystem.getUri();
- LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
- assertEquals("wrong filesystem of " + fsURI,
- contract.getScheme(), fsURI.getScheme());
- fileSystem.initialize(fsURI, conf);
- throw new Exception("Do not reach here");
- });
+ Configuration conf = super.createConfiguration();
+ //SSE-C must be configured with an encryption key
+ conf.set(Constants.S3_ENCRYPTION_ALGORITHM,
+ S3AEncryptionMethods.SSE_C.getMethod());
+ conf.set(Constants.S3_ENCRYPTION_KEY, "");
+ S3AContract contract = (S3AContract) createContract(conf);
+ contract.init();
+ //extract the test FS
+ FileSystem fileSystem = contract.getTestFileSystem();
+ assertNotNull("null filesystem", fileSystem);
+ URI fsURI = fileSystem.getUri();
+ LOG.info("Test filesystem = {} implemented by {}", fsURI, fileSystem);
+ assertEquals("wrong filesystem of " + fsURI,
+ contract.getScheme(), fsURI.getScheme());
+ fileSystem.initialize(fsURI, conf);
+ throw new Exception("Do not reach here");
Review comment:
&same
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 649910)
Time Spent: 40m (was: 0.5h)
> S3A CSE: minor tuning
> ---------------------
>
> Key: HADOOP-17871
> URL: https://issues.apache.org/jira/browse/HADOOP-17871
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Steve Loughran
> Assignee: Mehakmeet Singh
> Priority: Minor
> Labels: pull-request-available
> Time Spent: 40m
> Remaining Estimate: 0h
>
> Some minor tuning to the CSE encryption support before backporting to 3.3.x
> and so shipping this year
> * LogExactlyOnce an "please ignore the warning" message to a new log
> ("org.apache.hadoop.fs.s3a.encryption") which can be set to ERROR if you get
> bored of the message.
> * Extend testing_s3a.md and the SDK upgrade runbook: test CSE always
> * change property name of encryption key (maybe: fs.s3a.encryption) and add
> mapping in S3AFileSystem.addDeprecatedKeys ... docs will need updating too.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
