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 <[email protected]> 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 <[email protected]> Tested-by: Alexey Serbin <[email protected]> --- 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
