[ 
https://issues.apache.org/jira/browse/HADOOP-19971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18138168#comment-18138168
 ] 

ASF GitHub Bot commented on HADOOP-19971:
-----------------------------------------

hadoop-yetus commented on PR #8703:
URL: https://github.com/apache/hadoop/pull/8703#issuecomment-6639834490

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |  19m 19s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  |  The patch appears to 
include 5 new or modified test files.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   2m 37s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  53m  0s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  17m 55s |  |  trunk passed with JDK 
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu  |
   | +1 :green_heart: |  compile  |  18m 27s |  |  trunk passed with JDK 
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu  |
   | +1 :green_heart: |  checkstyle  |   6m  3s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m 36s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   2m 31s |  |  trunk passed with JDK 
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu  |
   | +1 :green_heart: |  javadoc  |   2m 22s |  |  trunk passed with JDK 
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu  |
   | +1 :green_heart: |  spotbugs  |   3m 13s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  34m 39s |  |  branch has no errors 
when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 30s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  6s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  17m  0s |  |  the patch passed with JDK 
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu  |
   | +1 :green_heart: |  javac  |  17m  0s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  18m 27s |  |  the patch passed with JDK 
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu  |
   | +1 :green_heart: |  javac  |  18m 27s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   5m 52s |  |  root: The patch generated 
0 new + 28 unchanged - 1 fixed = 28 total (was 29)  |
   | +1 :green_heart: |  mvnsite  |   2m 34s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 25s |  |  the patch passed with JDK 
Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu  |
   | +1 :green_heart: |  javadoc  |   2m 22s |  |  the patch passed with JDK 
Ubuntu-17.0.20.1+1-1-24.04-Ubuntu  |
   | +1 :green_heart: |  spotbugs  |   3m 49s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  35m 37s |  |  patch has no errors 
when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   3m 53s |  |  hadoop-auth in the patch 
passed.  |
   | +1 :green_heart: |  unit  |   1m 22s |  |  hadoop-mapreduce-client-shuffle 
in the patch passed.  |
   | +1 :green_heart: |  unit  |   2m 37s |  |  hadoop-yarn-services-api in the 
patch passed.  |
   | +1 :green_heart: |  asflicense  |   1m 17s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 270m 53s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.56 ServerAPI=1.56 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8703/9/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/8703 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 0a1991644fee 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 
15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 1ac936850e8000a3bb5be6488020cff9e9b3cd98 |
   | Default Java | Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
   | Multi-JDK versions | 
/usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu 
/usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
   |  Test Results | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8703/9/testReport/ |
   | Max. process+thread count | 604 (vs. ulimit of 10000) |
   | modules | C: hadoop-common-project/hadoop-auth 
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle
 
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-api
 U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8703/9/console |
   | versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
   | Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   




