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:03 UTC
[3/4] kudu git commit: KUDU-2541: Fill out basic sentry client API
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