From dcc3954195556486dd483cc3b865c5a7071a37ef Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Tue, 14 Jan 2020 21:49:36 +0530 Subject: [PATCH 1/6] server: add workers started stat (#9482) Signed-off-by: Rama Chavali --- docs/root/configuration/listeners/stats.rst | 3 +++ docs/root/intro/version_history.rst | 1 + source/server/listener_manager_impl.cc | 27 +++++++++++++++++---- source/server/listener_manager_impl.h | 10 ++++++-- test/server/listener_manager_impl_test.cc | 24 ++++++++++++++++++ 5 files changed, 58 insertions(+), 7 deletions(-) diff --git a/docs/root/configuration/listeners/stats.rst b/docs/root/configuration/listeners/stats.rst index 73be50156bea..58bc2f57e297 100644 --- a/docs/root/configuration/listeners/stats.rst +++ b/docs/root/configuration/listeners/stats.rst @@ -52,6 +52,8 @@ on either accepted or active connections. downstream_cx_total, Counter, Total connections on this handler. downstream_cx_active, Gauge, Total active connections on this handler. +.. _config_listener_manager_stats: + Listener manager ---------------- @@ -71,3 +73,4 @@ statistics. Any ``:`` character in the stats name is replaced with ``_``. total_listeners_warming, Gauge, Number of currently warming listeners total_listeners_active, Gauge, Number of currently active listeners total_listeners_draining, Gauge, Number of currently draining listeners + workers_started, Gauge, A boolean (1 if started and 0 otherwise) that indicates whether listeners have been initialized on workers. diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index cb2d7e61f10c..ea71619d2ed3 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -45,6 +45,7 @@ Version history * router: added :ref:`auto_sni ` to support setting SNI to transport socket for new upstream connections based on the downstream HTTP host/authority header. * server: added the :option:`--disable-extensions` CLI option, to disable extensions at startup. * server: fixed a bug in config validation for configs with runtime layers. +* server: added :ref:`workers_started ` that indicates whether listeners have been fully initialized on workers. * tcp_proxy: added :ref:`ClusterWeight.metadata_match`. * tcp_proxy: added :ref:`hash_policy`. * thrift_proxy: added support for cluster header based routing. diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 563c5917b632..65673ef9a00b 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -584,11 +584,12 @@ std::vector> ListenerManagerImpl return ret; } -void ListenerManagerImpl::addListenerToWorker(Worker& worker, ListenerImpl& listener) { - worker.addListener(listener, [this, &listener](bool success) -> void { +void ListenerManagerImpl::addListenerToWorker(Worker& worker, ListenerImpl& listener, + ListenerCompletionCallback completion_callback) { + worker.addListener(listener, [this, &listener, completion_callback](bool success) -> void { // The add listener completion runs on the worker thread. Post back to the main thread to // avoid locking. - server_.dispatcher().post([this, success, &listener]() -> void { + server_.dispatcher().post([this, success, &listener, completion_callback]() -> void { // It is theoretically possible for a listener to get added on 1 worker but not the others. // The below check with onListenerCreateFailure() is there to ensure we execute the // removal/logging/stats at most once on failure. Note also that drain/removal can race @@ -606,6 +607,9 @@ void ListenerManagerImpl::addListenerToWorker(Worker& worker, ListenerImpl& list if (success) { stats_.listener_create_success_.inc(); } + if (completion_callback) { + completion_callback(); + } }); }); } @@ -614,7 +618,7 @@ void ListenerManagerImpl::onListenerWarmed(ListenerImpl& listener) { // The warmed listener should be added first so that the worker will accept new connections // when it stops listening on the old listener. for (const auto& worker : workers_) { - addListenerToWorker(*worker, listener); + addListenerToWorker(*worker, listener, nullptr); } auto existing_active_listener = getListenerByName(active_listeners_, listener.name()); @@ -687,17 +691,30 @@ void ListenerManagerImpl::startWorkers(GuardDog& guard_dog) { ASSERT(!workers_started_); workers_started_ = true; uint32_t i = 0; + + // We can not use "Cleanup" to simplify this logic here, because it results in a issue if Envoy is + // killed before workers are actually started. Specifically the AdminRequestGetStatsAndKill test + // case in main_common_test fails with ASAN error if we use "Cleanup" here. + const auto listeners_pending_init = + std::make_shared>(workers_.size() * active_listeners_.size()); for (const auto& worker : workers_) { ENVOY_LOG(info, "starting worker {}", i); ASSERT(warming_listeners_.empty()); for (const auto& listener : active_listeners_) { - addListenerToWorker(*worker, *listener); + addListenerToWorker(*worker, *listener, [this, listeners_pending_init]() { + if (--(*listeners_pending_init) == 0) { + stats_.workers_started_.set(1); + } + }); } worker->start(guard_dog); if (enable_dispatcher_stats_) { worker->initializeStats(*scope_, fmt::format("worker_{}.", i++)); } } + if (active_listeners_.size() == 0) { + stats_.workers_started_.set(1); + } } void ListenerManagerImpl::stopListener(Network::ListenerConfig& listener, diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index 89885a4ec14c..988de5c5fe7a 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -113,7 +113,8 @@ using ListenerImplPtr = std::unique_ptr; COUNTER(listener_stopped) \ GAUGE(total_listeners_active, NeverImport) \ GAUGE(total_listeners_draining, NeverImport) \ - GAUGE(total_listeners_warming, NeverImport) + GAUGE(total_listeners_warming, NeverImport) \ + GAUGE(workers_started, NeverImport) /** * Struct definition for all listener manager stats. @see stats_macros.h @@ -154,6 +155,10 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable; + /** + * Callback invoked when a listener initialization is completed on worker. + */ + using ListenerCompletionCallback = std::function; bool addOrUpdateListenerInternal(const envoy::config::listener::v3alpha::Listener& config, const std::string& version_info, bool added_via_api, @@ -166,7 +171,8 @@ class ListenerManagerImpl : public ListenerManager, Logger::LoggablestartWorkers(guard_dog_); + // Validate that there are no active listeners and workers are started. + EXPECT_EQ(0, server_.stats_store_ + .gauge("listener_manager.total_active_listeners", + Stats::Gauge::ImportMode::NeverImport) + .value()); + EXPECT_EQ(1, server_.stats_store_ + .gauge("listener_manager.workers_started", Stats::Gauge::ImportMode::NeverImport) + .value()); const std::string proto_text = R"EOF( address: { @@ -758,12 +766,28 @@ version_info: version2 nanos: 2000000 )EOF"); + // Validate that workers_started stat is zero before calling startWorkers. + EXPECT_EQ(0, server_.stats_store_ + .gauge("listener_manager.workers_started", Stats::Gauge::ImportMode::NeverImport) + .value()); + // Start workers. EXPECT_CALL(*worker_, addListener(_, _)); EXPECT_CALL(*worker_, start(_)); manager_->startWorkers(guard_dog_); + // Validate that workers_started stat is still zero before workers set the status via + // completion callback. + EXPECT_EQ(0, server_.stats_store_ + .gauge("listener_manager.workers_started", Stats::Gauge::ImportMode::NeverImport) + .value()); worker_->callAddCompletion(true); + // Validate that workers_started stat is set to 1 after workers have responded with initialization + // status. + EXPECT_EQ(1, server_.stats_store_ + .gauge("listener_manager.workers_started", Stats::Gauge::ImportMode::NeverImport) + .value()); + // Update duplicate should be a NOP. EXPECT_FALSE( manager_->addOrUpdateListener(parseListenerFromV2Yaml(listener_foo_update1_yaml), "", true)); From 61ba6445bb2387b11806a4edf8241c2d53d463cb Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Tue, 14 Jan 2020 11:20:25 -0500 Subject: [PATCH 2/6] stats: tls and fuzzer cleanup (#9616) Signed-off-by: Joshua Marantz --- source/common/stats/thread_local_store.cc | 12 +++++++----- source/common/stats/thread_local_store.h | 4 ++-- test/common/stats/symbol_table_fuzz_test.cc | 19 ++++++++++++++++++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/source/common/stats/thread_local_store.cc b/source/common/stats/thread_local_store.cc index d6b0b8a86981..c35cd1e39f66 100644 --- a/source/common/stats/thread_local_store.cc +++ b/source/common/stats/thread_local_store.cc @@ -234,6 +234,11 @@ void ThreadLocalStoreImpl::releaseScopeCrossThread(ScopeImpl* scope) { } } +ThreadLocalStoreImpl::TlsCacheEntry& +ThreadLocalStoreImpl::TlsCache::insertScope(uint64_t scope_id) { + return scope_cache_[scope_id]; +} + void ThreadLocalStoreImpl::TlsCache::eraseScope(uint64_t scope_id) { scope_cache_.erase(scope_id); } void ThreadLocalStoreImpl::clearScopeFromCaches(uint64_t scope_id, @@ -248,12 +253,9 @@ void ThreadLocalStoreImpl::clearScopeFromCaches(uint64_t scope_id, } } -std::atomic ThreadLocalStoreImpl::ScopeImpl::next_scope_id_; - ThreadLocalStoreImpl::ScopeImpl::ScopeImpl(ThreadLocalStoreImpl& parent, const std::string& prefix) - : scope_id_(next_scope_id_++), parent_(parent), + : scope_id_(parent.next_scope_id_++), parent_(parent), prefix_(Utility::sanitizeStatsName(prefix), parent.symbolTable()), - // central_cache_(std::make_shared(parent.symbolTable())) {} central_cache_(new CentralCacheEntry(parent.symbolTable())) {} ThreadLocalStoreImpl::ScopeImpl::~ScopeImpl() { @@ -389,7 +391,7 @@ Counter& ThreadLocalStoreImpl::ScopeImpl::counterFromStatName(StatName name) { StatRefMap* tls_cache = nullptr; StatNameHashSet* tls_rejected_stats = nullptr; if (!parent_.shutting_down_ && parent_.tls_) { - TlsCacheEntry& entry = parent_.tls_->getTyped().scope_cache_[this->scope_id_]; + TlsCacheEntry& entry = parent_.tls_->getTyped().insertScope(this->scope_id_); tls_cache = &entry.counters_; tls_rejected_stats = &entry.rejected_stats_; } diff --git a/source/common/stats/thread_local_store.h b/source/common/stats/thread_local_store.h index d334fcf67bee..0f97f63ced9d 100644 --- a/source/common/stats/thread_local_store.h +++ b/source/common/stats/thread_local_store.h @@ -350,8 +350,6 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo std::unique_ptr& truncated_name_storage, std::vector& tags, std::string& tag_extracted_name); - static std::atomic next_scope_id_; - const uint64_t scope_id_; ThreadLocalStoreImpl& parent_; StatNameStorage prefix_; @@ -359,6 +357,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo }; struct TlsCache : public ThreadLocal::ThreadLocalObject { + TlsCacheEntry& insertScope(uint64_t scope_id); void eraseScope(uint64_t scope_id); // The TLS scope cache is keyed by scope ID. This is used to avoid complex circular references @@ -413,6 +412,7 @@ class ThreadLocalStoreImpl : Logger::Loggable, public StoreRo std::vector deleted_histograms_; Thread::ThreadSynchronizer sync_; + std::atomic next_scope_id_{}; }; } // namespace Stats diff --git a/test/common/stats/symbol_table_fuzz_test.cc b/test/common/stats/symbol_table_fuzz_test.cc index 724d0244e24e..e1a08a03e7a5 100644 --- a/test/common/stats/symbol_table_fuzz_test.cc +++ b/test/common/stats/symbol_table_fuzz_test.cc @@ -1,3 +1,5 @@ +#include + #include "common/common/assert.h" #include "common/common/base64.h" #include "common/stats/symbol_table_impl.h" @@ -13,18 +15,33 @@ namespace Fuzz { // Fuzzer for symbol tables. DEFINE_FUZZER(const uint8_t* buf, size_t len) { FuzzedDataProvider provider(buf, len); + FakeSymbolTableImpl fake_symbol_table; SymbolTableImpl symbol_table; StatNamePool pool(symbol_table); + StatNamePool fake_pool(fake_symbol_table); while (provider.remaining_bytes() != 0) { std::string next_data = provider.ConsumeRandomLengthString(provider.remaining_bytes()); StatName stat_name = pool.add(next_data); + StatName fake_stat_name = fake_pool.add(next_data); // We can add stat-names with trailing dots, but note that they will be // trimmed by the Symbol Table implementation, so we must trim the input // string before comparing. absl::string_view trimmed_fuzz_data = StringUtil::removeTrailingCharacters(next_data, '.'); FUZZ_ASSERT(trimmed_fuzz_data == symbol_table.toString(stat_name)); + FUZZ_ASSERT(trimmed_fuzz_data == fake_symbol_table.toString(fake_stat_name)); + + // Test all combinations of joins within each symbol table. + std::string joined = absl::StrCat(trimmed_fuzz_data, ".", trimmed_fuzz_data); + auto join = [](SymbolTable& table, StatName name1, StatName name2) -> std::string { + SymbolTable::StoragePtr storage = table.join({name1, name2}); + StatName stat_name(storage.get()); + return table.toString(stat_name); + }; + + FUZZ_ASSERT(joined == join(symbol_table, stat_name, stat_name)); + FUZZ_ASSERT(joined == join(fake_symbol_table, fake_stat_name, fake_stat_name)); // Also encode the string directly, without symbolizing it. TestUtil::serializeDeserializeString(next_data); @@ -32,7 +49,7 @@ DEFINE_FUZZER(const uint8_t* buf, size_t len) { // Grab the first few bytes from next_data to synthesize together a random uint64_t. if (next_data.size() > 1) { uint32_t num_bytes = (next_data[0] % 8) + 1; // random number between 1 and 8 inclusive. - num_bytes = std::min(static_cast(next_data.size()), + num_bytes = std::min(static_cast(next_data.size() - 1), num_bytes); // restrict number up to the size of next_data uint64_t number = 0; for (uint32_t i = 0; i < num_bytes; ++i) { From ce4a4ad64a7d0b6c344726ec0e7a0f209796fb97 Mon Sep 17 00:00:00 2001 From: htuch Date: Tue, 14 Jan 2020 15:52:14 -0500 Subject: [PATCH 3/6] api: document annotations and how to add an extension config. (#9672) Fix #9550 Partly addresses #5263 Signed-off-by: Harvey Tuch --- api/API_VERSIONING.md | 14 ++-------- api/STYLE.md | 64 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/api/API_VERSIONING.md b/api/API_VERSIONING.md index b31d9f723215..4684ed3e86e9 100644 --- a/api/API_VERSIONING.md +++ b/api/API_VERSIONING.md @@ -76,7 +76,7 @@ implementations within a major version should set explicit values for these fiel # API lifecycle -The API lifecycle follows a calendar clock. At the end of Q3 each year, a major API version +The API lifecycle follows a calendar clock. At the end of Q4 each year, a major API version increment may occur for any Envoy API package, in concert with the quarterly Envoy release. Envoy will support at most three major versions of any API package at all times: @@ -134,17 +134,9 @@ methods, depending on whether the change is mechanical or manual. ## Mechanical breaking changes -Field deprecations, renames, etc. are mechanical changes that will be supported by the +Field deprecations, renames, etc. are mechanical changes that are supported by the [protoxform](https://github.com/envoyproxy/envoy/tree/master/tools/protoxform) tool. These are -guided by annotations in protobuf. -* Deprecations are specified with the built-in protobuf deprecated option set on a message, enum, - field or enum value. No field may be marked as deprecated unless a replacement for this - functionality exists and the corresponding Envoy implementation is production ready. - -* Renames are specified with a `[(udpa.annotations.field_migrate).rename = ""]` annotation. - -* We anticipate that `protoxform` will also support `oneof` promotion, package movement, etc. via - similar annotations. +guided by [annotations](STYLE.md#api-annotations). ## Manual breaking changes diff --git a/api/STYLE.md b/api/STYLE.md index 94f6f49ba158..a293334a6179 100644 --- a/api/STYLE.md +++ b/api/STYLE.md @@ -113,3 +113,67 @@ for services and configs should be `//docs` (proto documentation tool). Extensions should use the regular hierarchy. For example, configuration for network filters belongs in a package under `envoy.config.filter.network`. + +## Adding an extension configuration to the API + +Extensions must currently be added as v2 APIs following the [package +organization](#package-organization) above. +To add an extension config to the API, the steps below should be followed: + +1. Place the v2 extension configuration `.proto` in `api/config`, e.g. + `api/config/filter/http/foobar/v2/foobar.proto` together with an initial BUILD file: + ``` + load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + + licenses(["notice"]) # Apache 2 + + api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], + ) + ``` +1. Add to the v2 extension config proto `import "udpa/annotations/migrate.proto";` +2. Add to the v2 extension config proto a package level `option (udpa.annotations.file_migrate).move_to_package = "envoy.extensions.filters.http.foobar.v3alpha";`. + This places the filter in the correct [v3 package hierarchy](#package-organization). +3. Add a reference to the v2 extension config in (1) in [api/docs/BUILD](docs/BUILD). +4. Run `./tools/proto_format fix`. This should regenerate the `BUILD` file, + reformat `foobar.proto` as needed and also generate the v3 extension config, + together with shadow API protos. +4. `git add api/ generated_api_shadow/` to add any new files to your Git index. + +## API annotations + +A number of annotations are used in the Envoy APIs to provide additional API +metadata. We describe these annotations below by category. + +### Field level +* `[deprecated = true]` to denote fields that are deprecated in a major version. + These fields are slated for removal at the next major cycle and follow the + [breaking change policy](../CONTRIBUTING.md#breaking-change-policy). +* `[envoy.annotations.disallowed_by_default = true]` to denote fields that have + been disallowed by default as per the [breaking change policy](../CONTRIBUTING.md#breaking-change-policy). +* `[(udpa.annotations.field_migrate).rename = ""]` to denote that + the field will be renamed to a given name in the next API major version. +* `[(udpa.annotations.field_migrate).oneof_promotion = ""]` to denote that + the field will be promoted to a given `oneof` in the next API major version. +* `[(udpa.annotations.sensitive) = true]` to denote sensitive fields that + should be redacted in output such as logging or configuration dumps. +* [PGV annotations](https://github.com/envoyproxy/protoc-gen-validate) to denote field + value constraints. + +### Enum value level +* `[(udpa.annotations.enum_value_migrate).rename = "new enum value name"]` to denote that + the enum value will be renamed to a given name in the next API major version. + +### Message level +* `option (udpa.annotations.versioning).previous_message_type = "";` to denote the previous type name for an upgraded message. You should + never have to write these manually, they are generated by `protoxform`. + +### Service level +* `option (envoy.annotations.resource).type = "";` to denote + the resource type for an xDS service definition. + +### File level +* `option (udpa.annotations.file_migrate).move_to_package = "";` + to denote that in the next major version of the API, the file will be moved to + the given package. This is consumed by `protoxform`. From 4400e53cf33ff5f2a30d3b59140e577723759dd6 Mon Sep 17 00:00:00 2001 From: zyfjeff Date: Wed, 15 Jan 2020 05:56:04 +0800 Subject: [PATCH 4/6] Shared subset any to avoid wasted memory and update overhead (#9632) Currently the subset lb may create multiple copies of a subset referring to all the hosts of a PrioritySubsetImpl, wasting memory. Since the subsets are functionally identical, merge them into a single copy. Risk Level: low Testing: existing unit tests Docs Changes: N/A Release Notes: N/A Signed-off-by: tianqian.zyf --- source/common/upstream/subset_lb.cc | 53 +++++++++++++++-------------- source/common/upstream/subset_lb.h | 3 +- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/source/common/upstream/subset_lb.cc b/source/common/upstream/subset_lb.cc index c29f0ae0a0e2..1714f440cd53 100644 --- a/source/common/upstream/subset_lb.cc +++ b/source/common/upstream/subset_lb.cc @@ -42,29 +42,24 @@ SubsetLoadBalancer::SubsetLoadBalancer( HostPredicate predicate; if (fallback_policy_ == envoy::config::cluster::v3alpha::Cluster::LbSubsetConfig::ANY_ENDPOINT) { - predicate = [](const Host&) -> bool { return true; }; - ENVOY_LOG(debug, "subset lb: creating any-endpoint fallback load balancer"); + initSubsetAnyOnce(); + fallback_subset_ = subset_any_; } else { predicate = [this](const Host& host) -> bool { return hostMatches(default_subset_metadata_, host); }; - ENVOY_LOG(debug, "subset lb: creating fallback load balancer for {}", describeMetadata(default_subset_metadata_)); + fallback_subset_ = std::make_shared(); + fallback_subset_->priority_subset_ = std::make_shared( + *this, predicate, locality_weight_aware_, scale_locality_weight_); } - - fallback_subset_ = std::make_shared(); - fallback_subset_->priority_subset_ = std::make_shared( - *this, predicate, locality_weight_aware_, scale_locality_weight_); } if (subsets.panicModeAny()) { - HostPredicate predicate = [](const Host&) -> bool { return true; }; - - panic_mode_subset_ = std::make_shared(); - panic_mode_subset_->priority_subset_ = std::make_shared( - *this, predicate, locality_weight_aware_, scale_locality_weight_); + initSubsetAnyOnce(); + panic_mode_subset_ = subset_any_; } // Create filtered default subset (if necessary) and other subsets based on current hosts. @@ -116,6 +111,15 @@ void SubsetLoadBalancer::refreshSubsets(uint32_t priority) { update(priority, host_sets[priority]->hosts(), {}); } +void SubsetLoadBalancer::initSubsetAnyOnce() { + if (!subset_any_) { + HostPredicate predicate = [](const Host&) -> bool { return true; }; + subset_any_ = std::make_shared(); + subset_any_->priority_subset_ = std::make_shared( + *this, predicate, locality_weight_aware_, scale_locality_weight_); + } +} + void SubsetLoadBalancer::initSubsetSelectorMap() { selectors_ = std::make_shared(); SubsetSelectorMapPtr selectors; @@ -156,12 +160,9 @@ void SubsetLoadBalancer::initSelectorFallbackSubset( LbSubsetSelectorFallbackPolicy& fallback_policy) { if (fallback_policy == envoy::config::cluster::v3alpha::Cluster::LbSubsetConfig:: LbSubsetSelector::ANY_ENDPOINT && - selector_fallback_subset_any_ == nullptr) { + subset_any_ == nullptr) { ENVOY_LOG(debug, "subset lb: creating any-endpoint fallback load balancer for selector"); - HostPredicate predicate = [](const Host&) -> bool { return true; }; - selector_fallback_subset_any_ = std::make_shared(); - selector_fallback_subset_any_->priority_subset_.reset( - new PrioritySubsetImpl(*this, predicate, locality_weight_aware_, scale_locality_weight_)); + initSubsetAnyOnce(); } else if (fallback_policy == envoy::config::cluster::v3alpha::Cluster::LbSubsetConfig:: LbSubsetSelector::DEFAULT_SUBSET && selector_fallback_subset_default_ == nullptr) { @@ -248,8 +249,8 @@ HostConstSharedPtr SubsetLoadBalancer::chooseHostForSelectorFallbackPolicy( const auto& fallback_policy = fallback_params.fallback_policy_; if (fallback_policy == envoy::config::cluster::v3alpha::Cluster::LbSubsetConfig:: LbSubsetSelector::ANY_ENDPOINT && - selector_fallback_subset_any_ != nullptr) { - return selector_fallback_subset_any_->priority_subset_->lb_->chooseHost(context); + subset_any_ != nullptr) { + return subset_any_->priority_subset_->lb_->chooseHost(context); } else if (fallback_policy == envoy::config::cluster::v3alpha::Cluster::LbSubsetConfig:: LbSubsetSelector::DEFAULT_SUBSET && selector_fallback_subset_default_ != nullptr) { @@ -332,8 +333,8 @@ SubsetLoadBalancer::LbSubsetEntryPtr SubsetLoadBalancer::findSubset( void SubsetLoadBalancer::updateFallbackSubset(uint32_t priority, const HostVector& hosts_added, const HostVector& hosts_removed) { - if (selector_fallback_subset_any_ != nullptr) { - selector_fallback_subset_any_->priority_subset_->update(priority, hosts_added, hosts_removed); + if (subset_any_ != nullptr) { + subset_any_->priority_subset_->update(priority, hosts_added, hosts_removed); } if (selector_fallback_subset_default_ != nullptr) { @@ -346,13 +347,13 @@ void SubsetLoadBalancer::updateFallbackSubset(uint32_t priority, const HostVecto return; } - // Add/remove hosts. - fallback_subset_->priority_subset_->update(priority, hosts_added, hosts_removed); + if (fallback_subset_ != subset_any_) { + // Add/remove hosts. + fallback_subset_->priority_subset_->update(priority, hosts_added, hosts_removed); + } // Same thing for the panic mode subset. - if (panic_mode_subset_ != nullptr) { - panic_mode_subset_->priority_subset_->update(priority, hosts_added, hosts_removed); - } + ASSERT(panic_mode_subset_ == nullptr || panic_mode_subset_ == subset_any_); } // Iterates over the added and removed hosts, looking up an LbSubsetEntryPtr for each. For every diff --git a/source/common/upstream/subset_lb.h b/source/common/upstream/subset_lb.h index a9b3439606ff..69b9b8b1209e 100644 --- a/source/common/upstream/subset_lb.h +++ b/source/common/upstream/subset_lb.h @@ -41,6 +41,7 @@ class SubsetLoadBalancer : public LoadBalancer, Logger::Loggable; struct SubsetSelectorFallbackParams; + void initSubsetAnyOnce(); void initSubsetSelectorMap(); void initSelectorFallbackSubset(const envoy::config::cluster::v3alpha::Cluster::LbSubsetConfig:: LbSubsetSelector::LbSubsetSelectorFallbackPolicy&); @@ -246,10 +247,10 @@ class SubsetLoadBalancer : public LoadBalancer, Logger::Loggable Date: Wed, 15 Jan 2020 06:20:40 +0800 Subject: [PATCH 5/6] Make only once abort or delay decision (#9597) In the original logic of fault filter, when the header contains the x-envoy-downstream-service-cluster, fault filter will execute the decision whether to abort or delay twice. As a result, the actual abort or delay ratio does not meet expectations. The expectations is p and the actual value will be 2p-p^2. For example, when the abort ratio is configured as 50%, if the header contains x-envoy-downstream-service-cluster, the actual abort ratio is 75%. This problem can be reproduced by enabling add_user_agent. The current PR fixes this problem. Risk Level: Low Testing: unit tests updated Docs Changes: n/a Release Notes: added Signed-off-by: wbpcode --- docs/root/intro/version_history.rst | 1 + .../filters/http/fault/fault_filter.cc | 16 +++++++--------- .../filters/http/fault/fault_filter_test.cc | 16 ---------------- 3 files changed, 8 insertions(+), 25 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index ea71619d2ed3..2519508afd51 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -57,6 +57,7 @@ Version history * tracing: added initial support for AWS X-Ray (local sampling rules only) :ref:`X-Ray Tracing `. * tracing: added tags for gRPC request path, authority, content-type and timeout. * udp: added initial support for :ref:`UDP proxy ` +* fault: fixed an issue where the http fault filter would repeatedly check the percentage of abort/delay when the `x-envoy-downstream-service-cluster` header was included in the request to ensure that the actual percentage of abort/delay matches the configuration of the filter. 1.12.2 (December 10, 2019) ========================== diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index 98209074643f..e3cfee08ad0e 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -225,23 +225,21 @@ bool FaultFilter::isDelayEnabled() { return false; } - bool enabled = config_->runtime().snapshot().featureEnabled( - fault_settings_->delayPercentRuntime(), fault_settings_->requestDelay()->percentage()); if (!downstream_cluster_delay_percent_key_.empty()) { - enabled |= config_->runtime().snapshot().featureEnabled( + return config_->runtime().snapshot().featureEnabled( downstream_cluster_delay_percent_key_, fault_settings_->requestDelay()->percentage()); } - return enabled; + return config_->runtime().snapshot().featureEnabled( + fault_settings_->delayPercentRuntime(), fault_settings_->requestDelay()->percentage()); } bool FaultFilter::isAbortEnabled() { - bool enabled = config_->runtime().snapshot().featureEnabled( - fault_settings_->abortPercentRuntime(), fault_settings_->abortPercentage()); if (!downstream_cluster_abort_percent_key_.empty()) { - enabled |= config_->runtime().snapshot().featureEnabled(downstream_cluster_abort_percent_key_, - fault_settings_->abortPercentage()); + return config_->runtime().snapshot().featureEnabled(downstream_cluster_abort_percent_key_, + fault_settings_->abortPercentage()); } - return enabled; + return config_->runtime().snapshot().featureEnabled(fault_settings_->abortPercentRuntime(), + fault_settings_->abortPercentage()); } absl::optional diff --git a/test/extensions/filters/http/fault/fault_filter_test.cc b/test/extensions/filters/http/fault/fault_filter_test.cc index e98d65cd69af..a268e1e120ca 100644 --- a/test/extensions/filters/http/fault/fault_filter_test.cc +++ b/test/extensions/filters/http/fault/fault_filter_test.cc @@ -382,10 +382,6 @@ TEST_F(FaultFilterTest, DelayForDownstreamCluster) { request_headers_.addCopy("x-envoy-downstream-service-cluster", "cluster"); // Delay related calls. - EXPECT_CALL(runtime_.snapshot_, - featureEnabled("fault.http.delay.fixed_delay_percent", - Matcher(Percent(100)))) - .WillOnce(Return(false)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("fault.http.cluster.delay.fixed_delay_percent", Matcher(Percent(100)))) @@ -403,10 +399,6 @@ TEST_F(FaultFilterTest, DelayForDownstreamCluster) { filter_->decodeHeaders(request_headers_, false)); // Abort related calls. - EXPECT_CALL(runtime_.snapshot_, - featureEnabled("fault.http.abort.abort_percent", - Matcher(Percent(0)))) - .WillOnce(Return(false)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("fault.http.cluster.abort.abort_percent", Matcher(Percent(0)))) @@ -443,10 +435,6 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortDownstream) { request_headers_.addCopy("x-envoy-downstream-service-cluster", "cluster"); // Delay related calls. - EXPECT_CALL(runtime_.snapshot_, - featureEnabled("fault.http.delay.fixed_delay_percent", - Matcher(Percent(100)))) - .WillOnce(Return(false)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("fault.http.cluster.delay.fixed_delay_percent", Matcher(Percent(100)))) @@ -467,10 +455,6 @@ TEST_F(FaultFilterTest, FixedDelayAndAbortDownstream) { EXPECT_EQ(1UL, config_->stats().active_faults_.value()); // Abort related calls - EXPECT_CALL(runtime_.snapshot_, - featureEnabled("fault.http.abort.abort_percent", - Matcher(Percent(100)))) - .WillOnce(Return(false)); EXPECT_CALL(runtime_.snapshot_, featureEnabled("fault.http.cluster.abort.abort_percent", Matcher(Percent(100)))) From 156d7c90083c196a206c07fc03b2de6be8260bd3 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Tue, 14 Jan 2020 19:48:05 -0500 Subject: [PATCH 6/6] tooling: restoring runtime fatal-by-defaults and changing to fully qualified (#9591) Changing from relative name to absolute name, and fixing the fatal-by-defaults that were broken by the v3 switch. The old way to allow fatal-by-defaults was envoy.deprecated_features:proto_file.proto:field_name the new way is envoy.deprecated_features:full.namespace.field_name When we switched to v3, all the hard-coded v2 names stopped working. This reinstates them via hopefully more permanent proto annotation. The only remaining ugly bit is that unfortunately the full namespace and field name are the v3 versions even if the original config was v2. Between @htuch and I we should fix that before merging. Risk Level: Medium Testing: added new unit tests Docs Changes: updated Release Notes: n/a Signed-off-by: Alyssa Wilk --- api/envoy/admin/v2alpha/BUILD | 1 + api/envoy/admin/v2alpha/server_info.proto | 7 ++-- api/envoy/admin/v3alpha/BUILD | 1 + api/envoy/admin/v3alpha/server_info.proto | 2 ++ api/envoy/annotations/deprecation.proto | 21 ++++++++++++ api/envoy/api/v2/core/BUILD | 1 + api/envoy/api/v2/core/config_source.proto | 4 ++- api/envoy/api/v2/route/BUILD | 1 + api/envoy/api/v2/route/route_components.proto | 6 ++-- api/envoy/config/bootstrap/v2/BUILD | 1 + api/envoy/config/bootstrap/v2/bootstrap.proto | 3 +- api/envoy/config/bootstrap/v3alpha/BUILD | 1 + .../config/bootstrap/v3alpha/bootstrap.proto | 1 + api/envoy/config/core/v3alpha/BUILD | 1 + .../config/core/v3alpha/config_source.proto | 4 ++- api/envoy/config/filter/fault/v2/BUILD | 1 + api/envoy/config/filter/fault/v2/fault.proto | 3 +- .../config/filter/http/ext_authz/v2/BUILD | 1 + .../filter/http/ext_authz/v2/ext_authz.proto | 3 +- .../filter/network/redis_proxy/v2/BUILD | 1 + .../network/redis_proxy/v2/redis_proxy.proto | 6 ++-- api/envoy/config/route/v3alpha/BUILD | 1 + .../route/v3alpha/route_components.proto | 1 + .../filters/common/fault/v3alpha/BUILD | 1 + .../filters/common/fault/v3alpha/fault.proto | 1 + .../filters/http/ext_authz/v3alpha/BUILD | 1 + .../http/ext_authz/v3alpha/ext_authz.proto | 1 + .../filters/network/redis_proxy/v3alpha/BUILD | 1 + .../redis_proxy/v3alpha/redis_proxy.proto | 1 + .../root/configuration/operations/runtime.rst | 13 ++++---- .../envoy/admin/v2alpha/BUILD | 1 + .../envoy/admin/v2alpha/server_info.proto | 7 ++-- .../envoy/admin/v3alpha/BUILD | 1 + .../envoy/admin/v3alpha/server_info.proto | 8 +++-- .../envoy/annotations/deprecation.proto | 21 ++++++++++++ generated_api_shadow/envoy/api/v2/core/BUILD | 1 + .../envoy/api/v2/core/config_source.proto | 4 ++- generated_api_shadow/envoy/api/v2/route/BUILD | 1 + .../envoy/api/v2/route/route_components.proto | 6 ++-- .../envoy/config/bootstrap/v2/BUILD | 1 + .../envoy/config/bootstrap/v2/bootstrap.proto | 3 +- .../envoy/config/bootstrap/v3alpha/BUILD | 1 + .../config/bootstrap/v3alpha/bootstrap.proto | 4 ++- .../envoy/config/core/v3alpha/BUILD | 1 + .../config/core/v3alpha/config_source.proto | 4 ++- .../envoy/config/filter/fault/v2/BUILD | 1 + .../envoy/config/filter/fault/v2/fault.proto | 3 +- .../config/filter/http/ext_authz/v2/BUILD | 1 + .../filter/http/ext_authz/v2/ext_authz.proto | 3 +- .../filter/network/redis_proxy/v2/BUILD | 1 + .../network/redis_proxy/v2/redis_proxy.proto | 6 ++-- .../envoy/config/route/v3alpha/BUILD | 1 + .../route/v3alpha/route_components.proto | 7 ++-- .../filters/common/fault/v3alpha/BUILD | 1 + .../filters/common/fault/v3alpha/fault.proto | 4 ++- .../filters/http/ext_authz/v3alpha/BUILD | 1 + .../http/ext_authz/v3alpha/ext_authz.proto | 4 ++- .../filters/network/redis_proxy/v3alpha/BUILD | 1 + .../redis_proxy/v3alpha/redis_proxy.proto | 7 ++-- include/envoy/runtime/runtime.h | 19 +++++++---- source/common/protobuf/BUILD | 1 + source/common/protobuf/utility.cc | 19 ++++++----- source/common/runtime/runtime_features.cc | 33 ------------------- source/common/runtime/runtime_features.h | 8 ----- source/common/runtime/runtime_impl.cc | 10 +++--- source/common/runtime/runtime_impl.h | 2 +- test/common/protobuf/utility_test.cc | 21 ++++++++++-- test/common/router/BUILD | 1 + test/common/router/config_impl_test.cc | 6 ++++ test/common/runtime/runtime_impl_test.cc | 7 +--- test/common/runtime/utility.h | 5 --- test/config_test/config_test.cc | 11 ++----- .../filters/network/redis_proxy/BUILD | 1 + .../network/redis_proxy/config_test.cc | 17 ++++++++++ test/integration/server.cc | 12 +++---- test/mocks/runtime/mocks.h | 2 +- test/proto/BUILD | 1 + test/proto/deprecated.proto | 9 +++-- test/server/server_fuzz_test.cc | 6 +--- tools/proto_sync.py | 5 ++- tools/protoxform/protoxform.py | 6 +++- 81 files changed, 258 insertions(+), 140 deletions(-) create mode 100644 api/envoy/annotations/deprecation.proto create mode 100644 generated_api_shadow/envoy/annotations/deprecation.proto diff --git a/api/envoy/admin/v2alpha/BUILD b/api/envoy/admin/v2alpha/BUILD index 3ced653bc328..a7253df510f8 100644 --- a/api/envoy/admin/v2alpha/BUILD +++ b/api/envoy/admin/v2alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/config/bootstrap/v2:pkg", "//envoy/service/tap/v2alpha:pkg", diff --git a/api/envoy/admin/v2alpha/server_info.proto b/api/envoy/admin/v2alpha/server_info.proto index 099e2f03bc8f..1052cb6296ee 100644 --- a/api/envoy/admin/v2alpha/server_info.proto +++ b/api/envoy/admin/v2alpha/server_info.proto @@ -4,6 +4,8 @@ package envoy.admin.v2alpha; import "google/protobuf/duration.proto"; +import "envoy/annotations/deprecation.proto"; + option java_package = "io.envoyproxy.envoy.admin.v2alpha"; option java_outer_classname = "ServerInfoProto"; option java_multiple_files = true; @@ -128,9 +130,10 @@ message CommandLineOptions { Mode mode = 19; // max_stats and max_obj_name_len are now unused and have no effect. - uint64 max_stats = 20 [deprecated = true]; + uint64 max_stats = 20 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; - uint64 max_obj_name_len = 21 [deprecated = true]; + uint64 max_obj_name_len = 21 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // See :option:`--disable-hot-restart` for details. bool disable_hot_restart = 22; diff --git a/api/envoy/admin/v3alpha/BUILD b/api/envoy/admin/v3alpha/BUILD index 32469e852fd4..b724bb8ad88f 100644 --- a/api/envoy/admin/v3alpha/BUILD +++ b/api/envoy/admin/v3alpha/BUILD @@ -7,6 +7,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ "//envoy/admin/v2alpha:pkg", + "//envoy/annotations:pkg", "//envoy/config/bootstrap/v3alpha:pkg", "//envoy/config/core/v3alpha:pkg", "//envoy/config/tap/v3alpha:pkg", diff --git a/api/envoy/admin/v3alpha/server_info.proto b/api/envoy/admin/v3alpha/server_info.proto index ccceb4e1b180..1136a4118a0e 100644 --- a/api/envoy/admin/v3alpha/server_info.proto +++ b/api/envoy/admin/v3alpha/server_info.proto @@ -6,6 +6,8 @@ import "google/protobuf/duration.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; + option java_package = "io.envoyproxy.envoy.admin.v3alpha"; option java_outer_classname = "ServerInfoProto"; option java_multiple_files = true; diff --git a/api/envoy/annotations/deprecation.proto b/api/envoy/annotations/deprecation.proto new file mode 100644 index 000000000000..813f1050bdeb --- /dev/null +++ b/api/envoy/annotations/deprecation.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package envoy.annotations; + +import "google/protobuf/descriptor.proto"; + +// Allows tagging proto fields as fatal by default. One Envoy release after +// deprecation, deprecated fields will be disallowed by default, a state which +// is reversible with :ref:`runtime overrides `. + +// Magic number in this file derived from top 28bit of SHA256 digest of +// "envoy.annotation.disallowed_by_default" +extend google.protobuf.FieldOptions { + bool disallowed_by_default = 189503207; +} + +// Magic number in this file derived from top 28bit of SHA256 digest of +// "envoy.annotation.disallowed_by_default_enum" +extend google.protobuf.EnumValueOptions { + bool disallowed_by_default_enum = 70100853; +} diff --git a/api/envoy/api/v2/core/BUILD b/api/envoy/api/v2/core/BUILD index 283a3ea63358..ca0c7c3f1e3d 100644 --- a/api/envoy/api/v2/core/BUILD +++ b/api/envoy/api/v2/core/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/type:pkg", "//envoy/type/matcher:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", diff --git a/api/envoy/api/v2/core/config_source.proto b/api/envoy/api/v2/core/config_source.proto index 5a082d8bc793..d5680d2e464d 100644 --- a/api/envoy/api/v2/core/config_source.proto +++ b/api/envoy/api/v2/core/config_source.proto @@ -7,6 +7,7 @@ import "envoy/api/v2/core/grpc_service.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -40,7 +41,8 @@ message ApiConfigSource { enum ApiType { // Ideally this would be 'reserved 0' but one can't reserve the default // value. Instead we throw an exception if this is ever used. - UNSUPPORTED_REST_LEGACY = 0 [deprecated = true]; + UNSUPPORTED_REST_LEGACY = 0 + [deprecated = true, (envoy.annotations.disallowed_by_default_enum) = true]; // REST-JSON v2 API. The `canonical JSON encoding // `_ for diff --git a/api/envoy/api/v2/route/BUILD b/api/envoy/api/v2/route/BUILD index b570ccfdeee3..b60e7c4ceb51 100644 --- a/api/envoy/api/v2/route/BUILD +++ b/api/envoy/api/v2/route/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/type:pkg", "//envoy/type/matcher:pkg", diff --git a/api/envoy/api/v2/route/route_components.proto b/api/envoy/api/v2/route/route_components.proto index 37f5aa1d5a22..d54a8673952d 100644 --- a/api/envoy/api/v2/route/route_components.proto +++ b/api/envoy/api/v2/route/route_components.proto @@ -14,6 +14,7 @@ import "google/protobuf/duration.proto"; import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -494,7 +495,8 @@ message CorsPolicy { // // **This field is deprecated**. Set the // :ref:`filter_enabled` field instead. - google.protobuf.BoolValue enabled = 7 [deprecated = true]; + google.protobuf.BoolValue enabled = 7 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Specifies the % of requests for which the CORS filter is enabled. // @@ -558,7 +560,7 @@ message RouteAction { // **This field is deprecated**. Set the // :ref:`runtime_fraction // ` field instead. - string runtime_key = 2 [deprecated = true]; + string runtime_key = 2 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // If both :ref:`runtime_key // ` and this field are not diff --git a/api/envoy/config/bootstrap/v2/BUILD b/api/envoy/config/bootstrap/v2/BUILD index ade709544815..4bdaf6046a52 100644 --- a/api/envoy/config/bootstrap/v2/BUILD +++ b/api/envoy/config/bootstrap/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2:pkg", "//envoy/api/v2/auth:pkg", "//envoy/api/v2/core:pkg", diff --git a/api/envoy/config/bootstrap/v2/bootstrap.proto b/api/envoy/config/bootstrap/v2/bootstrap.proto index bd33e601c701..cc12345efbae 100644 --- a/api/envoy/config/bootstrap/v2/bootstrap.proto +++ b/api/envoy/config/bootstrap/v2/bootstrap.proto @@ -16,6 +16,7 @@ import "google/protobuf/duration.proto"; import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.bootstrap.v2"; @@ -119,7 +120,7 @@ message Bootstrap { // Configuration for the runtime configuration provider (deprecated). If not // specified, a “null” provider will be used which will result in all defaults // being used. - Runtime runtime = 11 [deprecated = true]; + Runtime runtime = 11 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Configuration for the runtime configuration provider. If not // specified, a “null” provider will be used which will result in all defaults diff --git a/api/envoy/config/bootstrap/v3alpha/BUILD b/api/envoy/config/bootstrap/v3alpha/BUILD index 30bb6946b9bb..77e932583bff 100644 --- a/api/envoy/config/bootstrap/v3alpha/BUILD +++ b/api/envoy/config/bootstrap/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/bootstrap/v2:pkg", "//envoy/config/cluster/v3alpha:pkg", "//envoy/config/core/v3alpha:pkg", diff --git a/api/envoy/config/bootstrap/v3alpha/bootstrap.proto b/api/envoy/config/bootstrap/v3alpha/bootstrap.proto index 372dc8d7c6ef..9e3aa5670d5e 100644 --- a/api/envoy/config/bootstrap/v3alpha/bootstrap.proto +++ b/api/envoy/config/bootstrap/v3alpha/bootstrap.proto @@ -18,6 +18,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.bootstrap.v3alpha"; diff --git a/api/envoy/config/core/v3alpha/BUILD b/api/envoy/config/core/v3alpha/BUILD index 6cd088f299f2..59a487d549ae 100644 --- a/api/envoy/config/core/v3alpha/BUILD +++ b/api/envoy/config/core/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/type/matcher/v3alpha:pkg", "//envoy/type/v3alpha:pkg", diff --git a/api/envoy/config/core/v3alpha/config_source.proto b/api/envoy/config/core/v3alpha/config_source.proto index 718ca6cc191b..8628b7d85cb4 100644 --- a/api/envoy/config/core/v3alpha/config_source.proto +++ b/api/envoy/config/core/v3alpha/config_source.proto @@ -9,6 +9,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.core.v3alpha"; @@ -42,7 +43,8 @@ message ApiConfigSource { enum ApiType { // Ideally this would be 'reserved 0' but one can't reserve the default // value. Instead we throw an exception if this is ever used. - DEPRECATED_AND_UNAVAILABLE_DO_NOT_USE = 0 [deprecated = true]; + DEPRECATED_AND_UNAVAILABLE_DO_NOT_USE = 0 + [deprecated = true, (envoy.annotations.disallowed_by_default_enum) = true]; // REST-JSON v2 API. The `canonical JSON encoding // `_ for diff --git a/api/envoy/config/filter/fault/v2/BUILD b/api/envoy/config/filter/fault/v2/BUILD index 3535fe17b682..e2a45aba90ec 100644 --- a/api/envoy/config/filter/fault/v2/BUILD +++ b/api/envoy/config/filter/fault/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/type:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", ], diff --git a/api/envoy/config/filter/fault/v2/fault.proto b/api/envoy/config/filter/fault/v2/fault.proto index a687b358d5e5..02fb5646dcf1 100644 --- a/api/envoy/config/filter/fault/v2/fault.proto +++ b/api/envoy/config/filter/fault/v2/fault.proto @@ -6,6 +6,7 @@ import "envoy/type/percent.proto"; import "google/protobuf/duration.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -35,7 +36,7 @@ message FaultDelay { reserved 2; // Unused and deprecated. Will be removed in the next release. - FaultDelayType type = 1 [deprecated = true]; + FaultDelayType type = 1 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; oneof fault_delay_secifier { option (validate.required) = true; diff --git a/api/envoy/config/filter/http/ext_authz/v2/BUILD b/api/envoy/config/filter/http/ext_authz/v2/BUILD index 311b712bf464..1eb9a92e34aa 100644 --- a/api/envoy/config/filter/http/ext_authz/v2/BUILD +++ b/api/envoy/config/filter/http/ext_authz/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/type:pkg", "//envoy/type/matcher:pkg", diff --git a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto index 6c016d9269bb..94a87f3dde3c 100644 --- a/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto +++ b/api/envoy/config/filter/http/ext_authz/v2/ext_authz.proto @@ -8,6 +8,7 @@ import "envoy/api/v2/core/http_uri.proto"; import "envoy/type/http_status.proto"; import "envoy/type/matcher/string.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -50,7 +51,7 @@ message ExtAuthz { // useful when transitioning from alpha to release versions assuming that both definitions are // semantically compatible. Deprecation note: This field is deprecated and should only be used for // version upgrade. See release notes for more details. - bool use_alpha = 4 [deprecated = true]; + bool use_alpha = 4 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Enables filter to buffer the client request body and send it within the authorization request. // A ``x-envoy-auth-partial-body: false|true`` metadata header will be added to the authorization diff --git a/api/envoy/config/filter/network/redis_proxy/v2/BUILD b/api/envoy/config/filter/network/redis_proxy/v2/BUILD index 69168ad0cf24..55aaeb318f35 100644 --- a/api/envoy/config/filter/network/redis_proxy/v2/BUILD +++ b/api/envoy/config/filter/network/redis_proxy/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", ], diff --git a/api/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto b/api/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto index f78abdb03481..c7d7bf7e4057 100644 --- a/api/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto +++ b/api/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto @@ -7,6 +7,7 @@ import "envoy/api/v2/core/base.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -162,7 +163,8 @@ message RedisProxy { // This field is deprecated. Use a :ref:`catch_all // route` // instead. - string catch_all_cluster = 3 [deprecated = true]; + string catch_all_cluster = 3 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Optional catch-all route to forward commands that doesn't match any of the routes. The // catch-all route becomes required when no routes are specified. @@ -181,7 +183,7 @@ message RedisProxy { // This field is deprecated. Use a :ref:`catch_all // route` // instead. - string cluster = 2 [deprecated = true]; + string cluster = 2 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Network settings for the connection pool to the upstream clusters. ConnPoolSettings settings = 3 [(validate.rules).message = {required: true}]; diff --git a/api/envoy/config/route/v3alpha/BUILD b/api/envoy/config/route/v3alpha/BUILD index 8b724d7e8129..ffdcc0e309fd 100644 --- a/api/envoy/config/route/v3alpha/BUILD +++ b/api/envoy/config/route/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2:pkg", "//envoy/api/v2/route:pkg", "//envoy/config/core/v3alpha:pkg", diff --git a/api/envoy/config/route/v3alpha/route_components.proto b/api/envoy/config/route/v3alpha/route_components.proto index 56d5cf777fdc..c9f1232991c4 100644 --- a/api/envoy/config/route/v3alpha/route_components.proto +++ b/api/envoy/config/route/v3alpha/route_components.proto @@ -16,6 +16,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.route.v3alpha"; diff --git a/api/envoy/extensions/filters/common/fault/v3alpha/BUILD b/api/envoy/extensions/filters/common/fault/v3alpha/BUILD index 57f3af3bcbc0..26a3a595c93b 100644 --- a/api/envoy/extensions/filters/common/fault/v3alpha/BUILD +++ b/api/envoy/extensions/filters/common/fault/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/filter/fault/v2:pkg", "//envoy/type/v3alpha:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", diff --git a/api/envoy/extensions/filters/common/fault/v3alpha/fault.proto b/api/envoy/extensions/filters/common/fault/v3alpha/fault.proto index f556de47dce0..bdedd6d43ecd 100644 --- a/api/envoy/extensions/filters/common/fault/v3alpha/fault.proto +++ b/api/envoy/extensions/filters/common/fault/v3alpha/fault.proto @@ -8,6 +8,7 @@ import "google/protobuf/duration.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.common.fault.v3alpha"; diff --git a/api/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD b/api/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD index c1165f396f62..d2cfecf09783 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD +++ b/api/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/core/v3alpha:pkg", "//envoy/config/filter/http/ext_authz/v2:pkg", "//envoy/type/matcher/v3alpha:pkg", diff --git a/api/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto b/api/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto index d4367e5de601..ce9e764baccb 100644 --- a/api/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto +++ b/api/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto @@ -10,6 +10,7 @@ import "envoy/type/v3alpha/http_status.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.ext_authz.v3alpha"; diff --git a/api/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD b/api/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD index 66299959db23..73bce7c73fac 100644 --- a/api/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD +++ b/api/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/core/v3alpha:pkg", "//envoy/config/filter/network/redis_proxy/v2:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", diff --git a/api/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto b/api/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto index fbce339cbd63..9b60aaccd594 100644 --- a/api/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto +++ b/api/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto @@ -9,6 +9,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.network.redis_proxy.v3alpha"; diff --git a/docs/root/configuration/operations/runtime.rst b/docs/root/configuration/operations/runtime.rst index 47ebcfe7400c..3d405269991f 100644 --- a/docs/root/configuration/operations/runtime.rst +++ b/docs/root/configuration/operations/runtime.rst @@ -225,6 +225,8 @@ Lines starting with ``#`` as the first character are treated as comments. Comments can be used to provide context on an existing value. Comments are also useful in an otherwise empty file to keep a placeholder for deployment in a time of need. +.. _config_runtime_deprecation: + Using runtime overrides for deprecated features ----------------------------------------------- @@ -238,13 +240,12 @@ increments the :ref:`deprecated_feature_use ` runtime stat. Users are encouraged to go to :ref:`deprecated ` to see how to migrate to the new code path and make sure it is suitable for their use case. -In the second phase the message and filename will be added to -:repo:`runtime_features.cc ` -and use of that configuration field will cause the config to be rejected by default. -This fail-by-default mode can be overridden in runtime configuration by setting -envoy.deprecated_features.filename.proto:fieldname or envoy.deprecated_features.filename.proto:enum_value +In the second phase the field will be tagged as disallowed_by_default +and use of that configuration field will cause the config to be rejected by default. +This disallowed mode can be overridden in runtime configuration by setting +envoy.deprecated_features:full_fieldname or envoy.deprecated_features:full_enum_value to true. For example, for a deprecated field -``Foo.Bar.Eep`` in ``baz.proto`` set ``envoy.deprecated_features.baz.proto:Eep`` to +``Foo.Bar.Eep`` set ``envoy.deprecated_features:Foo.bar.Eep`` to ``true``. Use of this override is **strongly discouraged**. Fatal-by-default configuration indicates that the removal of the old code paths is imminent. It is far better for both Envoy users and for Envoy contributors if any bugs or feature gaps with the new diff --git a/generated_api_shadow/envoy/admin/v2alpha/BUILD b/generated_api_shadow/envoy/admin/v2alpha/BUILD index 3ced653bc328..a7253df510f8 100644 --- a/generated_api_shadow/envoy/admin/v2alpha/BUILD +++ b/generated_api_shadow/envoy/admin/v2alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/config/bootstrap/v2:pkg", "//envoy/service/tap/v2alpha:pkg", diff --git a/generated_api_shadow/envoy/admin/v2alpha/server_info.proto b/generated_api_shadow/envoy/admin/v2alpha/server_info.proto index 099e2f03bc8f..1052cb6296ee 100644 --- a/generated_api_shadow/envoy/admin/v2alpha/server_info.proto +++ b/generated_api_shadow/envoy/admin/v2alpha/server_info.proto @@ -4,6 +4,8 @@ package envoy.admin.v2alpha; import "google/protobuf/duration.proto"; +import "envoy/annotations/deprecation.proto"; + option java_package = "io.envoyproxy.envoy.admin.v2alpha"; option java_outer_classname = "ServerInfoProto"; option java_multiple_files = true; @@ -128,9 +130,10 @@ message CommandLineOptions { Mode mode = 19; // max_stats and max_obj_name_len are now unused and have no effect. - uint64 max_stats = 20 [deprecated = true]; + uint64 max_stats = 20 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; - uint64 max_obj_name_len = 21 [deprecated = true]; + uint64 max_obj_name_len = 21 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // See :option:`--disable-hot-restart` for details. bool disable_hot_restart = 22; diff --git a/generated_api_shadow/envoy/admin/v3alpha/BUILD b/generated_api_shadow/envoy/admin/v3alpha/BUILD index 32469e852fd4..b724bb8ad88f 100644 --- a/generated_api_shadow/envoy/admin/v3alpha/BUILD +++ b/generated_api_shadow/envoy/admin/v3alpha/BUILD @@ -7,6 +7,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ "//envoy/admin/v2alpha:pkg", + "//envoy/annotations:pkg", "//envoy/config/bootstrap/v3alpha:pkg", "//envoy/config/core/v3alpha:pkg", "//envoy/config/tap/v3alpha:pkg", diff --git a/generated_api_shadow/envoy/admin/v3alpha/server_info.proto b/generated_api_shadow/envoy/admin/v3alpha/server_info.proto index 5fcaf5a63a05..b2db7402c3f2 100644 --- a/generated_api_shadow/envoy/admin/v3alpha/server_info.proto +++ b/generated_api_shadow/envoy/admin/v3alpha/server_info.proto @@ -6,6 +6,8 @@ import "google/protobuf/duration.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; + option java_package = "io.envoyproxy.envoy.admin.v3alpha"; option java_outer_classname = "ServerInfoProto"; option java_multiple_files = true; @@ -135,9 +137,11 @@ message CommandLineOptions { Mode mode = 19; // max_stats and max_obj_name_len are now unused and have no effect. - uint64 hidden_envoy_deprecated_max_stats = 20 [deprecated = true]; + uint64 hidden_envoy_deprecated_max_stats = 20 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; - uint64 hidden_envoy_deprecated_max_obj_name_len = 21 [deprecated = true]; + uint64 hidden_envoy_deprecated_max_obj_name_len = 21 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // See :option:`--disable-hot-restart` for details. bool disable_hot_restart = 22; diff --git a/generated_api_shadow/envoy/annotations/deprecation.proto b/generated_api_shadow/envoy/annotations/deprecation.proto new file mode 100644 index 000000000000..813f1050bdeb --- /dev/null +++ b/generated_api_shadow/envoy/annotations/deprecation.proto @@ -0,0 +1,21 @@ +syntax = "proto3"; + +package envoy.annotations; + +import "google/protobuf/descriptor.proto"; + +// Allows tagging proto fields as fatal by default. One Envoy release after +// deprecation, deprecated fields will be disallowed by default, a state which +// is reversible with :ref:`runtime overrides `. + +// Magic number in this file derived from top 28bit of SHA256 digest of +// "envoy.annotation.disallowed_by_default" +extend google.protobuf.FieldOptions { + bool disallowed_by_default = 189503207; +} + +// Magic number in this file derived from top 28bit of SHA256 digest of +// "envoy.annotation.disallowed_by_default_enum" +extend google.protobuf.EnumValueOptions { + bool disallowed_by_default_enum = 70100853; +} diff --git a/generated_api_shadow/envoy/api/v2/core/BUILD b/generated_api_shadow/envoy/api/v2/core/BUILD index 283a3ea63358..ca0c7c3f1e3d 100644 --- a/generated_api_shadow/envoy/api/v2/core/BUILD +++ b/generated_api_shadow/envoy/api/v2/core/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/type:pkg", "//envoy/type/matcher:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", diff --git a/generated_api_shadow/envoy/api/v2/core/config_source.proto b/generated_api_shadow/envoy/api/v2/core/config_source.proto index 5a082d8bc793..d5680d2e464d 100644 --- a/generated_api_shadow/envoy/api/v2/core/config_source.proto +++ b/generated_api_shadow/envoy/api/v2/core/config_source.proto @@ -7,6 +7,7 @@ import "envoy/api/v2/core/grpc_service.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -40,7 +41,8 @@ message ApiConfigSource { enum ApiType { // Ideally this would be 'reserved 0' but one can't reserve the default // value. Instead we throw an exception if this is ever used. - UNSUPPORTED_REST_LEGACY = 0 [deprecated = true]; + UNSUPPORTED_REST_LEGACY = 0 + [deprecated = true, (envoy.annotations.disallowed_by_default_enum) = true]; // REST-JSON v2 API. The `canonical JSON encoding // `_ for diff --git a/generated_api_shadow/envoy/api/v2/route/BUILD b/generated_api_shadow/envoy/api/v2/route/BUILD index b570ccfdeee3..b60e7c4ceb51 100644 --- a/generated_api_shadow/envoy/api/v2/route/BUILD +++ b/generated_api_shadow/envoy/api/v2/route/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/type:pkg", "//envoy/type/matcher:pkg", diff --git a/generated_api_shadow/envoy/api/v2/route/route_components.proto b/generated_api_shadow/envoy/api/v2/route/route_components.proto index 37f5aa1d5a22..d54a8673952d 100644 --- a/generated_api_shadow/envoy/api/v2/route/route_components.proto +++ b/generated_api_shadow/envoy/api/v2/route/route_components.proto @@ -14,6 +14,7 @@ import "google/protobuf/duration.proto"; import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -494,7 +495,8 @@ message CorsPolicy { // // **This field is deprecated**. Set the // :ref:`filter_enabled` field instead. - google.protobuf.BoolValue enabled = 7 [deprecated = true]; + google.protobuf.BoolValue enabled = 7 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Specifies the % of requests for which the CORS filter is enabled. // @@ -558,7 +560,7 @@ message RouteAction { // **This field is deprecated**. Set the // :ref:`runtime_fraction // ` field instead. - string runtime_key = 2 [deprecated = true]; + string runtime_key = 2 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // If both :ref:`runtime_key // ` and this field are not diff --git a/generated_api_shadow/envoy/config/bootstrap/v2/BUILD b/generated_api_shadow/envoy/config/bootstrap/v2/BUILD index ade709544815..4bdaf6046a52 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v2/BUILD +++ b/generated_api_shadow/envoy/config/bootstrap/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2:pkg", "//envoy/api/v2/auth:pkg", "//envoy/api/v2/core:pkg", diff --git a/generated_api_shadow/envoy/config/bootstrap/v2/bootstrap.proto b/generated_api_shadow/envoy/config/bootstrap/v2/bootstrap.proto index bd33e601c701..cc12345efbae 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v2/bootstrap.proto +++ b/generated_api_shadow/envoy/config/bootstrap/v2/bootstrap.proto @@ -16,6 +16,7 @@ import "google/protobuf/duration.proto"; import "google/protobuf/struct.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.bootstrap.v2"; @@ -119,7 +120,7 @@ message Bootstrap { // Configuration for the runtime configuration provider (deprecated). If not // specified, a “null” provider will be used which will result in all defaults // being used. - Runtime runtime = 11 [deprecated = true]; + Runtime runtime = 11 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Configuration for the runtime configuration provider. If not // specified, a “null” provider will be used which will result in all defaults diff --git a/generated_api_shadow/envoy/config/bootstrap/v3alpha/BUILD b/generated_api_shadow/envoy/config/bootstrap/v3alpha/BUILD index 30bb6946b9bb..77e932583bff 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v3alpha/BUILD +++ b/generated_api_shadow/envoy/config/bootstrap/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/bootstrap/v2:pkg", "//envoy/config/cluster/v3alpha:pkg", "//envoy/config/core/v3alpha:pkg", diff --git a/generated_api_shadow/envoy/config/bootstrap/v3alpha/bootstrap.proto b/generated_api_shadow/envoy/config/bootstrap/v3alpha/bootstrap.proto index f0e718721c19..932bb3b16aa7 100644 --- a/generated_api_shadow/envoy/config/bootstrap/v3alpha/bootstrap.proto +++ b/generated_api_shadow/envoy/config/bootstrap/v3alpha/bootstrap.proto @@ -18,6 +18,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.bootstrap.v3alpha"; @@ -128,7 +129,8 @@ message Bootstrap { // Configuration for the runtime configuration provider (deprecated). If not // specified, a “null” provider will be used which will result in all defaults // being used. - Runtime hidden_envoy_deprecated_runtime = 11 [deprecated = true]; + Runtime hidden_envoy_deprecated_runtime = 11 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Configuration for the runtime configuration provider. If not // specified, a “null” provider will be used which will result in all defaults diff --git a/generated_api_shadow/envoy/config/core/v3alpha/BUILD b/generated_api_shadow/envoy/config/core/v3alpha/BUILD index 6cd088f299f2..59a487d549ae 100644 --- a/generated_api_shadow/envoy/config/core/v3alpha/BUILD +++ b/generated_api_shadow/envoy/config/core/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/type/matcher/v3alpha:pkg", "//envoy/type/v3alpha:pkg", diff --git a/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto b/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto index b9c58a30ae8d..12c7e7553964 100644 --- a/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto +++ b/generated_api_shadow/envoy/config/core/v3alpha/config_source.proto @@ -9,6 +9,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.core.v3alpha"; @@ -42,7 +43,8 @@ message ApiConfigSource { enum ApiType { // Ideally this would be 'reserved 0' but one can't reserve the default // value. Instead we throw an exception if this is ever used. - hidden_envoy_deprecated_UNSUPPORTED_REST_LEGACY = 0 [deprecated = true]; + hidden_envoy_deprecated_UNSUPPORTED_REST_LEGACY = 0 + [deprecated = true, (envoy.annotations.disallowed_by_default_enum) = true]; // REST-JSON v2 API. The `canonical JSON encoding // `_ for diff --git a/generated_api_shadow/envoy/config/filter/fault/v2/BUILD b/generated_api_shadow/envoy/config/filter/fault/v2/BUILD index 3535fe17b682..e2a45aba90ec 100644 --- a/generated_api_shadow/envoy/config/filter/fault/v2/BUILD +++ b/generated_api_shadow/envoy/config/filter/fault/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/type:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", ], diff --git a/generated_api_shadow/envoy/config/filter/fault/v2/fault.proto b/generated_api_shadow/envoy/config/filter/fault/v2/fault.proto index a687b358d5e5..02fb5646dcf1 100644 --- a/generated_api_shadow/envoy/config/filter/fault/v2/fault.proto +++ b/generated_api_shadow/envoy/config/filter/fault/v2/fault.proto @@ -6,6 +6,7 @@ import "envoy/type/percent.proto"; import "google/protobuf/duration.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -35,7 +36,7 @@ message FaultDelay { reserved 2; // Unused and deprecated. Will be removed in the next release. - FaultDelayType type = 1 [deprecated = true]; + FaultDelayType type = 1 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; oneof fault_delay_secifier { option (validate.required) = true; diff --git a/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/BUILD b/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/BUILD index 311b712bf464..1eb9a92e34aa 100644 --- a/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/BUILD +++ b/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "//envoy/type:pkg", "//envoy/type/matcher:pkg", diff --git a/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/ext_authz.proto b/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/ext_authz.proto index 6c016d9269bb..94a87f3dde3c 100644 --- a/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/ext_authz.proto +++ b/generated_api_shadow/envoy/config/filter/http/ext_authz/v2/ext_authz.proto @@ -8,6 +8,7 @@ import "envoy/api/v2/core/http_uri.proto"; import "envoy/type/http_status.proto"; import "envoy/type/matcher/string.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -50,7 +51,7 @@ message ExtAuthz { // useful when transitioning from alpha to release versions assuming that both definitions are // semantically compatible. Deprecation note: This field is deprecated and should only be used for // version upgrade. See release notes for more details. - bool use_alpha = 4 [deprecated = true]; + bool use_alpha = 4 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Enables filter to buffer the client request body and send it within the authorization request. // A ``x-envoy-auth-partial-body: false|true`` metadata header will be added to the authorization diff --git a/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/BUILD b/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/BUILD index 69168ad0cf24..55aaeb318f35 100644 --- a/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/BUILD +++ b/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2/core:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", ], diff --git a/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto b/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto index f78abdb03481..c7d7bf7e4057 100644 --- a/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto +++ b/generated_api_shadow/envoy/config/filter/network/redis_proxy/v2/redis_proxy.proto @@ -7,6 +7,7 @@ import "envoy/api/v2/core/base.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/wrappers.proto"; +import "envoy/annotations/deprecation.proto"; import "udpa/annotations/migrate.proto"; import "validate/validate.proto"; @@ -162,7 +163,8 @@ message RedisProxy { // This field is deprecated. Use a :ref:`catch_all // route` // instead. - string catch_all_cluster = 3 [deprecated = true]; + string catch_all_cluster = 3 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Optional catch-all route to forward commands that doesn't match any of the routes. The // catch-all route becomes required when no routes are specified. @@ -181,7 +183,7 @@ message RedisProxy { // This field is deprecated. Use a :ref:`catch_all // route` // instead. - string cluster = 2 [deprecated = true]; + string cluster = 2 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Network settings for the connection pool to the upstream clusters. ConnPoolSettings settings = 3 [(validate.rules).message = {required: true}]; diff --git a/generated_api_shadow/envoy/config/route/v3alpha/BUILD b/generated_api_shadow/envoy/config/route/v3alpha/BUILD index 8b724d7e8129..ffdcc0e309fd 100644 --- a/generated_api_shadow/envoy/config/route/v3alpha/BUILD +++ b/generated_api_shadow/envoy/config/route/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/api/v2:pkg", "//envoy/api/v2/route:pkg", "//envoy/config/core/v3alpha:pkg", diff --git a/generated_api_shadow/envoy/config/route/v3alpha/route_components.proto b/generated_api_shadow/envoy/config/route/v3alpha/route_components.proto index b1437d738d89..f7bd526225e1 100644 --- a/generated_api_shadow/envoy/config/route/v3alpha/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3alpha/route_components.proto @@ -16,6 +16,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.config.route.v3alpha"; @@ -521,7 +522,8 @@ message CorsPolicy { // **This field is deprecated**. Set the // :ref:`filter_enabled` field // instead. - google.protobuf.BoolValue hidden_envoy_deprecated_enabled = 7 [deprecated = true]; + google.protobuf.BoolValue hidden_envoy_deprecated_enabled = 7 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Specifies the % of requests for which the CORS filter is enabled. // @@ -591,7 +593,8 @@ message RouteAction { // :ref:`runtime_fraction // ` // field instead. - string hidden_envoy_deprecated_runtime_key = 2 [deprecated = true]; + string hidden_envoy_deprecated_runtime_key = 2 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // If both :ref:`runtime_key // ` and this diff --git a/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/BUILD b/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/BUILD index 57f3af3bcbc0..26a3a595c93b 100644 --- a/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/BUILD +++ b/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/filter/fault/v2:pkg", "//envoy/type/v3alpha:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", diff --git a/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/fault.proto b/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/fault.proto index 742da13ded9c..bc8ba2b1190e 100644 --- a/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/fault.proto +++ b/generated_api_shadow/envoy/extensions/filters/common/fault/v3alpha/fault.proto @@ -8,6 +8,7 @@ import "google/protobuf/duration.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.common.fault.v3alpha"; @@ -39,7 +40,8 @@ message FaultDelay { reserved 2; // Unused and deprecated. Will be removed in the next release. - FaultDelayType hidden_envoy_deprecated_type = 1 [deprecated = true]; + FaultDelayType hidden_envoy_deprecated_type = 1 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; oneof fault_delay_secifier { option (validate.required) = true; diff --git a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD index c1165f396f62..d2cfecf09783 100644 --- a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD +++ b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/core/v3alpha:pkg", "//envoy/config/filter/http/ext_authz/v2:pkg", "//envoy/type/matcher/v3alpha:pkg", diff --git a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto index 3e0aa10d1543..3e006b78e540 100644 --- a/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto +++ b/generated_api_shadow/envoy/extensions/filters/http/ext_authz/v3alpha/ext_authz.proto @@ -10,6 +10,7 @@ import "envoy/type/v3alpha/http_status.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.http.ext_authz.v3alpha"; @@ -52,7 +53,8 @@ message ExtAuthz { // useful when transitioning from alpha to release versions assuming that both definitions are // semantically compatible. Deprecation note: This field is deprecated and should only be used for // version upgrade. See release notes for more details. - bool hidden_envoy_deprecated_use_alpha = 4 [deprecated = true]; + bool hidden_envoy_deprecated_use_alpha = 4 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Enables filter to buffer the client request body and send it within the authorization request. // A ``x-envoy-auth-partial-body: false|true`` metadata header will be added to the authorization diff --git a/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD b/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD index 66299959db23..73bce7c73fac 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD +++ b/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/BUILD @@ -6,6 +6,7 @@ licenses(["notice"]) # Apache 2 api_proto_package( deps = [ + "//envoy/annotations:pkg", "//envoy/config/core/v3alpha:pkg", "//envoy/config/filter/network/redis_proxy/v2:pkg", "@com_github_cncf_udpa//udpa/annotations:pkg", diff --git a/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto b/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto index f925c67bf6f6..87861ff5b39b 100644 --- a/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto +++ b/generated_api_shadow/envoy/extensions/filters/network/redis_proxy/v3alpha/redis_proxy.proto @@ -9,6 +9,7 @@ import "google/protobuf/wrappers.proto"; import "udpa/annotations/versioning.proto"; +import "envoy/annotations/deprecation.proto"; import "validate/validate.proto"; option java_package = "io.envoyproxy.envoy.extensions.filters.network.redis_proxy.v3alpha"; @@ -177,7 +178,8 @@ message RedisProxy { // This field is deprecated. Use a :ref:`catch_all // route` // instead. - string hidden_envoy_deprecated_catch_all_cluster = 3 [deprecated = true]; + string hidden_envoy_deprecated_catch_all_cluster = 3 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Optional catch-all route to forward commands that doesn't match any of the routes. The // catch-all route becomes required when no routes are specified. @@ -196,7 +198,8 @@ message RedisProxy { // This field is deprecated. Use a :ref:`catch_all // route` // instead. - string hidden_envoy_deprecated_cluster = 2 [deprecated = true]; + string hidden_envoy_deprecated_cluster = 2 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; // Network settings for the connection pool to the upstream clusters. ConnPoolSettings settings = 3 [(validate.rules).message = {required: true}]; diff --git a/include/envoy/runtime/runtime.h b/include/envoy/runtime/runtime.h index 40b9c308ee3f..26ce3df3032e 100644 --- a/include/envoy/runtime/runtime.h +++ b/include/envoy/runtime/runtime.h @@ -103,13 +103,18 @@ class Snapshot { using OverrideLayerConstPtr = std::unique_ptr; - // Returns true if a deprecated feature is allowed. - // - // Fundamentally, deprecated features are boolean values. - // They are allowed by default or with explicit configuration to "true" via runtime configuration. - // They can be disallowed either by inclusion in the hard-coded disallowed_features[] list, or by - // configuration of "false" in runtime config. - virtual bool deprecatedFeatureEnabled(const std::string& key) const PURE; + /** + * Returns true if a deprecated feature is allowed. + * + * Fundamentally, deprecated features are boolean values. + * They are allowed by default or with explicit configuration to "true" via runtime configuration. + * They can be disallowed either by inclusion in the hard-coded disallowed_features[] list, or by + * configuration of "false" in runtime config. + * @param key supplies the key to lookup. + * @param default_value supplies the default value that will be used if either the key + * does not exist or it is not a boolean. + */ + virtual bool deprecatedFeatureEnabled(const std::string& key, bool default_enabled) const PURE; // Returns true if a runtime feature is enabled. // diff --git a/source/common/protobuf/BUILD b/source/common/protobuf/BUILD index 981fc375cb9f..b552b4795f58 100644 --- a/source/common/protobuf/BUILD +++ b/source/common/protobuf/BUILD @@ -68,6 +68,7 @@ envoy_cc_library( "//source/common/common:utility_lib", "//source/common/config:api_type_oracle_lib", "//source/common/config:version_converter_lib", + "@envoy_api//envoy/annotations:pkg_cc_proto", "@envoy_api//envoy/type/v3alpha:pkg_cc_proto", ], ) diff --git a/source/common/protobuf/utility.cc b/source/common/protobuf/utility.cc index 97bc51663a05..24ff6d2158ee 100644 --- a/source/common/protobuf/utility.cc +++ b/source/common/protobuf/utility.cc @@ -3,6 +3,7 @@ #include #include +#include "envoy/annotations/deprecation.pb.h" #include "envoy/protobuf/message_validator.h" #include "envoy/type/v3alpha/percent.pb.h" @@ -339,12 +340,13 @@ void checkForDeprecatedNonRepeatedEnumValue(const Protobuf::Message& message, #ifdef ENVOY_DISABLE_DEPRECATED_FEATURES bool warn_only = false; #else - bool warn_only = true; + bool warn_only = !enum_value_descriptor->options().GetExtension( + envoy::annotations::disallowed_by_default_enum); #endif - if (runtime && !runtime->snapshot().deprecatedFeatureEnabled(absl::StrCat( - "envoy.deprecated_features.", filename, ":", enum_value_descriptor->name()))) { - warn_only = false; + if (runtime) { + warn_only = runtime->snapshot().deprecatedFeatureEnabled( + absl::StrCat("envoy.deprecated_features:", enum_value_descriptor->full_name()), warn_only); } if (warn_only) { @@ -400,15 +402,14 @@ void checkForUnexpectedFields(const Protobuf::Message& message, #ifdef ENVOY_DISABLE_DEPRECATED_FEATURES bool warn_only = false; #else - bool warn_only = true; + bool warn_only = !field->options().GetExtension(envoy::annotations::disallowed_by_default); #endif // Allow runtime to be null both to not crash if this is called before server initialization, // and so proto validation works in context where runtime singleton is not set up (e.g. // standalone config validation utilities) - if (runtime && field->options().deprecated() && - !runtime->snapshot().deprecatedFeatureEnabled( - absl::StrCat("envoy.deprecated_features.", filename, ":", field->name()))) { - warn_only = false; + if (runtime && field->options().deprecated()) { + warn_only = runtime->snapshot().deprecatedFeatureEnabled( + absl::StrCat("envoy.deprecated_features:", field->full_name()), warn_only); } // If this field is deprecated, warn or throw an error. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 2b105c842a8b..0f2432524e26 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -49,40 +49,7 @@ constexpr const char* disabled_runtime_features[] = { "envoy.reloadable_features.http2_protocol_options.stream_error_on_invalid_http_messaging", }; -// This is a list of configuration fields which are disallowed by default in Envoy -// -// By default, use of proto fields marked as deprecated in their api/.../*.proto file will result -// in a logged warning, so that Envoy users have a warning that they are using deprecated fields. -// -// During the Envoy release cycle, the maintainer team runs a script which will upgrade currently -// deprecated features to be disallowed (adding them to the list below) at which point use of said -// feature will cause a hard-failure (ProtoValidationException) instead of a logged warning. -// -// The release cycle after a feature has been marked disallowed, it is officially removable, and -// the maintainer team will run a script creating a tracking issue for proto and code clean up. -constexpr const char* disallowed_features[] = { - // Acts as both a test entry for deprecated.proto and a marker for the Envoy - // deprecation scripts. - "envoy.deprecated_features.deprecated.proto:is_deprecated_fatal", - // 1.10.0 - "envoy.deprecated_features.config_source.proto:UNSUPPORTED_REST_LEGACY", - "envoy.deprecated_features.ext_authz.proto:use_alpha", - "envoy.deprecated_features.fault.proto:type", - "envoy.deprecated_features.route.proto:enabled", - "envoy.deprecated_features.route.proto:runtime_key", - // 1.11.0 - "envoy.deprecated_features.bootstrap.proto:runtime", - "envoy.deprecated_features.redis_proxy.proto:catch_all_cluster", - "envoy.deprecated_features.redis_proxy.proto:cluster", - "envoy.deprecated_features.server_info.proto:max_obj_name_len", - "envoy.deprecated_features.server_info.proto:max_stats", - "envoy.deprecated_features.v1_filter_json_config", -}; - RuntimeFeatures::RuntimeFeatures() { - for (auto& feature : disallowed_features) { - disallowed_features_.insert(feature); - } for (auto& feature : runtime_features) { enabled_features_.insert(feature); } diff --git a/source/common/runtime/runtime_features.h b/source/common/runtime/runtime_features.h index 56ffb73f4d91..e8c7b2bc4851 100644 --- a/source/common/runtime/runtime_features.h +++ b/source/common/runtime/runtime_features.h @@ -13,13 +13,6 @@ class RuntimeFeatures { public: RuntimeFeatures(); - // This tracks proto configured features, to determine if a given deprecated - // feature is still allowed, or has been made fatal-by-default per the Envoy - // deprecation process. - bool disallowedByDefault(absl::string_view feature) const { - return disallowed_features_.find(feature) != disallowed_features_.end(); - } - // This tracks config-guarded code paths, to determine if a given // runtime-guarded-code-path has the new code run by default or the old code. bool enabledByDefault(absl::string_view feature) const { @@ -32,7 +25,6 @@ class RuntimeFeatures { private: friend class RuntimeFeaturesPeer; - absl::flat_hash_set disallowed_features_; absl::flat_hash_set enabled_features_; absl::flat_hash_set disabled_features_; }; diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index da79775f19b8..f10d053b8957 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -199,12 +199,10 @@ std::string RandomGeneratorImpl::uuid() { return std::string(uuid, UUID_LENGTH); } -bool SnapshotImpl::deprecatedFeatureEnabled(const std::string& key) const { - const bool default_allowed = !RuntimeFeaturesDefaults::get().disallowedByDefault(key); - - // If the value is not explicitly set as a runtime boolean, the default value is based on - // disallowedByDefault. - if (!getBoolean(key, default_allowed)) { +bool SnapshotImpl::deprecatedFeatureEnabled(const std::string& key, bool default_value) const { + // If the value is not explicitly set as a runtime boolean, trust the proto annotations passed as + // default_value. + if (!getBoolean(key, default_value)) { // If either disallowed by default or configured off, the feature is not enabled. return false; } diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h index 8737793f120c..3631b6e10738 100644 --- a/source/common/runtime/runtime_impl.h +++ b/source/common/runtime/runtime_impl.h @@ -81,7 +81,7 @@ class SnapshotImpl : public Snapshot, std::vector&& layers); // Runtime::Snapshot - bool deprecatedFeatureEnabled(const std::string& key) const override; + bool deprecatedFeatureEnabled(const std::string& key, bool default_value) const override; bool runtimeFeatureEnabled(absl::string_view key) const override; bool featureEnabled(const std::string& key, uint64_t default_value, uint64_t random_value, uint64_t num_buckets) const override; diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index 7546efd5c0b4..7e54f3d89d29 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -791,7 +791,8 @@ TEST_P(DeprecatedFieldsTest, // Now create a new snapshot with this feature allowed. Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.deprecated_features.deprecated.proto:is_deprecated_fatal", "TrUe "}}); + {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated_fatal", + "TrUe "}}); // Now the same deprecation check should only trigger a warning. EXPECT_LOG_CONTAINS( @@ -811,7 +812,7 @@ TEST_P(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(DisallowViaRuntime)) { // Now create a new snapshot with this feature disallowed. Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.deprecated_features.deprecated.proto:is_deprecated", " false"}}); + {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.is_deprecated", " false"}}); EXPECT_THROW_WITH_REGEX( checkForDeprecation(base), ProtoValidationException, @@ -919,13 +920,27 @@ TEST_P(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(RuntimeOverrideEnumDefault) base.mutable_enum_container(); Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.deprecated_features.deprecated.proto:DEPRECATED_DEFAULT", "false"}}); + {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.DEPRECATED_DEFAULT", "false"}}); // Make sure this is set up right. EXPECT_THROW_WITH_REGEX(checkForDeprecation(base), ProtoValidationException, "Using the default now-deprecated value DEPRECATED_DEFAULT"); } +// Make sure the runtime overrides for allowing fatal enums work. +TEST_P(DeprecatedFieldsTest, DEPRECATED_FEATURE_TEST(FatalEnum)) { + envoy::test::deprecation_test::Base base; + base.mutable_enum_container()->set_deprecated_enum( + envoy::test::deprecation_test::Base::DEPRECATED_FATAL); + EXPECT_THROW_WITH_REGEX(checkForDeprecation(base), ProtoValidationException, + "Using deprecated value DEPRECATED_FATAL"); + + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.deprecated_features:envoy.test.deprecation_test.Base.DEPRECATED_FATAL", "true"}}); + + checkForDeprecation(base); +} + class TimestampUtilTest : public testing::Test, public ::testing::WithParamInterface {}; TEST_P(TimestampUtilTest, SystemClockToTimestampTest) { diff --git a/test/common/router/BUILD b/test/common/router/BUILD index 35797d4442a1..bfcf87ea8977 100644 --- a/test/common/router/BUILD +++ b/test/common/router/BUILD @@ -32,6 +32,7 @@ envoy_cc_test_library( "//test/mocks/server:server_mocks", "//test/test_common:environment_lib", "//test/test_common:registry_lib", + "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/route/v3alpha:pkg_cc_proto", "@envoy_api//envoy/type/v3alpha:pkg_cc_proto", diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index 27597eae9963..cb1e85db809c 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -25,6 +25,7 @@ #include "test/test_common/environment.h" #include "test/test_common/printers.h" #include "test/test_common/registry.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" @@ -2786,6 +2787,11 @@ class RouteConfigurationV2 : public testing::Test, public ConfigImplTestBase {}; // When removing runtime_key: this test can be removed. TEST_F(RouteConfigurationV2, DEPRECATED_FEATURE_TEST(RequestMirrorPolicy)) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.deprecated_features:envoy.config.route.v3alpha.RouteAction.RequestMirrorPolicy." + "hidden_envoy_deprecated_runtime_key", + "true"}}); const std::string yaml = R"EOF( name: foo virtual_hosts: diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc index 02482e6c7e61..5425c058ba34 100644 --- a/test/common/runtime/runtime_impl_test.cc +++ b/test/common/runtime/runtime_impl_test.cc @@ -233,13 +233,8 @@ TEST_F(DiskLoaderImplTest, All) { EXPECT_EQ(false, snapshot->runtimeFeatureEnabled("envoy.reloadable_features.test_feature_false")); // Deprecation -#ifdef ENVOY_DISABLE_DEPRECATED_FEATURES - EXPECT_EQ(false, snapshot->deprecatedFeatureEnabled("random_string_should_be_enabled")); -#else - EXPECT_EQ(true, snapshot->deprecatedFeatureEnabled("random_string_should_be_enabled")); -#endif EXPECT_EQ(false, snapshot->deprecatedFeatureEnabled( - "envoy.deprecated_features.deprecated.proto:is_deprecated_fatal")); + "envoy.deprecated_features.deprecated.proto:is_deprecated_fatal", false)); // Feature defaults via helper function. EXPECT_EQ(false, runtimeFeatureEnabled("envoy.reloadable_features.test_feature_false")); diff --git a/test/common/runtime/utility.h b/test/common/runtime/utility.h index f21cc42b457f..e442e8940e00 100644 --- a/test/common/runtime/utility.h +++ b/test/common/runtime/utility.h @@ -17,11 +17,6 @@ class RuntimeFeaturesPeer { const_cast(&Runtime::RuntimeFeaturesDefaults::get()) ->enabled_features_.erase(feature); } - static void setAllFeaturesAllowed() { - for (const std::string& feature : RuntimeFeaturesDefaults::get().disallowed_features_) { - Runtime::LoaderSingleton::getExisting()->mergeValues({{feature, "true"}}); - } - } }; } // namespace Runtime diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 6563c8235046..52d70ac6e7dc 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -74,14 +74,9 @@ class ConfigTest { // in production runtime is not setup until after the bootstrap config is loaded. This seems // better for configuration tests. ScopedRuntimeInjector scoped_runtime(server_.runtime()); - ON_CALL(server_.runtime_loader_.snapshot_, deprecatedFeatureEnabled(_)) - .WillByDefault(Invoke([](const std::string& key) { - if (Runtime::RuntimeFeaturesDefaults::get().disallowedByDefault(key)) { - return false; - } else { - return true; - } - })); + ON_CALL(server_.runtime_loader_.snapshot_, deprecatedFeatureEnabled(_, _)) + .WillByDefault( + Invoke([](const std::string&, bool default_value) { return default_value; })); envoy::config::bootstrap::v3alpha::Bootstrap bootstrap; Server::InstanceUtil::loadBootstrapConfig( diff --git a/test/extensions/filters/network/redis_proxy/BUILD b/test/extensions/filters/network/redis_proxy/BUILD index 97bfc8a4cba7..77c846dea3bc 100644 --- a/test/extensions/filters/network/redis_proxy/BUILD +++ b/test/extensions/filters/network/redis_proxy/BUILD @@ -97,6 +97,7 @@ envoy_extension_cc_test( "//source/common/protobuf:utility_lib", "//source/extensions/filters/network/redis_proxy:config", "//test/mocks/server:server_mocks", + "//test/test_common:test_runtime_lib", "@envoy_api//envoy/extensions/filters/network/redis_proxy/v3alpha:pkg_cc_proto", ], ) diff --git a/test/extensions/filters/network/redis_proxy/config_test.cc b/test/extensions/filters/network/redis_proxy/config_test.cc index e3c6f9eaded5..2537ca368b3f 100644 --- a/test/extensions/filters/network/redis_proxy/config_test.cc +++ b/test/extensions/filters/network/redis_proxy/config_test.cc @@ -6,6 +6,7 @@ #include "extensions/filters/network/redis_proxy/config.h" #include "test/mocks/server/mocks.h" +#include "test/test_common/test_runtime.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -69,6 +70,14 @@ settings: {} TEST(RedisProxyFilterConfigFactoryTest, DEPRECATED_FEATURE_TEST(RedisProxyCorrectProtoLegacyCluster)) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.deprecated_features:envoy.config.filter.network.redis_proxy.v2.RedisProxy.cluster", + "true"}, + {"envoy.deprecated_features:envoy.extensions.filters.network.redis_proxy.v3alpha.RedisProxy." + "hidden_envoy_deprecated_cluster", + "true"}}); + const std::string yaml = R"EOF( cluster: fake_cluster stat_prefix: foo @@ -89,6 +98,14 @@ stat_prefix: foo TEST(RedisProxyFilterConfigFactoryTest, DEPRECATED_FEATURE_TEST(RedisProxyCorrectProtoLegacyCatchAllCluster)) { + TestScopedRuntime scoped_runtime; + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.deprecated_features:envoy.config.filter.network.redis_proxy.v2.RedisProxy." + "PrefixRoutes.catch_all_cluster", + "true"}, + {"envoy.deprecated_features:envoy.extensions.filters.network.redis_proxy.v3alpha.RedisProxy." + "PrefixRoutes.hidden_envoy_deprecated_catch_all_cluster", + "true"}}); const std::string yaml = R"EOF( prefix_routes: catch_all_cluster: fake_cluster diff --git a/test/integration/server.cc b/test/integration/server.cc index 50554b867ab6..f61df5eedc1c 100644 --- a/test/integration/server.cc +++ b/test/integration/server.cc @@ -182,13 +182,11 @@ void IntegrationTestServer::threadRoutine( } void IntegrationTestServer::onRuntimeCreated() { - // Override runtime values to by default allow all disallowed features. - // - // Per #6288 we explicitly want to allow end to end testing of disallowed features until the code - // is removed from Envoy. - // - // This will revert as the runtime is torn down with the test Envoy server. - Runtime::RuntimeFeaturesPeer::setAllFeaturesAllowed(); + // TODO(alyssawilk) improve this. + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"envoy.deprecated_features:envoy.config.route.v3alpha.CorsPolicy." + "hidden_envoy_deprecated_enabled", + "true"}}); } void IntegrationTestServerImpl::createAndRunEnvoyServer( diff --git a/test/mocks/runtime/mocks.h b/test/mocks/runtime/mocks.h index 3ed731f903db..82e486f6c470 100644 --- a/test/mocks/runtime/mocks.h +++ b/test/mocks/runtime/mocks.h @@ -41,7 +41,7 @@ class MockSnapshot : public Snapshot { } } - MOCK_CONST_METHOD1(deprecatedFeatureEnabled, bool(const std::string& key)); + MOCK_CONST_METHOD2(deprecatedFeatureEnabled, bool(const std::string& key, bool default_value)); MOCK_CONST_METHOD1(runtimeFeatureEnabled, bool(absl::string_view key)); MOCK_CONST_METHOD2(featureEnabled, bool(const std::string& key, uint64_t default_value)); MOCK_CONST_METHOD3(featureEnabled, diff --git a/test/proto/BUILD b/test/proto/BUILD index d2aa8efa6249..7af0abc86c22 100644 --- a/test/proto/BUILD +++ b/test/proto/BUILD @@ -14,6 +14,7 @@ exports_files(["bookstore.proto"]) envoy_proto_library( name = "deprecated_proto", srcs = [":deprecated.proto"], + deps = ["@envoy_api//envoy/annotations:pkg"], ) envoy_proto_library( diff --git a/test/proto/deprecated.proto b/test/proto/deprecated.proto index ddc3849890a0..8e5638b73deb 100644 --- a/test/proto/deprecated.proto +++ b/test/proto/deprecated.proto @@ -2,14 +2,18 @@ syntax = "proto3"; package envoy.test.deprecation_test; +import "envoy/annotations/deprecation.proto"; + message Base { string not_deprecated = 1; string is_deprecated = 2 [deprecated = true]; - string is_deprecated_fatal = 3 [deprecated = true]; + string is_deprecated_fatal = 3 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; message InnerMessage { string inner_not_deprecated = 1; string inner_deprecated = 2 [deprecated = true]; - string inner_deprecated_fatal = 3 [deprecated = true]; + string inner_deprecated_fatal = 3 + [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; } InnerMessage deprecated_message = 4 [deprecated = true]; InnerMessage not_deprecated_message = 5; @@ -22,6 +26,7 @@ message Base { DEPRECATED_DEFAULT = 0 [deprecated = true]; NOT_DEPRECATED = 1; DEPRECATED_NOT_DEFAULT = 2 [deprecated = true]; + DEPRECATED_FATAL = 3 [deprecated = true, (envoy.annotations.disallowed_by_default_enum) = true]; } message InnerMessageWithDeprecationEnum { DeprecationEnum deprecated_enum = 1; diff --git a/test/server/server_fuzz_test.cc b/test/server/server_fuzz_test.cc index c00876caad6b..cb65c8872b35 100644 --- a/test/server/server_fuzz_test.cc +++ b/test/server/server_fuzz_test.cc @@ -65,13 +65,9 @@ makeHermeticPathsAndPorts(Fuzz::PerTestEnvironment& test_env, return output; } -class AllFeaturesHooks : public DefaultListenerHooks { - void onRuntimeCreated() override { Runtime::RuntimeFeaturesPeer::setAllFeaturesAllowed(); } -}; - DEFINE_PROTO_FUZZER(const envoy::config::bootstrap::v3alpha::Bootstrap& input) { testing::NiceMock options; - AllFeaturesHooks hooks; + DefaultListenerHooks hooks; testing::NiceMock restart; Stats::TestIsolatedStoreImpl stats_store; Thread::MutexBasicLockable fakelock; diff --git a/tools/proto_sync.py b/tools/proto_sync.py index b623eb23ef57..58f3c6ca24d9 100755 --- a/tools/proto_sync.py +++ b/tools/proto_sync.py @@ -280,7 +280,10 @@ def Sync(api_root, mode, labels, shadow): GenerateCurrentApiDir(api_root_path, current_api_dir) # These support files are handled manually. - for f in ['envoy/annotations/resource.proto', 'envoy/annotations/BUILD']: + for f in [ + 'envoy/annotations/resource.proto', 'envoy/annotations/deprecation.proto', + 'envoy/annotations/BUILD' + ]: copy_dst_dir = pathlib.Path(dst_dir, os.path.dirname(f)) copy_dst_dir.mkdir(exist_ok=True) shutil.copy(str(pathlib.Path(api_root, f)), str(copy_dst_dir)) diff --git a/tools/protoxform/protoxform.py b/tools/protoxform/protoxform.py index 8ce04fcc4376..289110e88cdc 100755 --- a/tools/protoxform/protoxform.py +++ b/tools/protoxform/protoxform.py @@ -33,6 +33,7 @@ # this also serves as whitelist of extended options. from google.api import annotations_pb2 as _ from validate import validate_pb2 as _ +from envoy.annotations import deprecation_pb2 as _ from envoy.annotations import resource_pb2 from udpa.annotations import migrate_pb2 @@ -207,7 +208,10 @@ def CamelCase(s): if idx in file_proto.public_dependency: public_imports.append(d) continue - elif d in ['envoy/annotations/resource.proto', 'udpa/annotations/migrate.proto']: + elif d in [ + 'envoy/annotations/resource.proto', 'envoy/annotations/deprecation.proto', + 'udpa/annotations/migrate.proto' + ]: infra_imports.append(d) elif d.startswith('envoy/'): # We ignore existing envoy/ imports, since these are computed explicitly