> Remove servlet and Jetty types from the secret providers, CertificateUtil, 
> the shuffle handler and the YARN services client
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-19971
>                 URL: https://issues.apache.org/jira/browse/HADOOP-19971
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: auth, common
>            Reporter: Jose Luis López
>            Assignee: Jose Luis López
>            Priority: Minor
>              Labels: pull-request-available
>
> h3. Why
> Hadoop will eventually move from the javax.servlet namespace to 
> jakarta.servlet. When it does, every class that mentions a servlet type has 
> to be revisited, and every class that mentions a Jetty type has to be 
> revisited again when the Jetty API changes underneath it.
> Several of those classes have no real connection to either. A component that 
> generates signing secrets was handed the web server's context even though it 
> only wanted somewhere to store an object. A certificate parser reported a bad 
> certificate as a web server error. A Netty-based shuffle handler borrowed two 
> header names from Jetty, and the YARN services client borrowed Jetty's URL 
> encoder.
> This change removes those references, so those classes drop out of the later 
> migration entirely.
> h3. What changes for anyone using Hadoop
> Nothing is removed, no signature changes shape, and no dependency, version or 
> setting changes. Code built against today's release keeps compiling and 
> running without being rebuilt.
> * Two entry points get a servlet-free alternative. The old ones stay until 
> the namespace change:
> ** {{SignerSecretProvider.init(Properties, ServletContext, long)}} -> 
> {{initialize(Properties, SecretProviderContext, long)}}. {{init}} is not 
> deprecated: every caller still has to go through it, because a caller that 
> called {{initialize}} directly would bypass a provider that overrides only 
> {{init}} and hit the default {{initialize}}, which throws.
> ** {{CertificateUtil.parseRSAPublicKey(String)}} -> 
> {{toRSAPublicKey(String)}}. {{parseRSAPublicKey}} is deprecated.
> * Providers that override {{init}} are still called directly. A provider that 
> extends Hadoop's rolling secret provider and calls {{super.init}} still gets 
> its rollover scheduler started. {{TestSignerSecretProviderCompatibility}} 
> pins this contract.
> * Exceptions are unchanged: same type, message and cause chain from 
> {{parseRSAPublicKey}} and from {{JWTRedirectAuthenticationHandler}} when the 
> PEM is corrupt.
> * A valid certificate whose public key is not RSA used to throw an unchecked 
> {{ClassCastException}}. {{toRSAPublicKey}} now throws 
> {{CertificateException}}, and {{parseRSAPublicKey}} reports it as 
> {{ServletException}}, with a message naming the key algorithm.
> * The shuffle response is unchanged, byte for byte: the same {{Connection}} 
> and {{Keep-Alive}} header names, capitalised as before.
> * The YARN services client now encodes {{user.name}} with 
> {{java.net.URLEncoder}}. That differs from Jetty's encoder for two characters 
> only: {{*}} is now sent as is where it used to be escaped, and {{~}} is now 
> escaped where it used to be sent as is. Every code point decodes to the same 
> value either way (checked across all of Unicode against Jetty 9.4.58), so the 
> server reads the same user name.
> Things a downstream project could notice:
> * A deprecation warning where code calls {{parseRSAPublicKey}}. Overriding 
> {{init}} produces none, because {{init}} is not deprecated.
> * A provider that implements neither {{init}} nor {{initialize}} now compiles 
> and fails at startup with {{UnsupportedOperationException}}, instead of 
> failing at build time.
> * {{getDeclaredMethod("init", ...)}} on the shipped providers now throws 
> {{NoSuchMethodException}}, because they declare {{initialize}} instead. 
> {{getMethod}} still finds {{init}}.
> * {{ZKSignerSecretProvider}} given a null ServletContext still throws a 
> NullPointerException on the first attribute access, as on trunk, but the 
> message now names the attribute. Every caller in Hadoop passes a real 
> context, and there the client is still stored as the ServletContext attribute 
> of the same name.
> h3. What changes inside
> || || before || after ||
> | modules using Jetty in main sources | 12 | 11 |
> | main-source files using Jetty | 26 | 24 |
> | hadoop-auth main classes naming a servlet | 14 | 11 |
> Counted on trunk at d2dd1a9736f. On top of HADOOP-19972 (3ca2871728d) the 
> same counts are 11 -> 10, 23 -> 21 and 15 -> 12.
> The MapReduce shuffle module no longer uses Jetty at all. In hadoop-auth, the 
> concrete secret providers ({{FileSignerSecretProvider}}, 
> {{RolloverSignerSecretProvider}}, {{ZKSignerSecretProvider}}, 
> {{RandomSignerSecretProvider}}) and {{AuthenticationToken}} no longer mention 
> the servlet API. They are initialised against a small two-method store, 
> {{SecretProviderContext}}, which holds whatever needs sharing; that is all 
> the one provider that used the servlet context ever did with it. The abstract 
> {{SignerSecretProvider}} keeps {{init(Properties, ServletContext, long)}} as 
> the compatibility bridge every caller still uses, and that is the only 
> servlet type left in the provider hierarchy. One new package-private class, 
> {{ServletSecretProviderContext}}, mentions the servlet API, and it exists 
> only to keep the old path working.
> h3. What this does not do
> No namespace change, no Jetty upgrade, no Jersey change, no EE environments. 
> The tree stays on Jetty 9.4, Jersey 2 and javax.servlet.
> The authentication handler classes still mention the servlet API, 
> deliberately. They are the extension point that HBase, Hive, Spark, Ozone and 
> Knox build against, and changing it is exactly the kind of break this issue 
> is meant to avoid. That belongs with the namespace change, where downstream 
> projects will expect it.
> h3. Ordering
> This is preparation for the jakarta move. It depends on nothing, is based on 
> trunk, and nothing in HADOOP-19972 depends on it. The plan is to land it with 
> or just before the namespace change in HADOOP-19395, in the next major 
> release.
> When rebasing onto a later trunk:
> * The {{jetty-http}} declaration in the shuffle module's pom comes from 
> HADOOP-20016 (as {{<optional>true</optional>}}) and from HADOOP-19972, not 
> from HADOOP-19970. It is there only because of the import this change 
> removes. When this branch is rebased after HADOOP-20016 lands, drop that 
> declaration here; the change then touches 17 files and includes one 
> dependency change.
> * The servlet API move in HADOOP-19970 (now 
> {{javax.servlet:javax.servlet-api}} 4.0.1, plus {{jetty-schemas}} 4.0.3 on 
> Jetty 12 ee8) does not affect this change.
> * If HADOOP-19972 has landed, re-check the shuffle headers and the 
> {{user.name}} encoding against whatever Jetty classes it left in those two 
> places.
> 16 files changed. Works on Jetty 9.4 and on the Jetty 12 ee8 environment of 
> HADOOP-19912, and commits the namespace change in HADOOP-19395 to neither 
> ee10 nor ee11.
> Contains content generated by Claude.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to