http: add server factory only factory support to geoip#46224
Conversation
|
/coverage |
|
Coverage for this Pull Request will be rendered here: https://storage.googleapis.com/envoy-cncf-pr/46224/coverage/index.html For comparison, current coverage on https://storage.googleapis.com/envoy-cncf-postsubmit/main/coverage/index.html The coverage results are (re-)rendered each time the CI |
Signed-off-by: wbpcode <wbphub@gmail.com>
0494671 to
b6cdb25
Compare
| virtual DriverSharedPtr | ||
| createGeoipProviderDriver(const Protobuf::Message& config, const std::string& stat_prefix, | ||
| Server::Configuration::FactoryContext& context) PURE; | ||
| Server::Configuration::ServerFactoryContext& context) PURE; |
There was a problem hiding this comment.
This may break our forks because this API changes if our forks have a custom GeoipProvider implementation, but I have no good idea to keep the compatbility.
I think it's acceptable because this is not an extension point like HTTP filter.
There was a problem hiding this comment.
And I checked the existing implementation of this extension point, it's possible the cached driver be shared across listeners, the server context is more appropriate for this API.
Anyway, I will let maintainers to check to this point.
There was a problem hiding this comment.
4 participants
Commit Message: http: add server factory only factory support to geoip
Additional Description:
This adds the server-factory-context-only filter factory creation method
(
createHttpFilterFactoryFromProtoTypedtaking only aServerFactoryContext,introduced in #45989) to the geoip HTTP filter(s).
This allows these filters to be created in contexts where only a
ServerFactoryContextis available (e.g. route/virtual-host level filterconfiguration) rather than requiring the full downstream
FactoryContext.The existing downstream
FactoryContextpath is preserved and now delegatesto a shared helper, so behavior for existing use cases is unchanged.
Generative AI (Claude) was used to assist with this change. I reviewed all changes.
Risk Level: mid, may break forks.
Testing: Existing/added unit config tests; CI.
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A