Skip to content

HADOOP-18711. Upgrade nimbus jwt jar due to issues in its embedded shaded json-smart code #5639

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 4 commits into from

Conversation

rohit-kb
Copy link
Contributor

@rohit-kb rohit-kb commented May 10, 2023

Description of PR

How was this patch tested?

For code changes:

  • Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
  • Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

@rohit-kb
Copy link
Contributor Author

rohit-kb commented May 10, 2023

Added filters because of the following error during compilation:

[INFO] --- maven-enforcer-plugin:3.0.0-M1:enforce (enforce-banned-dependencies) @ hadoop-client-check-test-invariants ---
[INFO] Adding ignorable dependency: org.apache.hadoop:hadoop-annotations:null
[INFO]   Adding ignore: *
[WARNING] Rule 1: org.apache.maven.plugins.enforcer.BanDuplicateClasses failed with message:
Duplicate classes found:

  Found in:
    org.apache.hadoop:hadoop-client-minicluster:jar:3.3.9-SNAPSHOT:compile
    org.apache.hadoop:hadoop-client-runtime:jar:3.3.9-SNAPSHOT:compile
  Duplicate classes:
    META-INF/versions/9/module-info.class

@ayushtkn
Copy link
Member

Why this filter isn’t required in trunk?

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 7m 31s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+0 🆗 xmllint 0m 1s xmllint was not available.
+0 🆗 shelldocs 0m 1s Shelldocs was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-3.3 Compile Tests _
+0 🆗 mvndep 15m 45s Maven dependency ordering for branch
+1 💚 mvninstall 23m 44s branch-3.3 passed
+1 💚 compile 18m 11s branch-3.3 passed
+1 💚 mvnsite 26m 14s branch-3.3 passed
+1 💚 javadoc 7m 43s branch-3.3 passed
+1 💚 shadedclient 35m 35s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 32s Maven dependency ordering for patch
-1 ❌ mvninstall 6m 27s /patch-mvninstall-root.txt root in the patch failed.
-1 ❌ compile 0m 26s /patch-compile-root.txt root in the patch failed.
-1 ❌ javac 0m 26s /patch-compile-root.txt root in the patch failed.
-1 ❌ blanks 0m 0s /blanks-eol.txt The patch has 1 line(s) that end in blanks. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
-1 ❌ mvnsite 0m 40s /patch-mvnsite-root.txt root in the patch failed.
+1 💚 shellcheck 0m 0s No new issues.
-1 ❌ javadoc 1m 35s /patch-javadoc-root.txt root in the patch failed.
-1 ❌ shadedclient 3m 30s patch has errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 5m 4s /patch-unit-root.txt root in the patch failed.
-1 ❌ asflicense 0m 51s /results-asflicense.txt The patch generated 1 ASF License warnings.
159m 8s
Subsystem Report/Notes
Docker ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/1/artifact/out/Dockerfile
GITHUB PR #5639
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint shellcheck shelldocs
uname Linux 8de93e424456 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision branch-3.3 / 13c8fea
Default Java Private Build-1.8.0_362-8u362-ga-0ubuntu1~18.04.1-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/1/testReport/
Max. process+thread count 550 (vs. ulimit of 5500)
modules C: hadoop-project hadoop-client-modules/hadoop-client-runtime hadoop-client-modules/hadoop-client-minicluster . U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/1/console
versions git=2.17.1 maven=3.6.0 shellcheck=0.4.6
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@rohit-kb
Copy link
Contributor Author

rohit-kb commented May 11, 2023

Why this filter isn’t required in trunk?

Probably because of this dependency version:

<dependency>
  <groupId>org.codehaus.mojo</groupId>
  <artifactId>extra-enforcer-rules</artifactId>
  <version>1.0-beta-3</version>
</dependency>

In trunk, the version is 1.5.1 and the upgrade is done in HADOOP-18131. This new version seems to have changes in enforcer rules.
cc @ayushtkn @steveloughran

@steveloughran
Copy link
Contributor

well, you get to cherrypick that and any followups you can see off the jira. that's the way it goes I'm afraid.

@rohit-kb
Copy link
Contributor Author

Actually, we have tried porting HADOOP-18131 to branch-3.3 previously and I checked it again too, but we were/are getting this compilation issue:

Dependency convergence error for log4j:log4j:jar:1.2.17:test paths to dependency are:
+-org.apache.hadoop:hadoop-yarn-server-timelineservice-documentstore:jar:3.3.9-SNAPSHOT
  +-org.mockito:mockito-core:jar:2.8.9:test
    +-net.bytebuddy:byte-buddy-agent:jar:1.6.14:test
      +-com.kohlschutter.junixsocket:junixsocket-native-common:jar:2.0.4:test
        +-com.kohlschutter.junixsocket:junixsocket-common:jar:2.0.4:test
          +-log4j:log4j:jar:1.2.17:test
