config: add support Any as opaque config #5435
Conversation
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
|
/retest |
|
🔨 rebuilding |
source/common/config/utility.cc
Outdated
| void Utility::translateOpaqueConfig(const ProtobufWkt::Any& typed_config, | ||
| const ProtobufWkt::Struct& config, | ||
| Protobuf::Message& out_proto) { | ||
| static const std::string struct_type = |
There was a problem hiding this comment.
Static initialization fiasco and non-POD?
There was a problem hiding this comment.
It shouldn't be problem since it is initialized in first call, but anyway it is also safe to make it as a reference.
| // Get the cluster with extension_protocol_options for an http filter factory. | ||
| TestHttpFilterConfigFactory factory(factoryBase); | ||
| Registry::InjectFactory<Server::Configuration::NamedHttpFilterConfigFactory> registry(factory); | ||
| clusters.push_back(makeCluster(typed_yaml)); |
There was a problem hiding this comment.
QQ: do we have a plan to convert all existing test configs and examples to Any? What about docs? Should we have a tracking bug?
There was a problem hiding this comment.
Friendly ping, any update on where we are with this and other comments?
There was a problem hiding this comment.
Yeah by the time we remove Struct configs we should convert all including examples, I'm planning add a doc in a follow up too.
There was a problem hiding this comment.
True, but ideally we convert the docs and examples ahead-of-time in the deprecation cycle, so we want to do this before then.
| if (type != struct_type || out_proto.GetDescriptor()->full_name() == struct_type) { | ||
| typed_config.UnpackTo(&out_proto); | ||
| } else { | ||
| ProtobufWkt::Struct struct_config; |
There was a problem hiding this comment.
Can you add tests for this branch? Looks like it is missing coverage.
There was a problem hiding this comment.
🤦♂️ it should be fixed now.
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
lizan
removed
the
stale
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
Signed-off-by: Lizan Zhou <lizan@tetrate.io>
|
/retest |
|
🔨 rebuilding |
| ProtobufWkt::Struct struct_config; | ||
| typed_config.UnpackTo(&struct_config); | ||
| MessageUtil::jsonConvert(struct_config, out_proto); | ||
| } |
There was a problem hiding this comment.
Shouldn't there be a return after this line?
There was a problem hiding this comment.
It wouldn't hurt, but I think the invariant is that !typed_config.empty ^ !config.empty, i.e. they should generally be disjoint via oneof.
Add support of Any as opaque config for extensions. Deprecates Struct configs. Fixes envoyproxy#4475. Risk Level: Low Testing: CI Docs Changes: Added. Release Notes: Added. Signed-off-by: Lizan Zhou <lizan@tetrate.io> Signed-off-by: Fred Douglas <fredlas@google.com>
Signed-off-by: Lizan Zhou lizan@tetrate.io
Description:
Add support of Any as opaque config for extensions. Deprecates Struct configs. Fixes #4475.
Risk Level: Low
Testing: CI
Docs Changes: Added.
Release Notes: Added.