From e1ca466bdbf330572eab4772446e004ae5151313 Mon Sep 17 00:00:00 2001
From: Viraj Jasani <vjasani@apache.org>
Date: Wed, 1 Mar 2023 16:02:07 -0800
Subject: [PATCH] HADOOP-18648. Avoid loading kms log4j properties dynamically
 by KMSWebServer (#5441)

---
 .../key/kms/server/KMSConfiguration.java      | 39 +++++++------------
 .../crypto/key/kms/server/KMSWebServer.java   |  2 +-
 .../main/libexec/shellprofile.d/hadoop-kms.sh |  2 +
 3 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSConfiguration.java b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSConfiguration.java
index 35ffb42981..353260f0f9 100644
--- a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSConfiguration.java
+++ b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSConfiguration.java
@@ -20,7 +20,7 @@
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
-import org.apache.log4j.PropertyConfigurator;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -104,8 +104,6 @@ public class KMSConfiguration {
 
   public static final boolean KEY_AUTHORIZATION_ENABLE_DEFAULT = true;
 
-  private static final String LOG4J_PROPERTIES = "kms-log4j.properties";
-
   static {
     Configuration.addDefaultResource(KMS_DEFAULT_XML);
     Configuration.addDefaultResource(KMS_SITE_XML);
@@ -163,31 +161,20 @@ public static boolean isACLsFileNewer(long time) {
     return newer;
   }
 
-  public static void initLogging() {
-    String confDir = System.getProperty(KMS_CONFIG_DIR);
-    if (confDir == null) {
-      throw new RuntimeException("System property '" +
-          KMSConfiguration.KMS_CONFIG_DIR + "' not defined");
+  /**
+   * Validate whether "kms.config.dir" and "log4j.configuration" are defined in the System
+   * properties. If not, abort the KMS WebServer.
+   */
+  public static void validateSystemProps() {
+    if (System.getProperty(KMS_CONFIG_DIR) == null) {
+      String errorMsg = "System property '" + KMS_CONFIG_DIR + "' not defined";
+      System.err.println("Aborting KMSWebServer because " + errorMsg);
+      throw new RuntimeException(errorMsg);
     }
     if (System.getProperty("log4j.configuration") == null) {
-      System.setProperty("log4j.defaultInitOverride", "true");
-      boolean fromClasspath = true;
-      File log4jConf = new File(confDir, LOG4J_PROPERTIES).getAbsoluteFile();
-      if (log4jConf.exists()) {
-        PropertyConfigurator.configureAndWatch(log4jConf.getPath(), 1000);
-        fromClasspath = false;
-      } else {
-        ClassLoader cl = Thread.currentThread().getContextClassLoader();
-        URL log4jUrl = cl.getResource(LOG4J_PROPERTIES);
-        if (log4jUrl != null) {
-          PropertyConfigurator.configure(log4jUrl);
-        }
-      }
-      LOG.debug("KMS log starting");
-      if (fromClasspath) {
-        LOG.warn("Log4j configuration file '{}' not found", LOG4J_PROPERTIES);
-        LOG.warn("Logging with INFO level to standard output");
-      }
+      String errorMsg = "System property 'log4j.configuration' not defined";
+      System.err.println("Aborting KMSWebServer because " + errorMsg);
+      throw new RuntimeException(errorMsg);
     }
   }
 }
diff --git a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java
index a6cab81eb8..5c9f23e9a0 100644
--- a/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java
+++ b/hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebServer.java
@@ -185,7 +185,7 @@ public URL getKMSUrl() {
   }
 
   public static void main(String[] args) throws Exception {
-    KMSConfiguration.initLogging();
+    KMSConfiguration.validateSystemProps();
     StringUtils.startupShutdownMessage(KMSWebServer.class, args, LOG);
     Configuration conf = KMSConfiguration.getKMSConf();
     Configuration sslConf = SSLFactory.readSSLConfiguration(conf, SSLFactory.Mode.SERVER);
diff --git a/hadoop-common-project/hadoop-kms/src/main/libexec/shellprofile.d/hadoop-kms.sh b/hadoop-common-project/hadoop-kms/src/main/libexec/shellprofile.d/hadoop-kms.sh
index 0d084bb36e..b54bf811d6 100755
--- a/hadoop-common-project/hadoop-kms/src/main/libexec/shellprofile.d/hadoop-kms.sh
+++ b/hadoop-common-project/hadoop-kms/src/main/libexec/shellprofile.d/hadoop-kms.sh
@@ -49,6 +49,8 @@ function hadoop_subcommand_kms
     "-Dkms.config.dir=${HADOOP_CONF_DIR}"
   hadoop_add_param HADOOP_OPTS "-Dkms.log.dir=" \
     "-Dkms.log.dir=${HADOOP_LOG_DIR}"
+  hadoop_add_param HADOOP_OPTS "-Dlog4j.configuration=" \
+    "-Dlog4j.configuration=file:${HADOOP_CONF_DIR}/kms-log4j.properties"
 
   if [[ "${HADOOP_DAEMON_MODE}" == "default" ]] ||
      [[ "${HADOOP_DAEMON_MODE}" == "start" ]]; then