Skip to content

Commit

Permalink
jwt_authn: RemoteJwks to support RetryPolicy config envoyproxy#16319 (e…
Browse files Browse the repository at this point in the history
…nvoyproxy#16924)

Commit Message: jwt_authn: RemoteJwks to support RetryPolicy config envoyproxy#16319 ( only for background fetches failing )
Additional Description: following up on comments and code pruned from background jwks refresh mechanism from envoyproxy#16912

Risk Level: Low, default number of retries set to zero. truncated exponential backoff requires explicit configuration.
Testing: Only mock-based unit tests so far.  Haven't been able to get it ( or the background jwks fetch option, for that matter )  built in [esp-v2](https://github.com/GoogleCloudPlatform/espv-2) so far.
Docs Changes:
Release Notes:
Platform Specific Features:
Fixes envoyproxy#16319

Signed-off-by: Anthony Lichnewsky <[email protected]>
  • Loading branch information
alichnewsky authored Jul 14, 2021
1 parent 2f54f36 commit b62dae2
Show file tree
Hide file tree
Showing 19 changed files with 543 additions and 181 deletions.
31 changes: 31 additions & 0 deletions api/envoy/extensions/filters/http/jwt_authn/v3/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,37 @@ message RemoteJwks {
// * Jwks is ready when the requests come, not need to wait for the Jwks fetching.
//
JwksAsyncFetch async_fetch = 3;

// Retry policy for fetching Jwks. optional. turned off by default.
//
// For example:
//
// .. code-block:: yaml
//
// retry_policy:
// retry_back_off:
// base_interval: 0.01s
// max_interval: 20s
// num_retries: 10
//
// will yield a randomized truncated exponential backoff policy with an initial delay of 10ms
// 10 maximum attempts spaced at most 20s seconds.
//
// .. code-block:: yaml
//
// retry_policy:
// num_retries:1
//
// uses the default :ref:`retry backoff strategy <envoy_v3_api_msg_config.core.v3.BackoffStrategy>`.
// with the default base interval is 1000 milliseconds. and the default maximum interval of 10 times the base interval.
//
// if num_retries is omitted, the default is to allow only one retry.
//
//
// If enabled, the retry policy will apply to all Jwks fetching approaches, e.g. on demand or asynchronously in background.
//
//
config.core.v3.RetryPolicy retry_policy = 4;
}

// Fetch Jwks asynchronously in the main thread when the filter config is parsed.
Expand Down
31 changes: 31 additions & 0 deletions api/envoy/extensions/filters/http/jwt_authn/v4alpha/config.proto

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

12 changes: 12 additions & 0 deletions envoy/http/async_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,12 @@ class AsyncClient {
return *this;
}

// this should be done with setBufferedBodyForRetry=true ?
StreamOptions& setRetryPolicy(const envoy::config::route::v3::RetryPolicy& p) {
retry_policy = p;
return *this;
}

