config: health checker timeout and interval should be greater than 0

api/envoy/api/v2/core/health_check.proto

@@ -21,10 +21,16 @@ option (gogoproto.equal_all) = true;
 message HealthCheck {
   // The time to wait for a health check response. If the timeout is reached the
   // health check attempt will be considered a failure.
-  google.protobuf.Duration timeout = 1 [(validate.rules).duration.required = true];
+  google.protobuf.Duration timeout = 1 [
+    (validate.rules).duration = {required: true, gt: {seconds: 0}},
+    (gogoproto.stdduration) = true
+  ];
 
   // The interval between health checks.
-  google.protobuf.Duration interval = 2 [(validate.rules).duration.required = true];
+  google.protobuf.Duration interval = 2 [
+    (validate.rules).duration = {required: true, gt: {seconds: 0}},
+    (gogoproto.stdduration) = true
+  ];
 
   // An optional jitter amount in millseconds. If specified, during every
   // internal Envoy will add 0 to interval_jitter to the wait time.

test/server/BUILD

@@ -174,6 +174,7 @@ envoy_cc_test(
     srcs = ["server_test.cc"],
     data = [
         ":cluster_dupe_bootstrap.yaml",
+        ":cluster_health_check_bootstrap.yaml",
         ":empty_bootstrap.yaml",
         ":node_bootstrap.yaml",
         "//test/config/integration:server.json",

test/server/cluster_health_check_bootstrap.yaml

@@ -0,0 +1,17 @@
+admin:
+  access_log_path: /dev/null
+  address:
+    socket_address:
+      address: {{ ntop_ip_loopback_address }}
+      port_value: 0
+static_resources:
+  clusters:
+  - name: service_google
+    connect_timeout: 0.25s
+    health_checks:
+      - timeout: {{ health_check_timeout }}s
+        interval: {{ health_check_interval }}s
+        unhealthy_threshold: 1
+        healthy_threshold: 1
+        http_health_check:
+          path: "/"

test/server/server_test.cc

@@ -117,6 +117,22 @@ class ServerInstanceImplTest : public testing::TestWithParam<Network::Address::I
     EXPECT_TRUE(server_->api().fileExists("/dev/null"));
   }
 
+  void initializeWithHealthCheckParams(const std::string& bootstrap_path, const double timeout,
+                                       const double interval) {
+    options_.config_path_ = TestEnvironment::temporaryFileSubstitute(
+        bootstrap_path,
+        {{"health_check_timeout", fmt::format("{}", timeout).c_str()},
+         {"health_check_interval", fmt::format("{}", interval).c_str()}},
+        {}, version_);
+    server_.reset(new InstanceImpl(
+        options_,
+        Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("127.0.0.1")),
+        hooks_, restart_, stats_store_, fakelock_, component_factory_,
+        std::make_unique<NiceMock<Runtime::MockRandomGenerator>>(), thread_local_));
+
+    EXPECT_TRUE(server_->api().fileExists("/dev/null"));
+  }
+
   Network::Address::IpVersion version_;
   testing::NiceMock<MockOptions> options_;
   DefaultTestHooks hooks_;
@@ -189,6 +205,41 @@ TEST_P(ServerInstanceImplTest, BootstrapClusterManagerInitializationFail) {
                             "cluster manager: duplicate cluster 'service_google'");
 }
 
+// Test for protoc-gen-validate constraint on invalid timeout entry of a health check config entry.
+TEST_P(ServerInstanceImplTest, BootstrapClusterHealthCheckInvalidTimeout) {
+  options_.v2_config_only_ = true;
+  EXPECT_THROW_WITH_REGEX(
+      initializeWithHealthCheckParams("test/server/cluster_health_check_bootstrap.yaml", 0, 0.25),
+      EnvoyException,
+      "HealthCheckValidationError.Timeout: \\[\"value must be greater than \" \"0s\"\\]");
+}
+
+// Test for protoc-gen-validate constraint on invalid interval entry of a health check config entry.
+TEST_P(ServerInstanceImplTest, BootstrapClusterHealthCheckInvalidInterval) {
+  options_.v2_config_only_ = true;
+  EXPECT_THROW_WITH_REGEX(
+      initializeWithHealthCheckParams("test/server/cluster_health_check_bootstrap.yaml", 0.5, 0),
+      EnvoyException,
+      "HealthCheckValidationError.Interval: \\[\"value must be greater than \" \"0s\"\\]");
+}
+
+// Test for protoc-gen-validate constraint on invalid timeout and interval entry of a health check
+// config entry.
+TEST_P(ServerInstanceImplTest, BootstrapClusterHealthCheckInvalidTimeoutAndInterval) {
+  options_.v2_config_only_ = true;
+  EXPECT_THROW_WITH_REGEX(
+      initializeWithHealthCheckParams("test/server/cluster_health_check_bootstrap.yaml", 0, 0),
+      EnvoyException,
+      "HealthCheckValidationError.Timeout: \\[\"value must be greater than \" \"0s\"\\]");
+}
+
+// Test for protoc-gen-validate constraint on valid interval entry of a health check config entry.
+TEST_P(ServerInstanceImplTest, BootstrapClusterHealthCheckValidTimeoutAndInterval) {
+  options_.v2_config_only_ = true;
+  EXPECT_NO_THROW(initializeWithHealthCheckParams("test/server/cluster_health_check_bootstrap.yaml",
+                                                  0.25, 0.5));
+}
+
 // Negative test for protoc-gen-validate constraints.
 TEST_P(ServerInstanceImplTest, ValidateFail) {
   options_.service_cluster_name_ = "some_cluster_name";