MB-23267: Prevent changing inapplicable options from cbepctl 99/76399/12
authorJames Harrison <00jamesh@gmail.com>
Thu, 6 Apr 2017 13:30:22 +0000 (14:30 +0100)
committerDave Rigby <daver@couchbase.com>
Wed, 12 Apr 2017 15:03:13 +0000 (15:03 +0000)
Check requirements in setFlushParam and setTapParam for the following
configuration parameters:

 "tap_keepalive"
 "access_scanner_enabled"
 "alog_sleep_time"
 "alog_task_time"
 "ephemeral_full_policy"

this will prevent them being set if their requirements are not met.

Change-Id: Ie70d062e5333393e12771d325d22438f5e865bdf
Reviewed-on: http://review.couchbase.org/76399
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dave Rigby <daver@couchbase.com>
src/ep_engine.cc
tests/module_tests/evp_engine_test.cc
tests/module_tests/evp_engine_test.h

index 517e673..226ddfe 100644 (file)
@@ -369,6 +369,7 @@ protocol_binary_response_status EventuallyPersistentEngine::setTapParam(
         if (strcmp(keyz, "tap_keepalive") == 0) {
             int v = std::stoi(valz);
             validate(v, 0, MAX_TAP_KEEP_ALIVE);
+            getConfiguration().requirementsMetOrThrow("tap_keepalive");
             setTapKeepAlive(static_cast<uint32_t>(v));
         } else if (strcmp(keyz, "replication_throttle_threshold") == 0) {
             getConfiguration().setReplicationThrottleThreshold(
@@ -517,10 +518,13 @@ protocol_binary_response_status EventuallyPersistentEngine::setFlushParam(
         } else if (strcmp(keyz, "exp_pager_initial_run_time") == 0) {
             getConfiguration().setExpPagerInitialRunTime(std::stoll(valz));
         } else if (strcmp(keyz, "access_scanner_enabled") == 0) {
+            getConfiguration().requirementsMetOrThrow("access_scanner_enabled");
             getConfiguration().setAccessScannerEnabled(cb_stob(valz));
         } else if (strcmp(keyz, "alog_sleep_time") == 0) {
+            getConfiguration().requirementsMetOrThrow("alog_sleep_time");
             getConfiguration().setAlogSleepTime(std::stoull(valz));
         } else if (strcmp(keyz, "alog_task_time") == 0) {
+            getConfiguration().requirementsMetOrThrow("alog_task_time");
             getConfiguration().setAlogTaskTime(std::stoull(valz));
         } else if (strcmp(keyz, "pager_active_vb_pcnt") == 0) {
             getConfiguration().setPagerActiveVbPcnt(std::stoull(valz));
@@ -577,6 +581,7 @@ protocol_binary_response_status EventuallyPersistentEngine::setFlushParam(
         } else if (strcmp(keyz, "vb_state_persist_run") == 0) {
             runVbStatePersistTask(std::stoi(valz));
         } else if (strcmp(keyz, "ephemeral_full_policy") == 0) {
+            getConfiguration().requirementsMetOrThrow("ephemeral_full_policy");
             getConfiguration().setEphemeralFullPolicy(valz);
         } else {
             msg = "Unknown config param";
index d851269..1403e0c 100644 (file)
@@ -25,6 +25,7 @@
 #include "programs/engine_testapp/mock_server.h"
 #include "tests/module_tests/test_helpers.h"
 
+#include <configuration_impl.h>
 #include <platform/dirutils.h>
 
 void EventuallyPersistentEngineTest::SetUp() {
@@ -86,3 +87,64 @@ void EventuallyPersistentEngineTest::store_item(uint16_t vbid,
 }
 
 const char EventuallyPersistentEngineTest::test_dbname[] = "ep_engine_ep_unit_tests_db";
+
+
+TEST_P(SetParamTest, requirements_bucket_type) {
+    std::string bucketType = engine->getConfiguration().getBucketType();
+
+    struct value_t {
+        std::string param;
+        std::string value;
+        std::string bucketType;
+    };
+
+    std::vector<value_t> values{
+            // Parameter, Example value, applicable bucket
+            {"access_scanner_enabled", "true", "persistent"},
+            {"alog_sleep_time", "1441", "persistent"},
+            {"alog_task_time", "3", "persistent"},
+            {"ephemeral_full_policy", "auto_delete", "ephemeral"},
+    };
+
+    std::string msg;
+
+    for (auto v : values) {
+        auto ret = engine->setFlushParam(v.param.c_str(), v.value.c_str(), msg);
+        if (bucketType == v.bucketType) {
+            EXPECT_EQ(PROTOCOL_BINARY_RESPONSE_SUCCESS, ret)
+                    << "Parameter " << v.param
+                    << "could not be set on bucket type \"" << bucketType
+                    << "\"";
+        } else {
+            EXPECT_EQ(PROTOCOL_BINARY_RESPONSE_EINVAL, ret)
+                    << "Setting parameter " << v.param
+                    << "should be invalid for bucket type \"" << bucketType
+                    << "\"";
+        }
+    }
+}
+
+TEST_F(EventuallyPersistentEngineTest, requirements_tap) {
+    Configuration& config = engine->getConfiguration();
+    config.setTap(true);
+
+    std::string msg;
+
+    EXPECT_EQ(PROTOCOL_BINARY_RESPONSE_SUCCESS,
+              engine->setTapParam("tap_keepalive", "0", msg))
+            << "tap is enabled but \"tap_keepalive\" could not be set";
+
+    config.setTap(false);
+
+    EXPECT_EQ(PROTOCOL_BINARY_RESPONSE_EINVAL,
+              engine->setTapParam("tap_keepalive", "0", msg))
+            << "Setting \"tap_keepalive\" should be invalid if tap is disabled";
+}
+
+// Test cases which run for persistent and ephemeral buckets
+INSTANTIATE_TEST_CASE_P(EphemeralOrPersistent,
+                        SetParamTest,
+                        ::testing::Values("persistent", "ephemeral"),
+                        [](const ::testing::TestParamInfo<std::string>& info) {
+                            return info.param;
+                        });
index 98eb453..83331c1 100644 (file)
@@ -56,3 +56,14 @@ protected:
     EventuallyPersistentEngine* engine;
     std::string bucketType;
 };
+
+/* Tests parameterised over ephemeral and persistent buckets
+ *
+ */
+class SetParamTest : public EventuallyPersistentEngineTest,
+                     public ::testing::WithParamInterface<std::string> {
+    void SetUp() override {
+        bucketType = GetParam();
+        EventuallyPersistentEngineTest::SetUp();
+    }
+};
\ No newline at end of file