Skip to content

Commit

Permalink
set_filter_state: enable per-route configuration override (#37507)
Browse files Browse the repository at this point in the history
Commit Message: set_filter_state: enable per-route configuration
override
Additional Description:
This extends this filter to support usage in route configuration. When
present, both the listener and route level settings are applied
(listener first, then route).

Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: John Howard <john.howard@solo.io>
  • Loading branch information
howardjohn authored Dec 13, 2024
1 parent 41f1901 commit 359b54b
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 2 deletions.
3 changes: 3 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ new_features:
change: |
Added new health check filter stats including total requests, successful/failed checks, cached responses, and
cluster health status counters. These stats help track health check behavior and cluster health state.
- area: filters
change: |
Updatd the ``set_filter_state`` :ref:`filter <config_http_filters_set_filter_state>` to support per-route overrides.
- area: grpc-json
change: |
Added a new http filter for :ref:`gRPC to JSON transcoding <config_http_filters_grpc_json_reverse_transcoder>`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ struct Value {
Formatter::FormatterConstSharedPtr value_;
};

class Config : public Logger::Loggable<Logger::Id::config> {
class Config : public ::Envoy::Router::RouteSpecificFilterConfig,
public Logger::Loggable<Logger::Id::config> {
public:
Config(const Protobuf::RepeatedPtrField<FilterStateValueProto>& proto_values, LifeSpan life_span,
Server::Configuration::GenericFactoryContext& context)
Expand Down
23 changes: 22 additions & 1 deletion source/extensions/filters/http/set_filter_state/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "envoy/formatter/substitution_formatter.h"
#include "envoy/registry/registry.h"

#include "source/common/http/utility.h"
#include "source/common/protobuf/utility.h"
#include "source/server/generic_factory_context.h"

Expand All @@ -19,7 +20,15 @@ SetFilterState::SetFilterState(const Filters::Common::SetFilterState::ConfigShar
: config_(config) {}

Http::FilterHeadersStatus SetFilterState::decodeHeaders(Http::RequestHeaderMap& headers, bool) {
config_->updateFilterState({&headers}, decoder_callbacks_->streamInfo());
// Apply listener level configuration first.
config_.get()->updateFilterState({&headers}, decoder_callbacks_->streamInfo());

// If configured, apply virtual host and then route level configuration next.
auto policies = Http::Utility::getAllPerFilterConfig<Filters::Common::SetFilterState::Config>(
decoder_callbacks_);
for (auto policy : policies) {
policy.get().updateFilterState({&headers}, decoder_callbacks_->streamInfo());
}
return Http::FilterHeadersStatus::Continue;
}

Expand All @@ -35,6 +44,18 @@ Http::FilterFactoryCb SetFilterStateConfig::createFilterFactoryFromProtoTyped(
};
}

absl::StatusOr<Router::RouteSpecificFilterConfigConstSharedPtr>
SetFilterStateConfig::createRouteSpecificFilterConfigTyped(
const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
Server::Configuration::ServerFactoryContext& context, ProtobufMessage::ValidationVisitor&) {

Server::GenericFactoryContextImpl generic_context(context, context.messageValidationVisitor());

return std::make_shared<const Filters::Common::SetFilterState::Config>(
proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain,
generic_context);
}

Http::FilterFactoryCb SetFilterStateConfig::createFilterFactoryFromProtoWithServerContextTyped(
const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
const std::string&, Server::Configuration::ServerFactoryContext& context) {
Expand Down
5 changes: 5 additions & 0 deletions source/extensions/filters/http/set_filter_state/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ class SetFilterStateConfig
const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;

absl::StatusOr<Router::RouteSpecificFilterConfigConstSharedPtr>
createRouteSpecificFilterConfigTyped(
const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
Server::Configuration::ServerFactoryContext&, ProtobufMessage::ValidationVisitor&) override;

Http::FilterFactoryCb createFilterFactoryFromProtoWithServerContextTyped(
const envoy::extensions::filters::http::set_filter_state::v3::Config& proto_config,
const std::string& stats_prefix,
Expand Down
85 changes: 85 additions & 0 deletions test/extensions/filters/http/set_filter_state/integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "gtest/gtest.h"

using testing::NiceMock;
using testing::Return;
using testing::ReturnRef;

namespace Envoy {
Expand Down Expand Up @@ -66,6 +67,45 @@ class SetMetadataIntegrationTest : public testing::Test {
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter->decodeHeaders(headers_, true));
}

void runPerRouteFilter(const std::string& filter_yaml_config,
const std::string& per_route_yaml_config) {
Server::GenericFactoryContextImpl generic_context(context_);

envoy::extensions::filters::http::set_filter_state::v3::Config filter_proto_config;
TestUtility::loadFromYaml(filter_yaml_config, filter_proto_config);
auto filter_config = std::make_shared<Filters::Common::SetFilterState::Config>(
filter_proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain,
generic_context);

envoy::extensions::filters::http::set_filter_state::v3::Config route_proto_config;
TestUtility::loadFromYaml(per_route_yaml_config, route_proto_config);
Filters::Common::SetFilterState::Config route_config(
route_proto_config.on_request_headers(), StreamInfo::FilterState::LifeSpan::FilterChain,
generic_context);

NiceMock<Http::MockStreamDecoderFilterCallbacks> decoder_callbacks;

EXPECT_CALL(decoder_callbacks, perFilterConfigs())
.WillOnce(testing::Invoke(
[&]() -> Router::RouteSpecificFilterConfigs { return {&route_config}; }));
auto filter = std::make_shared<SetFilterState>(filter_config);
filter->setDecoderFilterCallbacks(decoder_callbacks);
EXPECT_CALL(decoder_callbacks, streamInfo()).WillRepeatedly(ReturnRef(info_));
EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter->decodeHeaders(headers_, true));

// Test the factory method.
{
NiceMock<Server::Configuration::MockServerFactoryContext> context;
SetFilterStateConfig factory;
Router::RouteSpecificFilterConfigConstSharedPtr route_config =
factory
.createRouteSpecificFilterConfig(route_proto_config, context,
ProtobufMessage::getNullValidationVisitor())
.value();
EXPECT_TRUE(route_config.get());
}
}

NiceMock<Server::Configuration::MockFactoryContext> context_;
Http::TestRequestHeaderMapImpl headers_{{"test-header", "test-value"}};
NiceMock<StreamInfo::MockStreamInfo> info_;
Expand All @@ -85,6 +125,51 @@ TEST_F(SetMetadataIntegrationTest, FromHeader) {
EXPECT_EQ(foo->serializeAsString(), "test-value");
}

TEST_F(SetMetadataIntegrationTest, RouteLevel) {
const std::string filter_config = R"EOF(
on_request_headers:
- object_key: both
factory_key: envoy.string
format_string:
text_format_source:
inline_string: "filter-%REQ(test-header)%"
- object_key: filter-only
factory_key: envoy.string
format_string:
text_format_source:
inline_string: "filter"
)EOF";
const std::string route_config = R"EOF(
on_request_headers:
- object_key: both
factory_key: envoy.string
format_string:
text_format_source:
inline_string: "route-%REQ(test-header)%"
- object_key: route-only
factory_key: envoy.string
format_string:
text_format_source:
inline_string: "route"
)EOF";
runPerRouteFilter(filter_config, route_config);

const auto* both = info_.filterState()->getDataReadOnly<Router::StringAccessor>("both");
ASSERT_NE(nullptr, both);
// Route takes precedence
EXPECT_EQ(both->serializeAsString(), "route-test-value");

const auto* filter = info_.filterState()->getDataReadOnly<Router::StringAccessor>("filter-only");
ASSERT_NE(nullptr, filter);
// Only set on filter
EXPECT_EQ(filter->serializeAsString(), "filter");

const auto* route = info_.filterState()->getDataReadOnly<Router::StringAccessor>("route-only");
ASSERT_NE(nullptr, route);
// Only set on route
EXPECT_EQ(route->serializeAsString(), "route");
}

} // namespace SetFilterState
} // namespace HttpFilters
} // namespace Extensions
Expand Down

0 comments on commit 359b54b

Please sign in to comment.