// For gmock test
bool operator==(const StreamOptions& src) const {
return timeout == src.timeout && buffer_body_for_retry == src.buffer_body_for_retry &&
Expand All @@ -239,6 +245,8 @@ class AsyncClient {
ParentContext parent_context;

envoy::config::core::v3::Metadata metadata;

absl::optional<envoy::config::route::v3::RetryPolicy> retry_policy;
};

/**
Expand Down Expand Up @@ -274,6 +282,10 @@ class AsyncClient {
StreamOptions::setMetadata(m);
return *this;
}
RequestOptions& setRetryPolicy(const envoy::config::route::v3::RetryPolicy& p) {
StreamOptions::setRetryPolicy(p);
return *this;
}
RequestOptions& setParentSpan(Tracing::Span& parent_span) {
parent_span_ = &parent_span;
return *this;
Expand Down

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

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

3 changes: 1 addition & 2 deletions source/common/http/async_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const std::vector<std::reference_wrapper<const Router::RateLimitPolicyEntry>>
AsyncStreamImpl::NullRateLimitPolicy::rate_limit_policy_entry_;
const AsyncStreamImpl::NullHedgePolicy AsyncStreamImpl::RouteEntryImpl::hedge_policy_;
const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::RouteEntryImpl::rate_limit_policy_;
const AsyncStreamImpl::NullRetryPolicy AsyncStreamImpl::RouteEntryImpl::retry_policy_;
const Router::InternalRedirectPolicyImpl AsyncStreamImpl::RouteEntryImpl::internal_redirect_policy_;
const std::vector<Router::ShadowPolicyPtr> AsyncStreamImpl::RouteEntryImpl::shadow_policies_;
const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_host_;
Expand Down Expand Up @@ -86,7 +85,7 @@ AsyncStreamImpl::AsyncStreamImpl(AsyncClientImpl& parent, AsyncClient::StreamCal
stream_info_(Protocol::Http11, parent.dispatcher().timeSource(), nullptr),
tracing_config_(Tracing::EgressConfig::get()),
route_(std::make_shared<RouteImpl>(parent_.cluster_->name(), options.timeout,
options.hash_policy)),
options.hash_policy, options.retry_policy)),
send_xff_(options.send_xff) {

stream_info_.dynamicMetadata().MergeFrom(options.metadata);
Expand Down
22 changes: 17 additions & 5 deletions source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
#include "source/common/common/empty_string.h"
#include "source/common/common/linked_object.h"
#include "source/common/http/message_impl.h"
#include "source/common/protobuf/message_validator_impl.h"
#include "source/common/router/config_impl.h"
#include "source/common/router/router.h"
#include "source/common/stream_info/stream_info_impl.h"
#include "source/common/tracing/http_tracer_impl.h"
Expand Down Expand Up @@ -211,11 +213,19 @@ class AsyncStreamImpl : public AsyncClient::Stream,
RouteEntryImpl(
const std::string& cluster_name, const absl::optional<std::chrono::milliseconds>& timeout,
const Protobuf::RepeatedPtrField<envoy::config::route::v3::RouteAction::HashPolicy>&
hash_policy)
hash_policy,
const absl::optional<envoy::config::route::v3::RetryPolicy>& retry_policy)
: cluster_name_(cluster_name), timeout_(timeout) {
if (!hash_policy.empty()) {
hash_policy_ = std::make_unique<HashPolicyImpl>(hash_policy);
}
if (retry_policy.has_value()) {
// ProtobufMessage::getStrictValidationVisitor() ? how often do we do this?
retry_policy_ = std::make_unique<Router::RetryPolicyImpl>(
retry_policy.value(), ProtobufMessage::getNullValidationVisitor());
} else {
retry_policy_ = std::make_unique<NullRetryPolicy>();
}
}

// Router::RouteEntry
Expand Down Expand Up @@ -243,7 +253,7 @@ class AsyncStreamImpl : public AsyncClient::Stream,
return Upstream::ResourcePriority::Default;
}
const Router::RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; }
const Router::RetryPolicy& retryPolicy() const override { return retry_policy_; }
const Router::RetryPolicy& retryPolicy() const override { return *retry_policy_; }
const Router::InternalRedirectPolicy& internalRedirectPolicy() const override {
return internal_redirect_policy_;
}
Expand Down Expand Up @@ -307,9 +317,10 @@ class AsyncStreamImpl : public AsyncClient::Stream,
const Router::RouteEntry::UpgradeMap& upgradeMap() const override { return upgrade_map_; }
const std::string& routeName() const override { return route_name_; }
std::unique_ptr<const HashPolicyImpl> hash_policy_;
std::unique_ptr<Router::RetryPolicy> retry_policy_;

static const NullHedgePolicy hedge_policy_;
static const NullRateLimitPolicy rate_limit_policy_;
static const NullRetryPolicy retry_policy_;
static const Router::InternalRedirectPolicyImpl internal_redirect_policy_;
static const std::vector<Router::ShadowPolicyPtr> shadow_policies_;
static const NullVirtualHost virtual_host_;
Expand All @@ -330,8 +341,9 @@ class AsyncStreamImpl : public AsyncClient::Stream,
RouteImpl(const std::string& cluster_name,
const absl::optional<std::chrono::milliseconds>& timeout,
const Protobuf::RepeatedPtrField<envoy::config::route::v3::RouteAction::HashPolicy>&
hash_policy)
: route_entry_(cluster_name, timeout, hash_policy) {}
hash_policy,
const absl::optional<envoy::config::route::v3::RetryPolicy>& retry_policy)
: route_entry_(cluster_name, timeout, hash_policy, retry_policy) {}

// Router::Route
const Router::DirectResponseEntry* directResponseEntry() const override { return nullptr; }
Expand Down
1 change: 1 addition & 0 deletions source/extensions/filters/http/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ envoy_cc_library(
"//envoy/upstream:cluster_manager_interface",
"//source/common/http:utility_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
"@envoy_api//envoy/extensions/filters/http/jwt_authn/v3:pkg_cc_proto",
],
)

Expand Down
Loading

0 comments on commit b62dae2

Please sign in to comment.