xds: enable the is_optional field for HttpFilter
#16119
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
…d config 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>
soulxu
commented
Apr 22, 2021
| if (Runtime::runtimeFeatureEnabled( | ||
| "envoy.reloadable_features.check_unsupported_typed_per_filter_config")) { | ||
| throw EnvoyException( | ||
| fmt::format("The filter {} doesn't support virtual host-specific configurations", name)); |
There was a problem hiding this comment.
Should we ignore the unsupported typed per filter config for the optional http filter also? I assume there will be the same upgrade issue with the new filter. like, the filter added typed per filter config in the new version, the old version can't support it. Although there is a runtime flag here, the is_optional flag feels better.
There was a problem hiding this comment.
Yes, I think that if is_optional is set, we should ignore unknown per-route filter configs.
lizan
reviewed
Apr 29, 2021
|
sorry for the reply late, I will update the PR after the holidays. |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
… optional 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>
|
Can you resolve conflicts? Other than that LGTM |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Just resolved, thanks! |
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
lizan
approved these changes
May 18, 2021
|
@lizan thanks! |
leyao-daily
pushed a commit
to leyao-daily/envoy
that referenced
this pull request
Sep 30, 2021
Enable `is_optional` field for `HttpFilter`. The default value is `false`, it will keep the same behavior with the current implementation. The envoy will reject the unsupported HTTP filter, also will reject the unsupported HTTP filter in typed per filter config. When the value is `true`, the unsupported HTTP filter will be ignored by the envoy, also will be ignored by typed per filter config, with a warning log. Risk Level: low Testing: unit tests and integration tests are added Docs Changes: API doc is added Release Notes: added as new feature Fixes envoyproxy#15770 Fixes envoyproxy#15025 Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message: xds: enable the
is_optionalfield for HttpFilterAdditional Description:
Enable
is_optionalfield forHttpFilter. The default value isfalse, it will keep the same behavior with the current implementation. The envoy will reject the unsupported HTTP filter, also will reject the unsupported HTTP filter in typed per filter config. When the value istrue, the unsupported HTTP filter will be ignored by the envoy, also will be ignored by typed per filter config, with a warning log.Risk Level: low
Testing: unit tests and integration tests are added
Docs Changes: API doc is added
Release Notes: added as new feature
Fixes #15770
Fixes #15025