HADOOP-19103 : Add logic for verifying that the STS URL is in the correct format #6615

vnhive · 2024-03-07T06:00:03Z

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,

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

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.

Testing

Added a new integration test in ITestS3ATemporaryCredentials and verified that the existing tests ran fine.

mvn clean install verify -Dit.test=ITestS3ATemporaryCredentials -Dtest=none

…rect format At present an invalid URL supplied as an STS endpoint results in failing late with a unclear error message. Here we add support for failing fast with a clearer error message.

hadoop-yetus · 2024-03-07T07:24:02Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 22s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	32m 29s		trunk passed
+1 💚	compile	0m 25s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	compile	0m 22s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	checkstyle	0m 22s		trunk passed
+1 💚	mvnsite	0m 28s		trunk passed
+1 💚	javadoc	0m 20s		trunk passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 24s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	0m 47s		trunk passed
+1 💚	shadedclient	19m 34s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 19s		the patch passed
+1 💚	compile	0m 21s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javac	0m 21s		the patch passed
+1 💚	compile	0m 15s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	javac	0m 15s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 10s	/results-checkstyle-hadoop-tools_hadoop-aws.txt	hadoop-tools/hadoop-aws: The patch generated 4 new + 2 unchanged - 0 fixed = 6 total (was 2)
+1 💚	mvnsite	0m 20s		the patch passed
+1 💚	javadoc	0m 10s		the patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1
+1 💚	javadoc	0m 18s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	0m 41s		the patch passed
+1 💚	shadedclient	19m 18s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	2m 11s		hadoop-aws in the patch passed.
+1 💚	asflicense	0m 26s		The patch does not generate ASF License warnings.
		82m 58s

Subsystem	Report/Notes
Docker	ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/1/artifact/out/Dockerfile
GITHUB PR	#6615
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 760731d6fb15 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `f7bfc5e`
Default Java	Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/1/testReport/
Max. process+thread count	552 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/1/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

...s/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/STSClientFactory.java

ahmarsuhail

Looks fine mostly, agree @shameersss1 comments, and also left a few more.

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/STSClientFactory.java

...s/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java

...op-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java

...s/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java

hadoop-yetus · 2024-04-25T01:26:54Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
			_ Prechecks _
+1 💚	dupname	0m 00s		No case conflicting files found.
+0 🆗	spotbugs	0m 01s		spotbugs executables are not available.
+0 🆗	codespell	0m 01s		codespell was not available.
+0 🆗	detsecrets	0m 01s		detect-secrets was not available.
+1 💚	@author	0m 00s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 00s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	106m 34s		trunk passed
+1 💚	compile	5m 51s		trunk passed
+1 💚	checkstyle	5m 17s		trunk passed
+1 💚	mvnsite	5m 45s		trunk passed
+1 💚	javadoc	5m 24s		trunk passed
+1 💚	shadedclient	168m 41s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	3m 37s		the patch passed
+1 💚	compile	2m 40s		the patch passed
+1 💚	javac	2m 40s		the patch passed
+1 💚	blanks	0m 00s		The patch has no blanks issues.
+1 💚	checkstyle	2m 24s		the patch passed
+1 💚	mvnsite	2m 49s		the patch passed
+1 💚	javadoc	2m 31s		the patch passed
+1 💚	shadedclient	184m 37s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	asflicense	6m 13s		The patch does not generate ASF License warnings.
		487m 07s

Subsystem	Report/Notes
GITHUB PR	#6615
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	MINGW64_NT-10.0-17763 634fe718ba01 3.4.10-87d57229.x86_64 2024-02-14 20:17 UTC x86_64 Msys
Build tool	maven
Personality	/c/hadoop/dev-support/bin/hadoop.sh
git revision	trunk / `f7bfc5e`
Default Java	Azul Systems, Inc.-1.8.0_332-b09
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6615/1/testReport/
modules	C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6615/1/console
versions	git=2.44.0.windows.1
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

