From 3b3c392ae89b747a45db157801b80e2a35fea94c Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 7 May 2021 22:55:34 +0000 Subject: [PATCH 01/11] matching: disable hcm integration by default Signed-off-by: Snow Pettersen --- source/common/http/match_wrapper/config.cc | 4 ++ source/common/runtime/runtime_features.cc | 2 + test/common/http/match_wrapper/BUILD | 1 + test/common/http/match_wrapper/config_test.cc | 40 +++++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/source/common/http/match_wrapper/config.cc b/source/common/http/match_wrapper/config.cc index 85e394a56fe2a..fe1bf7efd34f0 100644 --- a/source/common/http/match_wrapper/config.cc +++ b/source/common/http/match_wrapper/config.cc @@ -87,6 +87,10 @@ Envoy::Http::FilterFactoryCb MatchWrapperConfig::createFilterFactoryFromProtoTyp const envoy::extensions::common::matching::v3::ExtensionWithMatcher& proto_config, const std::string& prefix, Server::Configuration::FactoryContext& context) { + if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.experimental_matching_api")) { + throw EnvoyException("Experimental matching API is not enabled"); + } + ASSERT(proto_config.has_extension_config()); auto& factory = Config::Utility::getAndCheckFactory( diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 01e83ce35caeb..4e7d105604446 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -115,6 +115,8 @@ constexpr const char* disabled_runtime_features[] = { "envoy.reloadable_features.remove_legacy_json", // Sentinel and test flag. "envoy.reloadable_features.test_feature_false", + // Allows the use of ExtensionWithMatcher to wrap a HTTP filter with a match tree. + "envoy.reloadable_features.experimental_matching_api", }; RuntimeFeatures::RuntimeFeatures() { diff --git a/test/common/http/match_wrapper/BUILD b/test/common/http/match_wrapper/BUILD index 52425ab72baa7..241c2b9d33fb1 100644 --- a/test/common/http/match_wrapper/BUILD +++ b/test/common/http/match_wrapper/BUILD @@ -15,5 +15,6 @@ envoy_cc_test( "//source/common/http/match_wrapper:config", "//test/mocks/server:factory_context_mocks", "//test/test_common:registry_lib", + "//test/test_common:test_runtime_lib", ], ) diff --git a/test/common/http/match_wrapper/config_test.cc b/test/common/http/match_wrapper/config_test.cc index 13cfb40df4e52..c2802e1eeaeb9 100644 --- a/test/common/http/match_wrapper/config_test.cc +++ b/test/common/http/match_wrapper/config_test.cc @@ -6,6 +6,7 @@ #include "test/mocks/server/factory_context.h" #include "test/test_common/registry.h" +#include "test/test_common/test_runtime.h" #include "gtest/gtest.h" @@ -47,7 +48,42 @@ struct TestFactory : public Envoy::Server::Configuration::NamedHttpFilterConfigF } }; +TEST(MatchWrapper, DisbaledByDefault) { + NiceMock factory_context; + + const auto config = + TestUtility::parseYaml(R"EOF( +extension_config: + name: test + typed_config: + "@type": type.googleapis.com/google.protobuf.StringValue +matcher: + matcher_tree: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: default-matcher-header + exact_match_map: + map: + match: + action: + name: skip + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.common.matcher.action.v3.SkipFilter +)EOF"); + + MatchWrapperConfig match_wrapper_config; + EXPECT_THROW_WITH_MESSAGE( + match_wrapper_config.createFilterFactoryFromProto(config, "", factory_context), + EnvoyException, "Experimental matching API is not enabled"); +} + TEST(MatchWrapper, WithMatcher) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.experimental_matching_api", "true"}}); + TestFactory test_factory; Envoy::Registry::InjectFactory inject_factory(test_factory); @@ -95,6 +131,10 @@ TEST(MatchWrapper, WithMatcher) { } TEST(MatchWrapper, WithMatcherInvalidDataInput) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.reloadable_features.experimental_matching_api", "true"}}); + TestFactory test_factory; Envoy::Registry::InjectFactory inject_factory(test_factory); From e136891d9e438719e2d3c0ab8c00de408e4d5443 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 7 May 2021 23:02:47 +0000 Subject: [PATCH 02/11] add version note Signed-off-by: Snow Pettersen --- docs/root/version_history/current.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 10ec44499448b..55386d2a9a49d 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -20,6 +20,9 @@ Minor Behavior Changes ``envoy.reloadable_features.send_strict_1xx_and_204_response_headers`` (do not send 1xx or 204 responses with these headers). Both are true by default. * http: serve HEAD requests from cache. +* http: disable the integration between :ref:`ExtensionWithMatcher ` + and HTTP filters by default to reflects its experimental status. This feature can be enabled by seting + `envoy.reloadable_features.experimental_matching_api` to true. * listener: respect the :ref:`connection balance config ` defined within the listener where the sockets are redirected to. Clear that field to restore the previous behavior. From 85e725203ecf53db03571945754a4986b35a982a Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 7 May 2021 23:03:10 +0000 Subject: [PATCH 03/11] reorder Signed-off-by: Snow Pettersen --- docs/root/version_history/current.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 55386d2a9a49d..01f292588e71b 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -14,15 +14,15 @@ Minor Behavior Changes *Changes that may cause incompatibilities for some users, but should not for most* * access_log: add new access_log command operator ``%REQUEST_TX_DURATION%``. +* http: disable the integration between :ref:`ExtensionWithMatcher ` + and HTTP filters by default to reflects its experimental status. This feature can be enabled by seting + `envoy.reloadable_features.experimental_matching_api` to true. * http: replaced setting ``envoy.reloadable_features.strict_1xx_and_204_response_headers`` with settings ``envoy.reloadable_features.require_strict_1xx_and_204_response_headers`` (require upstream 1xx or 204 responses to not have Transfer-Encoding or non-zero Content-Length headers) and ``envoy.reloadable_features.send_strict_1xx_and_204_response_headers`` (do not send 1xx or 204 responses with these headers). Both are true by default. * http: serve HEAD requests from cache. -* http: disable the integration between :ref:`ExtensionWithMatcher ` - and HTTP filters by default to reflects its experimental status. This feature can be enabled by seting - `envoy.reloadable_features.experimental_matching_api` to true. * listener: respect the :ref:`connection balance config ` defined within the listener where the sockets are redirected to. Clear that field to restore the previous behavior. From bc7540ea39b6fe139035b5eee0fafc01ebd23e5e Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 7 May 2021 23:03:29 +0000 Subject: [PATCH 04/11] format Signed-off-by: Snow Pettersen --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 01f292588e71b..a340bd8bf17ff 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -16,7 +16,7 @@ Minor Behavior Changes * access_log: add new access_log command operator ``%REQUEST_TX_DURATION%``. * http: disable the integration between :ref:`ExtensionWithMatcher ` and HTTP filters by default to reflects its experimental status. This feature can be enabled by seting - `envoy.reloadable_features.experimental_matching_api` to true. + ``envoy.reloadable_features.experimental_matching_api`` to true. * http: replaced setting ``envoy.reloadable_features.strict_1xx_and_204_response_headers`` with settings ``envoy.reloadable_features.require_strict_1xx_and_204_response_headers`` (require upstream 1xx or 204 responses to not have Transfer-Encoding or non-zero Content-Length headers) and From f4d2c71ee36236e8c9094c5067a749c9fcfb812b Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 7 May 2021 23:17:36 +0000 Subject: [PATCH 05/11] set runtime flag for integration test Signed-off-by: Snow Pettersen --- test/integration/integration_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index e6481e7e89613..17f74e02c1c7a 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -368,6 +368,8 @@ TEST_P(IntegrationTest, EnvoyProxying100ContinueWithDecodeDataPause) { // Verifies that we can construct a match tree with a filter, and that we are able to skip // filter invocation through the match tree. TEST_P(IntegrationTest, MatchingHttpFilterConstruction) { + config_helper_.addRuntimeOverride( + "envoy.reloadable_features.experimental_matching_api", "true"); config_helper_.addFilter(R"EOF( name: matcher typed_config: From ce4fabdd686ff137d486035394fde32a9bb348c0 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 7 May 2021 23:17:47 +0000 Subject: [PATCH 06/11] format Signed-off-by: Snow Pettersen --- test/integration/integration_test.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/integration_test.cc b/test/integration/integration_test.cc index 17f74e02c1c7a..c75bb2c497b74 100644 --- a/test/integration/integration_test.cc +++ b/test/integration/integration_test.cc @@ -368,8 +368,7 @@ TEST_P(IntegrationTest, EnvoyProxying100ContinueWithDecodeDataPause) { // Verifies that we can construct a match tree with a filter, and that we are able to skip // filter invocation through the match tree. TEST_P(IntegrationTest, MatchingHttpFilterConstruction) { - config_helper_.addRuntimeOverride( - "envoy.reloadable_features.experimental_matching_api", "true"); + config_helper_.addRuntimeOverride("envoy.reloadable_features.experimental_matching_api", "true"); config_helper_.addFilter(R"EOF( name: matcher typed_config: From 4e3ed2a271c235b3bad4cfb58f5010f01a761c04 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Sat, 8 May 2021 00:04:58 +0000 Subject: [PATCH 07/11] docs Signed-off-by: Snow Pettersen --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index a340bd8bf17ff..999be21203eb5 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -14,7 +14,7 @@ Minor Behavior Changes *Changes that may cause incompatibilities for some users, but should not for most* * access_log: add new access_log command operator ``%REQUEST_TX_DURATION%``. -* http: disable the integration between :ref:`ExtensionWithMatcher ` +* http: disable the integration between :ref:`ExtensionWithMatcher ` and HTTP filters by default to reflects its experimental status. This feature can be enabled by seting ``envoy.reloadable_features.experimental_matching_api`` to true. * http: replaced setting ``envoy.reloadable_features.strict_1xx_and_204_response_headers`` with settings From 2869d5ab7bdcdb4df2b7b0c9725edeb3abdc36f1 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Sat, 8 May 2021 00:05:32 +0000 Subject: [PATCH 08/11] spelling Signed-off-by: Snow Pettersen --- test/common/http/match_wrapper/config_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/http/match_wrapper/config_test.cc b/test/common/http/match_wrapper/config_test.cc index c2802e1eeaeb9..3a79a21b01aee 100644 --- a/test/common/http/match_wrapper/config_test.cc +++ b/test/common/http/match_wrapper/config_test.cc @@ -48,7 +48,7 @@ struct TestFactory : public Envoy::Server::Configuration::NamedHttpFilterConfigF } }; -TEST(MatchWrapper, DisbaledByDefault) { +TEST(MatchWrapper, DisabledByDefault) { NiceMock factory_context; const auto config = From 6ad8bd3f25f1556bab6a445d50e7f043ac25e8d5 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 10 May 2021 20:48:03 +0000 Subject: [PATCH 09/11] fix examples Signed-off-by: Snow Pettersen --- .../http/http_filters/_include/composite.yaml | 9 ++++++++- .../advanced/matching/_include/complicated.yaml | 8 ++++++++ .../advanced/matching/_include/request_response.yaml | 8 ++++++++ .../arch_overview/advanced/matching/_include/simple.yaml | 8 ++++++++ test/config_test/config_test.cc | 8 +++++++- 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/docs/root/configuration/http/http_filters/_include/composite.yaml b/docs/root/configuration/http/http_filters/_include/composite.yaml index 28f441bd73071..48109818710bf 100644 --- a/docs/root/configuration/http/http_filters/_include/composite.yaml +++ b/docs/root/configuration/http/http_filters/_include/composite.yaml @@ -2,7 +2,6 @@ admin: access_log_path: /tmp/admin_access.log address: socket_address: { address: 0.0.0.0, port_value: 9901 } - static_resources: listeners: - name: listener1 @@ -92,3 +91,11 @@ static_resources: socket_address: address: 127.0.0.1 port_value: 50051 + +layered_runtime: + layers: + - name: static-layer + static_layer: + envoy: + reloadable_features: + experimental_matching_api: true diff --git a/docs/root/intro/arch_overview/advanced/matching/_include/complicated.yaml b/docs/root/intro/arch_overview/advanced/matching/_include/complicated.yaml index c68d020f9c9df..0c385de903e42 100644 --- a/docs/root/intro/arch_overview/advanced/matching/_include/complicated.yaml +++ b/docs/root/intro/arch_overview/advanced/matching/_include/complicated.yaml @@ -91,3 +91,11 @@ static_resources: socket_address: address: 127.0.0.1 port_value: 8080 + +layered_runtime: + layers: + - name: static-layer + static_layer: + envoy: + reloadable_features: + experimental_matching_api: true diff --git a/docs/root/intro/arch_overview/advanced/matching/_include/request_response.yaml b/docs/root/intro/arch_overview/advanced/matching/_include/request_response.yaml index 4a9eaedf23e42..78ae0b18b1b79 100644 --- a/docs/root/intro/arch_overview/advanced/matching/_include/request_response.yaml +++ b/docs/root/intro/arch_overview/advanced/matching/_include/request_response.yaml @@ -77,3 +77,11 @@ static_resources: socket_address: address: 127.0.0.1 port_value: 8080 + +layered_runtime: + layers: + - name: static-layer + static_layer: + envoy: + reloadable_features: + experimental_matching_api: true diff --git a/docs/root/intro/arch_overview/advanced/matching/_include/simple.yaml b/docs/root/intro/arch_overview/advanced/matching/_include/simple.yaml index afd0aea63a8bb..9e00c6dba57d9 100644 --- a/docs/root/intro/arch_overview/advanced/matching/_include/simple.yaml +++ b/docs/root/intro/arch_overview/advanced/matching/_include/simple.yaml @@ -65,3 +65,11 @@ static_resources: socket_address: address: 127.0.0.1 port_value: 8080 + +layered_runtime: + layers: + - name: static-layer + static_layer: + envoy: + reloadable_features: + experimental_matching_api: true diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 705811dc29e15..896a27bde6c4e 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -85,6 +85,12 @@ class ConfigTest { ScopedRuntimeInjector scoped_runtime(server_.runtime()); ON_CALL(server_.runtime_loader_.snapshot_, deprecatedFeatureEnabled(_, _)) .WillByDefault(Invoke([](absl::string_view, bool default_value) { return default_value; })); + + // TODO(snowp): There's no way to override runtime flags per example file (since we mock out the runtime loader), + // so temporarily enable this flag explicitly here until we flip the default. This should allow the existing + // configuration examples to continue working despite the feature being disabled by default. + ON_CALL(*snapshot_, runtimeFeatureEnabled("envoy.reloadable_features.experimental_matching_api")) + .WillByDefault(Return(true)); ON_CALL(server_.runtime_loader_, threadsafeSnapshot()).WillByDefault(Invoke([this]() { return snapshot_; })); @@ -156,7 +162,7 @@ class ConfigTest { Server::ListenerManagerImpl listener_manager_{server_, component_factory_, worker_factory_, false}; Random::RandomGeneratorImpl random_; - Runtime::SnapshotConstSharedPtr snapshot_{std::make_shared>()}; + std::shared_ptr snapshot_{std::make_shared>()}; NiceMock os_sys_calls_; TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; NiceMock file_system_; From 2388add17ded6bad02d1c74c5263bcb6b4bf2dc0 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 11 May 2021 17:12:30 +0000 Subject: [PATCH 10/11] fix integration tests Signed-off-by: Snow Pettersen --- .../http/composite/composite_filter_integration_test.cc | 2 ++ test/integration/extension_discovery_integration_test.cc | 3 +++ 2 files changed, 5 insertions(+) diff --git a/test/extensions/filters/http/composite/composite_filter_integration_test.cc b/test/extensions/filters/http/composite/composite_filter_integration_test.cc index 8a290b633f963..de0abba85a078 100644 --- a/test/extensions/filters/http/composite/composite_filter_integration_test.cc +++ b/test/extensions/filters/http/composite/composite_filter_integration_test.cc @@ -15,6 +15,8 @@ class CompositeFilterIntegrationTest : public testing::TestWithParamadd_clusters(); From c3e947266b2658230d5b860bc3c774b42d93d49e Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 11 May 2021 17:13:31 +0000 Subject: [PATCH 11/11] format Signed-off-by: Snow Pettersen --- test/config_test/config_test.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 896a27bde6c4e..61cb500c1253f 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -86,10 +86,12 @@ class ConfigTest { ON_CALL(server_.runtime_loader_.snapshot_, deprecatedFeatureEnabled(_, _)) .WillByDefault(Invoke([](absl::string_view, bool default_value) { return default_value; })); - // TODO(snowp): There's no way to override runtime flags per example file (since we mock out the runtime loader), - // so temporarily enable this flag explicitly here until we flip the default. This should allow the existing - // configuration examples to continue working despite the feature being disabled by default. - ON_CALL(*snapshot_, runtimeFeatureEnabled("envoy.reloadable_features.experimental_matching_api")) + // TODO(snowp): There's no way to override runtime flags per example file (since we mock out the + // runtime loader), so temporarily enable this flag explicitly here until we flip the default. + // This should allow the existing configuration examples to continue working despite the feature + // being disabled by default. + ON_CALL(*snapshot_, + runtimeFeatureEnabled("envoy.reloadable_features.experimental_matching_api")) .WillByDefault(Return(true)); ON_CALL(server_.runtime_loader_, threadsafeSnapshot()).WillByDefault(Invoke([this]() { return snapshot_; @@ -162,7 +164,8 @@ class ConfigTest { Server::ListenerManagerImpl listener_manager_{server_, component_factory_, worker_factory_, false}; Random::RandomGeneratorImpl random_; - std::shared_ptr snapshot_{std::make_shared>()}; + std::shared_ptr snapshot_{ + std::make_shared>()}; NiceMock os_sys_calls_; TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; NiceMock file_system_;