and
+-org.apache.hadoop:hadoop-yarn-server-timelineservice-documentstore:jar:3.3.9-SNAPSHOT
  +-org.mockito:mockito-core:jar:2.8.9:test
    +-net.bytebuddy:byte-buddy-agent:jar:1.6.14:test
      +-com.kohlschutter.junixsocket:junixsocket-native-common:jar:2.0.4:test
        +-log4j:log4j:jar:1.2.17:test
and
+-org.apache.hadoop:hadoop-yarn-server-timelineservice-documentstore:jar:3.3.9-SNAPSHOT
  +-com.microsoft.azure:azure-cosmosdb:jar:2.4.5:compile
    +-com.fasterxml.uuid:java-uuid-generator:jar:3.1.4:compile
      +-log4j:log4j:jar:1.2.13:provided

It kind of comes down to log4j in trunk vs reload4j in branch-3.3. So, either we can try some exclusions/replacements while porting HADOOP-18131 or we can go with current filters in this PR.

@rohit-kb
Copy link
Contributor Author

Hi @ayushtkn , following up on above comment, I think there are two options to proceed further:

  1. One is to cherry-pick HADOOP-18131 but adding log4j exclusions due to log4j in trunk and reload4j in branch-3.3.
  2. Another one is to retain the filters in this PR.

I was thinking of going with second one for the moment as the first one might complicate things further. Please provide your input on the same.

@ayushtkn
Copy link
Member

ok, with the second option but your build is also failing with compilation error

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 7m 35s 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.
+0 🆗 xmllint 0m 0s xmllint was not available.
+0 🆗 shelldocs 0m 0s Shelldocs was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-3.3 Compile Tests _
+0 🆗 mvndep 36m 26s Maven dependency ordering for branch
+1 💚 mvninstall 24m 7s branch-3.3 passed
+1 💚 compile 17m 53s branch-3.3 passed
+1 💚 mvnsite 25m 14s branch-3.3 passed
+1 💚 javadoc 7m 0s branch-3.3 passed
+1 💚 shadedclient 30m 3s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 32s Maven dependency ordering for patch
+1 💚 mvninstall 30m 59s the patch passed
+1 💚 compile 17m 53s the patch passed
+1 💚 javac 17m 53s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 mvnsite 23m 13s the patch passed
+1 💚 shellcheck 0m 0s No new issues.
+1 💚 javadoc 6m 56s the patch passed
+1 💚 shadedclient 30m 1s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 691m 43s /patch-unit-root.txt root in the patch passed.
+1 💚 asflicense 1m 39s The patch does not generate ASF License warnings.
941m 16s
Reason Tests
Failed junit tests hadoop.yarn.server.resourcemanager.scheduler.fair.TestFairSchedulerOvercommit
hadoop.yarn.client.api.impl.TestAMRMClient
hadoop.yarn.sls.TestReservationSystemInvariants
hadoop.hdfs.server.federation.router.TestRouterWithSecureStartup
hadoop.hdfs.server.namenode.ha.TestStandbyCheckpoints
hadoop.hdfs.TestRollingUpgrade
hadoop.hdfs.server.namenode.TestLargeDirectoryDelete
hadoop.hdfs.tools.TestDFSZKFailoverController
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/2/artifact/out/Dockerfile
GITHUB PR #5639
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint shellcheck shelldocs
uname Linux eb4124b5fe03 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision branch-3.3 / b2a3aca
Default Java Private Build-1.8.0_362-8u372-gaus1-0ubuntu118.04-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/2/testReport/
Max. process+thread count 3159 (vs. ulimit of 5500)
modules C: hadoop-project hadoop-client-modules/hadoop-client-runtime hadoop-client-modules/hadoop-client-minicluster . U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/2/console
versions git=2.17.1 maven=3.6.0 shellcheck=0.4.6
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 35s 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.
+0 🆗 xmllint 0m 0s xmllint was not available.
+0 🆗 shelldocs 0m 0s Shelldocs was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-3.3 Compile Tests _
+0 🆗 mvndep 26m 22s Maven dependency ordering for branch
+1 💚 mvninstall 23m 43s branch-3.3 passed
+1 💚 compile 17m 46s branch-3.3 passed
+1 💚 mvnsite 25m 20s branch-3.3 passed
+1 💚 javadoc 7m 2s branch-3.3 passed
+1 💚 shadedclient 29m 59s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 34s Maven dependency ordering for patch
+1 💚 mvninstall 30m 42s the patch passed
+1 💚 compile 17m 26s the patch passed
+1 💚 javac 17m 26s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 mvnsite 20m 39s the patch passed
+1 💚 shellcheck 0m 0s No new issues.
+1 💚 javadoc 6m 41s the patch passed
+1 💚 shadedclient 30m 7s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 689m 34s /patch-unit-root.txt root in the patch passed.
+1 💚 asflicense 1m 40s The patch does not generate ASF License warnings.
918m 16s
Reason Tests
Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithHANameNodes
hadoop.hdfs.server.datanode.TestDataNodeRollingUpgrade
hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery
hadoop.hdfs.server.namenode.ha.TestHAAppend
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/3/artifact/out/Dockerfile
GITHUB PR #5639
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint shellcheck shelldocs
uname Linux b5780c582f46 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision branch-3.3 / b2a3aca
Default Java Private Build-1.8.0_362-8u372-gaus1-0ubuntu118.04-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/3/testReport/
Max. process+thread count 4131 (vs. ulimit of 5500)
modules C: hadoop-project hadoop-client-modules/hadoop-client-runtime hadoop-client-modules/hadoop-client-minicluster . U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/3/console
versions git=2.17.1 maven=3.6.0 shellcheck=0.4.6
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@pjfanning
Copy link
Contributor

