You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2019/05/03 18:22:08 UTC

[kudu] branch master updated (754de05 -> cc6b812)

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git.


    from 754de05  [authz] add scrubbing task for the privileges cache
     new 98d9765  [catalog_manager] update criterion to choose authz provider
     new 8005afc  authz: mark tserver flag non-experimental
     new cc6b812  [authz] validator for --sentry_service_rpc_addresses flag

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 src/kudu/integration-tests/master_sentry-itest.cc | 25 +++++++++++++++++++++++
 src/kudu/master/catalog_manager.cc                |  2 +-
 src/kudu/master/sentry_privileges_fetcher.cc      | 25 +++++++++++++++++++++++
 src/kudu/tserver/tablet_service.cc                |  1 -
 4 files changed, 51 insertions(+), 2 deletions(-)


[kudu] 01/03: [catalog_manager] update criterion to choose authz provider

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 98d9765e0514f8753731e04bddf7789c5bf2ca64
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu May 2 22:05:53 2019 -0700

    [catalog_manager] update criterion to choose authz provider
    
    Prior to this change, the hypothetical case when Kudu master is given
    Sentry server's RPC end-point but no HMS catalog RPC end-point, the
    fine-grained authorization was not be enabled since the catalog
    manager would use DefaultAuthzProvider instead of SentryAuthzProvider
    for authz decisions.
    
    Code-wise, SentryAuthzProvider does not explicitly depend on the HMS
    catalog.  From that perspective, the decision whether to use
    SentryAuthzProvider or DefaultAuthzProvider for authz decisions should
    be gated only by the presence of the Sentry server's RPC end-point
    in the corresponding runtime flags.
    
    From the design perspective, Kudu+Sentry fine-grain authz scheme
    logically depends on the integration with HMS catalog (that's why the
    case described above is pure hypothetical one).  The logical dependency
    will be addressed in a few follow-up changelists.
    
    Change-Id: Iee1760a8fe6ffc9d6822db2472da5ddef78aec8d
    Reviewed-on: http://gerrit.cloudera.org:8080/13223
    Reviewed-by: Andrew Wong <aw...@cloudera.com>
    Tested-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/master/catalog_manager.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index c5b9dcd..4a87f46 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -698,7 +698,7 @@ CatalogManager::CatalogManager(Master* master)
       leader_ready_term_(-1),
       hms_notification_log_event_id_(-1),
       leader_lock_(RWMutex::Priority::PREFER_WRITING) {
-  if (hms::HmsCatalog::IsEnabled() && SentryAuthzProvider::IsEnabled()) {
+  if (SentryAuthzProvider::IsEnabled()) {
     authz_provider_.reset(new SentryAuthzProvider(master_->metric_entity()));
   } else {
     authz_provider_.reset(new DefaultAuthzProvider);


[kudu] 03/03: [authz] validator for --sentry_service_rpc_addresses flag

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit cc6b812530ed312df4201ace62881f8c505297bb
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Thu May 2 18:02:47 2019 -0700

    [authz] validator for --sentry_service_rpc_addresses flag
    
    This patch introduces a group validator for the
    --sentry_service_rpc_addresses runtime flag.  It makes it necessary
    to set a non-empty value for the --hive_metastore_uris flag if the
    --sentry_service_rpc_addresses flag is set.  In other words, this patch
    makes it explicitly impossible to run Kudu master for a hypothetical
    configuration of Kudu+Sentry authz without HMS catalog.
    
    That reflects the logical dependency of the Kudu+Sentry fine-grain
    authz scheme on the HMS catalog.  The dependency is there by design.
    
    This patchs also contains a test for the introduced flag validator.
    As of now, all existing tests for Kudu+Sentry authz are run with
    configuration where the integration with HMS catalog is enabled as well.
    
    Change-Id: Iec0470f68e34edf72a9e8baf608eda1b83272921
    Reviewed-on: http://gerrit.cloudera.org:8080/13227
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    Tested-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/integration-tests/master_sentry-itest.cc | 25 +++++++++++++++++++++++
 src/kudu/master/sentry_privileges_fetcher.cc      | 25 +++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/src/kudu/integration-tests/master_sentry-itest.cc b/src/kudu/integration-tests/master_sentry-itest.cc
index 7d49965..02ea86a 100644
--- a/src/kudu/integration-tests/master_sentry-itest.cc
+++ b/src/kudu/integration-tests/master_sentry-itest.cc
@@ -937,4 +937,29 @@ TEST_F(AuthzCacheControlTest, ResetCacheNoSentryIntegration) {
   }
 }
 
+// A test for the ValidateSentryServiceRpcAddresses group flag validator.
+// The only existing test scenario covers only the negative case, while
+// several other Sentry-related (and not) tests provide good coverage
+// for all the positive cases.
+class MasterSentryAndHmsFlagsTest : public KuduTest {
+};
+
+TEST_F(MasterSentryAndHmsFlagsTest, MasterRefuseToStart) {
+  // The code below results in setting the --sentry_service_rpc_addresses flag
+  // to the mini-sentry's RPC address, but leaving the --hive_metastore_uris
+  // flag unset (i.e. its value is an empty string). Such a combination of flag
+  // settings makes it impossible to start Kudu master.
+  cluster::ExternalMiniClusterOptions opts;
+  opts.enable_kerberos = true;
+  opts.enable_sentry = true;
+  opts.hms_mode = HmsMode::NONE;
+
+  cluster::ExternalMiniCluster cluster(std::move(opts));
+  const auto s = cluster.Start();
+  const auto msg = s.ToString();
+  ASSERT_TRUE(s.IsRuntimeError()) << msg;
+  ASSERT_STR_CONTAINS(msg, "failed to start masters: Unable to start Master");
+  ASSERT_STR_CONTAINS(msg, "kudu-master: process exited with non-zero status 1");
+}
+
 } // namespace kudu
diff --git a/src/kudu/master/sentry_privileges_fetcher.cc b/src/kudu/master/sentry_privileges_fetcher.cc
index 0242cae..5afdf91 100644
--- a/src/kudu/master/sentry_privileges_fetcher.cc
+++ b/src/kudu/master/sentry_privileges_fetcher.cc
@@ -48,6 +48,7 @@
 #include "kudu/thrift/ha_client_metrics.h"
 #include "kudu/util/async_util.h"
 #include "kudu/util/flag_tags.h"
+#include "kudu/util/flag_validators.h"
 #include "kudu/util/malloc.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/net_util.h"
@@ -144,6 +145,7 @@ DEFINE_uint32(sentry_privileges_cache_max_scrubbed_entries_per_pass, 32,
 TAG_FLAG(sentry_privileges_cache_max_scrubbed_entries_per_pass, advanced);
 
 DECLARE_int64(authz_token_validity_seconds);
+DECLARE_string(hive_metastore_uris);
 DECLARE_string(kudu_service_name);
 DECLARE_string(server_name);
 
@@ -180,6 +182,29 @@ static bool ValidateAddresses(const char* flag_name, const string& addresses) {
 }
 DEFINE_validator(sentry_service_rpc_addresses, &ValidateAddresses);
 
+// This group flag validator enforces the logical dependency of the Sentry+Kudu
+// fine-grain authz scheme on the integration with HMS catalog.
+//
+// The validator makes it necessary to set the --hive_metastore_uris flag
+// if the --sentry_service_rpc_addresses flag is set.
+//
+// Even if Kudu could successfully fetch information on granted privileges from
+// Sentry to allow or deny commencing DML operations on already existing
+// tables, the information on privileges in Sentry would become inconsistent
+// after DDL operations (e.g., renaming a table).
+bool ValidateSentryServiceRpcAddresses() {
+  if (!FLAGS_sentry_service_rpc_addresses.empty() &&
+      FLAGS_hive_metastore_uris.empty()) {
+    LOG(ERROR) << "Hive Metastore catalog is required (--hive_metastore_uris) "
+                  "to run Kudu with Sentry-backed authorization scheme "
+                  "(--sentry_service_rpc_addresses).";
+    return false;
+  }
+  return true;
+}
+GROUP_FLAG_VALIDATOR(sentry_service_rpc_addresses,
+                     ValidateSentryServiceRpcAddresses);
+
 namespace {
 
 // Returns an authorizable based on the table identifier (in the format


[kudu] 02/03: authz: mark tserver flag non-experimental

Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 8005afcc6f0e882facedec1f3b81047acb74a326
Author: Andrew Wong <aw...@apache.org>
AuthorDate: Thu May 2 23:25:51 2019 -0700

    authz: mark tserver flag non-experimental
    
    This is a follow-up to b03411c, wherein master-side authorization flags
    were marked non-experimental.
    
    Change-Id: I2315e0d65f271608da0e5a80ea543da6e35aa253
    Reviewed-on: http://gerrit.cloudera.org:8080/13225
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
    Tested-by: Andrew Wong <aw...@cloudera.com>
---
 src/kudu/tserver/tablet_service.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/kudu/tserver/tablet_service.cc b/src/kudu/tserver/tablet_service.cc
index 60b83eb..a004742 100644
--- a/src/kudu/tserver/tablet_service.cc
+++ b/src/kudu/tserver/tablet_service.cc
@@ -150,7 +150,6 @@ TAG_FLAG(scanner_inject_service_unavailable_on_continue_scan, unsafe);
 DEFINE_bool(tserver_enforce_access_control, false,
             "If set, the server will apply fine-grained access control rules "
             "to client RPCs.");
-TAG_FLAG(tserver_enforce_access_control, experimental);
 TAG_FLAG(tserver_enforce_access_control, runtime);
 
 DEFINE_double(tserver_inject_invalid_authz_token_ratio, 0.0,