Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions api/envoy/api/v2/route/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ option (udpa.annotations.file_migrate).move_to_package = "envoy.config.route.v3"
// host header. This allows a single listener to service multiple top level domain path trees. Once
// a virtual host is selected based on the domain, the routes are processed in order to see which
// upstream cluster to route to or whether to perform a redirect.
// [#next-free-field: 19]
// [#next-free-field: 20]
message VirtualHost {
enum TlsRequirementType {
// No TLS requirement for the virtual host.
Expand Down Expand Up @@ -150,6 +150,14 @@ message VirtualHost {
// independently (e.g.: values are not inherited).
RetryPolicy retry_policy = 16;

// [#not-implemented-hide:]
// Implementation specific configuration which depends on the implementation being instantiated.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this description is pretty confusing for someone that doesn't know how the extensions usually work, maybe it should mention that this also defined what implementation is being instantiated?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused. RetryPolicy already has a typed config Any. Surely any implementation-specific extension to this should be nested in there?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RetryPolicy already has a typed config Any

I am not sure if this is true. I didn't find the field with typed Any

message RetryPolicy {
message RetryPriority {
string name = 1 [(validate.rules).string = {min_bytes: 1}];
oneof config_type {
google.protobuf.Struct config = 2 [deprecated = true];
google.protobuf.Any typed_config = 3;
}
}
message RetryHostPredicate {
string name = 1 [(validate.rules).string = {min_bytes: 1}];
oneof config_type {
google.protobuf.Struct config = 2 [deprecated = true];
google.protobuf.Any typed_config = 3;
}
}
message RetryBackOff {
// Specifies the base interval between retries. This parameter is required and must be greater
// than zero. Values less than 1 ms are rounded up to 1 ms.
// See :ref:`config_http_filters_router_x-envoy-max-retries` for a discussion of Envoy's
// back-off algorithm.
google.protobuf.Duration base_interval = 1 [(validate.rules).duration = {
required: true
gt {}
}];
// Specifies the maximum interval between retries. This parameter is optional, but must be
// greater than or equal to the `base_interval` if set. The default is 10 times the
// `base_interval`. See :ref:`config_http_filters_router_x-envoy-max-retries` for a discussion
// of Envoy's back-off algorithm.
google.protobuf.Duration max_interval = 2 [(validate.rules).duration = {gt {}}];
}
// Specifies the conditions under which retry takes place. These are the same
// conditions documented for :ref:`config_http_filters_router_x-envoy-retry-on` and
// :ref:`config_http_filters_router_x-envoy-retry-grpc-on`.
string retry_on = 1;
// Specifies the allowed number of retries. This parameter is optional and
// defaults to 1. These are the same conditions documented for
// :ref:`config_http_filters_router_x-envoy-max-retries`.
google.protobuf.UInt32Value num_retries = 2;
// Specifies a non-zero upstream timeout per retry attempt. This parameter is optional. The
// same conditions documented for
// :ref:`config_http_filters_router_x-envoy-upstream-rq-per-try-timeout-ms` apply.
//
// .. note::
//
// If left unspecified, Envoy will use the global
// :ref:`route timeout <envoy_api_field_route.RouteAction.timeout>` for the request.
// Consequently, when using a :ref:`5xx <config_http_filters_router_x-envoy-retry-on>` based
// retry policy, a request that times out will not be retried as the total timeout budget
// would have been exhausted.
google.protobuf.Duration per_try_timeout = 3;
// Specifies an implementation of a RetryPriority which is used to determine the
// distribution of load across priorities used for retries. Refer to
// :ref:`retry plugin configuration <arch_overview_http_retry_plugins>` for more details.
RetryPriority retry_priority = 4;
// Specifies a collection of RetryHostPredicates that will be consulted when selecting a host
// for retries. If any of the predicates reject the host, host selection will be reattempted.
// Refer to :ref:`retry plugin configuration <arch_overview_http_retry_plugins>` for more
// details.
repeated RetryHostPredicate retry_host_predicate = 5;
// The maximum number of times host selection will be reattempted before giving up, at which
// point the host that was last selected will be routed to. If unspecified, this will default to
// retrying once.
int64 host_selection_retry_max_attempts = 6;
// HTTP status codes that should trigger a retry in addition to those specified by retry_on.
repeated uint32 retriable_status_codes = 7;
// Specifies parameters that control retry back off. This parameter is optional, in which case the
// default base interval is 25 milliseconds or, if set, the current value of the
// `upstream.base_retry_backoff_ms` runtime parameter. The default maximum interval is 10 times
// the base interval. The documentation for :ref:`config_http_filters_router_x-envoy-max-retries`
// describes Envoy's back-off algorithm.
RetryBackOff retry_back_off = 8;
// HTTP response headers that trigger a retry if present in the response. A retry will be
// triggered if any of the header matches match the upstream response headers.
// The field is only consulted if 'retriable-headers' retry policy is active.
repeated HeaderMatcher retriable_headers = 9;
// HTTP headers which must be present in the request for retries to be attempted.
repeated HeaderMatcher retriable_request_headers = 10;
}

// See the supported retry policy implementations for further documentation. Note that setting a
// route level entry will take precedence over this config and it'll be treated
// independently (e.g.: values are not inherited). :ref:`Retry policy <envoy_api_field_route.VirtualHost.retry_policy>`
// should not be set if this field is used.
google.protobuf.Any retry_policy_extension = 19;
Comment thread
yxue marked this conversation as resolved.
Outdated

// Indicates the hedge policy for all routes in this virtual host. Note that setting a
// route level entry will take precedence over this config and it'll be treated
// independently (e.g.: values are not inherited).
Expand Down Expand Up @@ -535,7 +543,7 @@ message CorsPolicy {
core.RuntimeFractionalPercent shadow_enabled = 10;
}

// [#next-free-field: 33]
// [#next-free-field: 34]
message RouteAction {
enum ClusterNotFoundResponseCode {
// HTTP status code - 503 Service Unavailable.
Expand Down Expand Up @@ -878,6 +886,14 @@ message RouteAction {
// (e.g.: policies are not merged, most internal one becomes the enforced policy).
RetryPolicy retry_policy = 9;

// [#not-implemented-hide:]
// Implementation specific configuration which depends on the implementation being instantiated.
// See the supported retry policy implementations for further documentation. Note that if this is
// set, it'll take precedence over the virtual host level retry policy entirely (e.g.: policies
// are not merged, most internal one becomes the enforced policy). :ref:`Retry policy <envoy_api_field_route.VirtualHost.retry_policy>`
// should not be set if this field is used.
google.protobuf.Any retry_policy_extension = 33;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto. What does "See the supported retry policy implementations for further documentation." mean?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. I mean users can refer to retry policy extensions configuration for details since here is just an Any typed field.


// Indicates that the route has a request mirroring policy.
//
// .. attention::
Expand Down
20 changes: 18 additions & 2 deletions api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ option java_multiple_files = true;
// host header. This allows a single listener to service multiple top level domain path trees. Once
// a virtual host is selected based on the domain, the routes are processed in order to see which
// upstream cluster to route to or whether to perform a redirect.
// [#next-free-field: 19]
// [#next-free-field: 20]
message VirtualHost {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.VirtualHost";

Expand Down Expand Up @@ -147,6 +147,14 @@ message VirtualHost {
// independently (e.g.: values are not inherited).
RetryPolicy retry_policy = 16;

// [#not-implemented-hide:]
// Implementation specific configuration which depends on the implementation being instantiated.
// See the supported retry policy implementations for further documentation. Note that setting a
// route level entry will take precedence over this config and it'll be treated
// independently (e.g.: values are not inherited). :ref:`Retry policy <envoy_api_field_config.route.v3.VirtualHost.retry_policy>`
// should not be set if this field is used.
google.protobuf.Any retry_policy_extension = 19;

// Indicates the hedge policy for all routes in this virtual host. Note that setting a
// route level entry will take precedence over this config and it'll be treated
// independently (e.g.: values are not inherited).
Expand Down Expand Up @@ -496,7 +504,7 @@ message CorsPolicy {
core.v3.RuntimeFractionalPercent shadow_enabled = 10;
}

// [#next-free-field: 33]
// [#next-free-field: 34]
message RouteAction {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteAction";

Expand Down Expand Up @@ -853,6 +861,14 @@ message RouteAction {
// (e.g.: policies are not merged, most internal one becomes the enforced policy).
RetryPolicy retry_policy = 9;

// [#not-implemented-hide:]
// Implementation specific configuration which depends on the implementation being instantiated.
// See the supported retry policy implementations for further documentation. Note that if this is
// set, it'll take precedence over the virtual host level retry policy entirely (e.g.: policies
// are not merged, most internal one becomes the enforced policy). :ref:`Retry policy <envoy_api_field_config.route.v3.VirtualHost.retry_policy>`
// should not be set if this field is used.
google.protobuf.Any retry_policy_extension = 33;

// Indicates that the route has request mirroring policies.
repeated RequestMirrorPolicy request_mirror_policies = 30;

Expand Down
20 changes: 18 additions & 2 deletions generated_api_shadow/envoy/api/v2/route/route_components.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 18 additions & 2 deletions generated_api_shadow/envoy/config/route/v3/route_components.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 24 additions & 1 deletion source/common/config/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ class Utility {

/**
* Translate a nested config into a proto message provided by the implementation factory.
* @param extension_name name of extension corresponding to config.
* @param enclosing_message proto that contains a field 'config'. Note: the enclosing proto is
* provided because for statically registered implementations, a custom config is generally
* optional, which means the conversion must be done conditionally.
Expand All @@ -269,6 +268,30 @@ class Utility {
return config;
}

/**
* Translate the typed any field into a proto message provided by the implementation factory.
* @param typed_config typed configuration.
* @param validation_visitor message validation visitor instance.
* @param factory implementation factory with the method 'createEmptyConfigProto' to produce a
* proto to be filled with the translated configuration.
*/
template <class Factory>
static ProtobufTypes::MessagePtr
translateAnyToFactoryConfig(const ProtobufWkt::Any& typed_config,
Comment thread
yxue marked this conversation as resolved.
Outdated
ProtobufMessage::ValidationVisitor& validation_visitor,
Factory& factory) {
ProtobufTypes::MessagePtr config = factory.createEmptyConfigProto();

// Fail in an obvious way if a plugin does not return a proto.
RELEASE_ASSERT(config != nullptr, "");

// Check that the config type is not google.protobuf.Empty
RELEASE_ASSERT(config->GetDescriptor()->full_name() != "google.protobuf.Empty", "");

translateOpaqueConfig(typed_config, ProtobufWkt::Struct(), validation_visitor, *config);
return config;
}

/**
* Truncates the message to a length less than default GRPC trailers size limit (by default 8KiB).
*/
Expand Down
36 changes: 36 additions & 0 deletions test/common/config/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "common/config/well_known_names.h"
#include "common/protobuf/protobuf.h"

#include "test/mocks/config/mocks.h"
#include "test/mocks/grpc/mocks.h"
#include "test/mocks/local_info/mocks.h"
#include "test/mocks/stats/mocks.h"
Expand Down Expand Up @@ -287,6 +288,41 @@ TEST(UtilityTest, AnyWrongType) {
R"(Unable to unpack as google.protobuf.Timestamp: \[type.googleapis.com/google.protobuf.Duration\] .*)");
}

TEST(UtilityTest, TranslateAnyWrongToFactoryConfig) {
ProtobufWkt::Duration source_duration;
source_duration.set_seconds(42);
ProtobufWkt::Any typed_config;
typed_config.PackFrom(source_duration);

MockTypedFactory factory;
EXPECT_CALL(factory, createEmptyConfigProto()).WillOnce(Invoke([]() -> ProtobufTypes::MessagePtr {
return ProtobufTypes::MessagePtr{new ProtobufWkt::Timestamp()};
}));

EXPECT_THROW_WITH_REGEX(
Utility::translateAnyToFactoryConfig(typed_config,
ProtobufMessage::getStrictValidationVisitor(), factory),
EnvoyException,
R"(Unable to unpack as google.protobuf.Timestamp: \[type.googleapis.com/google.protobuf.Duration\] .*)");
}

TEST(UtilityTest, TranslateAnyToFactoryConfig) {
ProtobufWkt::Duration source_duration;
source_duration.set_seconds(42);
ProtobufWkt::Any typed_config;
typed_config.PackFrom(source_duration);

MockTypedFactory factory;
EXPECT_CALL(factory, createEmptyConfigProto()).WillOnce(Invoke([]() -> ProtobufTypes::MessagePtr {
return ProtobufTypes::MessagePtr{new ProtobufWkt::Duration()};
}));

auto config = Utility::translateAnyToFactoryConfig(
typed_config, ProtobufMessage::getStrictValidationVisitor(), factory);

EXPECT_THAT(*config, ProtoEq(source_duration));
}

void packTypedStructIntoAny(ProtobufWkt::Any& typed_config, const Protobuf::Message& inner) {
udpa::type::v1::TypedStruct typed_struct;
(*typed_struct.mutable_type_url()) =
Expand Down
11 changes: 11 additions & 0 deletions test/mocks/config/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "envoy/config/endpoint/v3/endpoint.pb.h"
#include "envoy/config/grpc_mux.h"
#include "envoy/config/subscription.h"
#include "envoy/config/typed_config.h"
#include "envoy/service/discovery/v3/discovery.pb.h"

#include "common/config/config_provider_impl.h"
Expand Down Expand Up @@ -119,5 +120,15 @@ class MockConfigProviderManager : public ConfigProviderManager {
const Envoy::Config::ConfigProviderManager::OptionalArg& optarg));
};

class MockTypedFactory : public TypedFactory {
public:
~MockTypedFactory() = default;
Comment thread
yxue marked this conversation as resolved.
Outdated

MOCK_METHOD(ProtobufTypes::MessagePtr, createEmptyConfigProto, ());
MOCK_METHOD(std::string, configType, ());
MOCK_METHOD(std::string, name, (), (const));
MOCK_METHOD(std::string, category, (), (const));
};

} // namespace Config
} // namespace Envoy