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/09/27 00:35:01 UTC

[1/4] kudu git commit: [DOCS] Added a version notice for changing managed Kudu table names in Impala

Repository: kudu
Updated Branches:
  refs/heads/master c9f1772e7 -> ecc4998cb


[DOCS] Added a version notice for changing managed Kudu table names in Impala

With IMPALA-5654, users can no longer change kudu.table_name property
for managed Kudu tables in Impala.

Change-Id: If26f103d931dd7ed57b56cf34b0010d14c098928
Reviewed-on: http://gerrit.cloudera.org:8080/11515
Reviewed-by: Alexey Serbin <as...@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/bfdcd973
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/bfdcd973
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/bfdcd973

Branch: refs/heads/master
Commit: bfdcd973f2338f1326456cccdb62fbee9530ece5
Parents: c9f1772
Author: Alex Rodoni <ar...@cloudera.com>
Authored: Tue Sep 25 15:19:36 2018 -0700
Committer: Alexey Serbin <as...@cloudera.com>
Committed: Wed Sep 26 22:28:31 2018 +0000

----------------------------------------------------------------------
 docs/kudu_impala_integration.adoc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/bfdcd973/docs/kudu_impala_integration.adoc
----------------------------------------------------------------------
diff --git a/docs/kudu_impala_integration.adoc b/docs/kudu_impala_integration.adoc
index f16711d..2fe7e27 100755
--- a/docs/kudu_impala_integration.adoc
+++ b/docs/kudu_impala_integration.adoc
@@ -683,8 +683,8 @@ underlying Kudu table.
 
 .Rename the underlying Kudu table for an internal table
 
-If a table is an internal table, the underlying Kudu table may be renamed by
-changing the `kudu.table_name` property:
+In Impala 2.11 and lower, the underlying Kudu table may be renamed by changing
+the `kudu.table_name` property:
 
 [source,sql]
 ----


[2/4] kudu git commit: KUDU-428: add Sentry to thirdparty, mini-sentry

Posted by da...@apache.org.
KUDU-428: add Sentry to thirdparty, mini-sentry

This commit adds Sentry to thirdparty, and fills out the MiniSentry
class with an initial implementation. Notable features that aren't
implemented:

- Stripped Sentry packaging. I've put an unmodified version of Sentry
  2.0.1 into thirdparty. It weighs in at almost 200MiB and takes about
  5s to startup on my laptop. We will probably want to add a stripped
  version later to reduce both of these.

- Kerberos support for mini-sentry. Right now Kerberos is disabled,
  which is an atypical configuration. A follow-up commit will add a
  Kerberos support configuration taking advantage of the mini KDC.

- The mini Sentry is not yet configured with the location of the HMS,
  which will be necessary to do anything non-trivial with it.

Change-Id: I03f39cf9b2c813c0c305d085e1ad3851636326f5
Reviewed-on: http://gerrit.cloudera.org:8080/11347
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/fad56a6f
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/fad56a6f
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/fad56a6f

Branch: refs/heads/master
Commit: fad56a6f85a4707f075f2334de21fb9025011795
Parents: bfdcd97
Author: Dan Burkert <da...@apache.org>
Authored: Thu Aug 23 16:40:24 2018 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Thu Sep 27 00:34:17 2018 +0000

----------------------------------------------------------------------
 build-support/dist_test.py            |   9 +-
 build-support/run_dist_test.py        |   3 +-
 src/kudu/hms/mini_hms.cc              |  37 ++-----
 src/kudu/sentry/CMakeLists.txt        |  10 ++
 src/kudu/sentry/mini_sentry.cc        | 154 +++++++++++++++++++++++++++++
 src/kudu/sentry/mini_sentry.h         |  47 +++++++++
 src/kudu/sentry/sentry_client-test.cc |  19 ++--
 src/kudu/util/test_util.cc            |  16 +++
 src/kudu/util/test_util.h             |   9 ++
 thirdparty/build-thirdparty.sh        |  10 +-
 thirdparty/download-thirdparty.sh     |   6 ++
 thirdparty/vars.sh                    |   4 +
 12 files changed, 281 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/build-support/dist_test.py
----------------------------------------------------------------------
diff --git a/build-support/dist_test.py b/build-support/dist_test.py
index c1176fe..b2e7db9 100755
--- a/build-support/dist_test.py
+++ b/build-support/dist_test.py
@@ -97,12 +97,13 @@ DEPS_FOR_ALL = \
      # Tests that require tooling require this.
      "build/latest/bin/kudu",
 
-     # The HMS tests require the Hadoop and Hive libraries. These files are just
-     # symlinks, but dist-test will copy the entire directories they point to.
-     # The symlinks themselves won't be recreated, so we point to them with
-     # environment variables in run_dist_test.py.
+     # The HMS and Sentry tests require the Hadoop, Hive, and Sentry libraries.
+     # These files are just symlinks, but dist-test will copy the entire
+     # directories they point to.  The symlinks themselves won't be recreated,
+     # so we point to them with environment variables in run_dist_test.py.
      "build/latest/bin/hive-home",
      "build/latest/bin/hadoop-home",
+     "build/latest/bin/sentry-home",
 
      # Add the Kudu HMS plugin.
      "build/latest/bin/hms-plugin.jar",

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/build-support/run_dist_test.py
----------------------------------------------------------------------
diff --git a/build-support/run_dist_test.py b/build-support/run_dist_test.py
index 297075d..1edcce2 100755
--- a/build-support/run_dist_test.py
+++ b/build-support/run_dist_test.py
@@ -147,9 +147,10 @@ def main():
       env[var_name] = os.environ.get(var_name, "") + " external_symbolizer_path=" + symbolizer_path
 
   # Add environment variables for Java dependencies. These environment variables
-  # are used in mini_hms.cc.
+  # are used in mini_hms.cc and mini_sentry.cc.
   env['HIVE_HOME'] = glob.glob(os.path.join(ROOT, "thirdparty/src/hive-*"))[0]
   env['HADOOP_HOME'] = glob.glob(os.path.join(ROOT, "thirdparty/src/hadoop-*"))[0]
