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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]