TEZ-4600: Secret managers in Tez should respect the algorithm set by hadoop by abstractdog · Pull Request #393 · apache/tez

abstractdog · 2025-02-19T14:54:50Z

This PR makes the JobTokenSecretManager in Tez able to use the algorithm specified in:

hadoop.security.secret-manager.key-generator.algorithm

An additional comment was needed to upgrade to hadoop 3.5.0-SNAPSHOT to compile against the changes of YARN-11738. This PR can be merged once Tez upgraded its hadoop.version to 3.5.0 (which needs a hadoop release first :) )

testing done: turned on SSL shuffle on a cluster having YARN-11738, DAG failed with exceptions mentioned on jira, then with the fix DAG ran successfully (as ShuffleScheduler + ShuffleManager used the non-default algorithm configured)

ayushtkn · 2025-02-25T10:17:59Z

+ } catch (NoSuchAlgorithmException nsa) {
+ throw new IllegalArgumentException("Can't find " + algorithm + " algorithm.", nsa);
+ } catch (InvalidKeyException ike) {
+ throw new IllegalArgumentException("Invalid key to HMAC computation", ike);


Do we want to hardcode HMAC over here? InvalidKeyException can be thrown for other algorithms as well, right?

oh, right, this is a leftover from the default HMAC-era

tez-yetus · 2025-02-25T13:32:18Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 38s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 10 new or modified test files.
		_ master Compile Tests _
+0 🆗	mvndep	2m 51s	Maven dependency ordering for branch
+1 💚	mvninstall	9m 44s	master passed
+1 💚	compile	5m 45s	master passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	compile	5m 24s	master passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	checkstyle	3m 58s	master passed
+1 💚	javadoc	4m 7s	master passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javadoc	3m 50s	master passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+0 🆗	spotbugs	5m 47s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	11m 29s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 57s	Maven dependency ordering for patch
+1 💚	mvninstall	7m 11s	the patch passed
+1 💚	compile	6m 6s	the patch passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javac	6m 6s	the patch passed
+1 💚	compile	5m 39s	the patch passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	javac	5m 39s	the patch passed
+1 💚	checkstyle	0m 28s	tez-api: The patch generated 0 new + 41 unchanged - 1 fixed = 41 total (was 42)
+1 💚	checkstyle	0m 31s	The patch passed checkstyle in tez-runtime-library
+1 💚	checkstyle	0m 25s	The patch passed checkstyle in tez-mapreduce
+1 💚	checkstyle	0m 35s	The patch passed checkstyle in tez-dag
+1 💚	checkstyle	0m 24s	The patch passed checkstyle in tez-ext-service-tests
+1 💚	checkstyle	0m 23s	The patch passed checkstyle in tez-aux-services
+1 💚	checkstyle	0m 58s	root: The patch generated 0 new + 329 unchanged - 1 fixed = 329 total (was 330)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	xml	0m 2s	The patch has no ill-formed XML file.
+1 💚	javadoc	3m 56s	the patch passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javadoc	3m 52s	the patch passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	findbugs	12m 23s	the patch passed
		_ Other Tests _
+1 💚	unit	0m 45s	tez-api in the patch passed.
+1 💚	unit	0m 36s	tez-runtime-library in the patch passed.
+1 💚	unit	0m 34s	tez-mapreduce in the patch passed.
+1 💚	unit	0m 41s	tez-dag in the patch passed.
+1 💚	unit	0m 35s	tez-ext-service-tests in the patch passed.
+1 💚	unit	0m 33s	tez-aux-services in the patch passed.
+1 💚	unit	3m 17s	root in the patch passed.
+1 💚	asflicense	2m 38s	The patch does not generate ASF License warnings.
		105m 9s

Subsystem	Report/Notes
Docker	ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/3/artifact/out/Dockerfile
GITHUB PR	#393
Optional Tests	dupname asflicense javac javadoc unit xml compile spotbugs findbugs checkstyle
uname	Linux 64af6b038ae0 5.15.0-130-generic #140-Ubuntu SMP Wed Dec 18 17:59:53 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `82b3476`
Default Java	Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/3/testReport/
Max. process+thread count	101 (vs. ulimit of 5500)
modules	C: tez-api tez-runtime-library tez-mapreduce tez-dag tez-ext-service-tests tez-plugins/tez-aux-services . U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/3/console
versions	git=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

abstractdog · 2025-02-25T13:52:48Z

tests passed
let me know @ayushtkn if this can be approved
we cannot merge until Hadoop 3.5.0, created TEZ-4606 for blocking this jira

ayushtkn

LGTM

…hadoop Change-Id: I236505c2439ba675f4a277aeb1f7b0faae2d1053

tez-yetus · 2025-02-26T11:46:23Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	25m 6s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 12 new or modified test files.
		_ master Compile Tests _
+0 🆗	mvndep	2m 0s	Maven dependency ordering for branch
+1 💚	mvninstall	10m 19s	master passed
+1 💚	compile	3m 34s	master passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	compile	3m 20s	master passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	checkstyle	2m 48s	master passed
+1 💚	javadoc	2m 57s	master passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javadoc	2m 43s	master passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+0 🆗	spotbugs	0m 46s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	5m 57s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 10s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 18s	the patch passed
+1 💚	compile	2m 19s	the patch passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javac	2m 19s	the patch passed
+1 💚	compile	2m 6s	the patch passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	javac	2m 6s	the patch passed
-0 ⚠️	checkstyle	0m 13s	tez-api: The patch generated 1 new + 41 unchanged - 1 fixed = 42 total (was 42)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	1m 32s	the patch passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javadoc	1m 29s	the patch passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	findbugs	5m 23s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 16s	tez-api in the patch passed.
+1 💚	unit	5m 47s	tez-runtime-library in the patch passed.
+1 💚	unit	1m 23s	tez-mapreduce in the patch passed.
+1 💚	unit	5m 5s	tez-dag in the patch passed.
+1 💚	unit	4m 32s	tez-ext-service-tests in the patch passed.
+1 💚	unit	2m 50s	tez-aux-services in the patch passed.
+1 💚	asflicense	0m 55s	The patch does not generate ASF License warnings.
		100m 56s

Subsystem	Report/Notes
Docker	ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/4/artifact/out/Dockerfile
GITHUB PR	#393
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux a4dd1a21cd29 5.15.0-131-generic #141-Ubuntu SMP Fri Jan 10 21:18:28 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `82b3476`
Default Java	Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
checkstyle	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/4/artifact/out/diff-checkstyle-tez-api.txt
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/4/testReport/
Max. process+thread count	2100 (vs. ulimit of 5500)
modules	C: tez-api tez-runtime-library tez-mapreduce tez-dag tez-ext-service-tests tez-plugins/tez-aux-services U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/4/console
versions	git=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

abstractdog · 2025-02-26T12:11:02Z

changed hadoop enums to strings until TEZ-4607
merging this soon assuming that @ayushtkn 's approval still holds

ayushtkn · 2025-03-03T23:03:00Z

there is a checkstyle warning that has shown up for the unused import, we need to fix it before we merge

abstractdog · 2025-03-04T07:24:20Z

there is a checkstyle warning that has shown up for the unused import, we need to fix it before we merge

makes sense, I just pushed a commit to remove it

tez-yetus · 2025-03-04T09:03:19Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	25m 44s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 12 new or modified test files.
		_ master Compile Tests _
+0 🆗	mvndep	2m 5s	Maven dependency ordering for branch
+1 💚	mvninstall	9m 45s	master passed
+1 💚	compile	3m 30s	master passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	compile	3m 13s	master passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	checkstyle	2m 43s	master passed
+1 💚	javadoc	2m 39s	master passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javadoc	2m 29s	master passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+0 🆗	spotbugs	0m 41s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	5m 29s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 9s	Maven dependency ordering for patch
+1 💚	mvninstall	2m 11s	the patch passed
+1 💚	compile	2m 7s	the patch passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javac	2m 7s	the patch passed
+1 💚	compile	1m 56s	the patch passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	javac	1m 56s	the patch passed
+1 💚	checkstyle	0m 12s	tez-api: The patch generated 0 new + 41 unchanged - 1 fixed = 41 total (was 42)
+1 💚	checkstyle	0m 16s	The patch passed checkstyle in tez-runtime-library
+1 💚	checkstyle	0m 10s	The patch passed checkstyle in tez-mapreduce
+1 💚	checkstyle	0m 19s	The patch passed checkstyle in tez-dag
+1 💚	checkstyle	0m 10s	The patch passed checkstyle in tez-ext-service-tests
+1 💚	checkstyle	0m 10s	The patch passed checkstyle in tez-aux-services
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	1m 22s	the patch passed with JDK Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04
+1 💚	javadoc	1m 21s	the patch passed with JDK Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
+1 💚	findbugs	5m 1s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 11s	tez-api in the patch passed.
+1 💚	unit	6m 17s	tez-runtime-library in the patch passed.
+1 💚	unit	1m 21s	tez-mapreduce in the patch passed.
+1 💚	unit	5m 1s	tez-dag in the patch passed.
+1 💚	unit	4m 43s	tez-ext-service-tests in the patch passed.
+1 💚	unit	2m 51s	tez-aux-services in the patch passed.
+1 💚	asflicense	0m 53s	The patch does not generate ASF License warnings.
		98m 44s

Subsystem	Report/Notes
Docker	ClientAPI=1.48 ServerAPI=1.48 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/5/artifact/out/Dockerfile
GITHUB PR	#393
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux c3870ff10f60 5.15.0-131-generic #141-Ubuntu SMP Fri Jan 10 21:18:28 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `82b3476`
Default Java	Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.26+4-post-Ubuntu-1ubuntu122.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_442-8u442-b06~~us1-0ubuntu1~~22.04-b06
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/5/testReport/
Max. process+thread count	1534 (vs. ulimit of 5500)
modules	C: tez-api tez-runtime-library tez-mapreduce tez-dag tez-ext-service-tests tez-plugins/tez-aux-services U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-393/5/console
versions	git=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

This comment was marked as outdated.
Sign in to view

abstractdog force-pushed the TEZ-4600 branch from 0a55dc8 to bd3ed36Compare February 25, 2025 09:27

abstractdog changed the title ~~TEZ-4600: JobTokenSecretManager should respect the algorithm set by hadoop~~ TEZ-4600: Secret managers in Tez should respect the algorithm set by hadoopFeb 25, 2025

abstractdog requested a review from ayushtkn February 25, 2025 09:30

ayushtkn reviewed Feb 25, 2025
View reviewed changes

This comment was marked as outdated.
Sign in to view

ayushtkn approved these changes Feb 25, 2025
View reviewed changes

abstractdog added 4 commits February 26, 2025 11:04

TEZ-4600: Secret managers in Tez should respect the algorithm set by …
258a4a5
…hadoop Change-Id: I236505c2439ba675f4a277aeb1f7b0faae2d1053

PR comments + checkstyle
b690de8

handle possible NPEs
bcda616

hardcoded config
a060cc4

abstractdog force-pushed the TEZ-4600 branch from 35a7574 to a060cc4Compare February 26, 2025 10:04

removed unused import from JobTokenSecretManager
50ddea5

ayushtkn merged commit 3aa2aa1 into apache:masterMar 4, 2025

Navigation Menu

Uh oh!

TEZ-4600: Secret managers in Tez should respect the algorithm set by hadoop - #393

TEZ-4600: Secret managers in Tez should respect the algorithm set by hadoop#393
ayushtkn merged 5 commits into
apache:masterfrom
abstractdog:TEZ-4600

abstractdog commented Feb 19, 2025•
edited
Loading

Uh oh!

This comment was marked as outdated.

ayushtknFeb 25, 2025

Uh oh!

abstractdogFeb 25, 2025

Uh oh!

This comment was marked as outdated.

tez-yetus commented Feb 25, 2025

Uh oh!

abstractdog commented Feb 25, 2025

Uh oh!

ayushtkn left a comment

Uh oh!

tez-yetus commented Feb 26, 2025

Uh oh!

abstractdog commented Feb 26, 2025

Uh oh!

ayushtkn commented Mar 3, 2025

Uh oh!

abstractdog commented Mar 4, 2025

Uh oh!

tez-yetus commented Mar 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

abstractdog commented Feb 19, 2025• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

ayushtknFeb 25, 2025

Choose a reason for hiding this comment

Uh oh!

abstractdogFeb 25, 2025

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

tez-yetus commented Feb 25, 2025

Uh oh!

abstractdog commented Feb 25, 2025

Uh oh!

ayushtkn left a comment

Choose a reason for hiding this comment

Uh oh!

tez-yetus commented Feb 26, 2025

Uh oh!

abstractdog commented Feb 26, 2025

Uh oh!

ayushtkn commented Mar 3, 2025

Uh oh!

abstractdog commented Mar 4, 2025

Uh oh!

tez-yetus commented Mar 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abstractdog commented Feb 19, 2025•
edited
Loading