Repository: hive Updated Branches: refs/heads/master a6fab1438 -> 4ec47a6e6
HIVE-18248: Clean up parameters (Janaki Lahorani, reviewed by Sahil Takiar) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/4ec47a6e Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/4ec47a6e Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/4ec47a6e Branch: refs/heads/master Commit: 4ec47a6e6ffb6a070295348308e3a8bcbc246190 Parents: a6fab14 Author: Janaki Lahorani <jan...@cloudera.com> Authored: Tue Dec 19 17:11:20 2017 -0800 Committer: Sahil Takiar <stak...@cloudera.com> Committed: Tue Dec 19 17:26:46 2017 -0800 ---------------------------------------------------------------------- .../org/apache/hadoop/hive/conf/HiveConf.java | 9 +- .../apache/hive/jdbc/TestRestrictedList.java | 192 +++++++++++++++++++ .../hadoop/hive/ql/session/SessionState.java | 7 + .../change_hive_hdfs_session_path.q | 2 + .../change_hive_local_session_path.q | 2 + .../change_hive_tmp_table_space.q | 2 + .../change_hive_hdfs_session_path.q.out | 2 + .../change_hive_local_session_path.q.out | 2 + .../change_hive_tmp_table_space.q.out | 2 + 9 files changed, 219 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java ---------------------------------------------------------------------- diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java index 28b52d4..8648a38 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java @@ -3582,6 +3582,7 @@ public class HiveConf extends Configuration { HIVE_MM_AVOID_GLOBSTATUS_ON_S3("hive.mm.avoid.s3.globstatus", true, "Whether to use listFiles (optimized on S3) instead of globStatus when on S3."), + // If a parameter is added to the restricted list, add a test in TestRestrictedList.Java HIVE_CONF_RESTRICTED_LIST("hive.conf.restricted.list", "hive.security.authenticator.manager,hive.security.authorization.manager," + "hive.security.metastore.authorization.manager,hive.security.metastore.authenticator.manager," + @@ -3610,7 +3611,9 @@ public class HiveConf extends Configuration { "bonecp.,"+ "hive.druid.broker.address.default,"+ "hive.druid.coordinator.address.default,"+ - "hikari.", + "hikari.,"+ + "hadoop.bin.path,"+ + "yarn.bin.path", "Comma separated list of configuration options which are immutable at runtime"), HIVE_CONF_HIDDEN_LIST("hive.conf.hidden.list", METASTOREPWD.varname + "," + HIVE_SERVER2_SSL_KEYSTORE_PASSWORD.varname @@ -4752,6 +4755,10 @@ public class HiveConf extends Configuration { hiveSiteURL = location; } + public static void setHivemetastoreSiteUrl(URL location) { + hivemetastoreSiteUrl = location; + } + public static URL getHiveSiteLocation() { return hiveSiteURL; } http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestRestrictedList.java ---------------------------------------------------------------------- diff --git a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestRestrictedList.java b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestRestrictedList.java new file mode 100644 index 0000000..1ec77b0 --- /dev/null +++ b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestRestrictedList.java @@ -0,0 +1,192 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hive.jdbc; + +import java.io.File; +import java.net.URL; +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.Statement; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.HashSet; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.conf.HiveConf.ConfVars; +import org.apache.hive.jdbc.miniHS2.MiniHS2; +import org.junit.AfterClass; +import org.junit.BeforeClass; + +import org.junit.Test; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class TestRestrictedList { + private static MiniHS2 miniHS2 = null; + private static URL oldHiveSiteURL = null; + private static URL oldHiveMetastoreSiteURL = null; + private static Map<String, String> expectedRestrictedMap = new HashMap<>(); + private static HiveConf hiveConf = null; + + @BeforeClass + public static void startServices() throws Exception { + Class.forName(MiniHS2.getJdbcDriverName()); + + oldHiveSiteURL = HiveConf.getHiveSiteLocation(); + oldHiveMetastoreSiteURL = HiveConf.getMetastoreSiteLocation(); + String confDir = "../../data/conf/rlist/"; + HiveConf.setHiveSiteLocation( + new URL("file://" + new File(confDir).toURI().getPath() + "/hive-site.xml")); + System.out.println("Setting hive-site: " + HiveConf.getHiveSiteLocation()); + HiveConf.setHivemetastoreSiteUrl( + new URL("file://" + new File(confDir).toURI().getPath() + "/hivemetastore-site.xml")); + System.out.println("Setting hive-site: " + HiveConf.getHiveSiteLocation()); + + hiveConf = new HiveConf(); + hiveConf.setIntVar(ConfVars.HIVE_SERVER2_THRIFT_MIN_WORKER_THREADS, 1); + hiveConf.setIntVar(ConfVars.HIVE_SERVER2_THRIFT_MAX_WORKER_THREADS, 1); + hiveConf.setBoolVar(ConfVars.HIVE_SUPPORT_CONCURRENCY, false); + + miniHS2 = new MiniHS2.Builder().withMiniMR().withRemoteMetastore().withConf(hiveConf).build(); + miniHS2.start(new HashMap<String, String>()); + + // Add the parameter here if it cannot change at runtime + addToExpectedRestrictedMap("hive.conf.restricted.list"); + addToExpectedRestrictedMap("hive.security.authenticator.manager"); + addToExpectedRestrictedMap("hive.security.authorization.manager"); + addToExpectedRestrictedMap("hive.security.metastore.authorization.manager"); + addToExpectedRestrictedMap("hive.security.metastore.authenticator.manager"); + addToExpectedRestrictedMap("hive.users.in.admin.role"); + addToExpectedRestrictedMap("hive.server2.xsrf.filter.enabled"); + addToExpectedRestrictedMap("hive.security.authorization.enabled"); + addToExpectedRestrictedMap("hive.distcp.privileged.doAs"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.baseDN"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.url"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.Domain"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.groupDNPattern"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.groupFilter"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.userDNPattern"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.userFilter"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.groupMembershipKey"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.userMembershipKey"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.groupClassKey"); + addToExpectedRestrictedMap("hive.server2.authentication.ldap.customLDAPQuery"); + addToExpectedRestrictedMap("hive.spark.client.channel.log.level"); + addToExpectedRestrictedMap("hive.spark.client.secret.bits"); + addToExpectedRestrictedMap("hive.spark.client.rpc.server.address"); + addToExpectedRestrictedMap("hive.spark.client.rpc.server.port"); + addToExpectedRestrictedMap("hive.spark.client.rpc.sasl.mechanisms"); + addToExpectedRestrictedMap("bonecp.test"); + addToExpectedRestrictedMap("hive.druid.broker.address.default"); + addToExpectedRestrictedMap("hive.druid.coordinator.address.default"); + addToExpectedRestrictedMap("hikari.test"); + addToExpectedRestrictedMap("hadoop.bin.path"); + addToExpectedRestrictedMap("yarn.bin.path"); + addToExpectedRestrictedMap("hive.spark.client.connect.timeout"); + addToExpectedRestrictedMap("hive.spark.client.server.connect.timeout"); + addToExpectedRestrictedMap("hive.spark.client.rpc.max.size"); + addToExpectedRestrictedMap("hive.spark.client.rpc.threads"); + addToExpectedRestrictedMap("_hive.local.session.path"); + addToExpectedRestrictedMap("_hive.tmp_table_space"); + addToExpectedRestrictedMap("_hive.hdfs.session.path"); + addToExpectedRestrictedMap("hive.spark.client.rpc.server.address"); + } + + @AfterClass + public static void stopServices() throws Exception { + if (miniHS2 != null && miniHS2.isStarted()) { + miniHS2.stop(); + } + HiveConf.setHivemetastoreSiteUrl(oldHiveMetastoreSiteURL); + HiveConf.setHiveSiteLocation(oldHiveSiteURL); + } + + @Test + public void testRestrictedList() throws Exception { + assertTrue("Test setup failed. MiniHS2 is not initialized", + miniHS2 != null && miniHS2.isStarted()); + + checkRestrictedListMatch(); + + try (Connection hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL(), "hive", "hive"); + Statement stmt = hs2Conn.createStatement();) { + for (Map.Entry<String, String> entry : expectedRestrictedMap.entrySet()) { + String parameter = entry.getKey(); + String value = entry.getValue(); + + try { + stmt.execute("set " + parameter + "=" + value); + fail("Exception not thrown for parameter: " + parameter); + } catch (Exception e1) { + assertTrue("Unexpected exception: " + e1.getMessage(), + e1.getMessage().contains("Error while processing statement: Cannot modify")); + } + } + } catch (Exception e2) { + fail("Unexpected Exception: " + e2.getMessage()); + } + } + + // This test will make sure that every entry in hive.conf.restricted.list, has a test here + private void checkRestrictedListMatch(){ + HiveConf.ConfVars restrictedConfVar = HiveConf.getConfVars("hive.conf.restricted.list"); + String definedRestrictedListString = HiveConf.getVar(hiveConf, restrictedConfVar); + Set<String> definedRestrictedSet = new HashSet<String>(); + + definedRestrictedSet.clear(); + assertTrue(definedRestrictedListString != null); + + // populate definedRestrictedSet with parameters defined in hive.conf.restricted.list + for (String entry : definedRestrictedListString.split(",")) { + definedRestrictedSet.add(entry.trim()); + } + + // remove all parameters that are tested. if the parameter is tested it is part of + // expectedRestrictedMap + definedRestrictedSet.removeAll(expectedRestrictedMap.keySet()); + + // the remaining parameters in definedRestrictedSet are starting parameter name + for (String definedRestrictedParameter : definedRestrictedSet) { + boolean definedRestrictedParameterTested = false; + for (String expectedRestrictedParameter : expectedRestrictedMap.keySet()) { + if (expectedRestrictedParameter.startsWith(definedRestrictedParameter)) { + definedRestrictedParameterTested = true; + break; + } + } + assertTrue(definedRestrictedParameter + " not tested.", definedRestrictedParameterTested); + } + } + + private static void addToExpectedRestrictedMap(String parameter) { + HiveConf.ConfVars confVars = HiveConf.getConfVars(parameter); + String value = "foo"; + + if (confVars != null) { + if (confVars.isType("foo") && confVars.validate("foo") == null) { + value = "foo"; + } else if (confVars.isType("1s") && confVars.validate("1s") == null) { + value = "1s"; + } else if (confVars.isType("1") && confVars.validate("1") == null) { + value = "1"; + } + } + expectedRestrictedMap.put(parameter, value); + } +} http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java index d03f5e3..ab040fe 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java @@ -703,6 +703,13 @@ public class SessionState { // Don't register with deleteOnExit createPath(conf, hdfsTmpTableSpace, scratchDirPermission, false, false); conf.set(TMP_TABLE_SPACE_KEY, hdfsTmpTableSpace.toUri().toString()); + + // _hive.tmp_table_space, _hive.hdfs.session.path, and _hive.local.session.path are respectively + // saved in hdfsTmpTableSpace, hdfsSessionPath and localSessionPath. Saving them as conf + // variables is useful to expose them to end users. But, end users shouldn't change them. + // Adding them to restricted list. + conf.addToRestrictList( + LOCAL_SESSION_PATH_KEY + "," + HDFS_SESSION_PATH_KEY + "," + TMP_TABLE_SPACE_KEY); } /** http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/ql/src/test/queries/clientnegative/change_hive_hdfs_session_path.q ---------------------------------------------------------------------- diff --git a/ql/src/test/queries/clientnegative/change_hive_hdfs_session_path.q b/ql/src/test/queries/clientnegative/change_hive_hdfs_session_path.q new file mode 100644 index 0000000..d3f3a21 --- /dev/null +++ b/ql/src/test/queries/clientnegative/change_hive_hdfs_session_path.q @@ -0,0 +1,2 @@ +set _hive.hdfs.session.path; +set _hive.hdfs.session.path=foo; http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/ql/src/test/queries/clientnegative/change_hive_local_session_path.q ---------------------------------------------------------------------- diff --git a/ql/src/test/queries/clientnegative/change_hive_local_session_path.q b/ql/src/test/queries/clientnegative/change_hive_local_session_path.q new file mode 100644 index 0000000..6477862 --- /dev/null +++ b/ql/src/test/queries/clientnegative/change_hive_local_session_path.q @@ -0,0 +1,2 @@ +set _hive.local.session.path; +set _hive.local.session.path=foo; http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/ql/src/test/queries/clientnegative/change_hive_tmp_table_space.q ---------------------------------------------------------------------- diff --git a/ql/src/test/queries/clientnegative/change_hive_tmp_table_space.q b/ql/src/test/queries/clientnegative/change_hive_tmp_table_space.q new file mode 100644 index 0000000..6fb82f5 --- /dev/null +++ b/ql/src/test/queries/clientnegative/change_hive_tmp_table_space.q @@ -0,0 +1,2 @@ +set _hive.tmp_table_space; +set _hive.tmp_table_space=foo; http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/ql/src/test/results/clientnegative/change_hive_hdfs_session_path.q.out ---------------------------------------------------------------------- diff --git a/ql/src/test/results/clientnegative/change_hive_hdfs_session_path.q.out b/ql/src/test/results/clientnegative/change_hive_hdfs_session_path.q.out new file mode 100644 index 0000000..d69d501 --- /dev/null +++ b/ql/src/test/results/clientnegative/change_hive_hdfs_session_path.q.out @@ -0,0 +1,2 @@ +#### A masked pattern was here #### +Query returned non-zero code: 1, cause: Cannot modify _hive.hdfs.session.path at runtime. It is in the list of parameters that can't be modified at runtime or is prefixed by a restricted variable http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/ql/src/test/results/clientnegative/change_hive_local_session_path.q.out ---------------------------------------------------------------------- diff --git a/ql/src/test/results/clientnegative/change_hive_local_session_path.q.out b/ql/src/test/results/clientnegative/change_hive_local_session_path.q.out new file mode 100644 index 0000000..2a7093f --- /dev/null +++ b/ql/src/test/results/clientnegative/change_hive_local_session_path.q.out @@ -0,0 +1,2 @@ +#### A masked pattern was here #### +Query returned non-zero code: 1, cause: Cannot modify _hive.local.session.path at runtime. It is in the list of parameters that can't be modified at runtime or is prefixed by a restricted variable http://git-wip-us.apache.org/repos/asf/hive/blob/4ec47a6e/ql/src/test/results/clientnegative/change_hive_tmp_table_space.q.out ---------------------------------------------------------------------- diff --git a/ql/src/test/results/clientnegative/change_hive_tmp_table_space.q.out b/ql/src/test/results/clientnegative/change_hive_tmp_table_space.q.out new file mode 100644 index 0000000..9d6abda --- /dev/null +++ b/ql/src/test/results/clientnegative/change_hive_tmp_table_space.q.out @@ -0,0 +1,2 @@ +#### A masked pattern was here #### +Query returned non-zero code: 1, cause: Cannot modify _hive.tmp_table_space at runtime. It is in the list of parameters that can't be modified at runtime or is prefixed by a restricted variable
