http: make ext authz/filter chain/fault/... exception free#46035
Merged
Conversation
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
wbpcode
requested review from
KBaichoo,
agrawroh,
ggreenway,
mattklein123,
ravenblackx,
tyxia,
yanavlasov and
yanjunxiang-google
as code owners
July 8, 2026 12:33
wbpcode
requested review from
Copilot
and removed request for
KBaichoo,
agrawroh,
ggreenway,
mattklein123,
ravenblackx,
tyxia,
yanavlasov and
yanjunxiang-google
July 8, 2026 12:33
tyxia
approved these changes
Jul 8, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates several HTTP filters’ config/factory codepaths to be exception-free by switching from throw-based validation to absl::Status / absl::StatusOr and updating affected tests to assert on status results instead of exceptions.
Changes:
- Convert
ext_authz,filter_chain,file_system_buffer,gcp_authn, andfaultfilter factories to returnabsl::StatusOr<Http::FilterFactoryCb>and propagate validation failures via status. - Refactor key config objects (e.g., per-route configs) to report construction failures through an
absl::Status&out-param rather than throwing. - Update unit/integration/fuzz tests and BUILD deps to use
test/test_common/status_utility.hmatchers (HasStatus) where appropriate.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/extensions/filters/http/filter_chain/config_test.cc | Updates tests to validate invalid configs via status rather than exceptions. |
| test/extensions/filters/http/filter_chain/BUILD | Adds status_utility_lib dependency for new status assertions. |
| test/extensions/filters/http/file_system_buffer/filter_test.cc | Adapts test helper to new status-returning config construction. |
| test/extensions/filters/http/file_system_buffer/config_test.cc | Converts invalid-path test to assert returned error status. |
| test/extensions/filters/http/file_system_buffer/BUILD | Adds status_utility_lib dependency for status assertions. |
| test/extensions/filters/http/ext_authz/http_filter_test_base.h | Updates test base to construct configs with absl::Status& validation. |
| test/extensions/filters/http/ext_authz/ext_authz_test.cc | Refactors per-route config construction in tests to handle status-based validation. |
| test/extensions/filters/http/ext_authz/ext_authz_integration_test.cc | Updates integration test to validate per-route config construction via status. |
| test/extensions/filters/http/ext_authz/ext_authz_fuzz_lib.cc | Converts fuzz setup from catch/throw to status-based validation and reporting. |
| test/extensions/filters/http/ext_authz/ext_authz_filter_test.cc | Updates filter tests to use status-validating per-route config construction. |
| test/extensions/filters/http/ext_authz/config_test.cc | Refactors config tests to build per-route configs with status-based validation. |
| test/extensions/filters/http/ext_authz/BUILD | Adds status_utility_lib dependency for new status assertions. |
| source/extensions/filters/http/gcp_authn/filter_config.h | Switches factory base and return type to exception-free (StatusOr). |
| source/extensions/filters/http/gcp_authn/filter_config.cc | Returns validation failures via RETURN_IF_NOT_OK instead of throwing. |
| source/extensions/filters/http/filter_chain/filter.h | Adds absl::Status& construction status plumbing to core filter-chain objects. |
| source/extensions/filters/http/filter_chain/filter.cc | Converts recursive-config rejection and factory creation failures into StatusOr. |
| source/extensions/filters/http/filter_chain/config.h | Switches factory base and return type to exception-free (StatusOr). |
| source/extensions/filters/http/filter_chain/config.cc | Constructs config with creation_status and returns failures via status. |
| source/extensions/filters/http/file_system_buffer/filter_config.h | Changes config construction to report failure via absl::Status&. |
| source/extensions/filters/http/file_system_buffer/filter_config.cc | Converts invalid path validation from throw to InvalidArgument status; retains one runtime throw. |
| source/extensions/filters/http/file_system_buffer/config.h | Switches factory base and return type to exception-free (StatusOr). |
| source/extensions/filters/http/file_system_buffer/config.cc | Plumbs creation_status through factory creation and route-specific config creation. |
| source/extensions/filters/http/fault/config.h | Switches factory base and return type to exception-free (StatusOr). |
| source/extensions/filters/http/fault/config.cc | Updates factory method signature to return StatusOr. |
| source/extensions/filters/http/ext_authz/ext_authz.h | Updates main/per-route config constructors to report errors via absl::Status&. |
| source/extensions/filters/http/ext_authz/ext_authz.cc | Converts duplicate config validation from throw to status assignment. |
| source/extensions/filters/http/ext_authz/config.h | Switches factory base and return type to exception-free (StatusOr). |
| source/extensions/filters/http/ext_authz/config.cc | Converts config creation to status-based validation; still has a throw in the gRPC callback path. |
yanavlasov
pushed a commit
that referenced
this pull request
Jul 16, 2026
…#46111) Migrate the ratelimit, rbac, and ext_authz network filter config factories from the throwing `Common::FactoryBase` to `Common::ExceptionFreeFactoryBase`, so config validation returns `absl::StatusOr` instead of throwing. This mirrors the already merged HTTP twins (#38629, #46035) and continues the exception free migration tracked in #27412. Error messages and failure semantics are unchanged (`EnvoyException` becomes `absl::InvalidArgumentError` with identical text). rbac's factory validation was the only throw in its directory, so its entry is removed from the `tools/code_format/config.yaml` exception allowlist, locking the directory throw free via `check_format`. ratelimit and ext_authz keep their entries: both retain throws outside the config validation surface (descriptor formatter, per connection client creation), left for a follow-up. The network read filter fuzz harness relied on factories throwing for invalid configs (`.value()` inside a `catch (EnvoyException)`). It now checks the returned status first, mirroring the HTTP fuzz harness, so error statuses keep exiting setup early instead of escaping as `BadStatusOrAccess` (caught by the `ext_authz_2` corpus entry, a V2 config). **Commit Message:** network: migrate ratelimit, rbac, ext_authz filters to exception free **Additional Description:** see above **Risk Level:** Low. Behavior preserving refactor, no user visible change. **Testing:** The three `config_test` targets and `//test/extensions/filters/network/common/fuzz:network_readfilter_fuzz_test` (which exercises the migrated error path via the `ext_authz_2` corpus) pass in the CI clang image. Added `DeprecatedV2TransportApiVersion` negative tests for ratelimit and ext_authz; rbac's `checkRule` expectations converted from `EXPECT_THROW` to status assertions. `check_format` passes with the rbac allowlist entry removed. **Docs Changes:** N/A **Release Notes:** N/A (internal refactor) **Platform Specific Features:** N/A Part of #27412 **Generative AI disclosure:** Developed with AI assistance (Claude Code); I reviewed and understand every line and take full ownership of the submission. Signed-off-by: David Vo <davidvo@lyft.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: make ext authz/filter chain/fault/... 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.