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
The following commit(s) were added to refs/heads/master by this push:
new b981b23 [master] Validate extra config properties for a table
b981b23 is described below
commit b981b2305f8b4a92ed9394ac37ca659a1335c688
Author: Yingchun Lai <[email protected]>
AuthorDate: Wed Sep 29 00:49:29 2021 +0800
[master] Validate extra config properties for a table
Prior to this patch, no validation was performed on the strings
representing the keys of extra configuration properties for a
table.
This patch adds the missing validation logic, so now an error
is returned on attempt to set unsupported extra configuration
properties for a table.
Change-Id: Iff5ccc553fdbe164cf342df6882ea0b78a9e10f9
Reviewed-on: http://gerrit.cloudera.org:8080/17914
Tested-by: Kudu Jenkins
Reviewed-by: Bankim Bhavsar <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/client/client-test.cc | 21 ++++++++++++++++++++-
src/kudu/common/wire_protocol.cc | 17 ++++++++++++++---
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/src/kudu/client/client-test.cc b/src/kudu/client/client-test.cc
index 9965f17..8eec04b 100644
--- a/src/kudu/client/client-test.cc
+++ b/src/kudu/client/client-test.cc
@@ -4780,7 +4780,26 @@ TEST_F(ClientTest, TestBasicAlterOperations) {
ASSERT_TRUE(tablet_replica->tablet()->metadata()->extra_config()->has_history_max_age_sec());
ASSERT_EQ(7200,
tablet_replica->tablet()->metadata()->extra_config()->history_max_age_sec());
}
- // 3. Reset history max age second to default.
+ // 3. Set an unexpected extra config.
+ {
+ map<string, string> extra_configs;
+ extra_configs["kudu.table.history_max_age_sec"] = "3600";
+ extra_configs["kudu.table.maintenance_priority_BAD_NAME"] = "2";
+ unique_ptr<KuduTableAlterer>
table_alterer(client_->NewTableAlterer(kTableName));
+ table_alterer->AlterExtraConfig(extra_configs);
+ auto s = table_alterer->Alter();
+ ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString();
+ ASSERT_STR_CONTAINS(s.ToString(),
+ "invalid extra configuration property: "
+ "kudu.table.maintenance_priority_BAD_NAME");
+ // Schema version and old properties are not changed.
+ ASSERT_EQ(10, tablet_replica->tablet()->metadata()->schema_version());
+ ASSERT_NE(boost::none,
tablet_replica->tablet()->metadata()->extra_config());
+
ASSERT_TRUE(tablet_replica->tablet()->metadata()->extra_config()->has_history_max_age_sec());
+ ASSERT_EQ(7200,
tablet_replica->tablet()->metadata()->extra_config()->history_max_age_sec());
+
ASSERT_FALSE(tablet_replica->tablet()->metadata()->extra_config()->has_maintenance_priority());
+ }
+ // 4. Reset history max age second to default.
{
map<string, string> extra_configs;
extra_configs["kudu.table.history_max_age_sec"] = "";
diff --git a/src/kudu/common/wire_protocol.cc b/src/kudu/common/wire_protocol.cc
index cdc5dde..8665fc0 100644
--- a/src/kudu/common/wire_protocol.cc
+++ b/src/kudu/common/wire_protocol.cc
@@ -24,6 +24,7 @@
#include <cstring>
#include <ostream>
#include <string>
+#include <unordered_set>
#include <vector>
#include <boost/optional/optional.hpp>
@@ -41,6 +42,7 @@
#include "kudu/common/wire_protocol.pb.h"
#include "kudu/consensus/metadata.pb.h"
#include "kudu/gutil/fixedarray.h"
+#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/strings/fastmem.h"
#include "kudu/gutil/strings/numbers.h"
@@ -67,6 +69,7 @@ using kudu::pb_util::SecureDebugString;
using kudu::pb_util::SecureShortDebugString;
using std::map;
using std::string;
+using std::unordered_set;
using std::vector;
using strings::Substitute;
@@ -647,8 +650,6 @@ Status ColumnPredicateFromPB(const Schema& schema,
return Status::OK();
}
-const char kTableHistoryMaxAgeSec[] = "kudu.table.history_max_age_sec";
-const char kTableMaintenancePriority[] = "kudu.table.maintenance_priority";
Status ExtraConfigPBToMap(const TableExtraConfigPB& pb, map<string, string>*
configs) {
Map<string, string> tmp;
RETURN_NOT_OK(ExtraConfigPBToPBMap(pb, &tmp));
@@ -665,11 +666,21 @@ Status ParseInt32Config(const string& name, const string&
value, int32_t* result
return Status::OK();
}
+static const std::string kTableHistoryMaxAgeSec =
"kudu.table.history_max_age_sec";
+static const std::string kTableMaintenancePriority =
"kudu.table.maintenance_priority";
+
Status ExtraConfigPBFromPBMap(const Map<string, string>& configs,
TableExtraConfigPB* pb) {
+ static const unordered_set<string> kSupportedConfigs({kTableHistoryMaxAgeSec,
+
kTableMaintenancePriority});
TableExtraConfigPB result;
for (const auto& config : configs) {
const string& name = config.first;
const string& value = config.second;
+ if (!ContainsKey(kSupportedConfigs, name)) {
+ return Status::InvalidArgument(
+ Substitute("invalid extra configuration property: $0", name));
+ }
+
if (name == kTableHistoryMaxAgeSec) {
if (!value.empty()) {
int32_t history_max_age_sec;
@@ -683,7 +694,7 @@ Status ExtraConfigPBFromPBMap(const Map<string, string>&
configs, TableExtraConf
result.set_maintenance_priority(maintenance_priority);
}
} else {
- LOG(WARNING) << "Unknown extra configuration property: " << name;
+ LOG(FATAL) << "Unknown extra configuration property: " << name;
}
}
*pb = std::move(result);
