Skip to content

Commit

Permalink
[hms] KUDU-1884 Add support for custom SASL protocol name
Browse files Browse the repository at this point in the history
This patch adds support to set a custom SASL protocol name
when using the KuduMetastorePlugin in the HMS. Because
there isn’t a good way to pass configuration the plugin
today, it uses an environment variable instead.

The patch updates the master_hms-itest to use the new
custom SASL protocol name feature to validate functionality.

Change-Id: Id8d9e208da1d767bb24470ab031a5266461d070b
Reviewed-on: http://gerrit.cloudera.org:8080/17350
Reviewed-by: Grant Henke <[email protected]>
Tested-by: Grant Henke <[email protected]>
  • Loading branch information
granthenke committed Apr 28, 2021
1 parent e495d6b commit d1a7044
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ public class KuduMetastorePlugin extends MetaStoreEventListener {
// are too many requests to the master.
static final String SYNC_ENABLED_ENV = "KUDU_HMS_SYNC_ENABLED";

// System env to set a custom sasl protocol name for the Kudu client.
// TODO(ghenke): Use a Hive config parameter from the KuduStorageHandler instead.
static final String SASL_PROTOCOL_NAME_ENV = "KUDU_SASL_PROTOCOL_NAME";

// Maps lists of master addresses to KuduClients to cache clients.
private static final Map<String, KuduClient> KUDU_CLIENTS =
new ConcurrentHashMap<String, KuduClient>();
Expand Down Expand Up @@ -512,7 +516,9 @@ private static KuduClient getKuduClient(String kuduMasters) {
try {
client = UserGroupInformation.getLoginUser().doAs(
(PrivilegedExceptionAction<KuduClient>) () ->
new KuduClient.KuduClientBuilder(kuduMasters).build()
new KuduClient.KuduClientBuilder(kuduMasters)
.saslProtocolName(getSaslProtocolName())
.build()
);
} catch (IOException | InterruptedException e) {
throw new RuntimeException("Failed to create the Kudu client");
Expand All @@ -521,4 +527,12 @@ private static KuduClient getKuduClient(String kuduMasters) {
}
return client;
}

private static String getSaslProtocolName() {
String saslProtocolName = System.getenv(SASL_PROTOCOL_NAME_ENV);
if (saslProtocolName == null || saslProtocolName.isEmpty()) {
saslProtocolName = "kudu";
}
return saslProtocolName;
}
}
8 changes: 8 additions & 0 deletions src/kudu/integration-tests/master_hms-itest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class MasterHmsTest : public ExternalMiniClusterITestBase {
opts.num_masters = 1;
opts.num_tablet_servers = 1;
opts.enable_kerberos = EnableKerberos();
opts.principal = Principal();
// Tune down the notification log poll period in order to speed up catalog convergence.
opts.extra_master_flags.emplace_back("--hive_metastore_notification_log_poll_period_seconds=1");
StartClusterWithOpts(std::move(opts));
Expand Down Expand Up @@ -164,6 +165,10 @@ class MasterHmsTest : public ExternalMiniClusterITestBase {
virtual bool EnableKerberos() {
return false;
}

virtual string Principal() {
return "kudu";
}
};

TEST_F(MasterHmsTest, TestCreateTable) {
Expand Down Expand Up @@ -769,6 +774,9 @@ class MasterHmsKerberizedTest : public MasterHmsTest {
bool EnableKerberos() override {
return true;
}
string Principal() override {
return "oryx";
}
};

// Checks that table ownership in a Kerberized cluster is set to the user
Expand Down
30 changes: 18 additions & 12 deletions src/kudu/integration-tests/security-itest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,18 +456,24 @@ TEST_F(SecurityITest, TestNonDefaultPrincipal) {
ASSERT_OK(StartCluster());

// A client with the default SASL proto shouldn't be able to connect
client::sp::shared_ptr<KuduClient> default_client;
Status s = cluster_->CreateClient(nullptr, &default_client);
ASSERT_TRUE(s.IsNotAuthorized());
ASSERT_STR_CONTAINS(s.ToString(), "not found in Kerberos database");

// Create a client with the matching SASL proto name and verify it's able to
// connect to the cluster and perform basic actions.
KuduClientBuilder b;
b.sasl_protocol_name(kPrincipal);
client::sp::shared_ptr<KuduClient> client;
ASSERT_OK(cluster_->CreateClient(&b, &client));
SmokeTestCluster(client);
{
client::sp::shared_ptr<KuduClient> client;
KuduClientBuilder builder;
for (auto i = 0; i < cluster_->num_masters(); ++i) {
builder.add_master_server_addr(cluster_->master(i)->bound_rpc_addr().ToString());
}
const auto s = builder.Build(&client);
ASSERT_TRUE(s.IsNotAuthorized());
ASSERT_STR_CONTAINS(s.ToString(), "not found in Kerberos database");
}

{
// Create a client with the matching SASL proto name and verify it's able to
// connect to the cluster and perform basic actions.
client::sp::shared_ptr<KuduClient> client;
ASSERT_OK(cluster_->CreateClient(nullptr, &client));
SmokeTestCluster(client);
}
}

TEST_F(SecurityITest, TestNonExistentPrincipal) {
Expand Down
5 changes: 5 additions & 0 deletions src/kudu/mini-cluster/external_mini_cluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,10 @@ Status ExternalMiniCluster::Start() {
"could not create keytab");
hms_->EnableKerberos(kdc_->GetEnvVars()["KRB5_CONFIG"], spn, ktpath,
rpc::SaslProtection::kAuthentication);

// Set the protocol name in the environment so that the KuduMetastorePlugin
// can communicate with Kudu when a custom name is used.
hms_->AddEnvVar("KUDU_SASL_PROTOCOL_NAME", opts_.principal);
}

RETURN_NOT_OK_PREPEND(hms_->Start(),
Expand Down Expand Up @@ -917,6 +921,7 @@ Status ExternalMiniCluster::CreateClient(client::KuduClientBuilder* builder,
for (const scoped_refptr<ExternalMaster>& master : masters_) {
builder->add_master_server_addr(master->bound_rpc_hostport().ToString());
}
builder->sasl_protocol_name(opts_.principal);
return builder->Build(client);
}

Expand Down

0 comments on commit d1a7044

Please sign in to comment.