xds: Relax the CHECK on factories for typed per-filter config #15965
Conversation
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
/wait I missed this comment, looks like people want to make filter as an optional #15770 (comment) |
| throw EnvoyException( | ||
| fmt::format("The filter {} doesn't support virtual host-specific configurations", name)); | ||
| } else { | ||
| ENVOY_LOG(warn, |
There was a problem hiding this comment.
We probably want to track these in a stats to allow automated monitoring.
|
@dmitri-d for first pass review. |
| @@ -1029,7 +1044,8 @@ class ConfigImpl : public Config { | |||
| public: | |||
| ConfigImpl(const envoy::config::route::v3::RouteConfiguration& config, | |||
| Server::Configuration::ServerFactoryContext& factory_context, | |||
| ProtobufMessage::ValidationVisitor& validator, bool validate_clusters_default); | |||
| ProtobufMessage::ValidationVisitor& validator, bool validate_clusters_default, | |||
| RdsStats* rds_stats = nullptr); | |||
There was a problem hiding this comment.
Why is rds_stats a nullptr by default? Can it ever be a nullptr?
There was a problem hiding this comment.
For the bootstrap config, there will be nullptr passed in, since there is no rds_stats for bootstrap config.
|
I still think that skipping over unknown configurations has a potential to make filter chains brittle, and I feel that debugging resulting filter chain configuration problems could be difficult too. I think that only supporting disabling of filters marked "optional" (see comment here: #15770 (comment)) would help. |
Yea, I got it, let me check with Mark if he need any help. :) |
Commit Message: xds: Relax the CHECK on factories for typed per-filter config
Additional Description:
Currently, if typed per-filter config includes an unknown filter, the exception will be raised. This PR aims to relax the check. A new runtime flag
envoy.reloadable_features.ignore_check_unknown_typed_per_filter_config_factoryis added, the default value is true. When it is true and unknown filter received from the rds API, instead of rejecting the update, a warning log will be emitted and increases a new counterunknown_per_filter_typed_config_factory's value.Risk Level: low
Testing: unit tests and integration tests are added
Docs Changes:
Release Notes: add a note as a new feature
Runtime guard: "envoy.reloadable_features.ignore_check_unknown_typed_per_filter_config_factory"
Fixes #15770