+  env['SENTRY_HOME'] = glob.glob(os.path.join(ROOT, "thirdparty/src/apache-sentry-*"))[0]
   env['JAVA_HOME'] = glob.glob("/usr/lib/jvm/java-1.8.0-*")[0]
 
   env['LD_LIBRARY_PATH'] = ":".join(

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/src/kudu/hms/mini_hms.cc
----------------------------------------------------------------------
diff --git a/src/kudu/hms/mini_hms.cc b/src/kudu/hms/mini_hms.cc
index cc7e6bc..253de09 100644
--- a/src/kudu/hms/mini_hms.cc
+++ b/src/kudu/hms/mini_hms.cc
@@ -19,7 +19,6 @@
 
 #include <algorithm>
 #include <csignal>
-#include <cstdlib>
 #include <map>
 #include <memory>
 #include <ostream>
@@ -34,7 +33,6 @@
 #include "kudu/util/slice.h"
 #include "kudu/util/status.h"
 #include "kudu/util/stopwatch.h"
-#include "kudu/util/string_case.h"
 #include "kudu/util/subprocess.h"
 #include "kudu/util/test_util.h"
 
@@ -76,25 +74,9 @@ void MiniHms::EnableKerberos(string krb5_conf,
 }
 
 void MiniHms::SetDataRoot(string data_root) {
-  data_root_ = data_root;
+  data_root_ = std::move(data_root);
 }
 
-namespace {
-Status FindHomeDir(const char* name, const string& bin_dir, string* home_dir) {
-  string name_upper;
-  ToUpperCase(name, &name_upper);
-
-  string env_var = Substitute("$0_HOME", name_upper);
-  const char* env = std::getenv(env_var.c_str());
-  *home_dir = env == nullptr ? JoinPathSegments(bin_dir, Substitute("$0-home", name)) : env;
-
-  if (!Env::Default()->FileExists(*home_dir)) {
-    return Status::NotFound(Substitute("$0 directory does not exist", env_var), *home_dir);
-  }
-  return Status::OK();
-}
-} // anonymous namespace
-
 Status MiniHms::Start() {
   SCOPED_LOG_SLOW_EXECUTION(WARNING, kHmsStartTimeoutMs / 2, "Starting HMS");
   CHECK(!hms_process_);
@@ -292,13 +274,13 @@ Status MiniHms::CreateHiveSite() const {
 </configuration>
   )";
 
-  string file_contents = strings::Substitute(kFileTemplate,
-                                             notification_log_ttl_.ToSeconds(),
-                                             data_root_,
-                                             !keytab_file_.empty(),
-                                             keytab_file_,
-                                             service_principal_,
-                                             SaslProtection::name_of(protection_));
+  string file_contents = Substitute(kFileTemplate,
+                                    notification_log_ttl_.ToSeconds(),
+                                    data_root_,
+                                    !keytab_file_.empty(),
+                                    keytab_file_,
+                                    service_principal_,
+                                    SaslProtection::name_of(protection_));
 
   return WriteStringToFile(Env::Default(),
                            file_contents,
@@ -323,8 +305,7 @@ Status MiniHms::CreateCoreSite() const {
 </configuration>
   )";
 
-  string file_contents = strings::Substitute(kFileTemplate,
-                                             keytab_file_.empty() ? "simple" : "kerberos");
+  string file_contents = Substitute(kFileTemplate, keytab_file_.empty() ? "simple" : "kerberos");
 
   return WriteStringToFile(Env::Default(),
                            file_contents,

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/src/kudu/sentry/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/CMakeLists.txt b/src/kudu/sentry/CMakeLists.txt
index 8c75614..8e96c97 100644
--- a/src/kudu/sentry/CMakeLists.txt
+++ b/src/kudu/sentry/CMakeLists.txt
@@ -45,6 +45,16 @@ target_link_libraries(kudu_sentry ${SENTRY_DEPS})
 # mini_sentry
 ##############################
 
+execute_process(COMMAND ln -nsf
+                "${CMAKE_SOURCE_DIR}/thirdparty/installed/common/opt/sentry"
+                "${EXECUTABLE_OUTPUT_PATH}/sentry-home")
+execute_process(COMMAND ln -nsf
+                "${CMAKE_SOURCE_DIR}/thirdparty/installed/common/opt/hadoop"
+                "${EXECUTABLE_OUTPUT_PATH}/hadoop-home")
+execute_process(COMMAND ln -nsf
+                "${JAVA_HOME}"
+                "${EXECUTABLE_OUTPUT_PATH}/java-home")
+
 set(MINI_SENTRY_SRCS
   mini_sentry.cc)
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/src/kudu/sentry/mini_sentry.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/mini_sentry.cc b/src/kudu/sentry/mini_sentry.cc
index 1b00099..865fd2d 100644
--- a/src/kudu/sentry/mini_sentry.cc
+++ b/src/kudu/sentry/mini_sentry.cc
@@ -17,7 +17,161 @@
 
 #include "kudu/sentry/mini_sentry.h"
 
+#include <algorithm>
+#include <csignal>
+#include <map>
+#include <memory>
+#include <ostream>
+#include <string>
+
+#include <glog/logging.h>
+
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/util/env.h"
+#include "kudu/util/monotime.h"
+#include "kudu/util/path_util.h"
+#include "kudu/util/slice.h"
+#include "kudu/util/status.h"
+#include "kudu/util/stopwatch.h"
+#include "kudu/util/subprocess.h"
+#include "kudu/util/test_util.h"
+
+using std::map;
+using std::string;
+using std::unique_ptr;
+using strings::Substitute;
+
+static constexpr int kSentryStartTimeoutMs = 60000;
+
 namespace kudu {
 namespace sentry {
+
+MiniSentry::MiniSentry() {
+}
+
+MiniSentry::~MiniSentry() {
+  WARN_NOT_OK(Stop(), "Failed to stop MiniSentry");
+}
+
+Status MiniSentry::Start() {
+  SCOPED_LOG_SLOW_EXECUTION(WARNING, kSentryStartTimeoutMs / 2, "Starting Sentry");
+  CHECK(!sentry_process_);
+
+  VLOG(1) << "Starting Sentry";
+
+  Env* env = Env::Default();
+
+  string exe;
+  RETURN_NOT_OK(env->GetExecutablePath(&exe));
+  const string bin_dir = DirName(exe);
+
+  string hadoop_home;
+  string sentry_home;
+  string java_home;
+  RETURN_NOT_OK(FindHomeDir("hadoop", bin_dir, &hadoop_home));
+  RETURN_NOT_OK(FindHomeDir("sentry", bin_dir, &sentry_home));
+  RETURN_NOT_OK(FindHomeDir("java", bin_dir, &java_home));
+
+  auto tmp_dir = GetTestDataDirectory();
+
+  RETURN_NOT_OK(CreateSentrySite(tmp_dir));
+
+  map<string, string> env_vars {
+      { "JAVA_HOME", java_home },
+      { "HADOOP_HOME", hadoop_home },
+  };
+
+  // Start Sentry.
+  sentry_process_.reset(new Subprocess({
+      Substitute("$0/bin/sentry", sentry_home),
+      "--command", "service",
+      "--conffile", JoinPathSegments(tmp_dir, "sentry-site.xml"),
+  }));
+
+  sentry_process_->SetEnvVars(env_vars);
+  RETURN_NOT_OK(sentry_process_->Start());
+
+  // Wait for Sentry to start listening on its ports and commencing operation.
+  VLOG(1) << "Waiting for Sentry ports";
+  Status wait = WaitForTcpBind(sentry_process_->pid(), &port_,
+                               MonoDelta::FromMilliseconds(kSentryStartTimeoutMs));
+  if (!wait.ok()) {
+    WARN_NOT_OK(sentry_process_->Kill(SIGQUIT), "failed to send SIGQUIT to Sentry");
+  }
+  return wait;
+}
+
+Status MiniSentry::Stop() {
+  if (sentry_process_) {
+    VLOG(1) << "Stopping Sentry";
+    unique_ptr<Subprocess> proc = std::move(sentry_process_);
+    RETURN_NOT_OK_PREPEND(proc->KillAndWait(SIGTERM), "failed to stop the Sentry service");
+  }
+  return Status::OK();
+}
+
+Status MiniSentry::Pause() {
+  CHECK(sentry_process_);
+  VLOG(1) << "Pausing Sentry";
+  RETURN_NOT_OK_PREPEND(sentry_process_->Kill(SIGSTOP),
+                        "failed to pause the Sentry service");
+  return Status::OK();
+}
+
+Status MiniSentry::Resume() {
+  CHECK(sentry_process_);
+  VLOG(1) << "Resuming Sentry";
+  RETURN_NOT_OK_PREPEND(sentry_process_->Kill(SIGCONT),
+                        "failed to unpause the Sentry service");
+  return Status::OK();
+}
+
+Status MiniSentry::CreateSentrySite(const string& tmp_dir) const {
+
+  // - sentry.store.jdbc.url
+  // - sentry.store.jdbc.password
+  //     Configures Sentry to use a local in-process Derby instance with a dummy
+  //     password value.
+  //
+  // - datanucleus.schema.autoCreateAll
+  // - sentry.verify.schema.version
+  //     Allow Sentry to startup and run without first running the schemaTool.
+  static const string kFileTemplate = R"(
+<configuration>
+
+  <property>
+    <name>sentry.service.security.mode</name>
+    <value>none</value>
+  </property>
+
+  <property>
+    <name>sentry.store.jdbc.url</name>
+    <value>jdbc:derby:$0/sentry;create=true</value>
+  </property>
+
+  <property>
+    <name>sentry.store.jdbc.password</name>
+    <value>_</value>
+  </property>
+
+  <property>
+    <name>datanucleus.schema.autoCreateAll</name>
+    <value>true</value>
+  </property>
+
+  <property>
+   <name>sentry.verify.schema.version</name>
+    <value>false</value>
+  </property>
+</configuration>
+  )";
+
+  string file_contents = Substitute(kFileTemplate, tmp_dir);
+
+  return WriteStringToFile(Env::Default(),
+                           file_contents,
+                           JoinPathSegments(tmp_dir, "sentry-site.xml"));
+}
+
 } // namespace sentry
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/src/kudu/sentry/mini_sentry.h
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/mini_sentry.h b/src/kudu/sentry/mini_sentry.h
index adc081d..fe3ccca 100644
--- a/src/kudu/sentry/mini_sentry.h
+++ b/src/kudu/sentry/mini_sentry.h
@@ -17,11 +17,58 @@
 
 #pragma once
 
+#include <cstdint>
+#include <memory>
+#include <string>
+
+#include "kudu/gutil/port.h"
+#include "kudu/util/net/net_util.h"
+#include "kudu/util/status.h"
+
 namespace kudu {
+
+class Subprocess;
+
 namespace sentry {
 
 class MiniSentry {
  public:
+
+  MiniSentry();
+
+  ~MiniSentry();
+
+  // Starts the mini Sentry service.
+  //
+  // If the MiniSentry has already been started and stopped, it will be restarted
+  // using the same listening port.
+  Status Start() WARN_UNUSED_RESULT;
+
+  // Stops the mini Sentry service.
+  Status Stop() WARN_UNUSED_RESULT;
+
+  // Pause the Sentry service.
+  Status Pause() WARN_UNUSED_RESULT;
+
+  // Unpause the Sentry service.
+  Status Resume() WARN_UNUSED_RESULT;
+
+  // Returns the address of the Sentry service. Should only be called after the
+  // Sentry service is started.
+  HostPort address() const {
+    return HostPort("127.0.0.1", port_);
+  }
+
+ private:
+
+  // Creates a sentry-site.xml for the mini Sentry.
+  Status CreateSentrySite(const std::string& tmp_dir) const WARN_UNUSED_RESULT;
+
+  // Waits for the metastore process to bind to a port.
+  Status WaitForSentryPorts() WARN_UNUSED_RESULT;
+
+  std::unique_ptr<Subprocess> sentry_process_;
+  uint16_t port_ = 0;
 };
 
 } // namespace sentry

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/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 6293ac1..8f3c560 100644
--- a/src/kudu/sentry/sentry_client-test.cc
+++ b/src/kudu/sentry/sentry_client-test.cc
@@ -15,12 +15,10 @@
 // specific language governing permissions and limitations
 // under the License.
 
-#include "kudu/sentry/sentry_client.h"
-
-#include <utility>
-
 #include <gtest/gtest.h>
 
+#include "kudu/sentry/mini_sentry.h"
+#include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
 
 namespace kudu {
@@ -30,10 +28,15 @@ class SentryClientTest : public KuduTest {
  public:
 };
 
-TEST_F(SentryClientTest, ItWorks) {
-  SentryClient client;
-  std::move(client);
-}
+TEST_F(SentryClientTest, TestMiniSentryLifecycle) {
+  MiniSentry mini_sentry;
+  ASSERT_OK(mini_sentry.Start());
 
+  ASSERT_OK(mini_sentry.Stop());
+  ASSERT_OK(mini_sentry.Start());
+
+  ASSERT_OK(mini_sentry.Pause());
+  ASSERT_OK(mini_sentry.Resume());
+}
 } // namespace sentry
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/src/kudu/util/test_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index c960441..214964d 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -54,6 +54,7 @@
 #include "kudu/util/slice.h"
 #include "kudu/util/spinlock_profiling.h"
 #include "kudu/util/status.h"
+#include "kudu/util/string_case.h"
 #include "kudu/util/subprocess.h"
 
 DEFINE_string(test_leave_files, "on_failure",
@@ -443,4 +444,19 @@ Status WaitForUdpBind(pid_t pid, uint16_t* port, MonoDelta timeout) {
   return WaitForBind(pid, port, "4UDP", timeout);
 }
 
+Status FindHomeDir(const string& name, const string& bin_dir, string* home_dir) {
+  string name_upper;
+  ToUpperCase(name, &name_upper);
+
+  string env_var = Substitute("$0_HOME", name_upper);
+  const char* env = std::getenv(env_var.c_str());
+  string dir = env == nullptr ? JoinPathSegments(bin_dir, Substitute("$0-home", name)) : env;
+
+  if (!Env::Default()->FileExists(dir)) {
+    return Status::NotFound(Substitute("$0 directory does not exist", env_var), dir);
+  }
+  *home_dir = dir;
+  return Status::OK();
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/src/kudu/util/test_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/test_util.h b/src/kudu/util/test_util.h
index 8090fbc..32dfacd 100644
--- a/src/kudu/util/test_util.h
+++ b/src/kudu/util/test_util.h
@@ -142,5 +142,14 @@ Status WaitForTcpBind(pid_t pid, uint16_t* port, MonoDelta timeout) WARN_UNUSED_
 // Waits for the subprocess to bind to any listening UDP port, and returns the port.
 Status WaitForUdpBind(pid_t pid, uint16_t* port, MonoDelta timeout) WARN_UNUSED_RESULT;
 
+// Find the home directory of a Java-style application, e.g. JAVA_HOME or
+// HADOOP_HOME.
+//
+// Checks the environment, or falls back to a symlink in the bin installation
+// directory.
+Status FindHomeDir(const std::string& name,
+                   const std::string& bin_dir,
+                   std::string* home_dir) WARN_UNUSED_RESULT;
+
 } // namespace kudu
 #endif

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/thirdparty/build-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/build-thirdparty.sh b/thirdparty/build-thirdparty.sh
index 8c2a44d..c13c95c 100755
--- a/thirdparty/build-thirdparty.sh
+++ b/thirdparty/build-thirdparty.sh
@@ -99,6 +99,7 @@ else
       "bison")        F_BISON=1 ;;
       "hadoop")       F_HADOOP=1 ;;
       "hive")         F_HIVE=1 ;;
+      "sentry")       F_SENTRY=1 ;;
       *)              echo "Unknown module: $arg"; exit 1 ;;
     esac
   done
@@ -244,8 +245,8 @@ if [ -n "$F_COMMON" -o -n "$F_BISON" ]; then
   build_bison
 fi
 
-# Install Hadoop and Hive by symlinking their source directories (which are
-# pre-built) into $PREFIX/opt.
+# Install Hadoop, Hive, and Sentry by symlinking their source directories (which
+# are pre-built) into $PREFIX/opt.
 if [ -n "$F_COMMON" -o -n "$F_HADOOP" ]; then
   mkdir -p $PREFIX/opt
   ln -nsf $HADOOP_SOURCE $PREFIX/opt/hadoop
@@ -256,6 +257,11 @@ if [ -n "$F_COMMON" -o -n "$F_HIVE" ]; then
   ln -nsf $HIVE_SOURCE $PREFIX/opt/hive
 fi
 
+if [ -n "$F_COMMON" -o -n "$F_SENTRY" ]; then
+  mkdir -p $PREFIX/opt
+  ln -nsf $SENTRY_SOURCE $PREFIX/opt/sentry
+fi
+
 ### Build C dependencies without instrumentation
 
 PREFIX=$PREFIX_DEPS

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/thirdparty/download-thirdparty.sh
----------------------------------------------------------------------
diff --git a/thirdparty/download-thirdparty.sh b/thirdparty/download-thirdparty.sh
index 7992065..9b19305 100755
--- a/thirdparty/download-thirdparty.sh
+++ b/thirdparty/download-thirdparty.sh
@@ -423,5 +423,11 @@ fetch_and_patch \
  $HADOOP_SOURCE \
  $HADOOP_PATCHLEVEL
 
+SENTRY_PATCHLEVEL=0
+fetch_and_patch \
+ $SENTRY_NAME.tar.gz \
+ $SENTRY_SOURCE \
+ $SENTRY_PATCHLEVEL
+
 echo "---------------"
 echo "Thirdparty dependencies downloaded successfully"

http://git-wip-us.apache.org/repos/asf/kudu/blob/fad56a6f/thirdparty/vars.sh
----------------------------------------------------------------------
diff --git a/thirdparty/vars.sh b/thirdparty/vars.sh
index 71beea6..fe25293 100644
--- a/thirdparty/vars.sh
+++ b/thirdparty/vars.sh
@@ -225,3 +225,7 @@ HIVE_SOURCE=$TP_SOURCE_DIR/$HIVE_NAME
 HADOOP_VERSION=2.8.2
 HADOOP_NAME=hadoop-$HADOOP_VERSION
 HADOOP_SOURCE=$TP_SOURCE_DIR/$HADOOP_NAME
+
+SENTRY_VERSION=2.0.1
+SENTRY_NAME=apache-sentry-$SENTRY_VERSION-bin
+SENTRY_SOURCE=$TP_SOURCE_DIR/$SENTRY_NAME


[4/4] kudu git commit: KUDU-2541: add Kerberos support to Sentry client and mini-cluster

Posted by da...@apache.org.
KUDU-2541: add Kerberos support to Sentry client and mini-cluster

More infrastructure work towards KUDU-428.

Change-Id: I3f7e27137135cb9b463a90b98e4aba864cece3c1
Reviewed-on: http://gerrit.cloudera.org:8080/11525
Reviewed-by: Hao Hao <ha...@cloudera.com>
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/ecc4998c
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/ecc4998c
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/ecc4998c

Branch: refs/heads/master
Commit: ecc4998cb921b97d919ce6acef2b0f623a75d653
Parents: 14f3e6f
Author: Dan Burkert <da...@apache.org>
Authored: Mon Sep 24 15:47:39 2018 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Thu Sep 27 00:34:32 2018 +0000

----------------------------------------------------------------------
 src/kudu/sentry/CMakeLists.txt        |  1 +
 src/kudu/sentry/mini_sentry.cc        | 53 +++++++++++++++++++++++++++---
 src/kudu/sentry/mini_sentry.h         | 10 ++++++
 src/kudu/sentry/sentry_client-test.cc | 41 +++++++++++++++++++----
 4 files changed, 95 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/ecc4998c/src/kudu/sentry/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/CMakeLists.txt b/src/kudu/sentry/CMakeLists.txt
index 910117c..588bf9f 100644
--- a/src/kudu/sentry/CMakeLists.txt
+++ b/src/kudu/sentry/CMakeLists.txt
@@ -72,6 +72,7 @@ target_link_libraries(mini_sentry
 if (NOT NO_TESTS)
   SET_KUDU_TEST_LINK_LIBS(
     kudu_sentry
+    mini_kdc
     mini_sentry)
 
   ADD_KUDU_TEST(sentry_client-test)

http://git-wip-us.apache.org/repos/asf/kudu/blob/ecc4998c/src/kudu/sentry/mini_sentry.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/mini_sentry.cc b/src/kudu/sentry/mini_sentry.cc
index ff54ad8..f50aa5b 100644
--- a/src/kudu/sentry/mini_sentry.cc
+++ b/src/kudu/sentry/mini_sentry.cc
@@ -53,6 +53,18 @@ MiniSentry::~MiniSentry() {
   WARN_NOT_OK(Stop(), "Failed to stop MiniSentry");
 }
 
+void MiniSentry::EnableKerberos(std::string krb5_conf,
+                                std::string service_principal,
+                                std::string keytab_file) {
+  CHECK(!sentry_process_);
+  CHECK(!krb5_conf.empty());
+  CHECK(!service_principal.empty());
+  CHECK(!keytab_file.empty());
+  krb5_conf_ = std::move(krb5_conf);
+  service_principal_ = std::move(service_principal);
+  keytab_file_ = std::move(keytab_file);
+}
+
 Status MiniSentry::Start() {
   SCOPED_LOG_SLOW_EXECUTION(WARNING, kSentryStartTimeoutMs / 2, "Starting Sentry");
   CHECK(!sentry_process_);
@@ -76,9 +88,16 @@ Status MiniSentry::Start() {
 
   RETURN_NOT_OK(CreateSentryConfigs(tmp_dir));
 
+  // List of JVM environment options to pass to the Sentry service.
+  string java_options;
+  if (!krb5_conf_.empty()) {
+    java_options += Substitute(" -Djava.security.krb5.conf=$0", krb5_conf_);
+  }
+
   map<string, string> env_vars {
       { "JAVA_HOME", java_home },
       { "HADOOP_HOME", hadoop_home },
+      { "JAVA_TOOL_OPTIONS", java_options },
   };
 
   // Start Sentry.
@@ -145,17 +164,31 @@ Status MiniSentry::CreateSentryConfigs(const string& tmp_dir) const {
   //
   // - sentry.service.admin.group
   //     Set up admin groups which have unrestricted privileges in Sentry.
+  //
+  // - sentry.service.allow.connect
+  //     Set of Kerberos principals which is allowed to connect to Sentry when
+  //     the Kerberos security mode is enabled.
   static const string kFileTemplate = R"(
 <configuration>
 
   <property>
     <name>sentry.service.security.mode</name>
-    <value>none</value>
+    <value>$0</value>
+  </property>
+
+  <property>
+    <name>sentry.service.server.principal</name>
+    <value>$1</value>
+  </property>
+
+  <property>
+    <name>sentry.service.server.keytab</name>
+    <value>$2</value>
   </property>
 
   <property>
     <name>sentry.store.jdbc.url</name>
-    <value>jdbc:derby:$0/sentry;create=true</value>
+    <value>jdbc:derby:$3/sentry;create=true</value>
   </property>
 
   <property>
@@ -180,18 +213,30 @@ Status MiniSentry::CreateSentryConfigs(const string& tmp_dir) const {
 
   <property>
     <name>sentry.store.group.mapping.resource</name>
-    <value>$1</value>
+    <value>$4</value>
   </property>
 
   <property>
     <name>sentry.service.admin.group</name>
     <value>admin</value>
   </property>
+
+  <property>
+    <name>sentry.service.allow.connect</name>
+    <value>kudu</value>
+  </property>
+
 </configuration>
   )";
 
   string users_ini_path = JoinPathSegments(tmp_dir, "users.ini");
-  string file_contents = Substitute(kFileTemplate, tmp_dir, users_ini_path);
+  string file_contents = Substitute(
+      kFileTemplate,
+      keytab_file_.empty() ? "none" : "kerberos",
+      service_principal_,
+      keytab_file_,
+      tmp_dir,
+      users_ini_path);
   RETURN_NOT_OK(WriteStringToFile(Env::Default(),
                                   file_contents,
                                   JoinPathSegments(tmp_dir, "sentry-site.xml")));

http://git-wip-us.apache.org/repos/asf/kudu/blob/ecc4998c/src/kudu/sentry/mini_sentry.h
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/mini_sentry.h b/src/kudu/sentry/mini_sentry.h
index bcf46aa..708ed6b 100644
--- a/src/kudu/sentry/mini_sentry.h
+++ b/src/kudu/sentry/mini_sentry.h
@@ -38,6 +38,11 @@ class MiniSentry {
 
   ~MiniSentry();
 
+  // Configures the mini Sentry service to use Kerberos.
+  void EnableKerberos(std::string krb5_conf,
+                      std::string service_principal,
+                      std::string keytab_file);
+
   // Starts the mini Sentry service.
   //
   // If the MiniSentry has already been started and stopped, it will be restarted
@@ -70,6 +75,11 @@ class MiniSentry {
 
   std::unique_ptr<Subprocess> sentry_process_;
   uint16_t port_ = 0;
+
+  // Kerberos configuration
+  std::string krb5_conf_;
+  std::string service_principal_;
+  std::string keytab_file_;
 };
 
 } // namespace sentry

http://git-wip-us.apache.org/repos/asf/kudu/blob/ecc4998c/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 7e3815b..d738a4b 100644
--- a/src/kudu/sentry/sentry_client-test.cc
+++ b/src/kudu/sentry/sentry_client-test.cc
@@ -17,10 +17,13 @@
 
 #include "kudu/sentry/sentry_client.h"
 
+#include <map>
 #include <string>
 
 #include <gtest/gtest.h>
 
+#include "kudu/rpc/sasl_common.h"
+#include "kudu/security/test/mini_kdc.h"
 #include "kudu/sentry/mini_sentry.h"
 #include "kudu/sentry/sentry_policy_service_types.h"
 #include "kudu/thrift/client.h"
@@ -28,14 +31,21 @@
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
 
+using std::string;
+
 namespace kudu {
 namespace sentry {
 
-class SentryClientTest : public KuduTest {
+class SentryClientTest : public KuduTest,
+                         public ::testing::WithParamInterface<bool> {
  public:
+  bool KerberosEnabled() const {
+    return GetParam();
+  }
 };
+INSTANTIATE_TEST_CASE_P(KerberosEnabled, SentryClientTest, ::testing::Bool());
 
-TEST_F(SentryClientTest, TestMiniSentryLifecycle) {
+TEST_P(SentryClientTest, TestMiniSentryLifecycle) {
   MiniSentry mini_sentry;
   ASSERT_OK(mini_sentry.Start());
 
@@ -50,11 +60,30 @@ TEST_F(SentryClientTest, TestMiniSentryLifecycle) {
 // test of Sentry's role handling, but instead verification that the client can
 // communicate with the Sentry service, and errors are converted to Status
 // instances.
-TEST_F(SentryClientTest, TestCreateDropRole) {
-  MiniSentry mini_sentry;
-  ASSERT_OK(mini_sentry.Start());
+TEST_P(SentryClientTest, TestCreateDropRole) {
+  MiniKdc kdc;
+  MiniSentry sentry;
+  thrift::ClientOptions sentry_client_opts;
+
+  if (KerberosEnabled()) {
+    ASSERT_OK(kdc.Start());
+
+    string spn = "sentry/127.0.0.1@KRBTEST.COM";
+    string ktpath;
+    ASSERT_OK(kdc.CreateServiceKeytab("sentry/127.0.0.1", &ktpath));
+
+    ASSERT_OK(rpc::SaslInit());
+    sentry.EnableKerberos(kdc.GetEnvVars()["KRB5_CONFIG"], spn, ktpath);
+
+    ASSERT_OK(kdc.CreateUserPrincipal("kudu"));
+    ASSERT_OK(kdc.Kinit("kudu"));
+    ASSERT_OK(kdc.SetKrb5Environment());
+    sentry_client_opts.enable_kerberos = true;
+    sentry_client_opts.service_principal = "sentry";
+  }
+  ASSERT_OK(sentry.Start());
 
-  SentryClient client(mini_sentry.address(), thrift::ClientOptions());
+  SentryClient client(sentry.address(), sentry_client_opts);
   ASSERT_OK(client.Start());
 
   { // Create a role


[3/4] kudu git commit: KUDU-2541: Fill out basic sentry client API

Posted by da...@apache.org.
KUDU-2541: Fill out basic sentry client API

This commit contains more plumbing for the upcoming Sentry integration
(KUDU-428). The Sentry client class is filled out with enough methods to
ensure the Thrift RPC is working correctly, as well as error conversion
from Sentry and Thrift's format to Kudu's Status.

Change-Id: I1004a11506ed109d1f23389ca73d95a34c32c194
Reviewed-on: http://gerrit.cloudera.org:8080/11524
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/14f3e6f6
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/14f3e6f6
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/14f3e6f6

Branch: refs/heads/master
Commit: 14f3e6f60b9fbfc7c7b41c6b5ce4ba8612088869
Parents: fad56a6
Author: Dan Burkert <da...@apache.org>
Authored: Fri Sep 21 10:43:05 2018 -0700
Committer: Dan Burkert <da...@apache.org>
Committed: Thu Sep 27 00:34:24 2018 +0000

----------------------------------------------------------------------
 src/kudu/sentry/CMakeLists.txt        |   1 +
 src/kudu/sentry/mini_sentry.cc        |  53 ++++++++++--
 src/kudu/sentry/mini_sentry.h         |   5 +-
 src/kudu/sentry/sentry_client-test.cc |  57 +++++++++++++
 src/kudu/sentry/sentry_client.cc      | 131 +++++++++++++++++++++++++++++
 src/kudu/sentry/sentry_client.h       |  63 ++++++++++++++
 6 files changed, 300 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/14f3e6f6/src/kudu/sentry/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/CMakeLists.txt b/src/kudu/sentry/CMakeLists.txt
index 8e96c97..910117c 100644
--- a/src/kudu/sentry/CMakeLists.txt
+++ b/src/kudu/sentry/CMakeLists.txt
@@ -35,6 +35,7 @@ set(SENTRY_SRCS
   sentry_client.cc)
 set(SENTRY_DEPS
   kudu_common
+  kudu_thrift
   kudu_util
   sentry_thrift)
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/14f3e6f6/src/kudu/sentry/mini_sentry.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/mini_sentry.cc b/src/kudu/sentry/mini_sentry.cc
index 865fd2d..ff54ad8 100644
--- a/src/kudu/sentry/mini_sentry.cc
+++ b/src/kudu/sentry/mini_sentry.cc
@@ -74,7 +74,7 @@ Status MiniSentry::Start() {
 
   auto tmp_dir = GetTestDataDirectory();
 
-  RETURN_NOT_OK(CreateSentrySite(tmp_dir));
+  RETURN_NOT_OK(CreateSentryConfigs(tmp_dir));
 
   map<string, string> env_vars {
       { "JAVA_HOME", java_home },
@@ -126,7 +126,7 @@ Status MiniSentry::Resume() {
   return Status::OK();
 }
 
-Status MiniSentry::CreateSentrySite(const string& tmp_dir) const {
+Status MiniSentry::CreateSentryConfigs(const string& tmp_dir) const {
 
   // - sentry.store.jdbc.url
   // - sentry.store.jdbc.password
@@ -136,6 +136,15 @@ Status MiniSentry::CreateSentrySite(const string& tmp_dir) const {
   // - datanucleus.schema.autoCreateAll
   // - sentry.verify.schema.version
   //     Allow Sentry to startup and run without first running the schemaTool.
+  //
+  // - sentry.store.group.mapping
+  //   sentry.store.group.mapping.resource
+  //     Production Sentry instances use Hadoop's UGI infrastructure to map users
+  //     to groups, but that's difficult to mock for tests, so we configure a
+  //     simpler static user/group mapping based on a generated INI file.
+  //
+  // - sentry.service.admin.group
+  //     Set up admin groups which have unrestricted privileges in Sentry.
   static const string kFileTemplate = R"(
 <configuration>
 
@@ -160,17 +169,45 @@ Status MiniSentry::CreateSentrySite(const string& tmp_dir) const {
   </property>
 
   <property>
-   <name>sentry.verify.schema.version</name>
+    <name>sentry.verify.schema.version</name>
     <value>false</value>
   </property>
+
+  <property>
+    <name>sentry.store.group.mapping</name>
+    <value>org.apache.sentry.provider.file.LocalGroupMappingService</value>
+  </property>
+
+  <property>
+    <name>sentry.store.group.mapping.resource</name>
+    <value>$1</value>
+  </property>
+
+  <property>
+    <name>sentry.service.admin.group</name>
+    <value>admin</value>
+  </property>
 </configuration>
   )";
 
-  string file_contents = Substitute(kFileTemplate, tmp_dir);
-
-  return WriteStringToFile(Env::Default(),
-                           file_contents,
-                           JoinPathSegments(tmp_dir, "sentry-site.xml"));
+  string users_ini_path = JoinPathSegments(tmp_dir, "users.ini");
+  string file_contents = Substitute(kFileTemplate, tmp_dir, users_ini_path);
+  RETURN_NOT_OK(WriteStringToFile(Env::Default(),
+                                  file_contents,
+                                  JoinPathSegments(tmp_dir, "sentry-site.xml")));
+
+  // Simple file format containing mapping of user to groups in INI syntax, see
+  // the LocalGroupMappingService class for more information.
+  static const string kUsers = R"(
+[users]
+test-admin=admin
+test-user=user
+kudu=admin
+joe-interloper=""
+)";
+
+  RETURN_NOT_OK(WriteStringToFile(Env::Default(), kUsers, users_ini_path));
+  return Status::OK();
 }
 
 } // namespace sentry

http://git-wip-us.apache.org/repos/asf/kudu/blob/14f3e6f6/src/kudu/sentry/mini_sentry.h
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/mini_sentry.h b/src/kudu/sentry/mini_sentry.h
index fe3ccca..bcf46aa 100644
--- a/src/kudu/sentry/mini_sentry.h
+++ b/src/kudu/sentry/mini_sentry.h
@@ -61,8 +61,9 @@ class MiniSentry {
 
  private:
 
-  // Creates a sentry-site.xml for the mini Sentry.
-  Status CreateSentrySite(const std::string& tmp_dir) const WARN_UNUSED_RESULT;
+  // Creates a sentry-site.xml for the mini Sentry, and other supporting
+  // configuration files.
+  Status CreateSentryConfigs(const std::string& tmp_dir) const WARN_UNUSED_RESULT;
 
   // Waits for the metastore process to bind to a port.
   Status WaitForSentryPorts() WARN_UNUSED_RESULT;

http://git-wip-us.apache.org/repos/asf/kudu/blob/14f3e6f6/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 8f3c560..7e3815b 100644
--- a/src/kudu/sentry/sentry_client-test.cc
+++ b/src/kudu/sentry/sentry_client-test.cc
@@ -15,9 +15,16 @@
 // specific language governing permissions and limitations
 // under the License.
 
+#include "kudu/sentry/sentry_client.h"
+
+#include <string>
+
 #include <gtest/gtest.h>
 
 #include "kudu/sentry/mini_sentry.h"
+#include "kudu/sentry/sentry_policy_service_types.h"
+#include "kudu/thrift/client.h"
+#include "kudu/util/status.h"
 #include "kudu/util/test_macros.h"
 #include "kudu/util/test_util.h"
 
@@ -38,5 +45,55 @@ TEST_F(SentryClientTest, TestMiniSentryLifecycle) {
   ASSERT_OK(mini_sentry.Pause());
   ASSERT_OK(mini_sentry.Resume());
 }
+
+// Basic functionality test of the Sentry client. The goal is not an exhaustive
+// test of Sentry's role handling, but instead verification that the client can
+// communicate with the Sentry service, and errors are converted to Status
+// instances.
+TEST_F(SentryClientTest, TestCreateDropRole) {
+  MiniSentry mini_sentry;
+  ASSERT_OK(mini_sentry.Start());
+
+  SentryClient client(mini_sentry.address(), thrift::ClientOptions());
+  ASSERT_OK(client.Start());
+
+  { // Create a role
+    ::sentry::TCreateSentryRoleRequest req;
+    req.requestorUserName = "test-admin";
+    req.roleName = "viewer";
+    ASSERT_OK(client.CreateRole(req));
+
+    // Attempt to create the role again.
+    Status s = client.CreateRole(req);
+    ASSERT_TRUE(s.IsAlreadyPresent()) << s.ToString();
+  }
+
+  { // Attempt to create a role as a non-admin user.
+    ::sentry::TCreateSentryRoleRequest req;
+    req.requestorUserName = "joe-interloper";
+    req.roleName = "fuzz";
+    Status s = client.CreateRole(req);
+    ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
+  }
+
+  { // Attempt to drop the role as a non-admin user.
+    ::sentry::TDropSentryRoleRequest req;
+    req.requestorUserName = "joe-interloper";
+    req.roleName = "viewer";
+    Status s = client.DropRole(req);
+    ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString();
+  }
+
+  { // Drop the role
+    ::sentry::TDropSentryRoleRequest req;
+    req.requestorUserName = "test-admin";
+    req.roleName = "viewer";
+    ASSERT_OK(client.DropRole(req));
+
+    // Attempt to drop the role again.
+    Status s = client.DropRole(req);
+    ASSERT_TRUE(s.IsNotFound()) << s.ToString();
+  }
+}
 } // namespace sentry
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/14f3e6f6/src/kudu/sentry/sentry_client.cc
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/sentry_client.cc b/src/kudu/sentry/sentry_client.cc
index 53d7928..530bd2e 100644
--- a/src/kudu/sentry/sentry_client.cc
+++ b/src/kudu/sentry/sentry_client.cc
@@ -17,7 +17,138 @@
 
 #include "kudu/sentry/sentry_client.h"
 
+#include <exception>
+#include <memory>
+#include <ostream>
+#include <string>
+
+#include <glog/logging.h>
+#include <thrift/Thrift.h>
+#include <thrift/protocol/TMultiplexedProtocol.h>
+#include <thrift/protocol/TProtocol.h>
+#include <thrift/transport/TTransport.h>
+#include <thrift/transport/TTransportException.h>
+
+#include "kudu/gutil/strings/substitute.h"
+#include "kudu/sentry/sentry_common_service_constants.h"
+#include "kudu/sentry/sentry_common_service_types.h"
+#include "kudu/sentry/sentry_policy_service_types.h"
+#include "kudu/thrift/client.h"
+#include "kudu/thrift/sasl_client_transport.h"
+#include "kudu/util/status.h"
+#include "kudu/util/stopwatch.h"
+
+using apache::thrift::TException;
+using apache::thrift::protocol::TMultiplexedProtocol;
+using apache::thrift::transport::TTransportException;
+using kudu::thrift::CreateClientProtocol;
+using kudu::thrift::SaslException;
+using std::make_shared;
+using strings::Substitute;
+
 namespace kudu {
 namespace sentry {
+
+const uint16_t SentryClient::kDefaultSentryPort = 8038;
+
+const int kSlowExecutionWarningThresholdMs = 1000;
+
+namespace {
+Status ConvertStatus(const ::sentry::TSentryResponseStatus& status) {
+  const auto& constants = ::sentry::g_sentry_common_service_constants;
+  // A switch isn't possible because these values aren't generated as real constants.
+  if (status.value == constants.TSENTRY_STATUS_OK) {
+    return Status::OK();
+  }
+  if (status.value == constants.TSENTRY_STATUS_ALREADY_EXISTS) {
+    return Status::AlreadyPresent(status.message, status.stack);
+  }
+  if (status.value == constants.TSENTRY_STATUS_NO_SUCH_OBJECT) {
+    return Status::NotFound(status.message, status.stack);
+  }
+  if (status.value == constants.TSENTRY_STATUS_RUNTIME_ERROR) {
+    return Status::RuntimeError(status.message, status.stack);
+  }
+  if (status.value == constants.TSENTRY_STATUS_INVALID_INPUT) {
+    return Status::InvalidArgument(status.message, status.stack);
+  }
+  if (status.value == constants.TSENTRY_STATUS_ACCESS_DENIED) {
+    return Status::NotAuthorized(status.message, status.stack);
+  }
+  if (status.value == constants.TSENTRY_STATUS_THRIFT_VERSION_MISMATCH) {
+    return Status::NotSupported(status.message, status.stack);
+  }
+  LOG(WARNING) << "Unknown error code in Sentry status: " << status;
+  return Status::RuntimeError(
+      Substitute("unknown error code: $0: $1", status.value, status.message), status.stack);
+}
+} // namespace
+
+// Wraps calls to the Sentry Thrift service, catching any exceptions and
+// converting the response status to a Kudu Status. If there is no
+// response_status then {} can be substituted.
+#define SENTRY_RET_NOT_OK(call, response_status, msg) \
+  try { \
+    (call); \
+    RETURN_NOT_OK_PREPEND(ConvertStatus(response_status), (msg)); \
+  } catch (const SaslException& e) { \
+    return e.status().CloneAndPrepend(msg); \
+  } catch (const TTransportException& e) { \
+    switch (e.getType()) { \
+      case TTransportException::TIMED_OUT: return Status::TimedOut((msg), e.what()); \
+      case TTransportException::BAD_ARGS: return Status::InvalidArgument((msg), e.what()); \
+      case TTransportException::CORRUPTED_DATA: return Status::Corruption((msg), e.what()); \
+      default: return Status::NetworkError((msg), e.what()); \
+    } \
+  } catch (const TException& e) { \
+    return Status::IOError((msg), e.what()); \
+  } catch (const std::exception& e) { \
+    return Status::RuntimeError((msg), e.what()); \
+  }
+
+SentryClient::SentryClient(const HostPort& address, const thrift::ClientOptions& options)
+      : client_(::sentry::SentryPolicyServiceClient(
+            make_shared<TMultiplexedProtocol>(CreateClientProtocol(address, options),
+                                              "SentryPolicyService"))) {
+}
+
+SentryClient::~SentryClient() {
+  WARN_NOT_OK(Stop(), "failed to shutdown Sentry client");
+}
+
+Status SentryClient::Start() {
+  SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs, "starting Sentry client");
+  SENTRY_RET_NOT_OK(client_.getOutputProtocol()->getTransport()->open(),
+                    {}, "failed to open Sentry connection");
+  return Status::OK();
+}
+
+Status SentryClient::Stop() {
+  SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs, "stopping Sentry client");
+  SENTRY_RET_NOT_OK(client_.getInputProtocol()->getTransport()->close(),
+                    {}, "failed to close Sentry connection");
+  return Status::OK();
+}
+
+bool SentryClient::IsConnected() {
+  return client_.getInputProtocol()->getTransport()->isOpen();
+}
+
+Status SentryClient::CreateRole(const ::sentry::TCreateSentryRoleRequest& request) {
+  SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs, "create Sentry role");
+  ::sentry::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) {
+  SCOPED_LOG_SLOW_EXECUTION(WARNING, kSlowExecutionWarningThresholdMs, "drop Sentry role");
+  ::sentry::TDropSentryRoleResponse response;
+  SENTRY_RET_NOT_OK(client_.drop_sentry_role(response, request),
+                    response.status, "failed to drop Sentry role");
+  return Status::OK();
+}
+
 } // namespace sentry
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/14f3e6f6/src/kudu/sentry/sentry_client.h
----------------------------------------------------------------------
diff --git a/src/kudu/sentry/sentry_client.h b/src/kudu/sentry/sentry_client.h
index 09d7916..e5d9226 100644
--- a/src/kudu/sentry/sentry_client.h
+++ b/src/kudu/sentry/sentry_client.h
@@ -17,11 +17,74 @@
 
 #pragma once
 
+#include <cstdint>
+
+#include "kudu/gutil/port.h"
+#include "kudu/sentry/SentryPolicyService.h"
+#include "kudu/util/status.h"
+
+namespace sentry {
+class TCreateSentryRoleRequest;
+class TDropSentryRoleRequest;
+}
+
 namespace kudu {
+
+class HostPort;
+
+namespace thrift {
+struct ClientOptions;
+} // namespace thrift
+
 namespace sentry {
 
+// A client for a Sentry service.
+//
+// All operations are synchronous, and may block.
+//
+// SentryClient is not thread safe.
+//
+// SentryClient wraps a single TCP connection to a Sentry service instance, and
+// does not attempt to handle or retry on failure. It's expected that a
+// higher-level component will wrap SentryClient to provide retry, pooling, and
+// HA deployment features if necessary.
+//
+// Note: see HmsClient for why TSocketPool is not used for transparently
+// handling connections to Sentry HA instances.
 class SentryClient {
  public:
+
+  static const uint16_t kDefaultSentryPort;
+
+  // Create a SentryClient connection to the provided Sentry service Thrift RPC address.
+  SentryClient(const HostPort& address, const thrift::ClientOptions& options);
+  ~SentryClient();
+
+  // Starts the Sentry service client.
+  //
+  // This method will open a synchronous TCP connection to the Sentry service.
+  // If the Sentry service can not be reached within the connection timeout
+  // interval, an error is returned.
+  //
+  // Must be called before any subsequent operations using the client.
+  Status Start() WARN_UNUSED_RESULT;
+
+  // Stops the Sentry service client.
+  //
+  // This is optional; if not called the destructor will stop the client.
+  Status Stop() WARN_UNUSED_RESULT;
+
+  // Returns 'true' if the client is connected to the remote server.
+  bool IsConnected() WARN_UNUSED_RESULT;
+
+  // Creates a new role in Sentry.
+  Status CreateRole(const ::sentry::TCreateSentryRoleRequest& request) WARN_UNUSED_RESULT;
+
+  // Drops a role in Sentry.
+  Status DropRole(const ::sentry::TDropSentryRoleRequest& request) WARN_UNUSED_RESULT;
+
+ private:
+  ::sentry::SentryPolicyServiceClient client_;
 };
 
 } // namespace sentry