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

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

steveloughran commented on code in PR #4537:
URL: https://github.com/apache/hadoop/pull/4537#discussion_r1044590462


##########
hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/util/PlatformName.java:
##########
@@ -33,16 +39,33 @@ public class PlatformName {
    * per the java-vm.
    */
   public static final String PLATFORM_NAME =
-      (System.getProperty("os.name").startsWith("Windows")
-      ? System.getenv("os") : System.getProperty("os.name"))
-      + "-" + System.getProperty("os.arch")
-      + "-" + System.getProperty("sun.arch.data.model");
+      (System.getProperty("os.name").startsWith("Windows") ?
+      System.getenv("os") : System.getProperty("os.name"))
+      + "-" + System.getProperty("os.arch") + "-"
+      + System.getProperty("sun.arch.data.model");
 
   /**
    * The java vendor name used in this platform.
    */
   public static final String JAVA_VENDOR_NAME = 
System.getProperty("java.vendor");
 
+  /**
+   * A concurrently accessible hashmap that saves re-computation of vendor 
checks.
+   */
+  private static final Map<String, Boolean> SYSTEM_CLASS_AVAILABILITY = new 
ConcurrentHashMap<>();

Review Comment:
   so this a static but mutable map. good to see it is concurrent *and* not 
visible outside the class.



##########
hadoop-common-project/hadoop-minikdc/src/test/java/org/apache/hadoop/minikdc/TestMiniKdc.java:
##########
@@ -38,8 +38,35 @@
 import java.util.Arrays;
 
 public class TestMiniKdc extends KerberosSecurityTestcase {
-  private static final boolean IBM_JAVA = System.getProperty("java.vendor")
-      .contains("IBM");
+  private static final boolean IBM_JAVA = shouldUseIbmPackages();
+  // duplicated to avoid cycles in the build
+  private static boolean shouldUseIbmPackages() {
+    final List<String> ibmTechnologyEditionSecurityModules = Arrays.asList(
+        "com.ibm.security.auth.module.JAASLoginModule",
+        "com.ibm.security.auth.module.Win64LoginModule",
+        "com.ibm.security.auth.module.NTLoginModule",
+        "com.ibm.security.auth.module.AIX64LoginModule",
+        "com.ibm.security.auth.module.LinuxLoginModule",
+        "com.ibm.security.auth.module.Krb5LoginModule"
+    );
+
+    if (System.getProperty("java.vendor").contains("IBM")) {
+      return ibmTechnologyEditionSecurityModules
+          .stream().anyMatch((module) -> isSystemClassAvailable(module));
+    }
+
+    return false;
+  }
+
+  private static boolean isSystemClassAvailable(String className) {

Review Comment:
   any reason for not using a copy of isSystemClassAvailable() here ... are you 
just waiting for feedback on the hadoop auth section?



##########
hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/util/PlatformName.java:
##########
@@ -33,21 +37,72 @@ public class PlatformName {
    * per the java-vm.
    */
   public static final String PLATFORM_NAME =
-      (System.getProperty("os.name").startsWith("Windows")
-      ? System.getenv("os") : System.getProperty("os.name"))
-      + "-" + System.getProperty("os.arch")
-      + "-" + System.getProperty("sun.arch.data.model");
+      (System.getProperty("os.name").startsWith("Windows") ?
+      System.getenv("os") : System.getProperty("os.name"))
+      + "-" + System.getProperty("os.arch") + "-"
+      + System.getProperty("sun.arch.data.model");
 
   /**
    * The java vendor name used in this platform.
    */
   public static final String JAVA_VENDOR_NAME = 
System.getProperty("java.vendor");
 
+  /**
+   * Define a system class accessor that is open to changes in underlying 
implementations
+   * of the system class loader modules.
+   */
+  private static final class SystemClassAccessor extends ClassLoader {
+    public Class<?> getSystemClass(String className) throws 
ClassNotFoundException {
+      return findSystemClass(className);
+    }
+  }
+
   /**
    * A public static variable to indicate the current java vendor is
-   * IBM java or not.
+   * IBM and the type is Java Technology Edition which provides its
+   * own implementations of many security packages and Cipher suites.
+   * Note that these are not provided in Semeru runtimes:
+   * See https://developer.ibm.com/languages/java/semeru-runtimes/
+   * The class used is present in any supported IBM JTE Runtimes.

Review Comment:
   not sure what this sentence means. easiest to cut it.





> Add support for IBM Semeru OE JRE 11.0.15.0 and greater
> -------------------------------------------------------
>
>                 Key: HADOOP-18329
>                 URL: https://issues.apache.org/jira/browse/HADOOP-18329
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: auth, common
>    Affects Versions: 3.0.0, 3.1.0, 3.0.1, 3.2.0, 3.0.2, 3.1.1, 3.0.3, 3.3.0, 
> 3.1.2, 3.2.1, 3.1.3, 3.1.4, 3.2.2, 3.3.1, 3.2.3, 3.3.2, 3.3.3
>         Environment: Running Hadoop (or Apache Spark 3.2.1 or above) on IBM 
> Semeru runtimes open edition 11.0.15.0 or greater.
>            Reporter: Jack
>            Priority: Major
>              Labels: pull-request-available
>   Original Estimate: 1h
>          Time Spent: 2.5h
>  Remaining Estimate: 0h
>
> There are checks within the PlatformName class that use the Vendor property 
> of the provided runtime JVM specifically looking for `IBM` within the name. 
> Whilst this check worked for IBM's [java technology 
> edition|https://www.ibm.com/docs/en/sdk-java-technology] it fails to work on 
> [Semeru|https://developer.ibm.com/languages/java/semeru-runtimes/] since 
> 11.0.15.0 due to the following change:
> h4. java.vendor system property
> In this release, the {{java.vendor}} system property has been changed from 
> "International Business Machines Corporation" to "IBM Corporation".
> Modules such as the below are not provided in these runtimes.
> com.ibm.security.auth.module.JAASLoginModule



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

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

Reply via email to