drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ve...@apache.org
Subject [2/2] drill git commit: DRILL-3037: Fix impersonation issue with HDFS based filesystem plugins.
Date Wed, 13 May 2015 21:43:14 GMT
DRILL-3037: Fix impersonation issue with HDFS based filesystem plugins.

We have common code where we impersonate the owner of scan (proxy user) while creating
a filesystem object. When impersonation is enabled the proxy user is either query
user or view owner (if the query involves views). When impersonation is disabled the
proxy user is always the user who started Drillbit (processUser). This causes problems
as "processUser" tries to impersonate "processUser" when impersonation is disabled.
HDFS based filesystems which have impersonation disabled prohibit this (even the self
impersonation).

Fix is don't impersonate "processUser". If the given proxy user name is same as
"processUser", then return the process UserGroupInformation directly instead of
proxy UserGroupInformation.


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/0328c4c1
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/0328c4c1
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/0328c4c1

Branch: refs/heads/master
Commit: 0328c4c1b06c00529f17b468b389284cee515a56
Parents: 97a01b2
Author: vkorukanti <venki.korukanti@gmail.com>
Authored: Tue May 12 22:57:11 2015 -0700
Committer: vkorukanti <venki.korukanti@gmail.com>
Committed: Wed May 13 11:53:26 2015 -0700

----------------------------------------------------------------------
 .../drill/exec/util/ImpersonationUtil.java      |  5 ++
 .../impersonation/BaseTestImpersonation.java    | 26 ++++++-
 .../TestImpersonationDisabledWithMiniDFS.java   | 80 ++++++++++++++++++++
 .../TestImpersonationMetadata.java              | 14 ++++
 4 files changed, 121 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/0328c4c1/exec/java-exec/src/main/java/org/apache/drill/exec/util/ImpersonationUtil.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/util/ImpersonationUtil.java
b/exec/java-exec/src/main/java/org/apache/drill/exec/util/ImpersonationUtil.java
index 9997178..499b740 100644
--- a/exec/java-exec/src/main/java/org/apache/drill/exec/util/ImpersonationUtil.java
+++ b/exec/java-exec/src/main/java/org/apache/drill/exec/util/ImpersonationUtil.java
@@ -73,6 +73,11 @@ public class ImpersonationUtil {
         throw new DrillRuntimeException("Invalid value for proxy user name");
       }
 
