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:05 UTC

[1/3] kudu git commit: sentry module: introduce using declarations for generated types

Repository: kudu
Updated Branches:
  refs/heads/master 36d09f187 -> 082bbfe20


sentry module: introduce using declarations for generated types

This cleans up the .cc files in the sentry module with 'using'
declarations, as suggested by Adar.

Change-Id: I51f61eedc1069d362fa1ce26ce0449cce8f0846b
Reviewed-on: http://gerrit.cloudera.org:8080/11712
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: 7607e2354bf61957e4d3168d75d4990169dc2472
Parents: 36d09f1
Author: Dan Burkert <da...@apache.org>
Authored: Wed Oct 17 12:46:03 2018 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Wed Oct 17 23:02:24 2018 +0000

----------------------------------------------------------------------
 src/kudu/sentry/sentry_client-test.cc    | 55 ++++++++++++++++-----------
 src/kudu/sentry/sentry_client.cc         | 55 ++++++++++++++++-----------
 src/kudu/sentry/thrift_operators-test.cc | 34 +++++++----------
 3 files changed, 79 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/7607e235/src/kudu/sentry/sentry_client-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/sentry_client-test.cc b/src/kudu/sentry/sentry_client-test.cc
index d8c845d..10a4134 100644
--- a/src/kudu/sentry/sentry_client-test.cc
+++ b/src/kudu/sentry/sentry_client-test.cc
@@ -35,6 +35,17 @@
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
 
+using sentry::TAlterSentryRoleAddGroupsRequest;
+using sentry::TAlterSentryRoleAddGroupsResponse;
+using sentry::TAlterSentryRoleGrantPrivilegeRequest;
+using sentry::TAlterSentryRoleGrantPrivilegeResponse;
+using sentry::TCreateSentryRoleRequest;
+using sentry::TDropSentryRoleRequest;
+using sentry::TListSentryPrivilegesRequest;
+using sentry::TListSentryPrivilegesResponse;
+using sentry::TSentryAuthorizable;
+using sentry::TSentryGroup;
+using sentry::TSentryPrivilege;
 using std::set;
 using std::string;
 using std::unique_ptr;
