ahmarsuhail commented on code in PR #6316:
URL: https://github.com/apache/hadoop/pull/6316#discussion_r1417123542
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/ITestAssumeRole.java:
##########
@@ -181,6 +189,10 @@ protected Configuration createValidRoleConf() throws
JsonProcessingException {
conf.set(ASSUMED_ROLE_ARN, roleARN);
conf.set(ASSUMED_ROLE_SESSION_NAME, "valid");
conf.set(ASSUMED_ROLE_SESSION_DURATION, "45m");
+ // disable create session so there's no need to
+ // add a role policy for it.
+ disableCreateSession(conf);
Review Comment:
what was happening without this? I am seeing the same failure on trunk and
on this branch. for eg `testPartialDelete` fails on list for
`/test/testPartialDelete/file-1/`
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java:
##########
@@ -493,23 +507,72 @@ public static void skipIfNotEnabled(final Configuration
configuration,
}
/**
- * Skip a test if storage class tests are disabled.
+ * Skip a test if storage class tests are disabled,
+ * or the bucket is an S3Express bucket.
* @param configuration configuration to probe
*/
public static void skipIfStorageClassTestsDisabled(
Configuration configuration) {
skipIfNotEnabled(configuration, KEY_STORAGE_CLASS_TESTS_ENABLED,
"Skipping storage class tests");
+ skipIfS3ExpressBucket(configuration);
}
/**
- * Skip a test if ACL class tests are disabled.
+ * Skip a test if ACL class tests are disabled,
+ * or the bucket is an S3Express bucket.
* @param configuration configuration to probe
*/
public static void skipIfACLTestsDisabled(
Configuration configuration) {
skipIfNotEnabled(configuration, KEY_ACL_TESTS_ENABLED,
"Skipping storage class ACL tests");
+ skipIfS3ExpressBucket(configuration);
+ }
+
+ /**
+ * Skip a test if the test bucket is an S3Express bucket.
+ * @param configuration configuration to probe
+ */
+ public static void skipIfS3ExpressBucket(
+ Configuration configuration) {
+ assume("Skipping test as bucket is an S3Express bucket",
+ !isS3ExpressTestBucket(configuration));
+ }
+
+ /**
+ * Is the test bucket an S3Express bucket?
+ * @param conf configuration
+ * @return true if the bucket is an S3Express bucket.
+ */
+ public static boolean isS3ExpressTestBucket(final Configuration conf) {
+ return S3ExpressStorage.isS3ExpressStore(getTestBucketName(conf), "");
+ }
+
+ /**
+ * Skip a test if the filesystem lacks a required capability.
Review Comment:
nit: update javadoc, i think this will skip if it has a capability
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/auth/delegation/ITestSessionDelegationInFilesystem.java:
##########
@@ -147,40 +150,51 @@ protected Configuration createConfiguration() {
// disable if assume role opts are off
assumeSessionTestsEnabled(conf);
disableFilesystemCaching(conf);
- String s3EncryptionMethod;
- try {
- s3EncryptionMethod =
- getEncryptionAlgorithm(getTestBucketName(conf), conf).getMethod();
- } catch (IOException e) {
- throw new UncheckedIOException("Failed to lookup encryption algorithm.",
- e);
- }
- String s3EncryptionKey = getS3EncryptionKey(getTestBucketName(conf), conf);
+ final String bucket = getTestBucketName(conf);
+ final boolean isS3Express = isS3ExpressTestBucket(conf);
+
removeBaseAndBucketOverrides(conf,
DELEGATION_TOKEN_BINDING,
Constants.S3_ENCRYPTION_ALGORITHM,
Constants.S3_ENCRYPTION_KEY,
SERVER_SIDE_ENCRYPTION_ALGORITHM,
- SERVER_SIDE_ENCRYPTION_KEY);
+ SERVER_SIDE_ENCRYPTION_KEY,
+ S3EXPRESS_CREATE_SESSION);
conf.set(HADOOP_SECURITY_AUTHENTICATION,
UserGroupInformation.AuthenticationMethod.KERBEROS.name());
enableDelegationTokens(conf, getDelegationBinding());
conf.set(AWS_CREDENTIALS_PROVIDER, " ");
// switch to CSE-KMS(if specified) else SSE-KMS.
- if (conf.getBoolean(KEY_ENCRYPTION_TESTS, true)) {
+ if (!isS3Express && conf.getBoolean(KEY_ENCRYPTION_TESTS, true)) {
+ String s3EncryptionMethod;
+ try {
+ s3EncryptionMethod =
+ getEncryptionAlgorithm(bucket, conf).getMethod();
+ } catch (IOException e) {
+ throw new UncheckedIOException("Failed to lookup encryption
algorithm.",
+ e);
+ }
+ String s3EncryptionKey = getS3EncryptionKey(bucket, conf);
+
conf.set(Constants.S3_ENCRYPTION_ALGORITHM, s3EncryptionMethod);
// KMS key ID a must if CSE-KMS is being tested.
conf.set(Constants.S3_ENCRYPTION_KEY, s3EncryptionKey);
}
// set the YARN RM up for YARN tests.
conf.set(YarnConfiguration.RM_PRINCIPAL, YARN_RM);
- // turn on ACLs so as to verify role DT permissions include
- // write access.
- conf.set(CANNED_ACL, LOG_DELIVERY_WRITE);
+
+ if (conf.getBoolean(KEY_ACL_TESTS_ENABLED, false)
+ && !isS3Express) {
+ // turn on ACLs so as to verify role DT permissions include
+ // write access.
+ conf.set(CANNED_ACL, LOG_DELIVERY_WRITE);
+ }
+ // disable create session so there's no need to
+ // add a role policy for it.
+ disableCreateSession(conf);
Review Comment:
same here, I didn't see a difference in failure for
`testDelegatedFileSystem` with and without this.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestBucketTool.java:
##########
@@ -118,7 +121,10 @@ public void testRecreateTestBucketS3Express() throws
Throwable {
fsURI));
if (ex instanceof AWSBadRequestException) {
// owned error
- assertExceptionContains(OWNED, ex);
+ if (!ex.getMessage().contains(OWNED)
+ && !ex.getMessage().contains(INVALID_LOCATION)) {
Review Comment:
when does the INVALID_LOCATION get thrown?
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java:
##########
@@ -493,23 +507,72 @@ public static void skipIfNotEnabled(final Configuration
configuration,
}
/**
- * Skip a test if storage class tests are disabled.
+ * Skip a test if storage class tests are disabled,
+ * or the bucket is an S3Express bucket.
* @param configuration configuration to probe
*/
public static void skipIfStorageClassTestsDisabled(
Configuration configuration) {
skipIfNotEnabled(configuration, KEY_STORAGE_CLASS_TESTS_ENABLED,
"Skipping storage class tests");
+ skipIfS3ExpressBucket(configuration);
}
/**
- * Skip a test if ACL class tests are disabled.
+ * Skip a test if ACL class tests are disabled,
+ * or the bucket is an S3Express bucket.
* @param configuration configuration to probe
*/
public static void skipIfACLTestsDisabled(
Configuration configuration) {
skipIfNotEnabled(configuration, KEY_ACL_TESTS_ENABLED,
"Skipping storage class ACL tests");
+ skipIfS3ExpressBucket(configuration);
+ }
+
+ /**
+ * Skip a test if the test bucket is an S3Express bucket.
+ * @param configuration configuration to probe
+ */
+ public static void skipIfS3ExpressBucket(
+ Configuration configuration) {
+ assume("Skipping test as bucket is an S3Express bucket",
+ !isS3ExpressTestBucket(configuration));
+ }
+
+ /**
+ * Is the test bucket an S3Express bucket?
+ * @param conf configuration
+ * @return true if the bucket is an S3Express bucket.
+ */
+ public static boolean isS3ExpressTestBucket(final Configuration conf) {
+ return S3ExpressStorage.isS3ExpressStore(getTestBucketName(conf), "");
+ }
+
+ /**
+ * Skip a test if the filesystem lacks a required capability.
+ * @param fs filesystem
+ * @param capability capability
+ */
+ public static void assumePathCapability(FileSystem fs, String capability) {
+ try {
+ assume("Filesystem lacks capability " + capability,
+ fs.hasPathCapability(new Path("/"), capability));
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ } /**
+ * Skip a test if the filesystem lacks a required capability.
+ * @param fs filesystem
+ * @param capability capability
+ */
+ public static void assumePathCapabilityFalse(FileSystem fs, String
capability) {
+ try {
+ assume("Filesystem lacks capability " + capability,
Review Comment:
think this should be "filesystem has capability"
--
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]