+      // If the request proxy user is same as process user name, return the process UGI.
+      if (proxyUserName.equals(getProcessUserName())) {
+        return getProcessUserUGI();
+      }
+
       return UserGroupInformation.createProxyUser(proxyUserName, UserGroupInformation.getLoginUser());
     } catch(IOException e) {
       final String errMsg = "Failed to create proxy user UserGroupInformation object: " +
e.getMessage();

http://git-wip-us.apache.org/repos/asf/drill/blob/0328c4c1/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
index 274f5f7..56b7bde 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/BaseTestImpersonation.java
@@ -41,7 +41,23 @@ public class BaseTestImpersonation extends PlanTestBase {
   protected static Configuration conf;
   protected static String miniDfsStoragePath;
 
+  /**
+   * Start a MiniDFS cluster backed Drillbit cluster with impersonation enabled.
+   * @param testClass
+   * @throws Exception
+   */
   protected static void startMiniDfsCluster(String testClass) throws Exception {
+    startMiniDfsCluster(testClass, true);
+  }
+
+  /**
+   * Start a MiniDFS cluster backed Drillbit cluster
+   * @param testClass
+   * @param isImpersonationEnabled Enable impersonation in the cluster?
+   * @throws Exception
+   */
+  protected static void startMiniDfsCluster(
+      final String testClass, final boolean isImpersonationEnabled) throws Exception {
     Preconditions.checkArgument(!Strings.isNullOrEmpty(testClass), "Expected a non-null and
non-empty test class name");
     conf = new Configuration();
 
@@ -50,9 +66,11 @@ public class BaseTestImpersonation extends PlanTestBase {
     miniDfsStoragePath = System.getProperty("java.io.tmpdir") + Path.SEPARATOR + testClass;
     conf.set("hdfs.minidfs.basedir", miniDfsStoragePath);
 
-    // Set the proxyuser settings so that the user who is running the Drillbits/MiniDfs can
impersonate other users.
-    conf.set(String.format("hadoop.proxyuser.%s.hosts", processUser), "*");
-    conf.set(String.format("hadoop.proxyuser.%s.groups", processUser), "*");
+    if (isImpersonationEnabled) {
+      // Set the proxyuser settings so that the user who is running the Drillbits/MiniDfs
can impersonate other users.
+      conf.set(String.format("hadoop.proxyuser.%s.hosts", processUser), "*");
+      conf.set(String.format("hadoop.proxyuser.%s.groups", processUser), "*");
+    }
 
     // Start the MiniDfs cluster
     dfsCluster = new MiniDFSCluster.Builder(conf)
@@ -61,7 +79,7 @@ public class BaseTestImpersonation extends PlanTestBase {
         .build();
 
     final Properties props = cloneDefaultTestConfigProperties();
-    props.setProperty(ExecConstants.IMPERSONATION_ENABLED, "true");
+    props.setProperty(ExecConstants.IMPERSONATION_ENABLED, Boolean.toString(isImpersonationEnabled));
 
     updateTestCluster(1, DrillConfig.create(props));
   }

http://git-wip-us.apache.org/repos/asf/drill/blob/0328c4c1/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationDisabledWithMiniDFS.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationDisabledWithMiniDFS.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationDisabledWithMiniDFS.java
new file mode 100644
index 0000000..e38d6da
--- /dev/null
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationDisabledWithMiniDFS.java
@@ -0,0 +1,80 @@
+/**
+ * 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.drill.exec.impersonation;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import org.apache.drill.exec.store.StoragePluginRegistry;
+import org.apache.drill.exec.store.dfs.FileSystemConfig;
+import org.apache.drill.exec.store.dfs.WorkspaceConfig;
+import org.apache.hadoop.fs.FileSystem;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.Map;
+
+public class TestImpersonationDisabledWithMiniDFS extends BaseTestImpersonation {
+  private static final String MINIDFS_STORAGE_PLUGIN_NAME =
+      "minidfs" + TestImpersonationDisabledWithMiniDFS.class.getSimpleName();
+
+  @BeforeClass
+  public static void addMiniDfsBasedStorage() throws Exception {
+    startMiniDfsCluster(TestImpersonationDisabledWithMiniDFS.class.getSimpleName(), false);
+
+    // Create a HDFS based storage plugin based on local storage plugin and add it to plugin
registry (connection string
+    // for mini dfs is varies for each run).
+    final StoragePluginRegistry pluginRegistry = getDrillbitContext().getStorage();
+    final FileSystemConfig lfsPluginConfig = (FileSystemConfig) pluginRegistry.getPlugin("dfs_test").getConfig();
+
+    final FileSystemConfig miniDfsPluginConfig = new FileSystemConfig();
+    miniDfsPluginConfig.connection = conf.get(FileSystem.FS_DEFAULT_NAME_KEY);
+
+    Map<String, WorkspaceConfig> workspaces = Maps.newHashMap(lfsPluginConfig.workspaces);
+
+    miniDfsPluginConfig.workspaces = workspaces;
+    miniDfsPluginConfig.formats = ImmutableMap.copyOf(lfsPluginConfig.formats);
+    miniDfsPluginConfig.setEnabled(true);
+
+    pluginRegistry.createOrUpdate(MINIDFS_STORAGE_PLUGIN_NAME, miniDfsPluginConfig, true);
+
+    // Create test table in minidfs.tmp schema for use in test queries
+    test(String.format("CREATE TABLE %s.tmp.dfsRegion AS SELECT * FROM cp.`region.json`",
MINIDFS_STORAGE_PLUGIN_NAME));
+  }
+
+  @Test // DRILL-3037
+  public void testSimpleQuery() throws Exception {
+    final String query =
+        String.format("SELECT sales_city, sales_country FROM tmp.dfsRegion ORDER BY region_id
DESC LIMIT 2");
+
+    testBuilder()
+        .optionSettingQueriesForTestQuery(String.format("USE %s", MINIDFS_STORAGE_PLUGIN_NAME))
+        .sqlQuery(query)
+        .unOrdered()
+        .baselineColumns("sales_city", "sales_country")
+        .baselineValues("Santa Fe", "Mexico")
+        .baselineValues("Santa Anita", "Mexico")
+        .go();
+  }
+
+  @AfterClass
+  public static void removeMiniDfsBasedStorage() throws Exception {
+    getDrillbitContext().getStorage().deletePlugin(MINIDFS_STORAGE_PLUGIN_NAME);
+    stopMiniDfsCluster();
+  }
+}

http://git-wip-us.apache.org/repos/asf/drill/blob/0328c4c1/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java
----------------------------------------------------------------------
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java
b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java
index 122542a..27e737d 100644
--- a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java
+++ b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java
@@ -104,6 +104,20 @@ public class TestImpersonationMetadata extends BaseTestImpersonation
{
     createAndAddWorkspace(fs, "drillTestGrp1_700", "/drillTestGrp1_700", (short)0700, user1,
group1, workspaces);
   }
 
+  @Test // DRILL-3037
+  public void testImpersonatingProcessUser() throws Exception {
+    updateClient(processUser);
+
+    // Process user start the mini dfs, he has read/write permissions by default
+    final String viewName = String.format("%s.drillTestGrp0_700.testView", MINIDFS_STORAGE_PLUGIN_NAME);
+    try {
+      test("CREATE VIEW " + viewName + " AS SELECT * FROM cp.`region.json`");
+      test("SELECT * FROM " + viewName + " LIMIT 2");
+    } finally {
+      test("DROP VIEW " + viewName);
+    }
+  }
+
   @Test
   public void testShowFilesInWSWithUserAndGroupPermissionsForQueryUser() throws Exception
{
     updateClient(user1);


Mime
View raw message