You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by jd...@apache.org on 2017/05/12 15:05:46 UTC

[1/2] kudu git commit: KUDU-1941: more validation for RPC auth flags

Repository: kudu
Updated Branches:
  refs/heads/branch-1.3.x ad9f6de83 -> a1234c05a


KUDU-1941: more validation for RPC auth flags

With this patch, both master and tserver refuse to start if
authentication is 'required' but no authentication method is configured.

Prior to this patch, the inconsistency with the run-time configuration
could be detected at a later stage when a client would try to connect
to Kudu cluster.

Change-Id: I3c088fd6d7a695234e2955e09ca53626078b4e51
Reviewed-on: http://gerrit.cloudera.org:8080/6851
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit 87ddf0ae2584f2394bb26d36c01c16e6719659db)
Reviewed-on: http://gerrit.cloudera.org:8080/6862
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/436b3a4e
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/436b3a4e
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/436b3a4e

Branch: refs/heads/branch-1.3.x
Commit: 436b3a4e614720ad918f5eee23fe6155666aecd9
Parents: ad9f6de
Author: Alexey Serbin <as...@cloudera.com>
Authored: Wed May 10 18:04:25 2017 -0700
Committer: Jean-Daniel Cryans <jd...@apache.org>
Committed: Fri May 12 15:03:18 2017 +0000

----------------------------------------------------------------------
 src/kudu/rpc/messenger.cc | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/436b3a4e/src/kudu/rpc/messenger.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index f13adb2..d307ce8 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -164,11 +164,20 @@ static bool ValidateRpcAuthnFlags() {
     return false;
   }
 
+  const bool has_keytab = !FLAGS_keytab_file.empty();
+  const bool has_cert = !FLAGS_rpc_certificate_file.empty();
+  if (authentication == RpcAuthentication::REQUIRED && !has_keytab && !has_cert) {
+    LOG(ERROR) << "RPC authentication (--rpc_authentication) may not be "
+                  "required unless Kerberos (--keytab_file) or external PKI "
+                  "(--rpc_certificate_file et al) are configured";
+    return false;
+  }
+
   return true;
 }
 GROUP_FLAG_VALIDATOR(rpc_authn_flags, ValidateRpcAuthnFlags);
 
