diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConfUtil.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConfUtil.java index ae6fa43df0..ca02f1d88c 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConfUtil.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConfUtil.java @@ -42,6 +42,9 @@ import java.util.StringTokenizer; import java.util.stream.Stream; +import static org.apache.hive.common.util.HiveStringUtils.COMMA; +import static org.apache.hive.common.util.HiveStringUtils.EQUALS; + /** * Hive Configuration utils */ @@ -215,7 +218,7 @@ public static void updateJobCredentialProviders(Configuration jobConf) { // Hide sensitive configuration values from MR HistoryUI by telling MR to redact the following list. jobConf.set(MRJobConfig.MR_JOB_REDACTED_PROPERTIES, - StringUtils.join(redactedProperties, ",")); + StringUtils.join(redactedProperties, COMMA)); } } } @@ -241,14 +244,13 @@ public static String getJobCredentialProviderPassword(Configuration conf) { return null; } - private static void addKeyValuePair(Configuration jobConf, String property, String keyName, - String newKeyValue) { + private static void addKeyValuePair(Configuration jobConf, String property, String keyName, String newKeyValue) { String existingValue = jobConf.get(property); - if (existingValue == null) { - jobConf.set(property, (keyName + "=" + newKeyValue)); + + if (StringUtils.isBlank(existingValue)) { + jobConf.set(property, (keyName + EQUALS + newKeyValue)); return; } - String propertyValue = HiveStringUtils.insertValue(keyName, newKeyValue, existingValue); jobConf.set(property, propertyValue); } diff --git a/common/src/java/org/apache/hive/common/util/HiveStringUtils.java b/common/src/java/org/apache/hive/common/util/HiveStringUtils.java index a4923f9f1b..196b9c457b 100644 --- a/common/src/java/org/apache/hive/common/util/HiveStringUtils.java +++ b/common/src/java/org/apache/hive/common/util/HiveStringUtils.java @@ -510,33 +510,41 @@ public static String getFormattedTimeWithDiff(DateFormat dateFormat, } /** - * In a given string of comma-separated key=value pairs insert a new value of a given key + * In a given string of comma-separated key=value pairs associates the specified value with + * the specified key. + * If the `string` previously contained a mapping for the key, the old value is replaced. * - * @param key The key whose value needs to be replaced - * @param newValue The new value of the key + * @param key key with which the specified value is to be associated + * @param value value to be associated with the specified key * @param strKvPairs Comma separated key=value pairs Eg: "k1=v1, k2=v2, k3=v3" - * @return Comma separated string of key=value pairs with the new value for key keyName + * @return Updated comma separated string of key=value pairs */ - public static String insertValue(String key, String newValue, - String strKvPairs) { + public static String insertValue(String key, String value, String strKvPairs) { + boolean keyNotFound = true; + String[] keyValuePairs = HiveStringUtils.split(strKvPairs); StringBuilder sb = new StringBuilder(); + for (int i = 0; i < keyValuePairs.length; i++) { String[] pair = HiveStringUtils.split(keyValuePairs[i], ESCAPE_CHAR, EQUALS); if (pair.length != 2) { throw new RuntimeException("Error parsing the keyvalue pair " + keyValuePairs[i]); } - sb.append(pair[0]); - sb.append(EQUALS); + sb.append(pair[0]).append(EQUALS); if (pair[0].equals(key)) { - sb.append(newValue); + sb.append(value); + keyNotFound = false; } else { sb.append(pair[1]); } - if (i < (keyValuePairs.length - 1)) { + if (i < (keyValuePairs.length - 1) || keyNotFound) { sb.append(COMMA); } } + + if (keyNotFound) { + sb.append(key).append(EQUALS).append(value); + } return sb.toString(); } diff --git a/ql/src/test/org/apache/hadoop/hive/ql/exec/TestHiveCredentialProviders.java b/ql/src/test/org/apache/hadoop/hive/ql/exec/TestHiveCredentialProviders.java index 4f49190df0..7af8542d04 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/exec/TestHiveCredentialProviders.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/exec/TestHiveCredentialProviders.java @@ -50,7 +50,8 @@ private static final Collection REDACTED_PROPERTIES = Arrays.asList( JobConf.MAPRED_MAP_TASK_ENV, - JobConf.MAPRED_REDUCE_TASK_ENV); + JobConf.MAPRED_REDUCE_TASK_ENV, + "yarn.app.mapreduce.am.admin.user.env"); private Configuration jobConf; @@ -102,6 +103,9 @@ public void testJobCredentialProvider() throws Exception { Assert.assertEquals(HIVE_JOB_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertEquals(HIVE_JOB_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( + jobConf.get("yarn.app.mapreduce.am.admin.user.env"), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertTrue(jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) .containsAll(REDACTED_PROPERTIES)); } @@ -126,6 +130,9 @@ public void testHadoopCredentialProvider() throws Exception { Assert.assertEquals(HADOOP_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertEquals(HADOOP_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( + jobConf.get("yarn.app.mapreduce.am.admin.user.env"), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertTrue(jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) .containsAll(REDACTED_PROPERTIES)); } @@ -146,6 +153,9 @@ public void testNoCredentialProviderWithPassword() throws Exception { Assert.assertNull(getValueFromJobConf(jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertNull(getValueFromJobConf(jobConf.get("yarn.app.mapreduce.am.admin.user.env"), + HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + REDACTED_PROPERTIES.forEach(property -> Assert.assertFalse( jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) .contains(property))); @@ -169,6 +179,9 @@ public void testJobCredentialProviderWithDefaultPassword() throws Exception { Assert.assertEquals(HADOOP_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertEquals(HADOOP_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( + jobConf.get("yarn.app.mapreduce.am.admin.user.env"), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertTrue(jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) .containsAll(REDACTED_PROPERTIES)); } @@ -186,6 +199,7 @@ public void testCredentialProviderWithNoPasswords() throws Exception { jobConf.get(HADOOP_CREDENTIAL_PROVIDER_PATH_CONFIG)); Assert.assertNull(jobConf.get(JobConf.MAPRED_MAP_TASK_ENV)); Assert.assertNull(jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV)); + Assert.assertNull(jobConf.get("yarn.app.mapreduce.am.admin.user.env")); REDACTED_PROPERTIES.forEach(property -> Assert.assertFalse( jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) @@ -199,6 +213,7 @@ public void testCredentialProviderWithNoPasswords() throws Exception { jobConf.get(HADOOP_CREDENTIAL_PROVIDER_PATH_CONFIG)); Assert.assertNull(jobConf.get(JobConf.MAPRED_MAP_TASK_ENV)); Assert.assertNull(jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV)); + Assert.assertNull(jobConf.get("yarn.app.mapreduce.am.admin.user.env")); REDACTED_PROPERTIES.forEach(property -> Assert.assertFalse( jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) @@ -223,6 +238,9 @@ public void testJobCredentialProviderUnset() throws Exception { assertEquals(HADOOP_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertEquals(HADOOP_CREDSTORE_PASSWORD_ENVVAR_VAL, getValueFromJobConf( + jobConf.get("yarn.app.mapreduce.am.admin.user.env"), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + Assert.assertTrue(jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) .containsAll(REDACTED_PROPERTIES)); } @@ -243,6 +261,9 @@ public void testNoCredentialProvider() throws Exception { assertNull(getValueFromJobConf(jobConf.get(JobConf.MAPRED_REDUCE_TASK_ENV), HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + assertNull(getValueFromJobConf(jobConf.get("yarn.app.mapreduce.am.admin.user.env"), + HADOOP_CREDENTIAL_PASSWORD_ENVVAR)); + REDACTED_PROPERTIES.forEach(property -> Assert.assertFalse( jobConf.getStringCollection(MRJobConfig.MR_JOB_REDACTED_PROPERTIES) .contains(property)));