hadoop-yetus · 2024-06-25T02:40:02Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 48s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	49m 38s		trunk passed
+1 💚	compile	0m 46s		trunk passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	compile	0m 35s		trunk passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	checkstyle	0m 32s		trunk passed
+1 💚	mvnsite	0m 42s		trunk passed
+1 💚	javadoc	0m 28s		trunk passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	javadoc	0m 34s		trunk passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	spotbugs	1m 9s		trunk passed
+1 💚	shadedclient	38m 58s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 31s		the patch passed
+1 💚	compile	0m 37s		the patch passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	javac	0m 37s		the patch passed
+1 💚	compile	0m 26s		the patch passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	javac	0m 26s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 20s	/results-checkstyle-hadoop-tools_hadoop-aws.txt	hadoop-tools/hadoop-aws: The patch generated 4 new + 2 unchanged - 0 fixed = 6 total (was 2)
+1 💚	mvnsite	0m 32s		the patch passed
+1 💚	javadoc	0m 16s		the patch passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	javadoc	0m 26s		the patch passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	spotbugs	1m 8s		the patch passed
+1 💚	shadedclient	39m 57s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	2m 50s		hadoop-aws in the patch passed.
+1 💚	asflicense	0m 36s		The patch does not generate ASF License warnings.
		143m 10s

Subsystem	Report/Notes
Docker	ClientAPI=1.45 ServerAPI=1.45 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/2/artifact/out/Dockerfile
GITHUB PR	#6615
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 1326e1c3c987 5.15.0-107-generic #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `0de7044`
Default Java	Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/2/testReport/
Max. process+thread count	528 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/2/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

…rect format Address the review comments on the PR.

vnhive

Thank you very much for the review comments.
I really appreciate the attention to detail in the review.
I have addressed all the comments and raised a new PR.
Please do check and revert back for any changes you want.

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/STSClientFactory.java

...s/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java

...op-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java

...s/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java

...op-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java

vnhive · 2024-06-27T11:53:56Z

Thank you very much for the review comments.

I really appreciate the attention to detail in the review.

I have addressed all the comments and raised a new PR.

Please do check and revert back for any changes you want.

Please note the following test output

[INFO] Running org.apache.hadoop.fs.s3a.ITestS3ATemporaryCredentials
[INFO] Tests run: 15, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 107.068 s - in org.apache.hadoop.fs.s3a.ITestS3ATemporaryCredentials
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 15, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:53 min
[INFO] Finished at: 2024-06-27T16:45:34+05:30
[INFO] ------------------------------------------------------------------------

hadoop-yetus · 2024-06-27T13:49:57Z

🎊 +1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 48s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 1s		codespell was not available.
+0 🆗	detsecrets	0m 1s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	50m 35s		trunk passed
+1 💚	compile	0m 44s		trunk passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	compile	0m 35s		trunk passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	checkstyle	0m 32s		trunk passed
+1 💚	mvnsite	0m 42s		trunk passed
+1 💚	javadoc	0m 28s		trunk passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	javadoc	0m 34s		trunk passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	spotbugs	1m 11s		trunk passed
+1 💚	shadedclient	39m 19s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 31s		the patch passed
+1 💚	compile	0m 36s		the patch passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	javac	0m 36s		the patch passed
+1 💚	compile	0m 27s		the patch passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	javac	0m 27s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
-0 ⚠️	checkstyle	0m 20s	/results-checkstyle-hadoop-tools_hadoop-aws.txt	hadoop-tools/hadoop-aws: The patch generated 40 new + 2 unchanged - 0 fixed = 42 total (was 2)
+1 💚	mvnsite	0m 33s		the patch passed
+1 💚	javadoc	0m 16s		the patch passed with JDK Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2
+1 💚	javadoc	0m 25s		the patch passed with JDK Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
+1 💚	spotbugs	1m 8s		the patch passed
+1 💚	shadedclient	39m 2s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	2m 50s		hadoop-aws in the patch passed.
+1 💚	asflicense	0m 37s		The patch does not generate ASF License warnings.
		143m 31s

