Created
April 8, 2020 22:10
-
-
Save andrwng/957dde774a77af30e9e835a7f40e2a27 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit 6fee34dc46a6dd19112a42ab0f51046f23913c30 | |
Author: Andrew Wong <awong@cloudera.com> | |
Date: Wed Apr 8 15:08:14 2020 -0700 | |
reuse Sentry/Ranger-agnostic code | |
Change-Id: I826d89b4289c3c6116b6a7a540ed12827e21f4b6 | |
diff --git a/src/kudu/integration-tests/master_authz-itest.cc b/src/kudu/integration-tests/master_authz-itest.cc | |
index bfc1b0405..8de02cacb 100644 | |
--- a/src/kudu/integration-tests/master_authz-itest.cc | |
+++ b/src/kudu/integration-tests/master_authz-itest.cc | |
@@ -154,6 +154,8 @@ struct PrivilegeParams { | |
class MasterAuthzITestHarness { | |
public: | |
+ virtual ~MasterAuthzITestHarness() {} | |
+ | |
static Status GetTableLocationsWithTableId(const string& table_name, | |
const optional<const string&>& table_id, | |
const unique_ptr<ExternalMiniCluster>& cluster) { | |
@@ -303,37 +305,18 @@ class MasterAuthzITestHarness { | |
return Status::OK(); | |
} | |
- // The function argument passed to SetUpCluster initializer needs to | |
- // initialize the ExternalMiniCluster with the passed options and return a | |
- // pointer to the initialized cluster. | |
- void SetUpCluster(const function<const unique_ptr<ExternalMiniCluster>& ( | |
- ExternalMiniClusterOptions)>& cluster_start_func) { | |
- cluster::ExternalMiniClusterOptions opts; | |
+ ExternalMiniClusterOptions GetClusterOpts() { | |
+ ExternalMiniClusterOptions opts; | |
// Always enable Kerberos, as Sentry/Ranger deployment does not make much | |
// sense in a non-Kerberized environment. | |
bool enable_kerberos = true; | |
opts.enable_kerberos = enable_kerberos; | |
- | |
// Add 'impala' as trusted user who may access the cluster without being | |
// authorized. | |
opts.extra_master_flags.emplace_back("--trusted_user_acl=impala"); | |
opts.extra_master_flags.emplace_back("--user_acl=test-user,impala"); | |
- | |
SetUpExternalMiniServiceOpts(&opts); | |
- const unique_ptr<ExternalMiniCluster>& cluster = cluster_start_func(std::move(opts)); | |
- shared_ptr<KuduClient> client; | |
- cluster->CreateClient(nullptr, &client); | |
- | |
- // Create principals 'impala' and 'kudu'. Configure to use the latter. | |
- ASSERT_OK(cluster->kdc()->CreateUserPrincipal(kImpalaUser)); | |
- ASSERT_OK(cluster->kdc()->CreateUserPrincipal("kudu")); | |
- ASSERT_OK(cluster->kdc()->Kinit("kudu")); | |
- | |
- ASSERT_OK(SetUpExternalServiceClients(enable_kerberos, cluster)); | |
- | |
- ASSERT_OK(SetUpCredentials()); | |
- | |
- ASSERT_OK(SetUpTables(cluster, client)); | |
+ return opts; | |
} | |
virtual Status GrantCreateTablePrivilege(const PrivilegeParams& p) = 0; | |
@@ -341,10 +324,14 @@ class MasterAuthzITestHarness { | |
virtual Status GrantAlterTablePrivilege(const PrivilegeParams& p) = 0; | |
virtual Status GrantRenameTablePrivilege(const PrivilegeParams& p) = 0; | |
virtual Status GrantGetMetadataTablePrivilege(const PrivilegeParams& p) = 0; | |
+ virtual Status GrantGetMetadataDatabasePrivilege(const PrivilegeParams& p) = 0; | |
virtual Status CreateTable(const OperationParams& p, | |
const shared_ptr<KuduClient>& client) = 0; | |
+ virtual void CheckTableDoesNotExist(const string& database_name, const string& table_name, | |
+ shared_ptr<KuduClient> client) = 0; | |
+ virtual Status StartAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) = 0; | |
+ virtual Status StopAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) = 0; | |
virtual void TearDown() = 0; | |
- protected: | |
virtual void SetUpExternalMiniServiceOpts(ExternalMiniClusterOptions* opts) = 0; | |
virtual Status SetUpCredentials() = 0; | |
virtual Status SetUpExternalServiceClients(bool enable_kerberos, | |
@@ -358,19 +345,24 @@ class SentryITestHarness : public MasterAuthzITestHarness, | |
public: | |
using MasterAuthzITestHarness::CreateKuduTable; | |
using HmsITestHarness::CheckTable; | |
- Status StopAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) { | |
+ Status StopAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) override { | |
RETURN_NOT_OK(sentry_client_->Stop()); | |
RETURN_NOT_OK(cluster->sentry()->Stop()); | |
return Status::OK(); | |
} | |
- Status StartAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) { | |
+ Status StartAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) override { | |
RETURN_NOT_OK(cluster->sentry()->Start()); | |
RETURN_NOT_OK(cluster->kdc()->Kinit("kudu")); | |
RETURN_NOT_OK(sentry_client_->Start()); | |
return Status::OK(); | |
} | |
+ void CheckTableDoesNotExist(const string& database_name, const string& table_name, | |
+ shared_ptr<KuduClient> client) override { | |
+ // TODO | |
+ } | |
+ | |
Status GrantCreateTablePrivilege(const PrivilegeParams& p) override { | |
return AlterRoleGrantPrivilege(sentry_client_.get(), kDevRole, | |
GetDatabasePrivilege(p.db_name, "CREATE")); | |
@@ -398,6 +390,11 @@ class SentryITestHarness : public MasterAuthzITestHarness, | |
GetTablePrivilege(p.db_name, p.table_name, "METADATA")); | |
} | |
+ Status GrantGetMetadataDatabasePrivilege(const PrivilegeParams& p) override { | |
+ return AlterRoleGrantPrivilege(sentry_client_.get(), kDevRole, | |
+ GetDatabasePrivilege(p.db_name, "METADATA")); | |
+ } | |
+ | |
Status CreateTable(const OperationParams& p, | |
const shared_ptr<KuduClient>& client) override { | |
Slice hms_database; | |
@@ -425,9 +422,6 @@ class SentryITestHarness : public MasterAuthzITestHarness, | |
} | |
} | |
- unique_ptr<SentryClient> sentry_client_; | |
- | |
- protected: | |
void SetUpExternalMiniServiceOpts(ExternalMiniClusterOptions* opts) override { | |
opts->enable_sentry = true; | |
// Configure the timeout to reduce the run time of tests that involve | |
@@ -468,6 +462,17 @@ class SentryITestHarness : public MasterAuthzITestHarness, | |
TSentryGrantOption::DISABLED); | |
return AlterRoleGrantPrivilege(sentry_client_.get(), kAdminRole, privilege); | |
} | |
+ | |
+ protected: | |
+ unique_ptr<SentryClient> sentry_client_; | |
+}; | |
+ | |
+class SentryWithCacheITestHarness : public SentryITestHarness { | |
+ public: | |
+ void SetUpExternalMiniServiceOpts(ExternalMiniClusterOptions* opts) override { | |
+ NO_FATALS(SentryITestHarness::SetUpExternalMiniServiceOpts(opts)); | |
+ opts->extra_master_flags.emplace_back("--sentry_privileges_cache_capacity_mb=1"); | |
+ } | |
}; | |
class RangerITestHarness : public MasterAuthzITestHarness { | |
@@ -494,6 +499,11 @@ class RangerITestHarness : public MasterAuthzITestHarness { | |
return Status::OK(); | |
} | |
+ void CheckTableDoesNotExist(const string& database_name, const string& table_name, | |
+ shared_ptr<KuduClient> client) override { | |
+ // TODO | |
+ } | |
+ | |
Status GrantAlterTablePrivilege(const PrivilegeParams& p) override { | |
AuthorizationPolicy policy; | |
policy.databases.emplace_back(p.db_name); | |
@@ -501,7 +511,6 @@ class RangerITestHarness : public MasterAuthzITestHarness { | |
policy.items.emplace_back(PolicyItem({kTestUser}, {ActionPB::ALTER})); | |
RETURN_NOT_OK(ranger_->AddPolicy(move(policy))); | |
SleepFor(MonoDelta::FromMilliseconds(1500)); | |
- | |
return Status::OK(); | |
} | |
@@ -517,9 +526,16 @@ class RangerITestHarness : public MasterAuthzITestHarness { | |
policy.tables.emplace_back(p.table_name); | |
policy.items.emplace_back(PolicyItem({kTestUser}, {ActionPB::ALL})); | |
RETURN_NOT_OK(ranger_->AddPolicy(move(policy))); | |
- | |
SleepFor(MonoDelta::FromMilliseconds(1500)); | |
+ return Status::OK(); | |
+ } | |
+ Status GrantGetMetadataDatabasePrivilege(const PrivilegeParams& p) override { | |
+ AuthorizationPolicy policy; | |
+ policy.databases.emplace_back(p.db_name); | |
+ policy.items.emplace_back(PolicyItem({kTestUser}, {ActionPB::METADATA})); | |
+ RETURN_NOT_OK(ranger_->AddPolicy(move(policy))); | |
+ SleepFor(MonoDelta::FromMilliseconds(1500)); | |
return Status::OK(); | |
} | |
@@ -530,7 +546,6 @@ class RangerITestHarness : public MasterAuthzITestHarness { | |
policy.items.emplace_back(PolicyItem({kTestUser}, {ActionPB::METADATA})); | |
RETURN_NOT_OK(ranger_->AddPolicy(move(policy))); | |
SleepFor(MonoDelta::FromMilliseconds(1500)); | |
- | |
return Status::OK(); | |
} | |
@@ -543,11 +558,11 @@ class RangerITestHarness : public MasterAuthzITestHarness { | |
return CreateKuduTable(hms_database.ToString(), hms_table.ToString(), client); | |
} | |
- Status StartAuthzProvider() { | |
+ Status StartAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) override { | |
return ranger_->Start(); | |
} | |
- Status StopAuthzProvider() { | |
+ Status StopAuthzProvider(const unique_ptr<ExternalMiniCluster>& cluster) override { | |
return ranger_->Stop(); | |
} | |
@@ -579,19 +594,45 @@ class RangerITestHarness : public MasterAuthzITestHarness { | |
MiniRanger* ranger_; | |
}; | |
+enum HarnessEnum { | |
+ kSentry, | |
+ kSentryWithCache, | |
+ kRanger, | |
+}; | |
+ | |
// Test basic master authorization enforcement with Sentry and HMS integration | |
// enabled. | |
-template<class AuthzITestHarness> | |
-class MasterAuthzITest : public ExternalMiniClusterITestBase { | |
+class MasterAuthzITestBase : public ExternalMiniClusterITestBase { | |
public: | |
- void SetUp() override { | |
+ void SetUp() override = 0; | |
+ void SetUpHarness(HarnessEnum harness) { | |
ExternalMiniClusterITestBase::SetUp(); | |
- harness_.SetUpCluster([&] (ExternalMiniClusterOptions opts) -> | |
- const unique_ptr<ExternalMiniCluster>& { | |
- AddExtraOpts(&opts); | |
- StartClusterWithOpts(opts); | |
- return cluster_; | |
- }); | |
+ switch (harness) { | |
+ case kSentry: | |
+ harness_.reset(new SentryITestHarness()); | |
+ break; | |
+ case kSentryWithCache: | |
+ harness_.reset(new SentryWithCacheITestHarness()); | |
+ break; | |
+ case kRanger: | |
+ harness_.reset(new RangerITestHarness()); | |
+ break; | |
+ default: | |
+ LOG(FATAL) << "unknown harness"; | |
+ } | |
+ ExternalMiniClusterOptions opts = harness_->GetClusterOpts(); | |
+ NO_FATALS(StartClusterWithOpts(std::move(opts))); | |
+ | |
+ shared_ptr<KuduClient> client; | |
+ cluster_->CreateClient(nullptr, &client); | |
+ // Create principals 'impala' and 'kudu'. Configure to use the latter. | |
+ ASSERT_OK(cluster_->kdc()->CreateUserPrincipal(kImpalaUser)); | |
+ ASSERT_OK(cluster_->kdc()->CreateUserPrincipal("kudu")); | |
+ ASSERT_OK(cluster_->kdc()->Kinit("kudu")); | |
+ | |
+ ASSERT_OK(harness_->SetUpExternalServiceClients(/*enable_kerberos*/true, cluster_)); | |
+ ASSERT_OK(harness_->SetUpCredentials()); | |
+ ASSERT_OK(harness_->SetUpTables(cluster_, client)); | |
// Log in as 'test-user' and reset the client to pick up the change in user. | |
ASSERT_OK(cluster_->kdc()->Kinit(kTestUser)); | |
@@ -599,193 +640,119 @@ class MasterAuthzITest : public ExternalMiniClusterITestBase { | |
} | |
void TearDown() override { | |
- harness_.TearDown(); | |
+ harness_->TearDown(); | |
ExternalMiniClusterITestBase::TearDown(); | |
} | |
Status GetTableLocationsWithTableId(const string& table_name, | |
optional<const string&> table_id) { | |
- return harness_.GetTableLocationsWithTableId(table_name, table_id, cluster_); | |
+ return harness_->GetTableLocationsWithTableId(table_name, table_id, cluster_); | |
} | |
Status GrantCreateTablePrivilege(const PrivilegeParams& p) { | |
- return harness_.GrantCreateTablePrivilege(p); | |
+ return harness_->GrantCreateTablePrivilege(p); | |
} | |
Status GrantDropTablePrivilege(const PrivilegeParams& p) { | |
- return harness_.GrantDropTablePrivilege(p); | |
+ return harness_->GrantDropTablePrivilege(p); | |
} | |
Status GrantAlterTablePrivilege(const PrivilegeParams& p) { | |
- return harness_.GrantAlterTablePrivilege(p); | |
+ return harness_->GrantAlterTablePrivilege(p); | |
} | |
Status GrantRenameTablePrivilege(const PrivilegeParams& p) { | |
- return harness_.GrantRenameTablePrivilege(p); | |
+ return harness_->GrantRenameTablePrivilege(p); | |
} | |
Status GrantGetMetadataTablePrivilege(const PrivilegeParams& p) { | |
- return harness_.GrantGetMetadataTablePrivilege(p); | |
+ return harness_->GrantGetMetadataTablePrivilege(p); | |
+ } | |
+ | |
+ Status GrantGetMetadataDatabasePrivilege(const PrivilegeParams& p) { | |
+ return harness_->GrantGetMetadataDatabasePrivilege(p); | |
} | |
Status CreateTable(const OperationParams& p) { | |
- return harness_.CreateTable(p, client_); | |
+ return harness_->CreateTable(p, client_); | |
} | |
Status IsCreateTableDone(const OperationParams& p) { | |
- return harness_.IsCreateTableDone(p, client_); | |
+ return harness_->IsCreateTableDone(p, client_); | |
} | |
Status DropTable(const OperationParams& p) { | |
- return harness_.DropTable(p, client_); | |
+ return harness_->DropTable(p, client_); | |
} | |
Status AlterTable(const OperationParams& p) { | |
- return harness_.AlterTable(p, client_); | |
+ return harness_->AlterTable(p, client_); | |
} | |
Status IsAlterTableDone(const OperationParams& p) { | |
- return harness_.IsAlterTableDone(p, client_); | |
+ return harness_->IsAlterTableDone(p, client_); | |
} | |
Status RenameTable(const OperationParams& p) { | |
- return harness_.RenameTable(p, client_); | |
+ return harness_->RenameTable(p, client_); | |
} | |
Status GetTableSchema(const OperationParams& p) { | |
- return harness_.GetTableSchema(p, client_); | |
+ return harness_->GetTableSchema(p, client_); | |
} | |
Status GetTableLocations(const OperationParams& p) { | |
- return harness_.GetTableLocations(p, cluster_); | |
+ return harness_->GetTableLocations(p, cluster_); | |
} | |
Status GetTabletLocations(const OperationParams& p) { | |
- return harness_.GetTabletLocations(p, cluster_); | |
+ return harness_->GetTabletLocations(p, cluster_); | |
} | |
Status CreateKuduTable(const std::string& database_name, | |
const std::string& table_name, | |
MonoDelta timeout = {}) { | |
- return harness_.CreateKuduTable(database_name, table_name, client_, timeout); | |
+ return harness_->CreateKuduTable(database_name, table_name, client_, timeout); | |
} | |
void CheckTable(const std::string& database_name, | |
const std::string& table_name, | |
boost::optional<const std::string&> user, | |
const std::string& table_type = hms::HmsClient::kManagedTable) { | |
- harness_.CheckTable(database_name, table_name, user, cluster_, client_, table_type); | |
+ harness_->CheckTable(database_name, table_name, user, cluster_, client_, table_type); | |
} | |
void CheckTableDoesNotExist(const std::string& database_name, | |
const std::string& table_name) { | |
- harness_.CheckTableDoesNotExist(database_name, table_name, client_); | |
+ harness_->CheckTableDoesNotExist(database_name, table_name, client_); | |
} | |
Status StartAuthzProvider() { | |
- return harness_.StartAuthzProvider(cluster_); | |
+ return harness_->StartAuthzProvider(cluster_); | |
} | |
Status StopAuthzProvider() { | |
- return harness_.StopAuthzProvider(cluster_); | |
- } | |
- | |
- Status StartHms() { | |
- return harness_.StartHms(cluster_); | |
- } | |
- | |
- Status StopHms() { | |
- return harness_.StopHms(cluster_); | |
- } | |
- | |
- virtual bool IsAuthzPrivilegeCacheEnabled() const { | |
- return false; | |
+ return harness_->StopAuthzProvider(cluster_); | |
} | |
protected: | |
- AuthzITestHarness harness_; | |
+ std::unique_ptr<MasterAuthzITestHarness> harness_; | |
private: | |
virtual void AddExtraOpts(ExternalMiniClusterOptions* opts) {} | |
}; | |
-typedef MasterAuthzITest<SentryITestHarness> MasterSentryTest; | |
-typedef MasterAuthzITest<RangerITestHarness> MasterRangerTest; | |
- | |
-template<> | |
-void MasterSentryTest::AddExtraOpts(ExternalMiniClusterOptions* opts) { | |
- // Enable/disable caching of authz privileges received from Sentry. | |
- opts->extra_master_flags.emplace_back( | |
- Substitute("--sentry_privileges_cache_capacity_mb=$0", | |
- IsAuthzPrivilegeCacheEnabled() ? 1 : 0)); | |
-} | |
- | |
-// We unfortunately need to duplicate a bunch of code here as we can't | |
-// templatize variables and typedefs. | |
- | |
-// Functor that performs a certain operation (e.g. Create, Rename) on a table | |
-// given its name and its desired new name, if necessary (only used for Rename). | |
-typedef function<Status(MasterSentryTest*, const OperationParams&)> OperatorFuncSentry; | |
-typedef function<Status(MasterRangerTest*, const OperationParams&)> OperatorFuncRanger; | |
- | |
-// Functor that grants the required permission for an operation (e.g Create, | |
-// Rename) on a table given the database the table belongs to and the name of | |
-// the table, if applicable. | |
-typedef function<Status(MasterSentryTest*, const PrivilegeParams&)> PrivilegeFuncSentry; | |
-typedef function<Status(MasterRangerTest*, const PrivilegeParams&)> PrivilegeFuncRanger; | |
- | |
-// A description of the operation function that describes a particular action | |
-// on a table a user can perform, as well as the privilege granting function | |
-// that grants the required permission to the user to perform the action. | |
-struct AuthzFuncsSentry { | |
- OperatorFuncSentry do_action; | |
- PrivilegeFuncSentry grant_privileges; | |
- string description; | |
-}; | |
-ostream& operator <<(ostream& out, const AuthzFuncsSentry& d) { | |
- out << d.description; | |
- return out; | |
-} | |
- | |
-struct AuthzFuncsRanger { | |
- OperatorFuncRanger do_action; | |
- PrivilegeFuncRanger grant_privileges; | |
- string description; | |
-}; | |
-ostream& operator <<(ostream& out, const AuthzFuncsRanger& d) { | |
- out << d.description; | |
- return out; | |
-} | |
- | |
- | |
-// A description of an authorization process, including the protected resource (table), | |
-// the operation function, as well as the privilege granting function. | |
-struct AuthzDescriptorSentry { | |
- AuthzFuncsSentry funcs; | |
- string database; | |
- string table_name; | |
- string new_table_name; | |
-}; | |
-ostream& operator <<(ostream& out, const AuthzDescriptorSentry& d) { | |
- out << d.funcs.description; | |
- return out; | |
-} | |
- | |
-struct AuthzDescriptorRanger { | |
- AuthzFuncsRanger funcs; | |
- string database; | |
- string table_name; | |
- string new_table_name; | |
+class MasterAuthzITest : public MasterAuthzITestBase, | |
+ public ::testing::WithParamInterface<HarnessEnum> { | |
+ public: | |
+ void SetUp() override { | |
+ NO_FATALS(SetUpHarness(GetParam())); | |
+ } | |
}; | |
-ostream& operator <<(ostream& out, const AuthzDescriptorRanger& d) { | |
- out << d.funcs.description; | |
- return out; | |
-} | |
-typedef ::testing::Types<SentryITestHarness, RangerITestHarness> MasterAuthzITestTypes; | |
-TYPED_TEST_CASE(MasterAuthzITest, MasterAuthzITestTypes); | |
+INSTANTIATE_TEST_CASE_P(AuthzProvides, MasterAuthzITest, ::testing::Values(kSentry, kRanger)); | |
-TEST_F(MasterRangerTest, TestCreateTableAuthorized) { | |
+TEST_P(MasterAuthzITest, TestCreateTableAuthorized) { | |
ASSERT_OK(cluster_->kdc()->Kinit(kAdminUser)); | |
ASSERT_OK(cluster_->CreateClient(nullptr, &client_)); | |
@@ -803,7 +770,7 @@ TEST_F(MasterRangerTest, TestCreateTableAuthorized) { | |
.Create()); | |
} | |
-TEST_F(MasterRangerTest, TestCreateTableUnauthorized) { | |
+TEST_P(MasterAuthzITest, TestCreateTableUnauthorized) { | |
ASSERT_OK(cluster_->kdc()->Kinit(kTestUser)); | |
ASSERT_OK(cluster_->CreateClient(nullptr, &client_)); | |
@@ -822,7 +789,7 @@ TEST_F(MasterRangerTest, TestCreateTableUnauthorized) { | |
} | |
// Test that the trusted user can access the cluster without being authorized. | |
-TYPED_TEST(MasterAuthzITest, TestTrustedUserAcl) { | |
+TEST_P(MasterAuthzITest, TestTrustedUserAcl) { | |
// Log in as 'impala' and reset the client to pick up the change in user. | |
ASSERT_OK(this->cluster_->kdc()->Kinit(kImpalaUser)); | |
ASSERT_OK(this->cluster_->CreateClient(nullptr, &this->client_)); | |
@@ -839,7 +806,7 @@ TYPED_TEST(MasterAuthzITest, TestTrustedUserAcl) { | |
make_optional<const string&>(kImpalaUser))); | |
} | |
-TYPED_TEST(MasterAuthzITest, TestAuthzListTables) { | |
+TEST_P(MasterAuthzITest, TestAuthzListTables) { | |
// ListTables is not parameterized as other operations below, because non-authorized | |
// tables will be filtered instead of returning NOT_AUTHORIZED error. | |
const auto table_name = Substitute("$0.$1", kDatabaseName, kTableName); | |
@@ -865,7 +832,7 @@ TYPED_TEST(MasterAuthzITest, TestAuthzListTables) { | |
// When authorizing ListTables, if there is a concurrent rename, we may not end | |
// up showing the table. | |
-TYPED_TEST(MasterAuthzITest, TestAuthzListTablesConcurrentRename) { | |
+TEST_P(MasterAuthzITest, TestAuthzListTablesConcurrentRename) { | |
ASSERT_OK(this->cluster_->SetFlag(this->cluster_->master(), | |
"catalog_manager_inject_latency_list_authz_ms", "3000"));; | |
const auto table_name = Substitute("$0.$1", kDatabaseName, kTableName); | |
@@ -888,7 +855,7 @@ TYPED_TEST(MasterAuthzITest, TestAuthzListTablesConcurrentRename) { | |
ASSERT_EQ(sec_table_name, tables[0]); | |
} | |
-TEST_F(MasterSentryTest, TestTableOwnership) { | |
+TEST_P(MasterAuthzITest, TestTableOwnership) { | |
ASSERT_OK(GrantCreateTablePrivilege({ kDatabaseName })); | |
ASSERT_OK(CreateKuduTable(kDatabaseName, "new_table")); | |
NO_FATALS(CheckTable(kDatabaseName, "new_table", | |
@@ -905,7 +872,7 @@ TEST_F(MasterSentryTest, TestTableOwnership) { | |
} | |
// Checks Sentry privileges are synchronized upon table rename in the HMS. | |
-TEST_F(MasterSentryTest, TestRenameTablePrivilegeTransfer) { | |
+TEST_P(MasterAuthzITest, TestRenameTablePrivilegeTransfer) { | |
ASSERT_OK(GrantRenameTablePrivilege({ kDatabaseName, kTableName })); | |
ASSERT_OK(RenameTable({ Substitute("$0.$1", kDatabaseName, kTableName), | |
Substitute("$0.$1", kDatabaseName, "b") })); | |
@@ -928,138 +895,56 @@ TEST_F(MasterSentryTest, TestRenameTablePrivilegeTransfer) { | |
NO_FATALS(CheckTable(kDatabaseName, "b", make_optional<const string&>(kAdminUser))); | |
} | |
-class TestAuthzTableSentry : | |
- public MasterSentryTest, | |
- public ::testing::WithParamInterface<AuthzDescriptorSentry> { | |
- // TODO(aserbin): update the test to introduce authz privilege caching | |
-}; | |
- | |
-TEST_P(TestAuthzTableSentry, TestAuthorizeTable) { | |
- const AuthzDescriptorSentry& desc = GetParam(); | |
- const auto table_name = Substitute("$0.$1", desc.database, desc.table_name); | |
- const auto new_table_name = Substitute("$0.$1", | |
- desc.database, desc.new_table_name); | |
- const OperationParams action_params = { table_name, new_table_name }; | |
- const PrivilegeParams privilege_params = { desc.database, desc.table_name }; | |
+// We unfortunately need to duplicate a bunch of code here as we can't | |
+// templatize variables and typedefs. | |
- // User 'test-user' attempts to operate on the table without proper privileges. | |
- Status s = desc.funcs.do_action(this, action_params); | |
- ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString(); | |
- ASSERT_STR_CONTAINS(s.ToString(), "unauthorized action"); | |
+// Functor that performs a certain operation (e.g. Create, Rename) on a table | |
+// given its name and its desired new name, if necessary (only used for Rename). | |
+typedef function<Status(MasterAuthzITestBase*, const OperationParams&)> OperatorFunc; | |
- // User 'test-user' can operate on the table after obtaining proper privileges. | |
- ASSERT_OK(desc.funcs.grant_privileges(this, privilege_params)); | |
- ASSERT_OK(desc.funcs.do_action(this, action_params)); | |
+// Functor that grants the required permission for an operation (e.g Create, | |
+// Rename) on a table given the database the table belongs to and the name of | |
+// the table, if applicable. | |
+typedef function<Status(MasterAuthzITestBase*, const PrivilegeParams&)> PrivilegeFunc; | |
- // Ensure that operating on a table while the Sentry is unreachable fails. | |
- ASSERT_OK(StopAuthzProvider()); | |
- s = desc.funcs.do_action(this, action_params); | |
- ASSERT_TRUE(s.IsNetworkError()) << s.ToString(); | |
+// A description of the operation function that describes a particular action | |
+// on a table a user can perform, as well as the privilege granting function | |
+// that grants the required permission to the user to perform the action. | |
+struct AuthzFuncs { | |
+ OperatorFunc do_action; | |
+ PrivilegeFunc grant_privileges; | |
+ string description; | |
+}; | |
+ostream& operator <<(ostream& out, const AuthzFuncs& d) { | |
+ out << d.description; | |
+ return out; | |
} | |
-static const AuthzDescriptorSentry kAuthzCombinationsSentry[] = { | |
- { | |
- { | |
- &MasterSentryTest::CreateTable, | |
- &MasterSentryTest::GrantCreateTablePrivilege, | |
- "CreateTable", | |
- }, | |
- kDatabaseName, | |
- "new_table", | |
- "" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::DropTable, | |
- &MasterSentryTest::GrantDropTablePrivilege, | |
- "DropTable", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::AlterTable, | |
- &MasterSentryTest::GrantAlterTablePrivilege, | |
- "AlterTable", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::RenameTable, | |
- &MasterSentryTest::GrantRenameTablePrivilege, | |
- "RenameTable", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "new_table" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::GetTableSchema, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "GetTableSchema", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::GetTableLocations, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "GetTableLocations", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::GetTabletLocations, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "GetTabletLocations", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::IsCreateTableDone, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "IsCreateTableDone", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "" | |
- }, | |
- { | |
- { | |
- &MasterSentryTest::IsAlterTableDone, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "IsAlterTableDone", | |
- }, | |
- kDatabaseName, | |
- kTableName, | |
- "" | |
- }, | |
+// A description of an authorization process, including the protected resource (table), | |
+// the operation function, as well as the privilege granting function. | |
+struct AuthzDescriptor { | |
+ AuthzFuncs funcs; | |
+ string database; | |
+ string table_name; | |
+ string new_table_name; | |
}; | |
-INSTANTIATE_TEST_CASE_P(AuthzCombinationsSentry, | |
- TestAuthzTableSentry, | |
- ::testing::ValuesIn(kAuthzCombinationsSentry)); | |
+ostream& operator <<(ostream& out, const AuthzDescriptor& d) { | |
+ out << d.funcs.description; | |
+ return out; | |
+} | |
-class TestAuthzTableRanger : | |
- public MasterRangerTest, | |
- public ::testing::WithParamInterface<AuthzDescriptorRanger> { | |
+class TestAuthzTable : | |
+ public MasterAuthzITestBase, | |
+ public ::testing::WithParamInterface<std::tuple<HarnessEnum, AuthzDescriptor>> { | |
+ public: | |
+ void SetUp() override { | |
+ HarnessEnum h = std::get<0>(GetParam()); | |
+ NO_FATALS(SetUpHarness(h)); | |
+ } | |
}; | |
-TEST_P(TestAuthzTableRanger, TestAuthorizeTable) { | |
- const AuthzDescriptorRanger& desc = GetParam(); | |
+TEST_P(TestAuthzTable, TestAuthorizeTable) { | |
+ const AuthzDescriptor& desc = std::get<1>(GetParam()); | |
const auto table_name = Substitute("$0.$1", desc.database, desc.table_name); | |
const auto new_table_name = Substitute("$0.$1", | |
desc.database, desc.new_table_name); | |
@@ -1069,18 +954,23 @@ TEST_P(TestAuthzTableRanger, TestAuthorizeTable) { | |
// User 'test-user' attempts to operate on the table without proper privileges. | |
Status s = desc.funcs.do_action(this, action_params); | |
ASSERT_TRUE(s.IsNotAuthorized()) << s.ToString(); | |
- ASSERT_STR_MATCHES(s.ToString(), "[Uu]nauthorized action"); | |
+ ASSERT_STR_CONTAINS(s.ToString(), "unauthorized action"); | |
// User 'test-user' can operate on the table after obtaining proper privileges. | |
ASSERT_OK(desc.funcs.grant_privileges(this, privilege_params)); | |
ASSERT_OK(desc.funcs.do_action(this, action_params)); | |
+ | |
+ // Ensure that operating on a table while the Sentry is unreachable fails. | |
+ ASSERT_OK(StopAuthzProvider()); | |
+ s = desc.funcs.do_action(this, action_params); | |
+ ASSERT_TRUE(s.IsNetworkError()) << s.ToString(); | |
} | |
-static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
+static const AuthzDescriptor kAuthzCombinations[] = { | |
{ | |
{ | |
- &MasterRangerTest::CreateTable, | |
- &MasterRangerTest::GrantCreateTablePrivilege, | |
+ &MasterAuthzITestBase::CreateTable, | |
+ &MasterAuthzITestBase::GrantCreateTablePrivilege, | |
"CreateTable", | |
}, | |
kDatabaseName, | |
@@ -1089,8 +979,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::DropTable, | |
- &MasterRangerTest::GrantDropTablePrivilege, | |
+ &MasterAuthzITestBase::DropTable, | |
+ &MasterAuthzITestBase::GrantDropTablePrivilege, | |
"DropTable", | |
}, | |
kDatabaseName, | |
@@ -1099,8 +989,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::AlterTable, | |
- &MasterRangerTest::GrantAlterTablePrivilege, | |
+ &MasterAuthzITestBase::AlterTable, | |
+ &MasterAuthzITestBase::GrantAlterTablePrivilege, | |
"AlterTable", | |
}, | |
kDatabaseName, | |
@@ -1109,8 +999,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::RenameTable, | |
- &MasterRangerTest::GrantRenameTablePrivilege, | |
+ &MasterAuthzITestBase::RenameTable, | |
+ &MasterAuthzITestBase::GrantRenameTablePrivilege, | |
"RenameTable", | |
}, | |
kDatabaseName, | |
@@ -1119,8 +1009,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::GetTableSchema, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::GetTableSchema, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"GetTableSchema", | |
}, | |
kDatabaseName, | |
@@ -1129,8 +1019,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::GetTableLocations, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::GetTableLocations, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"GetTableLocations", | |
}, | |
kDatabaseName, | |
@@ -1139,8 +1029,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::GetTabletLocations, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::GetTabletLocations, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"GetTabletLocations", | |
}, | |
kDatabaseName, | |
@@ -1149,8 +1039,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::IsCreateTableDone, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::IsCreateTableDone, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"IsCreateTableDone", | |
}, | |
kDatabaseName, | |
@@ -1159,8 +1049,8 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
}, | |
{ | |
{ | |
- &MasterRangerTest::IsAlterTableDone, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::IsAlterTableDone, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"IsAlterTableDone", | |
}, | |
kDatabaseName, | |
@@ -1168,16 +1058,18 @@ static const AuthzDescriptorRanger kAuthzCombinationsRanger[] = { | |
"" | |
}, | |
}; | |
-INSTANTIATE_TEST_CASE_P(AuthzCombinationsRanger, | |
- TestAuthzTableRanger, | |
- ::testing::ValuesIn(kAuthzCombinationsRanger)); | |
+INSTANTIATE_TEST_CASE_P(AuthzCombinations, | |
+ TestAuthzTable, | |
+ ::testing::Combine( | |
+ ::testing::Values(kSentry, kRanger), | |
+ ::testing::ValuesIn(kAuthzCombinations))); | |
// Test that when the client passes a table identifier with the table name | |
// and table ID refer to different tables, the client needs permission on | |
// both tables for returning TABLE_NOT_FOUND error to avoid leaking table | |
// existence. | |
-TYPED_TEST(MasterAuthzITest, TestMismatchedTable) { | |
+TEST_P(MasterAuthzITest, TestMismatchedTable) { | |
const auto table_name_a = Substitute("$0.$1", kDatabaseName, kTableName); | |
const auto table_name_b = Substitute("$0.$1", kDatabaseName, kSecondTable); | |
@@ -1207,14 +1099,25 @@ TYPED_TEST(MasterAuthzITest, TestMismatchedTable) { | |
ASSERT_STR_CONTAINS(s.ToString(), "the table ID refers to a different table"); | |
} | |
-class AuthzErrorHandlingTestSentry : | |
- public MasterSentryTest, | |
- public ::testing::WithParamInterface<AuthzFuncsSentry> { | |
+class MasterSentryTest : public MasterAuthzITestBase { | |
+ public: | |
+ void SetUp() override { | |
+ NO_FATALS(SetUpHarness(kSentry)); | |
+ } | |
+}; | |
+ | |
+class AuthzErrorHandlingTest : | |
+ public MasterAuthzITestBase, | |
+ public ::testing::WithParamInterface<std::tuple<HarnessEnum, AuthzFuncs>> { | |
// TODO(aserbin): update the test to introduce authz privilege caching | |
+ public: | |
+ void SetUp() override { | |
+ NO_FATALS(SetUpHarness(std::get<0>(GetParam()))); | |
+ } | |
}; | |
-TEST_P(AuthzErrorHandlingTestSentry, TestNonExistentTable) { | |
+TEST_P(AuthzErrorHandlingTest, TestNonExistentTable) { | |
static constexpr const char* const kTableName = "non_existent"; | |
- const AuthzFuncsSentry& funcs = GetParam(); | |
+ const AuthzFuncs& funcs = std::get<1>(GetParam()); | |
const auto table_name = Substitute("$0.$1", kDatabaseName, kTableName); | |
const auto new_table_name = Substitute("$0.$1", kDatabaseName, "b"); | |
const OperationParams action_params = { table_name, new_table_name }; | |
@@ -1228,7 +1131,7 @@ TEST_P(AuthzErrorHandlingTestSentry, TestNonExistentTable) { | |
ASSERT_STR_MATCHES(s.ToString(), "[Uu]nauthorized action"); | |
// Ensure that operating on a non-existent table fails | |
- // while Sentry is unreachable. | |
+ // while Sentry/Ranger is unreachable. | |
ASSERT_OK(StopAuthzProvider()); | |
s = funcs.do_action(this, action_params); | |
ASSERT_TRUE(s.IsNetworkError()) << s.ToString(); | |
@@ -1245,122 +1148,56 @@ TEST_P(AuthzErrorHandlingTestSentry, TestNonExistentTable) { | |
ASSERT_TRUE(s.IsNotFound()) << s.ToString(); | |
} | |
-static const AuthzFuncsSentry kAuthzFuncCombinationsSentry[] = { | |
- { | |
- &MasterSentryTest::DropTable, | |
- &MasterSentryTest::GrantDropTablePrivilege, | |
- "DropTable" | |
- }, | |
- { | |
- &MasterSentryTest::AlterTable, | |
- &MasterSentryTest::GrantAlterTablePrivilege, | |
- "AlterTable" | |
- }, | |
- { | |
- &MasterSentryTest::RenameTable, | |
- &MasterSentryTest::GrantRenameTablePrivilege, | |
- "RenameTable" | |
- }, | |
- { | |
- &MasterSentryTest::GetTableSchema, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "GetTableSchema" | |
- }, | |
- { | |
- &MasterSentryTest::GetTableLocations, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "GetTableLocations" | |
- }, | |
- { | |
- &MasterSentryTest::IsCreateTableDone, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "IsCreateTableDone" | |
- }, | |
- { | |
- &MasterSentryTest::IsAlterTableDone, | |
- &MasterSentryTest::GrantGetMetadataTablePrivilege, | |
- "IsAlterTableDone" | |
- }, | |
-}; | |
- | |
-INSTANTIATE_TEST_CASE_P(AuthzFuncCombinationsSentry, | |
- AuthzErrorHandlingTestSentry, | |
- ::testing::ValuesIn(kAuthzFuncCombinationsSentry)); | |
- | |
-class AuthzErrorHandlingTestRanger : | |
- public MasterRangerTest, | |
- public ::testing::WithParamInterface<AuthzFuncsRanger> { | |
- // TODO(aserbin): update the test to introduce authz privilege caching | |
-}; | |
-TEST_P(AuthzErrorHandlingTestRanger, TestNonExistentTable) { | |
- static constexpr const char* const kTableName = "non_existent"; | |
- const AuthzFuncsRanger& funcs = GetParam(); | |
- const auto table_name = Substitute("$0.$1", kDatabaseName, kTableName); | |
- const auto new_table_name = Substitute("$0.$1", kDatabaseName, "b"); | |
- const OperationParams action_params = { table_name, new_table_name }; | |
- const PrivilegeParams privilege_params = { kDatabaseName, kTableName }; | |
- | |
- // Ensure that operating on a non-existent table without proper privileges gives | |
- // a NOT_AUTHORIZED error, instead of leaking the table existence by giving a | |
- // TABLE_NOT_FOUND error. | |
- Status s = funcs.do_action(this, action_params); | |
- ASSERT_TRUE(s.IsNotAuthorized()); | |
- ASSERT_STR_MATCHES(s.ToString(), "[Uu]nauthorized action"); | |
- | |
- ASSERT_OK(funcs.grant_privileges(this, privilege_params)); | |
- s = funcs.do_action(this, action_params); | |
- ASSERT_TRUE(s.IsNotFound()) << s.ToString(); | |
-} | |
- | |
-static const AuthzFuncsRanger kAuthzFuncCombinationsRanger[] = { | |
+static const AuthzFuncs kAuthzFuncCombinations[] = { | |
{ | |
- &MasterRangerTest::DropTable, | |
- &MasterRangerTest::GrantDropTablePrivilege, | |
+ &MasterAuthzITestBase::DropTable, | |
+ &MasterAuthzITestBase::GrantDropTablePrivilege, | |
"DropTable" | |
}, | |
{ | |
- &MasterRangerTest::AlterTable, | |
- &MasterRangerTest::GrantAlterTablePrivilege, | |
+ &MasterAuthzITestBase::AlterTable, | |
+ &MasterAuthzITestBase::GrantAlterTablePrivilege, | |
"AlterTable" | |
}, | |
{ | |
- &MasterRangerTest::RenameTable, | |
- &MasterRangerTest::GrantRenameTablePrivilege, | |
+ &MasterAuthzITestBase::RenameTable, | |
+ &MasterAuthzITestBase::GrantRenameTablePrivilege, | |
"RenameTable" | |
}, | |
{ | |
- &MasterRangerTest::GetTableSchema, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::GetTableSchema, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"GetTableSchema" | |
}, | |
{ | |
- &MasterRangerTest::GetTableLocations, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::GetTableLocations, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"GetTableLocations" | |
}, | |
{ | |
- &MasterRangerTest::IsCreateTableDone, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::IsCreateTableDone, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"IsCreateTableDone" | |
}, | |
{ | |
- &MasterRangerTest::IsAlterTableDone, | |
- &MasterRangerTest::GrantGetMetadataTablePrivilege, | |
+ &MasterAuthzITestBase::IsAlterTableDone, | |
+ &MasterAuthzITestBase::GrantGetMetadataTablePrivilege, | |
"IsAlterTableDone" | |
}, | |
}; | |
-INSTANTIATE_TEST_CASE_P(AuthzFuncCombinationsRanger, | |
- AuthzErrorHandlingTestRanger, | |
- ::testing::ValuesIn(kAuthzFuncCombinationsRanger)); | |
- | |
+INSTANTIATE_TEST_CASE_P(AuthzFuncCombinations, | |
+ AuthzErrorHandlingTest, | |
+ ::testing::Combine( | |
+ ::testing::Values(kSentry, kRanger), | |
+ ::testing::ValuesIn(kAuthzFuncCombinations))); | |
// Class for test scenarios verifying functionality of managing AuthzProvider's | |
// privileges cache via Kudu RPC. | |
-class SentryAuthzProviderCacheITest : public MasterSentryTest { | |
+class SentryAuthzProviderCacheITest : public MasterAuthzITestBase { | |
public: | |
- bool IsAuthzPrivilegeCacheEnabled() const override { | |
- return true; | |
+ void SetUp() override { | |
+ NO_FATALS(SetUpHarness(kSentryWithCache)); | |
} | |
Status ResetCache() { | |
@@ -1514,9 +1351,7 @@ TEST_F(SentryAuthzProviderCacheITest, DISABLED_CreateTables) { | |
// Grant CREATE TABLE and METADATA privileges on the database. | |
ASSERT_OK(GrantCreateTablePrivilege({ kDatabaseName })); | |
- ASSERT_OK(AlterRoleGrantPrivilege( | |
- harness_.sentry_client_.get(), kDevRole, | |
- GetDatabasePrivilege(kDatabaseName, "METADATA"))); | |
+ ASSERT_OK(GrantGetMetadataDatabasePrivilege({ kDatabaseName })); | |
// Make sure it's possible to create a table in the database. This also | |
// populates the privileges cache with information on the privileges |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment