[
https://issues.apache.org/jira/browse/HADOOP-18997?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17793672#comment-17793672
]
ASF GitHub Bot commented on HADOOP-18997:
-----------------------------------------
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"
> S3A: Add option fs.s3a.s3express.create.session to enable/disable
> CreateSession
> -------------------------------------------------------------------------------
>
> Key: HADOOP-18997
> URL: https://issues.apache.org/jira/browse/HADOOP-18997
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Steve Loughran
> Assignee: Steve Loughran
> Priority: Minor
> Labels: pull-request-available
>
> add a way to disable the need to use the createsession call, so as to allow
> for
> * simplifying our role test runs
> * benchmarking the performance hit
> * troubleshooting IAM permissions
> this can also be disabled from the sysprop "aws.disableS3ExpressAuth"
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
