-
Notifications
You must be signed in to change notification settings - Fork 5.5k
udp: add router for UDP proxy #18791
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 67 commits
4cfa9c5
9891c45
367db00
3bd3085
4c1b126
3f762f7
95e1094
b592612
804aedd
37def08
77fb547
9c71fe4
b4cbd8d
4a201e7
b5086f2
42ce9c3
0b14da0
744d74b
cf61bb7
cc24008
f5fd481
f613a87
4a2f3f8
115b532
db04638
e9f56d7
786386b
f615fd5
854176a
7cff9bf
b0fb7d6
8cfdf65
cfee9b0
063cf40
35b0f5e
7303e6f
8184199
8f3e8e0
ecddff7
fae4e72
69376a9
0fc55bf
e779a36
fb2731e
68dc8fb
8de2770
e9cf799
03005e5
e162cae
d62539e
e49181a
b04a059
87d1361
bd7b917
b00cf9e
45165c6
5173760
0e19161
8b30f25
d3db96e
b501e34
bc33bd0
2b9fe8d
9d1abd2
628855d
cb3d52c
d6911cc
29e86f8
4478d77
c1dc8b1
086ea89
cf77be6
4830ec7
5ff8de5
43a1b80
2be0b73
03b55aa
549a487
a08f1e1
d368ef3
9b46d46
79d1748
f959f11
b9ac01e
96d1ca1
f0eead0
a5fc56d
00a84ef
eede13d
0617a91
4eeea14
dc59d27
3f9c62c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| syntax = "proto3"; | ||
|
|
||
| package envoy.extensions.filters.udp.udp_proxy.v3; | ||
|
|
||
| import "udpa/annotations/status.proto"; | ||
| import "validate/validate.proto"; | ||
|
|
||
| option java_package = "io.envoyproxy.envoy.extensions.filters.udp.udp_proxy.v3"; | ||
| option java_outer_classname = "RouteProto"; | ||
| option java_multiple_files = true; | ||
| option go_package = "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/udp/udp_proxy/v3;udp_proxyv3"; | ||
| option (udpa.annotations.file_status).package_version_status = ACTIVE; | ||
|
|
||
| // [#protodoc-title: UDP proxy route configuration] | ||
| // UDP proxy :ref:`configuration overview <config_udp_listener_filters_udp_proxy>`. | ||
|
|
||
| message Route { | ||
| // Indicates the upstream cluster to which the request should be routed. | ||
| string cluster = 1 [(validate.rules).string = {min_len: 1}]; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,7 @@ New Features | |
| useful in support systems such as CI, CD, etc. The | ||
| :ref:`schema validator check tool <install_tools_schema_validator_check_tool>` has been added | ||
| to the tools image. | ||
| * udp_proxy: added :ref:`matcher <envoy_v3_api_field_extensions.filters.udp.udp_proxy.v3.UdpProxyConfig.matcher>` to support matching and routing to different clusters. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a note in deprecated below for the deprecated field.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, added. |
||
|
|
||
| Deprecated | ||
| ---------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| load( | ||
| "//bazel:envoy_build_system.bzl", | ||
| "envoy_cc_library", | ||
| "envoy_extension_package", | ||
| ) | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| envoy_extension_package() | ||
|
|
||
| envoy_cc_library( | ||
| name = "router_interface", | ||
| hdrs = ["router.h"], | ||
| ) | ||
|
|
||
| envoy_cc_library( | ||
| name = "router_lib", | ||
| srcs = ["router_impl.cc"], | ||
| hdrs = ["router_impl.h"], | ||
| deps = [ | ||
| ":router_interface", | ||
| "//source/common/common:empty_string", | ||
| "//source/common/matcher:matcher_lib", | ||
| "//source/common/matcher:validation_visitor_lib", | ||
| "//source/common/network/matching:inputs_lib", | ||
| "@envoy_api//envoy/extensions/filters/udp/udp_proxy/v3:pkg_cc_proto", | ||
| "@envoy_api//envoy/type/matcher/v3:pkg_cc_proto", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,45 @@ | ||||||
| #pragma once | ||||||
|
|
||||||
| #include <memory> | ||||||
| #include <string> | ||||||
| #include <vector> | ||||||
|
|
||||||
| #include "envoy/common/pure.h" | ||||||
| #include "envoy/network/address.h" | ||||||
|
|
||||||
| namespace Envoy { | ||||||
| namespace Extensions { | ||||||
| namespace UdpFilters { | ||||||
| namespace UdpProxy { | ||||||
| namespace Router { | ||||||
|
|
||||||
| /** | ||||||
| * The router which holds all clusters and determines the cluster which UDP data should be routed | ||||||
| * to. | ||||||
| */ | ||||||
| class Router { | ||||||
| public: | ||||||
| virtual ~Router() = default; | ||||||
|
|
||||||
| /** | ||||||
| * Based on the source address of the incoming UDP data, determine the target route for the data. | ||||||
| * @return the cluster name or empty string if there is not matching route for the data. | ||||||
| */ | ||||||
| virtual const std::string | ||||||
| route(Network::Address::InstanceConstSharedPtr source_address) const PURE; | ||||||
|
|
||||||
| /** | ||||||
| * Returns all cluster names in the router. The UDP proxy filter requires every cluster names for | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, changed. |
||||||
| * initialization which will call this method on construction. | ||||||
| * @return vector of all cluster names. | ||||||
| */ | ||||||
| virtual const std::vector<std::string>& allClusterNames() const PURE; | ||||||
| }; | ||||||
|
|
||||||
| using RouterConstSharedPtr = std::shared_ptr<const Router>; | ||||||
|
|
||||||
| } // namespace Router | ||||||
| } // namespace UdpProxy | ||||||
| } // namespace UdpFilters | ||||||
| } // namespace Extensions | ||||||
| } // namespace Envoy | ||||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,93 @@ | ||||
| #include "source/extensions/filters/udp/udp_proxy/router/router_impl.h" | ||||
|
|
||||
| #include "envoy/extensions/filters/udp/udp_proxy/v3/route.pb.h" | ||||
| #include "envoy/extensions/filters/udp/udp_proxy/v3/route.pb.validate.h" | ||||
| #include "envoy/type/matcher/v3/network_inputs.pb.h" | ||||
|
|
||||
| #include "source/common/common/empty_string.h" | ||||
| #include "source/common/network/matching/data_impl.h" | ||||
| #include "source/common/network/matching/inputs.h" | ||||
|
|
||||
| namespace Envoy { | ||||
| namespace Extensions { | ||||
| namespace UdpFilters { | ||||
| namespace UdpProxy { | ||||
| namespace Router { | ||||
|
|
||||
| Matcher::ActionFactoryCb RouteMatchActionFactory::createActionFactoryCb( | ||||
| const Protobuf::Message& config, RouteActionContext& context, | ||||
| ProtobufMessage::ValidationVisitor& validation_visitor) { | ||||
| const auto& route_config = MessageUtil::downcastAndValidate< | ||||
| const envoy::extensions::filters::udp::udp_proxy::v3::Route&>(config, validation_visitor); | ||||
| const auto& cluster = route_config.cluster(); | ||||
|
|
||||
| // Emplace cluster names to context to get all cluster names | ||||
| context.cluster_name_.emplace(cluster); | ||||
|
|
||||
| return [cluster]() { return std::make_unique<RouteMatchAction>(cluster); }; | ||||
| } | ||||
|
|
||||
| REGISTER_FACTORY(RouteMatchActionFactory, Matcher::ActionFactory<RouteActionContext>); | ||||
|
|
||||
| absl::Status RouteActionValidationVisitor::performDataInputValidation( | ||||
| const Matcher::DataInputFactory<Network::MatchingData>&, absl::string_view type_url) { | ||||
| static std::string source_ip_input_name = TypeUtil::descriptorFullNameToTypeUrl( | ||||
| envoy::type::matcher::v3::SourceIpMatchInput::descriptor()->full_name()); | ||||
| if (type_url == source_ip_input_name) { | ||||
| return absl::OkStatus(); | ||||
| } | ||||
|
|
||||
| return absl::InvalidArgumentError( | ||||
| fmt::format("Route table can only match on source IP, saw {}", type_url)); | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the source IP input is the only thing registered for this match tree I don't think we need this additional validation, just means more work whenever a new input is added
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently we only support sip input, but I think I have registered sip, dip, sport, dport in the match tree for future use.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can we just remove them for now and add them later? I would rather only add the code that is actually used. It will also make this PR smaller.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I will remove these data inputs, but I still think the validation is necessary since a HTTP data input is also syntax valid for the UDP router and the validator will keep the gate.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way the factory registrations work is that registrations are unique per data input type, so since you're using the NetworkDataInput it won't share registrations with HttpDataInput registrations
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, I also think matcher is expected to work that way, but the test shows Envoy will not stop until the validation visitor returns the error. I guess the registration of HttpDataInput is shared with NetworkDataInput, do I miss something?
|
||||
| } | ||||
|
|
||||
| RouterImpl::RouterImpl(const envoy::extensions::filters::udp::udp_proxy::v3::UdpProxyConfig& config, | ||||
| Server::Configuration::ServerFactoryContext& factory_context) { | ||||
| if (config.has_cluster()) { | ||||
| cluster_ = config.cluster(); | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be a bit cleaner to just synthesize matcher config for this case. Then you can use unified code for the rest of it? (Basically it makes the deprecated path cheaper to maintain and understand as the only special case is here.)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am sorry but I cannot catch your idea here. Do you mention we can move
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I catch your thought finally, I will submit the commit soon. |
||||
| cluster_names_.push_back(config.cluster()); | ||||
| } else { | ||||
| RouteActionContext context{}; | ||||
| RouteActionValidationVisitor validation_visitor; | ||||
| Matcher::MatchTreeFactory<Network::MatchingData, RouteActionContext> factory( | ||||
| context, factory_context, validation_visitor); | ||||
| matcher_ = factory.create(config.matcher())(); | ||||
|
|
||||
| if (!validation_visitor.errors().empty()) { | ||||
| // TODO(snowp): Output all violations. | ||||
| throw EnvoyException(fmt::format("requirement violation while creating route match tree: {}", | ||||
| validation_visitor.errors()[0])); | ||||
| } | ||||
|
|
||||
| // Copy all clusters names | ||||
| cluster_names_.insert(cluster_names_.end(), context.cluster_name_.begin(), | ||||
| context.cluster_name_.end()); | ||||
| } | ||||
| } | ||||
|
|
||||
| const std::string RouterImpl::route(Network::Address::InstanceConstSharedPtr source_address) const { | ||||
| if (cluster_.has_value()) { | ||||
| return cluster_.value(); | ||||
| } | ||||
|
|
||||
| if (source_address->ip()) { | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Technically isn't it possible to send UDP/datagrams over a pipe? Or do we block this in config? Either way it seems like this should be handled elsewhere as a config load issue vs. on every route request?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is redundant. I will remove it, thanks. |
||||
| Network::Matching::MatchingDataImpl data(source_address->ip()); | ||||
|
|
||||
| auto result = matcher_->match(data); | ||||
| if (result.match_state_ == Matcher::MatchState::MatchComplete) { | ||||
| if (result.on_match_.has_value()) { | ||||
| return result.on_match_.value().action_cb_()->getTyped<RouteMatchAction>().cluster(); | ||||
| } | ||||
| } | ||||
| } | ||||
|
|
||||
| return EMPTY_STRING; | ||||
| } | ||||
|
|
||||
| const std::vector<std::string>& RouterImpl::allClusterNames() const { return cluster_names_; } | ||||
|
|
||||
| } // namespace Router | ||||
| } // namespace UdpProxy | ||||
| } // namespace UdpFilters | ||||
| } // namespace Extensions | ||||
| } // namespace Envoy | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| #pragma once | ||
|
|
||
| #include "envoy/extensions/filters/udp/udp_proxy/v3/route.pb.h" | ||
| #include "envoy/extensions/filters/udp/udp_proxy/v3/udp_proxy.pb.h" | ||
| #include "envoy/network/filter.h" | ||
| #include "envoy/server/factory_context.h" | ||
|
|
||
| #include "source/common/matcher/matcher.h" | ||
| #include "source/common/matcher/validation_visitor.h" | ||
| #include "source/extensions/filters/udp/udp_proxy/router/router.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Extensions { | ||
| namespace UdpFilters { | ||
| namespace UdpProxy { | ||
| namespace Router { | ||
|
|
||
| struct RouteActionContext { | ||
| absl::flat_hash_set<std::string> cluster_name_; | ||
| }; | ||
|
|
||
| class RouteMatchAction | ||
| : public Matcher::ActionBase<envoy::extensions::filters::udp::udp_proxy::v3::Route> { | ||
| public: | ||
| explicit RouteMatchAction(const std::string& cluster) : cluster_(cluster) {} | ||
|
|
||
| const std::string& cluster() const { return cluster_; } | ||
|
|
||
| private: | ||
| const std::string cluster_; | ||
| }; | ||
|
|
||
| class RouteMatchActionFactory : public Matcher::ActionFactory<RouteActionContext> { | ||
| public: | ||
| Matcher::ActionFactoryCb | ||
| createActionFactoryCb(const Protobuf::Message& config, RouteActionContext& context, | ||
| ProtobufMessage::ValidationVisitor& validation_visitor) override; | ||
| std::string name() const override { return "route"; } | ||
| ProtobufTypes::MessagePtr createEmptyConfigProto() override { | ||
| return std::make_unique<envoy::extensions::filters::udp::udp_proxy::v3::Route>(); | ||
| } | ||
| }; | ||
|
|
||
| class RouteActionValidationVisitor | ||
| : public Matcher::MatchTreeValidationVisitor<Network::MatchingData> { | ||
| public: | ||
| absl::Status | ||
| performDataInputValidation(const Matcher::DataInputFactory<Network::MatchingData>& data_input, | ||
| absl::string_view type_url) override; | ||
| }; | ||
|
|
||
| class RouterImpl : public Router { | ||
| public: | ||
| RouterImpl(const envoy::extensions::filters::udp::udp_proxy::v3::UdpProxyConfig& config, | ||
| Server::Configuration::ServerFactoryContext& context); | ||
|
|
||
| // Router::Router | ||
| const std::string route(Network::Address::InstanceConstSharedPtr source_address) const override; | ||
| const std::vector<std::string>& allClusterNames() const override; | ||
|
|
||
| private: | ||
| absl::optional<std::string> cluster_; | ||
| Matcher::MatchTreeSharedPtr<Network::MatchingData> matcher_; | ||
| std::vector<std::string> cluster_names_; | ||
| }; | ||
|
|
||
| } // namespace Router | ||
| } // namespace UdpProxy | ||
| } // namespace UdpFilters | ||
| } // namespace Extensions | ||
| } // namespace Envoy |
Uh oh!
There was an error while loading. Please reload this page.