http: migrate grpc_stats/jwt_authn/... to exception free#46037
Open
wbpcode wants to merge 1 commit into
Open
Conversation
Signed-off-by: wbpcode/wangbaiping <wbphub@gmail.com>
wbpcode
requested review from
adisuissa,
botengyao,
kyessenov,
mattklein123,
nezdolik,
tyxia and
yanavlasov
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request migrates several HTTP filters (notably jwt_authn, plus grpc_web, grpc_stats, health_check, and kill_request) toward Envoy’s exception-free configuration model by switching factories and internal construction paths from throwing EnvoyException to returning absl::Status/absl::StatusOr.
Changes:
- Convert multiple filter config factories to
Common::ExceptionFreeFactoryBaseand returnabsl::StatusOr<Http::FilterFactoryCb>. - Update
jwt_authncore components (FilterConfigImpl,Verifier,Matcher,JwksCache) to propagate configuration/creation failures viaabsl::Status/StatusOrinstead of exceptions. - Update affected unit tests/fuzz test to use status-based assertions rather than
EXPECT_THROW.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| test/extensions/filters/http/jwt_authn/provider_verifier_test.cc | Updates verifier/config construction to status-based APIs and adjusts expectations for invalid config. |
| test/extensions/filters/http/jwt_authn/matcher_test.cc | Updates matcher creation to StatusOr API. |
| test/extensions/filters/http/jwt_authn/jwt_authn_fuzz_test.cc | Replaces exception handling with status-based filter config construction in fuzz harness. |
| test/extensions/filters/http/jwt_authn/jwks_cache_test.cc | Updates JWKS cache creation to StatusOr API. |
| test/extensions/filters/http/jwt_authn/group_verifier_test.cc | Updates verifier creation to StatusOr API. |
| test/extensions/filters/http/jwt_authn/filter_factory_test.cc | Updates factory failure expectations to match returned status instead of exceptions. |
| test/extensions/filters/http/jwt_authn/filter_config_test.cc | Updates config construction and error assertions to use absl::Status. |
| test/extensions/filters/http/jwt_authn/BUILD | Adds dependency on status_utility test helpers. |
| test/extensions/filters/http/jwt_authn/authenticator_test.cc | Updates filter config construction to status-based API. |
| test/extensions/filters/http/jwt_authn/all_verifier_test.cc | Updates verifier/config creation to status-based APIs. |
| test/extensions/filters/http/health_check/config_test.cc | Updates factory error expectations to status-based assertions. |
| test/extensions/filters/http/health_check/BUILD | Adds dependency on status_utility test helpers. |
| source/extensions/filters/http/kill_request/kill_request_config.h | Switches filter factory to exception-free base and StatusOr factory callback. |
| source/extensions/filters/http/kill_request/kill_request_config.cc | Implements StatusOr<Http::FilterFactoryCb> factory callback return. |
| source/extensions/filters/http/jwt_authn/verifier.h | Changes verifier factory method to return absl::StatusOr<VerifierConstPtr>. |
| source/extensions/filters/http/jwt_authn/verifier.cc | Propagates verifier creation failures via status instead of throwing. |
| source/extensions/filters/http/jwt_authn/matcher.h | Changes matcher factory method to return absl::StatusOr<MatcherConstPtr>. |
| source/extensions/filters/http/jwt_authn/matcher.cc | Propagates path-match-policy creation failures via status instead of throwing. |
| source/extensions/filters/http/jwt_authn/jwks_cache.h | Changes JWKS cache factory to return absl::StatusOr<JwksCachePtr>. |
| source/extensions/filters/http/jwt_authn/jwks_cache.cc | Propagates JWKS cache construction/validation failures via status. |
| source/extensions/filters/http/jwt_authn/filter_factory.h | Switches jwt_authn factory to exception-free base and StatusOr callback return. |
| source/extensions/filters/http/jwt_authn/filter_factory.cc | Converts inline-JWKS validation and config creation to status-based flow. |
| source/extensions/filters/http/jwt_authn/filter_config.h | Updates FilterConfigImpl ctor to accept absl::Status& for construction status. |
| source/extensions/filters/http/jwt_authn/filter_config.cc | Threads status propagation through verifier/matcher/JWKS cache construction. |
| source/extensions/filters/http/health_check/config.h | Switches health_check factory to exception-free base and StatusOr callback return. |
| source/extensions/filters/http/health_check/config.cc | Returns InvalidArgument status instead of throwing for invalid config. |
| source/extensions/filters/http/grpc_web/config.h | Switches grpc_web factory to exception-free base and StatusOr callback return. |
| source/extensions/filters/http/grpc_web/config.cc | Updates factory callback return type to StatusOr. |
| source/extensions/filters/http/grpc_stats/grpc_stats_filter.h | Switches grpc_stats factory to exception-free base and StatusOr callback return. |
| source/extensions/filters/http/grpc_stats/grpc_stats_filter.cc | Updates factory callback return type to StatusOr. |
Comment on lines
+43
to
+49
| absl::Status creation_status = absl::OkStatus(); | ||
| filter_config_ = | ||
| std::make_unique<FilterConfigImpl>(proto_config_, "", mock_factory_ctx_, creation_status); | ||
| ASSERT_TRUE(creation_status.ok()); | ||
| verifier_ = Verifier::create(proto_config_.rules(0).requires_(), proto_config_.providers(), | ||
| *filter_config_); | ||
| *filter_config_) | ||
| .value(); |
Comment on lines
+262
to
265
| absl::Status creation_status = absl::OkStatus(); | ||
| FilterConfigImpl filter_config(proto_config_, "", mock_factory_ctx_, creation_status); | ||
| EXPECT_FALSE(creation_status.ok()); | ||
| } |
Comment on lines
+50
to
+53
| absl::Status creation_status = absl::OkStatus(); | ||
| filter_config_ = | ||
| std::make_unique<FilterConfigImpl>(proto_config_, "", mock_factory_ctx_, creation_status); | ||
| ASSERT_TRUE(creation_status.ok()); |
Comment on lines
79
to
82
| verifier_ = Verifier::create(proto_config_.rules(0).requires_(), proto_config_.providers(), | ||
| mock_factory_); | ||
| mock_factory_) | ||
| .value(); | ||
| } |
Comment on lines
82
to
85
| verifier_ = Verifier::create(proto_config_.rules(0).requires_(), proto_config_.providers(), | ||
| *filter_config_); | ||
| *filter_config_) | ||
| .value(); | ||
| } |
Comment on lines
+99
to
100
| cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_).value(); | ||
|
|
Comment on lines
+118
to
119
| cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_).value(); | ||
|
|
Comment on lines
+135
to
136
| cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_).value(); | ||
|
|
Comment on lines
+149
to
150
| cache_ = JwksCache::create(config_, context_, mock_fetcher_.AsStdFunction(), stats_).value(); | ||
|
|
Comment on lines
22
to
26
| MatcherConstPtr createMatcher(const char* config) { | ||
| RequirementRule rule; | ||
| TestUtility::loadFromYaml(config, rule); | ||
| return Matcher::create(rule, context_); | ||
| return Matcher::create(rule, context_).value(); | ||
| } |
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 grpc_stats/jwt_authn/... 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.