-static bool ValidatePkiFlags() {
+static bool ValidateExternalPkiFlags() {
   bool has_cert = !FLAGS_rpc_certificate_file.empty();
   bool has_key = !FLAGS_rpc_private_key_file.empty();
   bool has_ca = !FLAGS_rpc_ca_certificate_file.empty();
@@ -182,7 +191,7 @@ static bool ValidatePkiFlags() {
 
   return true;
 }
-GROUP_FLAG_VALIDATOR(pki_flags, ValidatePkiFlags);
+GROUP_FLAG_VALIDATOR(external_pki_flags, ValidateExternalPkiFlags);
 
 MessengerBuilder::MessengerBuilder(std::string name)
     : name_(std::move(name)),


[2/2] kudu git commit: [flags] fixed typo in group flag validation logic

Posted by jd...@apache.org.
[flags] fixed typo in group flag validation logic

This is a follow-up for e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3.

This patch fixes the typo in the original commit and adds test coverage
for the case of multiple group validators in the validation pipeline.

Change-Id: Idfdc25d491941674fd7b86d8ffa33ff2e046703b
Reviewed-on: http://gerrit.cloudera.org:8080/6860
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit a0006dbaa77c276c6498dc4edc0228cae049738c)
Reviewed-on: http://gerrit.cloudera.org:8080/6864
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a1234c05
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a1234c05
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a1234c05

Branch: refs/heads/branch-1.3.x
Commit: a1234c05a03c2fbb23cd28b60b4061d9aa20761b
Parents: 436b3a4
Author: Alexey Serbin <as...@cloudera.com>
Authored: Thu May 11 12:20:59 2017 -0700
Committer: Jean-Daniel Cryans <jd...@apache.org>
Committed: Fri May 12 15:03:37 2017 +0000

----------------------------------------------------------------------
 src/kudu/util/flag_validators-test.cc | 81 ++++++++++++++++++++++++++++--
 src/kudu/util/flags.cc                |  2 +-
 2 files changed, 78 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/a1234c05/src/kudu/util/flag_validators-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/flag_validators-test.cc b/src/kudu/util/flag_validators-test.cc
index 0bf553f..3e7fb5b 100644
--- a/src/kudu/util/flag_validators-test.cc
+++ b/src/kudu/util/flag_validators-test.cc
@@ -28,10 +28,12 @@
 
 DEFINE_string(grouped_0, "", "First flag to set.");
 DEFINE_string(grouped_1, "", "Second flag to set.");
+DEFINE_string(grouped_2, "", "Third flag to set.");
+DEFINE_string(grouped_3, "", "Fourth flag to set.");
 
 namespace kudu {
 
-static bool CheckGroupedFlags() {
+static bool CheckGroupedFlags01() {
   const bool is_set_0 = !FLAGS_grouped_0.empty();
   const bool is_set_1 = !FLAGS_grouped_1.empty();
 
@@ -42,7 +44,20 @@ static bool CheckGroupedFlags() {
 
   return true;
 }
-GROUP_FLAG_VALIDATOR(test_group_validator, CheckGroupedFlags)
+GROUP_FLAG_VALIDATOR(test_group_validator01, CheckGroupedFlags01)
+
+static bool CheckGroupedFlags23() {
+  const bool is_set_2 = !FLAGS_grouped_2.empty();
+  const bool is_set_3 = !FLAGS_grouped_3.empty();
+
+  if (is_set_2 != is_set_3) {
+    LOG(ERROR) << "--grouped_2 and --grouped_3 must be set as a group";
+    return false;
+  }
+
+  return true;
+}
+GROUP_FLAG_VALIDATOR(test_group_validator23, CheckGroupedFlags23)
 
 class FlagsValidatorsBasicTest : public KuduTest {
  public:
@@ -55,8 +70,10 @@ class FlagsValidatorsBasicTest : public KuduTest {
 
 TEST_F(FlagsValidatorsBasicTest, Grouped) {
   const auto& validators = GetFlagValidators();
-  ASSERT_EQ(1, validators.size());
-  const auto& validator = validators.begin()->second;
+  ASSERT_EQ(2, validators.size());
+  const auto& it = validators.find("test_group_validator01");
+  ASSERT_NE(validators.end(), it);
+  const auto& validator = it->second;
   EXPECT_TRUE(validator());
   FLAGS_grouped_0 = "0";
   EXPECT_FALSE(validator());
@@ -117,6 +134,16 @@ TEST_F(FlagsValidatorsDeathTest, GroupedSuccessSimple) {
       "--grouped_1=",
       "--grouped_0=",
     },
+    {
+      "argv_set_4",
+      "--grouped_2=2",
+      "--grouped_3=3",
+    },
+    {
+      "argv_set_5",
+      "--grouped_3=",
+      "--grouped_2=",
+    },
   };
   for (auto argv : argv_sets) {
     RunSuccess(argv, kArgvSize);
@@ -134,6 +161,52 @@ TEST_F(FlagsValidatorsDeathTest, GroupedFailureSimple) {
       "argv_set_1",
       "--grouped_1=b",
     },
+    {
+      "argv_set_2",
+      "--grouped_2=2",
+    },
+    {
+      "argv_set_3",
+      "--grouped_3=3",
+    },
+  };
+  for (auto argv : argv_sets) {
+    RunFailure(argv, kArgvSize);
+  }
+}
+
+// Test for correct behavior when only one of two group validators is failing.
+TEST_F(FlagsValidatorsDeathTest, GroupedFailureOneOfTwoValidators) {
+  static const size_t kArgvSize = 4 + 1;
+  const char* argv_sets[][kArgvSize] = {
+    {
+      "argv_set_0",
+      "--grouped_0=0",
+      "--grouped_1=1",
+      "--grouped_2=",
+      "--grouped_3=3",
+    },
+    {
+      "argv_set_1",
+      "--grouped_2=",
+      "--grouped_3=3",
+      "--grouped_0=0",
+      "--grouped_1=1",
+    },
+    {
+      "argv_set_2",
+      "--grouped_0=0",
+      "--grouped_1=",
+      "--grouped_2=2",
+      "--grouped_3=3",
+    },
+    {
+      "argv_set_3",
+      "--grouped_3=3",
+      "--grouped_2=2",
+      "--grouped_1=1",
+      "--grouped_0=",
+    },
   };
   for (auto argv : argv_sets) {
     RunFailure(argv, kArgvSize);

http://git-wip-us.apache.org/repos/asf/kudu/blob/a1234c05/src/kudu/util/flags.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/flags.cc b/src/kudu/util/flags.cc
index ed9b34b..6be68ac 100644
--- a/src/kudu/util/flags.cc
+++ b/src/kudu/util/flags.cc
@@ -425,7 +425,7 @@ void RunCustomValidators() {
   const auto& validators(GetFlagValidators());
   bool found_inconsistency = false;
   for (const auto& e : validators) {
-    found_inconsistency = !e.second();
+    found_inconsistency |= !e.second();
   }
   if (found_inconsistency) {
     LOG(ERROR) << "Detected inconsistency in command-line flags; exiting";