Subsystem	Report/Notes
Docker	ClientAPI=1.46 ServerAPI=1.46 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/3/artifact/out/Dockerfile
GITHUB PR	#6615
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 664f927b0047 5.15.0-107-generic #117-Ubuntu SMP Fri Apr 26 12:26:49 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `086e0b6`
Default Java	Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.23+9-post-Ubuntu-1ubuntu120.04.2 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_412-8u412-ga-1~20.04.1-b08
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/3/testReport/
Max. process+thread count	528 (vs. ulimit of 5500)
modules	C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6615/3/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

vnhive · 2024-06-27T14:37:57Z

Looks fine mostly, agree @shameersss1 comments, and also left a few more.

Really appreciate the detailed comments and reviews. Please do check the PR with the comments addressed.

shameersss1 · 2024-06-27T14:45:14Z

...op-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java

@@ -327,14 +440,11 @@ public void testSessionCredentialsRegionBadEndpoint() throws Throwable {
    describe("Create a session with a bad region and expect fast failure");
    IllegalArgumentException ex
        = expectedSessionRequestFailure(
-        IllegalArgumentException.class,


nit: can we avoid non intentional changes?

shameersss1 · 2024-06-27T14:45:40Z

...op-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ATemporaryCredentials.java

        " ",
        EU_IRELAND,
        "");
    LOG.info("Outcome: ", ex);
-    if (!(ex.getCause() instanceof URISyntaxException)) {


Why are we removing this ?

HADOOP-19103 : Add logic for verifying that the STS URL is in the cor…

f7bfc5e

…rect format At present an invalid URL supplied as an STS endpoint results in failing late with a unclear error message. Here we add support for failing fast with a clearer error message.

github-actions bot added trunk TOOLS AWS labels Mar 7, 2024

shameersss1 suggested changes Mar 7, 2024

View reviewed changes

...s/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/delegation/DelegationConstants.java Outdated Show resolved Hide resolved

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/STSClientFactory.java Outdated Show resolved Hide resolved

shameersss1 suggested changes Mar 7, 2024

View reviewed changes

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/STSClientFactory.java Outdated Show resolved Hide resolved

ahmarsuhail requested changes Mar 13, 2024

View reviewed changes

Merge branch 'apache:trunk' into HADOOP-19103

0de7044

HADOOP-19103 : Add logic for verifying that the STS URL is in the cor…

086e0b6

…rect format Address the review comments on the PR.

vnhive commented Jun 27, 2024

View reviewed changes

shameersss1 reviewed Jun 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-19103 : Add logic for verifying that the STS URL is in the correct format #6615

HADOOP-19103 : Add logic for verifying that the STS URL is in the correct format #6615

vnhive commented Mar 7, 2024

hadoop-yetus commented Mar 7, 2024

ahmarsuhail left a comment

hadoop-yetus commented Apr 25, 2024

hadoop-yetus commented Jun 25, 2024

vnhive left a comment

vnhive commented Jun 27, 2024

hadoop-yetus commented Jun 27, 2024

vnhive commented Jun 27, 2024

shameersss1 Jun 27, 2024

shameersss1 Jun 27, 2024

HADOOP-19103 : Add logic for verifying that the STS URL is in the correct format #6615

Are you sure you want to change the base?

HADOOP-19103 : Add logic for verifying that the STS URL is in the correct format #6615

Conversation

vnhive commented Mar 7, 2024

hadoop-yetus commented Mar 7, 2024

ahmarsuhail left a comment

Choose a reason for hiding this comment

hadoop-yetus commented Apr 25, 2024

hadoop-yetus commented Jun 25, 2024

vnhive left a comment

Choose a reason for hiding this comment

vnhive commented Jun 27, 2024

hadoop-yetus commented Jun 27, 2024

vnhive commented Jun 27, 2024

shameersss1 Jun 27, 2024

Choose a reason for hiding this comment

shameersss1 Jun 27, 2024

Choose a reason for hiding this comment