@rohit-kb could you rebase this? The issue with the module-info classes has recently been resolved in branch-3.3.

@rohit-kb
Copy link
Contributor Author

Sure @pjfanning, will do, thanks for the info.

pjfanning and others added 4 commits October 18, 2023 14:29
…aded json-smart code. (apache#5573). Contributed by PJ Fanning.

Signed-off-by: Ayush Saxena <ayushsaxena@apache.org>
@pjfanning
Copy link
Contributor

pjfanning commented Oct 18, 2023

@rohit-kb could you remove the edits to the 2 pom files? You have 4 edited files in this PR while you only need 2 (the ones that change the nimbus version). See #5573

And you didn't rebase - so this PR is based on a very of date starting commit - one that doesn't have the module-info class fixes.

@rohit-kb
Copy link
Contributor Author

@pjfanning my bad, kind of went a bit overboard while avoiding those blanks related checks assuming that these changes won't matter.

Also, a bit doubtful regarding the rebase, I did rebase these commits after taking a recent pull from branch-3.3 locally. Should I add a new commit entirely reflecting the recent date for this PR? Thanks.

@pjfanning
Copy link
Contributor

@rohit-kb I created #6201 - feel free to continue with this PR but I created 6201 just in case.

@hadoop-yetus
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 25s 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.
+0 🆗 xmllint 0m 0s xmllint was not available.
+0 🆗 shelldocs 0m 0s Shelldocs was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
-1 ❌ test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ branch-3.3 Compile Tests _
+0 🆗 mvndep 14m 33s Maven dependency ordering for branch
+1 💚 mvninstall 22m 26s branch-3.3 passed
+1 💚 compile 11m 56s branch-3.3 passed
+1 💚 mvnsite 19m 25s branch-3.3 passed
+1 💚 javadoc 5m 5s branch-3.3 passed
+1 💚 shadedclient 27m 35s branch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 28s Maven dependency ordering for patch
+1 💚 mvninstall 32m 48s the patch passed
+1 💚 compile 11m 33s the patch passed
+1 💚 javac 11m 33s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 mvnsite 14m 58s the patch passed
+1 💚 shellcheck 0m 0s No new issues.
+1 💚 javadoc 4m 50s the patch passed
+1 💚 shadedclient 27m 48s patch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌ unit 612m 37s /patch-unit-root.txt root in the patch passed.
+1 💚 asflicense 1m 18s The patch does not generate ASF License warnings.
801m 31s
Reason Tests
Failed junit tests hadoop.hdfs.tools.TestDFSAdmin
hadoop.hdfs.server.datanode.TestDirectoryScanner
hadoop.util.curator.TestZKCuratorManager
hadoop.yarn.sls.TestReservationSystemInvariants
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/4/artifact/out/Dockerfile
GITHUB PR #5639
Optional Tests dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint shellcheck shelldocs
uname Linux 5e824957dafc 4.15.0-213-generic #224-Ubuntu SMP Mon Jun 19 13:30:12 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/bin/hadoop.sh
git revision branch-3.3 / d3d24c6
Default Java Private Build-1.8.0_362-8u372-gaus1-0ubuntu118.04-b09
Test Results https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/4/testReport/
Max. process+thread count 3245 (vs. ulimit of 5500)
modules C: hadoop-project hadoop-client-modules/hadoop-client-runtime hadoop-client-modules/hadoop-client-minicluster . U: .
Console output https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5639/4/console
versions git=2.17.1 maven=3.6.0 shellcheck=0.4.6
Powered by Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@pjfanning
Copy link
Contributor

#6201 was merged - this can be closed

@rohit-kb rohit-kb closed this Dec 8, 2023
@rohit-kb rohit-kb deleted the mr_nimbus branch December 8, 2023 04:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants