You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2018/10/17 23:03:07 UTC

[3/3] kudu git commit: HMS integration: provide Java API to override owner during table create

HMS integration: provide Java API to override owner during table create

This is necessary so that Impala can override the table owner, instead
of using the inferred username of the connection (which would be
'impala'). This API is marked as unstable since it's not clear we will
want to support it long term, and there are some open questions about
the fine-grained privileges required to use it.

Change-Id: I21069d81a4b54e55c399933b69789bf83bd58de0
Reviewed-on: http://gerrit.cloudera.org:8080/11413
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/082bbfe2
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/082bbfe2
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/082bbfe2

Branch: refs/heads/master
Commit: 082bbfe2048279518a9103e4edcd91f815705294
Parents: 1e8ef0e
Author: Dan Burkert <da...@apache.org>
Authored: Mon Sep 10 16:02:54 2018 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Wed Oct 17 23:02:37 2018 +0000

----------------------------------------------------------------------
 java/kudu-client/build.gradle                   |  1 +
 .../apache/kudu/client/CreateTableOptions.java  | 23 +++++++
 .../client/TestHiveMetastoreIntegration.java    | 72 ++++++++++++++++++++
 src/kudu/master/catalog_manager.cc              |  4 +-
 src/kudu/master/master.proto                    |  4 ++
 5 files changed, 102 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/082bbfe2/java/kudu-client/build.gradle
----------------------------------------------------------------------
diff --git a/java/kudu-client/build.gradle b/java/kudu-client/build.gradle
index 077e050..363135b 100644
--- a/java/kudu-client/build.gradle
+++ b/java/kudu-client/build.gradle
@@ -37,6 +37,7 @@ dependencies {
 
   testCompile project(":kudu-test-utils")
   testCompile libs.hamcrestCore
+  testCompile libs.hiveMetastore
   testCompile libs.junit
   testCompile libs.log4j
   testCompile libs.mockitoCore

http://git-wip-us.apache.org/repos/asf/kudu/blob/082bbfe2/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java b/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
index cb2ba52..2f137a8 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
@@ -19,6 +19,8 @@ package org.apache.kudu.client;
 
 import java.util.List;
 
+import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -209,6 +211,27 @@ public class CreateTableOptions {
     return this;
   }
 
+  /**
+   * Set the table owner as the provided username in configured external catalogs
+   * such as the Hive Metastore. Overrides the default of the currently logged-in
+   * username or Kerberos principal.
+   *
+   * This is an unstable method because it is not yet clear whether this should
+   * be supported directly in the long run, rather than requiring the table creator
+   * to re-assign ownership explicitly.
+   *
+   * @param owner the username to set as the table owner in external catalogs
+   * @return this instance
+   */
+  @InterfaceAudience.LimitedPrivate("Impala")
+  @InterfaceStability.Unstable
+  public CreateTableOptions setOwner(String owner) {
+    Preconditions.checkArgument(!Strings.isNullOrEmpty(owner),
+                                "table owner must not be null or empty");
+    pb.setOwner(owner);
+    return this;
+  }
+
   Master.CreateTableRequestPB.Builder getBuilder() {
     if (!splitRows.isEmpty() || !rangePartitions.isEmpty()) {
       pb.setSplitRowsRangeBounds(new Operation.OperationsEncoder()

http://git-wip-us.apache.org/repos/asf/kudu/blob/082bbfe2/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java
new file mode 100644
index 0000000..daf2396
--- /dev/null
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestHiveMetastoreIntegration.java
@@ -0,0 +1,72 @@
+// 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.kudu.client;
+
+import static org.junit.Assert.assertEquals;
+
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
+import org.apache.hadoop.hive.metastore.IMetaStoreClient;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.kudu.test.ClientTestUtil;
+import org.apache.kudu.test.KuduTestHarness;
+
+public class TestHiveMetastoreIntegration {
+
+  @Rule
+  public KuduTestHarness harness = new KuduTestHarness(
+      KuduTestHarness.getBaseClusterBuilder().enableHiveMetastoreIntegration());
+
+  private KuduClient client;
+
+  @Before
+  public void setUp() {
+    client = harness.getClient();
+  }
+
+  @Test(timeout = 100000)
+  public void testOverrideTableOwner() throws Exception {
+    // Create a table with an overridden owner.
+    String tableName = "default.testOverrideTableOwner";
+    String owner = "alice";
+    CreateTableOptions options = ClientTestUtil.getBasicCreateTableOptions();
+    options.setOwner(owner);
+    client.createTable(tableName, ClientTestUtil.getBasicSchema(), options);
+
+    // Create an HMS client. Kudu doesn't provide an API to look up the table owner,
+    // so it's necessary to use the HMS APIs directly.
+    HiveMetastoreConfig hmsConfig = client.getHiveMetastoreConfig();
+    HiveConf hiveConf = new HiveConf();
+    hiveConf.setVar(HiveConf.ConfVars.METASTOREURIS, hmsConfig.getHiveMetastoreUris());
+    hiveConf.setBoolVar(
+        HiveConf.ConfVars.METASTORE_USE_THRIFT_SASL,
+        hmsConfig.getHiveMetastoreSaslEnabled());
+
+    // Check that the owner of the table in the HMS matches.
+    IMetaStoreClient hmsClient = new HiveMetaStoreClient(hiveConf);
+    assertEquals(owner, hmsClient.getTable("default", "testOverrideTableOwner").getOwner());
+
+    // Altering the table should not result in a change of ownership.
+    client.alterTable(
+        tableName, new AlterTableOptions().renameTable("default.testOverrideTableOwner_renamed"));
+    assertEquals(owner, hmsClient.getTable("default", "testOverrideTableOwner_renamed").getOwner());
+  }
+}

http://git-wip-us.apache.org/repos/asf/kudu/blob/082bbfe2/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index be42e47..bffb4bc 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1574,8 +1574,8 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
   // It is critical that this step happen before writing the table to the sys catalog,
   // since this step validates that the table name is available in the HMS catalog.
   if (hms_catalog_) {
-    Status s = hms_catalog_->CreateTable(table->id(), normalized_table_name,
-                                         rpc->remote_user().username(), schema);
+    string owner = req.has_owner() ? req.owner() : rpc->remote_user().username();
+    Status s = hms_catalog_->CreateTable(table->id(), normalized_table_name, owner, schema);
     if (!s.ok()) {
       s = s.CloneAndPrepend(Substitute("an error occurred while creating table $0 in the HMS",
                                        normalized_table_name));

http://git-wip-us.apache.org/repos/asf/kudu/blob/082bbfe2/src/kudu/master/master.proto
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index 4eb6156..b525786 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -420,6 +420,10 @@ message CreateTableRequestPB {
   optional RowOperationsPB split_rows_range_bounds = 6;
   optional PartitionSchemaPB partition_schema = 7;
   optional int32 num_replicas = 4;
+
+  // If set, uses the provided value as the table owner when creating the table
+  // in external catalogs such as the Hive Metastore.
+  optional string owner = 8;
 }
 
 message CreateTableResponsePB {