From 6d65e138cff97e166fcf84add45429d98391a17a Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Wed, 18 Jul 2018 15:32:04 -0400 Subject: [PATCH 1/3] Remove deprecated endpoint field for the health check filter Signed-off-by: Matt Rice --- .../http/health_check/v2/health_check.proto | 7 +-- source/common/config/filter_json.cc | 5 +- .../filters/http/health_check/config.cc | 15 +----- .../filters/http/health_check/config_test.cc | 48 ++++--------------- 4 files changed, 16 insertions(+), 59 deletions(-) diff --git a/api/envoy/config/filter/http/health_check/v2/health_check.proto b/api/envoy/config/filter/http/health_check/v2/health_check.proto index 88106e93136cd..4efc0fbda1d71 100644 --- a/api/envoy/config/filter/http/health_check/v2/health_check.proto +++ b/api/envoy/config/filter/http/health_check/v2/health_check.proto @@ -19,11 +19,8 @@ message HealthCheck { // Specifies whether the filter operates in pass through mode or not. google.protobuf.BoolValue pass_through_mode = 1 [(validate.rules).message.required = true]; - // Specifies the incoming HTTP endpoint that should be considered the - // health check endpoint. For example */healthcheck*. - // Note that this field is deprecated in favor of - // :ref:`headers `. - string endpoint = 2 [deprecated = true]; + reserved 2; + reserved "endpoint"; // If operating in pass through mode, the amount of time in milliseconds // that the filter should cache the upstream response. diff --git a/source/common/config/filter_json.cc b/source/common/config/filter_json.cc index a00b452800884..e8462b92642de 100644 --- a/source/common/config/filter_json.cc +++ b/source/common/config/filter_json.cc @@ -302,7 +302,10 @@ void FilterJson::translateHealthCheckFilter( JSON_UTIL_SET_BOOL(json_config, proto_config, pass_through_mode); JSON_UTIL_SET_DURATION(json_config, proto_config, cache_time); - JSON_UTIL_SET_STRING(json_config, proto_config, endpoint); + std::string endpoint = json_config.getString("endpoint"); + auto& header = *proto_config.add_headers(); + header.set_name(":path"); + header.set_exact_match(endpoint); } void FilterJson::translateGrpcJsonTranscoder( diff --git a/source/extensions/filters/http/health_check/config.cc b/source/extensions/filters/http/health_check/config.cc index 04e28992198f2..56b682dafa382 100644 --- a/source/extensions/filters/http/health_check/config.cc +++ b/source/extensions/filters/http/health_check/config.cc @@ -20,25 +20,12 @@ Http::FilterFactoryCb HealthCheckFilterConfig::createFilterFactoryFromProtoTyped const bool pass_through_mode = proto_config.pass_through_mode().value(); const int64_t cache_time_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config, cache_time, 0); - const std::string hc_endpoint = proto_config.endpoint(); auto header_match_data = std::make_shared>(); - // TODO(mrice32): remove endpoint field at the end of the 1.7.0 deprecation cycle. - const bool endpoint_set = !proto_config.endpoint().empty(); - if (endpoint_set) { - envoy::api::v2::route::HeaderMatcher matcher; - matcher.set_name(Http::Headers::get().Path.get()); - matcher.set_exact_match(proto_config.endpoint()); - header_match_data->emplace_back(matcher); - } - for (const envoy::api::v2::route::HeaderMatcher& matcher : proto_config.headers()) { Http::HeaderUtility::HeaderData single_header_match(matcher); - // Ignore any path header matchers if the endpoint field has been set. - if (!(endpoint_set && single_header_match.name_ == Http::Headers::get().Path)) { - header_match_data->push_back(std::move(single_header_match)); - } + header_match_data->push_back(std::move(single_header_match)); } if (!pass_through_mode && cache_time_ms) { diff --git a/test/extensions/filters/http/health_check/config_test.cc b/test/extensions/filters/http/health_check/config_test.cc index dd743334991f8..a966cae896c85 100644 --- a/test/extensions/filters/http/health_check/config_test.cc +++ b/test/extensions/filters/http/health_check/config_test.cc @@ -71,8 +71,10 @@ TEST(HealthCheckFilterConfig, FailsWhenNotPassThroughButTimeoutSetProto) { NiceMock context; config.mutable_pass_through_mode()->set_value(false); - config.set_endpoint("foo"); config.mutable_cache_time()->set_seconds(10); + envoy::api::v2::route::HeaderMatcher& header = *config.add_headers(); + header.set_name(":path"); + header.set_exact_match("foo"); EXPECT_THROW( healthCheckFilterConfig.createFilterFactoryFromProto(config, "dummy_stats_prefix", context), @@ -85,7 +87,9 @@ TEST(HealthCheckFilterConfig, NotFailingWhenNotPassThroughAndTimeoutNotSetProto) NiceMock context; config.mutable_pass_through_mode()->set_value(false); - config.set_endpoint("foo"); + envoy::api::v2::route::HeaderMatcher& header = *config.add_headers(); + header.set_name(":path"); + header.set_exact_match("foo"); healthCheckFilterConfig.createFilterFactoryFromProto(config, "dummy_stats_prefix", context); } @@ -97,7 +101,9 @@ TEST(HealthCheckFilterConfig, HealthCheckFilterWithEmptyProto) { healthCheckFilterConfig.createEmptyConfigProto().get()); config.mutable_pass_through_mode()->set_value(false); - config.set_endpoint("foo"); + envoy::api::v2::route::HeaderMatcher& header = *config.add_headers(); + header.set_name(":path"); + header.set_exact_match("foo"); healthCheckFilterConfig.createFilterFactoryFromProto(config, "dummy_stats_prefix", context); } @@ -195,42 +201,6 @@ TEST(HealthCheckFilterConfig, HealthCheckFilterHeaderMatchMissingHeader) { testHealthCheckHeaderMatch(config, headers, false); } -// If an endpoint is specified and the path matches, it should match regardless of any :path -// conditions given in the headers field. -TEST(HealthCheckFilterConfig, HealthCheckFilterEndpoint) { - envoy::config::filter::http::health_check::v2::HealthCheck config; - - config.mutable_pass_through_mode()->set_value(false); - - config.set_endpoint("foo"); - - envoy::api::v2::route::HeaderMatcher& header = *config.add_headers(); - header.set_name(Http::Headers::get().Path.get()); - header.set_exact_match("bar"); - - Http::TestHeaderMapImpl headers{{Http::Headers::get().Path.get(), "foo"}}; - - testHealthCheckHeaderMatch(config, headers, true); -} - -// If an endpoint is specified and the path does not match, the filter should not match regardless -// of any :path conditions given in the headers field. -TEST(HealthCheckFilterConfig, HealthCheckFilterEndpointOverride) { - envoy::config::filter::http::health_check::v2::HealthCheck config; - - config.mutable_pass_through_mode()->set_value(false); - - config.set_endpoint("foo"); - - envoy::api::v2::route::HeaderMatcher& header = *config.add_headers(); - header.set_name(Http::Headers::get().Path.get()); - header.set_exact_match("bar"); - - Http::TestHeaderMapImpl headers{{Http::Headers::get().Path.get(), "bar"}}; - - testHealthCheckHeaderMatch(config, headers, false); -} - // Conditions for the same header should match if they are both satisfied. TEST(HealthCheckFilterConfig, HealthCheckFilterDuplicateMatch) { envoy::config::filter::http::health_check::v2::HealthCheck config; From df4529edf4402f6ea84915b1145576d9683b454d Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Mon, 23 Jul 2018 17:56:49 -0400 Subject: [PATCH 2/3] Fix docs. Signed-off-by: Matt Rice --- .../config/filter/http/health_check/v2/health_check.proto | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/envoy/config/filter/http/health_check/v2/health_check.proto b/api/envoy/config/filter/http/health_check/v2/health_check.proto index 4efc0fbda1d71..0f584b451f68a 100644 --- a/api/envoy/config/filter/http/health_check/v2/health_check.proto +++ b/api/envoy/config/filter/http/health_check/v2/health_check.proto @@ -33,8 +33,6 @@ message HealthCheck { // Specifies a set of health check request headers to match on. The health check filter will // check a request’s headers against all the specified headers. To specify the health check - // endpoint, set the ``:path`` header to match on. Note that if the - // :ref:`endpoint ` - // field is set, it will overwrite any ``:path`` header to match. + // endpoint, set the ``:path`` header to match on. repeated envoy.api.v2.route.HeaderMatcher headers = 5; } From a5888df4ac892ef8cae58f3a0a1b05c5ceada79d Mon Sep 17 00:00:00 2001 From: Matt Rice Date: Tue, 24 Jul 2018 10:03:49 -0400 Subject: [PATCH 3/3] Fix docs again. Signed-off-by: Matt Rice --- docs/root/intro/version_history.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index e6f55e59f3c5f..31a53991ec5f9 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -91,8 +91,7 @@ Version history * health check: health check connections can now be configured to use http/2. * health check http filter: added :ref:`generic header matching ` - to trigger health check response. Deprecated the - :ref:`endpoint option `. + to trigger health check response. Deprecated the endpoint option. * http: filters can now optionally support :ref:`virtual host `, :ref:`route `, and