@@ -106,12 +117,12 @@ TEST_P(SentryClientTest, TestMiniSentryLifecycle) {
                          sentry_client_opts));
   auto smoketest = [&]() -> Status {
     return client.Execute([](SentryClient* client) -> Status {
-        ::sentry::TCreateSentryRoleRequest create_req;
+        TCreateSentryRoleRequest create_req;
         create_req.requestorUserName = "test-admin";
         create_req.roleName = "test-role";
         RETURN_NOT_OK(client->CreateRole(create_req));
 
-        ::sentry::TDropSentryRoleRequest drop_req;
+        TDropSentryRoleRequest drop_req;
         drop_req.requestorUserName = "test-admin";
         drop_req.roleName = "test-role";
         RETURN_NOT_OK(client->DropRole(drop_req));
@@ -137,7 +148,7 @@ TEST_P(SentryClientTest, TestMiniSentryLifecycle) {
 TEST_P(SentryClientTest, TestCreateDropRole) {
 
   { // Create a role
-    ::sentry::TCreateSentryRoleRequest req;
+    TCreateSentryRoleRequest req;
     req.requestorUserName = "test-admin";
     req.roleName = "viewer";
     ASSERT_OK(sentry_client_->CreateRole(req));
@@ -148,7 +159,7 @@ TEST_P(SentryClientTest, TestCreateDropRole) {
   }
 
   { // Attempt to create a role as a non-admin user.
-    ::sentry::TCreateSentryRoleRequest req;
+    TCreateSentryRoleRequest req;
     req.requestorUserName = "joe-interloper";
     req.roleName = "fuzz";
     Status s = sentry_client_->CreateRole(req);
@@ -156,7 +167,7 @@ TEST_P(SentryClientTest, TestCreateDropRole) {
   }
 
   { // Attempt to drop the role as a non-admin user.
-    ::sentry::TDropSentryRoleRequest req;
+    TDropSentryRoleRequest req;
     req.requestorUserName = "joe-interloper";
     req.roleName = "viewer";
     Status s = sentry_client_->DropRole(req);
@@ -164,7 +175,7 @@ TEST_P(SentryClientTest, TestCreateDropRole) {
   }
 
   { // Drop the role
-    ::sentry::TDropSentryRoleRequest req;
+    TDropSentryRoleRequest req;
     req.requestorUserName = "test-admin";
     req.roleName = "viewer";
     ASSERT_OK(sentry_client_->DropRole(req));
@@ -180,15 +191,15 @@ TEST_P(SentryClientTest, TestCreateDropRole) {
 // instances.
 TEST_P(SentryClientTest, TestListPrivilege) {
   // Attempt to access Sentry privileges by a non admin user.
-  ::sentry::TSentryAuthorizable authorizable;
+  TSentryAuthorizable authorizable;
   authorizable.server = "server";
   authorizable.db = "db";
   authorizable.table = "table";
-  ::sentry::TListSentryPrivilegesRequest request;
+  TListSentryPrivilegesRequest request;
   request.requestorUserName = "joe-interloper";
   request.authorizableHierarchy = authorizable;
   request.__set_principalName("viewer");
-  ::sentry::TListSentryPrivilegesResponse response;
+  TListSentryPrivilegesResponse response;
   Status s = sentry_client_->ListPrivilegesByUser(request, &response);
   ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
 
@@ -214,13 +225,13 @@ TEST_P(SentryClientTest, TestListPrivilege) {
 TEST_P(SentryClientTest, TestGrantRole) {
   // Attempt to alter role by a non admin user.
 
-  ::sentry::TSentryGroup group;
+  TSentryGroup group;
   group.groupName = "user";
-  set<::sentry::TSentryGroup> groups;
+  set<TSentryGroup> groups;
   groups.insert(group);
 
-  ::sentry::TAlterSentryRoleAddGroupsRequest group_request;
-  ::sentry::TAlterSentryRoleAddGroupsResponse group_response;
+  TAlterSentryRoleAddGroupsRequest group_request;
+  TAlterSentryRoleAddGroupsResponse group_response;
   group_request.requestorUserName = "joe-interloper";
   group_request.roleName = "viewer";
   group_request.groups = groups;
@@ -234,7 +245,7 @@ TEST_P(SentryClientTest, TestGrantRole) {
   ASSERT_TRUE(s.IsNotFound()) << s.ToString();
 
   // Alter role 'viewer' to add group 'user'.
-  ::sentry::TCreateSentryRoleRequest role_request;
+  TCreateSentryRoleRequest role_request;
   role_request.requestorUserName = "test-admin";
   role_request.roleName = "viewer";
   ASSERT_OK(sentry_client_->CreateRole(role_request));
@@ -247,29 +258,29 @@ TEST_P(SentryClientTest, TestGrantRole) {
 TEST_P(SentryClientTest, TestGrantPrivilege) {
   // Alter role 'viewer' to add group 'user'.
 
-  ::sentry::TSentryGroup group;
+  TSentryGroup group;
   group.groupName = "user";
-  set<::sentry::TSentryGroup> groups;
+  set<TSentryGroup> groups;
   groups.insert(group);
 
-  ::sentry::TAlterSentryRoleAddGroupsRequest group_request;
-  ::sentry::TAlterSentryRoleAddGroupsResponse group_response;
+  TAlterSentryRoleAddGroupsRequest group_request;
+  TAlterSentryRoleAddGroupsResponse group_response;
   group_request.requestorUserName = "test-admin";
   group_request.roleName = "viewer";
   group_request.groups = groups;
 
-  ::sentry::TCreateSentryRoleRequest role_request;
+  TCreateSentryRoleRequest role_request;
   role_request.requestorUserName = "test-admin";
   role_request.roleName = "viewer";
   ASSERT_OK(sentry_client_->CreateRole(role_request));
   ASSERT_OK(sentry_client_->AlterRoleAddGroups(group_request, &group_response));
 
   // Attempt to alter role by a non admin user.
-  ::sentry::TAlterSentryRoleGrantPrivilegeRequest privilege_request;
-  ::sentry::TAlterSentryRoleGrantPrivilegeResponse privilege_response;
+  TAlterSentryRoleGrantPrivilegeRequest privilege_request;
+  TAlterSentryRoleGrantPrivilegeResponse privilege_response;
   privilege_request.requestorUserName = "joe-interloper";
   privilege_request.roleName = "viewer";
-  ::sentry::TSentryPrivilege privilege;
+  TSentryPrivilege privilege;
   privilege.serverName = "server";
   privilege.dbName = "db";
   privilege.tableName = "table";

http://git-wip-us.apache.org/repos/asf/kudu/blob/7607e235/src/kudu/sentry/sentry_client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/sentry_client.cc b/src/kudu/sentry/sentry_client.cc
index ff29e23..919fb07 100644
--- a/src/kudu/sentry/sentry_client.cc
+++ b/src/kudu/sentry/sentry_client.cc
@@ -43,6 +43,19 @@ using apache::thrift::protocol::TMultiplexedProtocol;
 using apache::thrift::transport::TTransportException;
 using kudu::thrift::CreateClientProtocol;
 using kudu::thrift::SaslException;
+using sentry::SentryPolicyServiceClient;
+using sentry::TAlterSentryRoleAddGroupsRequest;
+using sentry::TAlterSentryRoleAddGroupsResponse;
+using sentry::TAlterSentryRoleGrantPrivilegeRequest;
+using sentry::TAlterSentryRoleGrantPrivilegeResponse;
+using sentry::TCreateSentryRoleRequest;
+using sentry::TCreateSentryRoleResponse;
+using sentry::TDropSentryRoleRequest;
+using sentry::TDropSentryRoleResponse;
+using sentry::TListSentryPrivilegesRequest;
+using sentry::TListSentryPrivilegesResponse;
+using sentry::TSentryResponseStatus;
+using sentry::g_sentry_common_service_constants;
 using std::make_shared;
 using strings::Substitute;
 
@@ -56,28 +69,27 @@ const char* const SentryClient::kServiceName = "Sentry";
 const int kSlowExecutionWarningThresholdMs = 1000;
 
 namespace {
-Status ConvertStatus(const ::sentry::TSentryResponseStatus& status) {
-  const auto& constants = ::sentry::g_sentry_common_service_constants;
+Status ConvertStatus(const TSentryResponseStatus& status) {
   // A switch isn't possible because these values aren't generated as real constants.
-  if (status.value == constants.TSENTRY_STATUS_OK) {
+  if (status.value == g_sentry_common_service_constants.TSENTRY_STATUS_OK) {
     return Status::OK();
   }
-  if (status.value == constants.TSENTRY_STATUS_ALREADY_EXISTS) {
+  if (status.value == g_sentry_common_service_constants.TSENTRY_STATUS_ALREADY_EXISTS) {
     return Status::AlreadyPresent(status.message, status.stack);
   }
-  if (status.value == constants.TSENTRY_STATUS_NO_SUCH_OBJECT) {
+  if (status.value == g_sentry_common_service_constants.TSENTRY_STATUS_NO_SUCH_OBJECT) {
     return Status::NotFound(status.message, status.stack);
   }
-  if (status.value == constants.TSENTRY_STATUS_RUNTIME_ERROR) {
+  if (status.value == g_sentry_common_service_constants.TSENTRY_STATUS_RUNTIME_ERROR) {
     return Status::RuntimeError(status.message, status.stack);
   }
-  if (status.value == constants.TSENTRY_STATUS_INVALID_INPUT) {
+  if (status.value == g_sentry_common_service_constants.TSENTRY_STATUS_INVALID_INPUT) {
     return Status::InvalidArgument(status.message, status.stack);
   }
-  if (status.value == constants.TSENTRY_STATUS_ACCESS_DENIED) {
+  if (status.value == g_sentry_common_service_constants.TSENTRY_STATUS_ACCESS_DENIED) {
     return Status::NotAuthorized(status.message, status.stack);
   }
-  if (status.value == constants.TSENTRY_STATUS_THRIFT_VERSION_MISMATCH) {
+  if (status.value == g_sentry_common_service_constants.TSENTRY_STATUS_THRIFT_VERSION_MISMATCH) {
     return Status::NotSupported(status.message, status.stack);
   }
   LOG(WARNING) << "Unknown error code in Sentry status: " << status;
@@ -109,7 +121,7 @@ Status ConvertStatus(const ::sentry::TSentryResponseStatus& status) {
   }
 
 SentryClient::SentryClient(const HostPort& address, const thrift::ClientOptions& options)
-      : client_(::sentry::SentryPolicyServiceClient(
+      : client_(SentryPolicyServiceClient(
             make_shared<TMultiplexedProtocol>(CreateClientProtocol(address, options),
                                               "SentryPolicyService"))) {
 }
@@ -136,25 +148,24 @@ bool SentryClient::IsConnected() {
   return client_.getInputProtocol()->getTransport()->isOpen();
 }
 
-Status SentryClient::CreateRole(const ::sentry::TCreateSentryRoleRequest& request) {
+Status SentryClient::CreateRole(const TCreateSentryRoleRequest& request) {
   SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs, "create Sentry role");
-  ::sentry::TCreateSentryRoleResponse response;
+  TCreateSentryRoleResponse response;
   SENTRY_RET_NOT_OK(client_.create_sentry_role(response, request),
                     response.status, "failed to create Sentry role");
   return Status::OK();
 }
 
-Status SentryClient::DropRole(const ::sentry::TDropSentryRoleRequest& request) {
+Status SentryClient::DropRole(const TDropSentryRoleRequest& request) {
   SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs, "drop Sentry role");
-  ::sentry::TDropSentryRoleResponse response;
+  TDropSentryRoleResponse response;
   SENTRY_RET_NOT_OK(client_.drop_sentry_role(response, request),
                     response.status, "failed to drop Sentry role");
   return Status::OK();
 }
 
-Status SentryClient::ListPrivilegesByUser(
-    const ::sentry::TListSentryPrivilegesRequest& request,
-    ::sentry::TListSentryPrivilegesResponse* response)  {
+Status SentryClient::ListPrivilegesByUser(const TListSentryPrivilegesRequest& request,
+                                          TListSentryPrivilegesResponse* response)  {
   SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs,
                             "list Sentry privilege by user");
   SENTRY_RET_NOT_OK(client_.list_sentry_privileges_by_user_and_itsgroups(*response, request),
@@ -162,9 +173,8 @@ Status SentryClient::ListPrivilegesByUser(
   return Status::OK();
 }
 
-Status SentryClient::AlterRoleAddGroups(
-    const ::sentry::TAlterSentryRoleAddGroupsRequest& request,
-    ::sentry::TAlterSentryRoleAddGroupsResponse* response)  {
+Status SentryClient::AlterRoleAddGroups(const TAlterSentryRoleAddGroupsRequest& request,
+                                        TAlterSentryRoleAddGroupsResponse* response)  {
   SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs,
                             "alter Sentry role add groups");
   SENTRY_RET_NOT_OK(client_.alter_sentry_role_add_groups(*response, request),
@@ -172,9 +182,8 @@ Status SentryClient::AlterRoleAddGroups(
   return Status::OK();
 }
 
-Status SentryClient::AlterRoleGrantPrivilege(
-    const ::sentry::TAlterSentryRoleGrantPrivilegeRequest& request,
-    ::sentry::TAlterSentryRoleGrantPrivilegeResponse* response)  {
+Status SentryClient::AlterRoleGrantPrivilege(const TAlterSentryRoleGrantPrivilegeRequest& request,
+                                             TAlterSentryRoleGrantPrivilegeResponse* response)  {
   SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs,
                             "alter Sentry role grant privileges");
   SENTRY_RET_NOT_OK(client_.alter_sentry_role_grant_privilege(*response, request),

http://git-wip-us.apache.org/repos/asf/kudu/blob/7607e235/src/kudu/sentry/thrift_operators-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/thrift_operators-test.cc b/src/kudu/sentry/thrift_operators-test.cc
index 46bd6c8..4ef3873 100644
--- a/src/kudu/sentry/thrift_operators-test.cc
+++ b/src/kudu/sentry/thrift_operators-test.cc
@@ -23,17 +23,12 @@
 
 #include "kudu/sentry/sentry_policy_service_types.h"
 #include "kudu/util/test_macros.h"
-#include "kudu/util/test_util.h"
 
-using std::string;
 using std::set;
+using std::string;
 
-namespace kudu {
 namespace sentry {
 
-class ThriftOperatorsTest : public KuduTest {
-};
-
 template<typename T>
 void AssertCompareRequirements(const T& a, const T& b) {
   // Values must not be less than themselves.
@@ -46,57 +41,56 @@ void AssertCompareRequirements(const T& a, const T& b) {
   }
 }
 
-TEST_F(ThriftOperatorsTest, TestOperatorLt) {
+TEST(ThriftOperatorsTest, TestOperatorLt) {
   // TSentryRole::operator<
-  ::sentry::TSentryRole role_a;
+  TSentryRole role_a;
   role_a.__set_roleName("a");
 
-  ::sentry::TSentryRole role_b;
+  TSentryRole role_b;
   role_b.__set_roleName("b");
 
   NO_FATALS(AssertCompareRequirements(role_a, role_b));
-  set<::sentry::TSentryRole> roles { role_a, role_b };
+  set<TSentryRole> roles { role_a, role_b };
   ASSERT_EQ(2, roles.size()) << roles;
 
   // TSentryGroup::operator<
-  ::sentry::TSentryGroup group_a;
+  TSentryGroup group_a;
   group_a.__set_groupName("a");
 
-  ::sentry::TSentryGroup group_b;
+  TSentryGroup group_b;
   group_b.__set_groupName("b");
 
   NO_FATALS(AssertCompareRequirements(group_a, group_b));
-  set<::sentry::TSentryGroup> groups { group_a, group_b };
+  set<TSentryGroup> groups { group_a, group_b };
   ASSERT_EQ(2, groups.size()) << groups;
 
   // TSentryPrivilege::operator<
-  ::sentry::TSentryPrivilege db_priv;
+  TSentryPrivilege db_priv;
   db_priv.__set_serverName("server1");
   db_priv.__set_dbName("db1");
 
-  ::sentry::TSentryPrivilege tbl_priv;
+  TSentryPrivilege tbl_priv;
   tbl_priv.__set_serverName("server1");
   tbl_priv.__set_dbName("db1");
   tbl_priv.__set_tableName("tbl1");
 
   NO_FATALS(AssertCompareRequirements(db_priv, tbl_priv));
-  set<::sentry::TSentryPrivilege> privileges { db_priv, tbl_priv };
+  set<TSentryPrivilege> privileges { db_priv, tbl_priv };
   ASSERT_EQ(2, privileges.size()) << privileges;
 
 
   // TSentryAuthorizable::operator<
-  ::sentry::TSentryAuthorizable db_authorizable;
+  TSentryAuthorizable db_authorizable;
   db_authorizable.__set_server("server1");
   db_authorizable.__set_db("db1");
 
-  ::sentry::TSentryAuthorizable tbl_authorizable;
+  TSentryAuthorizable tbl_authorizable;
   tbl_authorizable.__set_server("server1");
   tbl_authorizable.__set_db("db1");
   tbl_authorizable.__set_table("tbl1");
 
   NO_FATALS(AssertCompareRequirements(db_authorizable, tbl_authorizable));
-  set<::sentry::TSentryAuthorizable> authorizables { db_authorizable, tbl_authorizable };
+  set<TSentryAuthorizable> authorizables { db_authorizable, tbl_authorizable };
   ASSERT_EQ(2, authorizables.size()) << authorizables;
 }
 } // namespace sentry
-} // namespace kudu


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

Posted by da...@apache.org.
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 {


[2/3] kudu git commit: thrift operators: add more test coverage

Posted by da...@apache.org.
thrift operators: add more test coverage

This is a follow-up to Adar's suggestions in
https://gerrit.cloudera.org/c/11693/.

Change-Id: Ifdba996a9c4e6e01f55c257bfd10fb3f74f0f894
Reviewed-on: http://gerrit.cloudera.org:8080/11713
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins


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

Branch: refs/heads/master
Commit: 1e8ef0eac6d411a225bd3a4101a7e19346eb1048
Parents: 7607e23
Author: Dan Burkert <da...@apache.org>
Authored: Wed Oct 17 12:53:05 2018 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Wed Oct 17 23:02:28 2018 +0000

----------------------------------------------------------------------
 src/kudu/sentry/thrift_operators-test.cc | 56 +++++++++++++++++----------
 1 file changed, 36 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/1e8ef0ea/src/kudu/sentry/thrift_operators-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/thrift_operators-test.cc b/src/kudu/sentry/thrift_operators-test.cc
index 4ef3873..d4aa070 100644
--- a/src/kudu/sentry/thrift_operators-test.cc
+++ b/src/kudu/sentry/thrift_operators-test.cc
@@ -65,32 +65,48 @@ TEST(ThriftOperatorsTest, TestOperatorLt) {
   ASSERT_EQ(2, groups.size()) << groups;
 
   // TSentryPrivilege::operator<
+  const string kServer = "server1";
+  const string kDatabase = "db1";
+
   TSentryPrivilege db_priv;
-  db_priv.__set_serverName("server1");
-  db_priv.__set_dbName("db1");
+  db_priv.__set_serverName(kServer);
+  db_priv.__set_dbName(kDatabase);
 
-  TSentryPrivilege tbl_priv;
-  tbl_priv.__set_serverName("server1");
-  tbl_priv.__set_dbName("db1");
-  tbl_priv.__set_tableName("tbl1");
+  TSentryPrivilege tbl1_priv;
+  tbl1_priv.__set_serverName(kServer);
+  tbl1_priv.__set_dbName(kDatabase);
+  tbl1_priv.__set_tableName("tbl1");
 
-  NO_FATALS(AssertCompareRequirements(db_priv, tbl_priv));
-  set<TSentryPrivilege> privileges { db_priv, tbl_priv };
-  ASSERT_EQ(2, privileges.size()) << privileges;
+  TSentryPrivilege tbl2_priv;
+  tbl2_priv.__set_serverName(kServer);
+  tbl2_priv.__set_dbName(kDatabase);
+  tbl2_priv.__set_tableName("tbl2");
 
+  NO_FATALS(AssertCompareRequirements(db_priv, tbl1_priv));
+  NO_FATALS(AssertCompareRequirements(db_priv, tbl2_priv));
+  NO_FATALS(AssertCompareRequirements(tbl1_priv, tbl2_priv));
+  set<TSentryPrivilege> privileges { db_priv, tbl1_priv, tbl2_priv };
+  ASSERT_EQ(3, privileges.size()) << privileges;
 
   // TSentryAuthorizable::operator<
   TSentryAuthorizable db_authorizable;
-  db_authorizable.__set_server("server1");
-  db_authorizable.__set_db("db1");
-
-  TSentryAuthorizable tbl_authorizable;
-  tbl_authorizable.__set_server("server1");
-  tbl_authorizable.__set_db("db1");
-  tbl_authorizable.__set_table("tbl1");
-
-  NO_FATALS(AssertCompareRequirements(db_authorizable, tbl_authorizable));
-  set<TSentryAuthorizable> authorizables { db_authorizable, tbl_authorizable };
-  ASSERT_EQ(2, authorizables.size()) << authorizables;
+  db_authorizable.__set_server(kServer);
+  db_authorizable.__set_db(kDatabase);
+
+  TSentryAuthorizable tbl1_authorizable;
+  tbl1_authorizable.__set_server(kServer);
+  tbl1_authorizable.__set_db(kDatabase);
+  tbl1_authorizable.__set_table("tbl1");
+
+  TSentryAuthorizable tbl2_authorizable;
+  tbl2_authorizable.__set_server(kServer);
+  tbl2_authorizable.__set_db(kDatabase);
+  tbl2_authorizable.__set_table("tbl2");
+
+  NO_FATALS(AssertCompareRequirements(db_authorizable, tbl1_authorizable));
+  NO_FATALS(AssertCompareRequirements(db_authorizable, tbl2_authorizable));
+  NO_FATALS(AssertCompareRequirements(tbl1_authorizable, tbl2_authorizable));
+  set<TSentryAuthorizable> authorizables { db_authorizable, tbl1_authorizable, tbl2_authorizable };
+  ASSERT_EQ(3, authorizables.size()) << authorizables;
 }
 } // namespace sentry