http: migrate set_filter_state/rbac/... to exception free#46039
Open
wbpcode wants to merge 2 commits into
Open
http: migrate set_filter_state/rbac/... to exception free#46039wbpcode wants to merge 2 commits into
wbpcode wants to merge 2 commits into
Conversation
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates several HTTP filter factories and related helper code to Envoy’s exception-free factory pattern by switching to Common::ExceptionFreeFactoryBase and returning absl::StatusOr<FilterFactoryCb> / absl::Status on creation-time failures, and updates tests to validate status-based failures.
Changes:
- Convert multiple HTTP filter factories (set_metadata, set_filter_state, rbac, rate_limit_quota, proto_message_extraction) to exception-free creation (
absl::StatusOr). - Refactor RLQS global client/TLS store construction and proto message extraction config building to propagate failures via
absl::Statusinstead of throwing. - Update unit tests and Bazel deps to use
StatusHelpers::HasStatusfor error assertions.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/extensions/filters/http/rate_limit_quota/config_test.cc | Updates config tests for status-returning factory methods (also introduces a missing Bazel dep and still expects exceptions in one test). |
| test/extensions/filters/http/rate_limit_quota/client_test.cc | Updates client test to use creation-status out-parameter for exception-free construction. |
| test/extensions/filters/http/proto_message_extraction/filter_test.cc | Updates filter test to pass/verify creation status when building FilterConfig. |
| test/extensions/filters/http/proto_message_extraction/filter_config_test.cc | Converts exception-based assertions to status-based assertions and adds a helper for config creation (needs a small correctness tweak). |
| test/extensions/filters/http/proto_message_extraction/BUILD | Adds status_utility_lib dependency for new status assertions. |
| source/extensions/filters/http/set_metadata/config.h | Switches factory to ExceptionFreeFactoryBase and returns StatusOr. |
| source/extensions/filters/http/set_metadata/config.cc | Implements StatusOr factory creation for set_metadata. |
| source/extensions/filters/http/set_filter_state/config.h | Switches factory to ExceptionFreeFactoryBase and returns StatusOr. |
| source/extensions/filters/http/set_filter_state/config.cc | Implements StatusOr factory creation for set_filter_state. |
| source/extensions/filters/http/rbac/config.h | Switches RBAC HTTP factory to ExceptionFreeFactoryBase and returns StatusOr. |
| source/extensions/filters/http/rbac/config.cc | Updates signatures/returns for exception-free RBAC factory creation (still vulnerable to thrown exceptions from deeper construction). |
| source/extensions/filters/http/rate_limit_quota/global_client_impl.h | Converts global client creation helper to return StatusOr<unique_ptr<...>>. |
| source/extensions/filters/http/rate_limit_quota/global_client_impl.cc | Converts global client ctor failure paths from throwing to setting a status out-parameter. |
| source/extensions/filters/http/rate_limit_quota/filter_persistence.h | Updates TLS store getter to return StatusOr to propagate failures. |
| source/extensions/filters/http/rate_limit_quota/filter_persistence.cc | Propagates TLS/global client init failures via StatusOr. |
| source/extensions/filters/http/rate_limit_quota/config.h | Switches RLQ factory to ExceptionFreeFactoryBase and returns StatusOr. |
| source/extensions/filters/http/rate_limit_quota/config.cc | Propagates validation/TLS-store errors via status (but matcher construction can still throw). |
| source/extensions/filters/http/proto_message_extraction/filter_config.h | Updates FilterConfig to report creation failures via status out-parameter; converts init helpers to return absl::Status. |
| source/extensions/filters/http/proto_message_extraction/filter_config.cc | Implements status-returning initialization and converts multiple throws to status errors. |
| source/extensions/filters/http/proto_message_extraction/config.h | Switches factory to ExceptionFreeFactoryBase and returns StatusOr. |
| source/extensions/filters/http/proto_message_extraction/config.cc | Propagates filter config creation failures via status-returning factory methods. |
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
This file contains hidden or 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
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: http: migrate set_filter_state/rbac/... to exception free
Additional Description:
Migrate exceptions of these filters to absl::Status and absl::StatusOr.
NOTE: This is AI assisted. But I reviewed all the changes and tests, and also did some manually update.
Risk Level: low.
Testing: unit/integration.
Docs Changes: n/a.
Release Notes: n/a.
Platform Specific Features: n/a.