[
https://issues.apache.org/jira/browse/HADOOP-19103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17826017#comment-17826017
]
ASF GitHub Bot commented on HADOOP-19103:
-----------------------------------------
ahmarsuhail commented on code in PR #6615:
URL: https://github.com/apache/hadoop/pull/6615#discussion_r1523052340
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java:
##########
@@ -325,16 +356,13 @@ public void testSessionCredentialsRegionNoEndpoint()
throws Throwable {
@Test
public void testSessionCredentialsRegionBadEndpoint() throws Throwable {
describe("Create a session with a bad region and expect fast failure");
- IllegalArgumentException ex
+ IOException ex
Review Comment:
revert to IllegalArgumentException
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java:
##########
@@ -160,6 +160,18 @@ public final class DelegationConstants {
*/
public static final String STS_STANDARD = "sts.amazonaws.com";
+ /**
+ * The format of the STS Endpoint
+ */
Review Comment:
you can move both of these into STSClientFactory as they are only relevant
for that class.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java:
##########
@@ -160,6 +160,18 @@ public final class DelegationConstants {
*/
public static final String STS_STANDARD = "sts.amazonaws.com";
+ /**
+ * The format of the STS Endpoint
+ */
+ public static final String STS_ENDPOINT_URI_PATTERN =
"^sts\\..*\\.amazonaws\\.com$";
Review Comment:
your pattern will need to handle china as well.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/STSClientFactory.java:
##########
@@ -125,6 +125,14 @@ public static StsClientBuilder builder(
public static StsClientBuilder builder(final AwsCredentialsProvider
credentials,
final Configuration conf, final String stsEndpoint, final String
stsRegion,
final String bucket) throws IOException {
+ // If an STS endpoint is provided and if it is not STS_STANDARD
(sts.amazonaws.com)
+ // it should match E_INVALID_STS_ENDPOINT_PATTERN.
+ if (!isEmpty(stsEndpoint) &&
+ !STS_STANDARD.equals(stsEndpoint) &&
+ !stsEndpoint.matches(STS_ENDPOINT_URI_PATTERN)) {
+ throw new
IOException(String.format(E_INVALID_STS_ENDPOINT_PATTERN,stsEndpoint));
Review Comment:
Yes, please use `IllegalArgumentException`
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java:
##########
@@ -163,6 +163,37 @@ public void testSTS() throws IOException {
}
}
+ /**
+ * Test use of Invalid STS for requesting temporary credentials.
+ *
+ * The property test.sts.endpoint can be set to point this at different
+ * STS endpoints. This test will use the AWS credentials (if provided) for
+ * S3A tests to request temporary credentials, then attempt to use those
+ * credentials instead.
+ *
+ * @throws IOException failure
+ */
+ @Test
+ public void testSTSInvalid() throws IOException {
+ Configuration conf = getContract().getConf();
+ S3AFileSystem testFS = getFileSystem();
+ credentials = getS3AInternals().shareCredentials("testSTS");
+
+ String bucket = testFS.getBucket();
+ try {
Review Comment:
Look at the other tests in this class,
`testSessionCredentialsRegionBadEndpoint()` for example. We can write this
similarly.
You will also need to add a test case to check if China endpoint works.
> Add logic for verifying that the STS URL is in the correct format
> -----------------------------------------------------------------
>
> Key: HADOOP-19103
> URL: https://issues.apache.org/jira/browse/HADOOP-19103
> Project: Hadoop Common
> Issue Type: Bug
> Components: fs/s3
> Reporter: Narayanan Venkateswaran
> Priority: Minor
> Labels: pull-request-available
>
> * At present an invalid URL can be supplied as an STS endpoint. It will
> attempt to create an STSClient with it and then fail with,
> {quote}java.net.UnknownHostException: request session credentials:
> software.amazon.awssdk.core.exception.SdkClientException: Received an
> UnknownHostException when attempting to interact with a service. See cause
> for the exact endpoint that is failing to resolve. If this is happening on an
> endpoint that previously worked, there may be a network connectivity issue or
> your DNS cache could be storing endpoints for too long.:
> software.amazon.awssdk.core.exception.SdkClientException: Received an
> UnknownHostException when attempting to interact with a service. See cause
> for the exact endpoint that is failing to resolve. If this is happening on an
> endpoint that previously worked, there may be a network connectivity issue or
> your DNS cache could be storing endpoints for too long.: https
> {quote} * This is inefficient. An invalid URL can be parsed much earlier and
> can be failed based on the URL format itself.
> * The error message is not very clear and does not indicate a problem in the
> URL format.
> * In this Jira issue, we attempt to parse the STS URL and fail fast with a
> more relevant error message.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
