Templatized utilities to improve proto and factory experience

[eric846]

Investigate strongly typed ConfigValidator

Can the ConfigValidator [0] interface be rewritten somehow so that it knows the plugin-specific proto type, rather than having to try unpacking a Message?

[0] https://github.com/envoyproxy/nighthawk/blob/master/include/nighthawk/adaptive_load/config_validator.h

Create a templatized helper for the following

  const auto& any = dynamic_cast<const Envoy::ProtobufWkt::Any&>(message);
  nighthawk::adaptive_load::LinearScoringFunctionConfig config;
  // set fields of config here
  Envoy::MessageUtil::unpackTo(any, config);

(for all values of LinearScoringFunctionConfig)

Make it easy to create a TypedExtensionConfig for a given plugin

TypedExtensionConfig [1] has fields name and typed_config. It costs multiple lines of code to set the fields of the plugin-specific config, pack it into an Any, and set name and typed_config on the final proto. And there is no guarantee that you used a config proto that matches the plugin name.

[1] https://github.com/envoyproxy/envoy/blob/e8216a8cf79c54e3e0a77ab729ebf27f4e79eb1b/api/envoy/config/core/v3/extension.proto

It's easy to make a helper function:

template<typename CustomConfigProtoT>
TypedExtensionConfig MakePluginConfig(
    absl::string_view plugin_name, 
    std::function<void(CustomConfigProtoT&)> field_setting_lambda);

You would call it like:

TypedExtensionConfig config = MakePluginConfig("com.plugins.a", [](AConfigProto& inner_config) {
  inner_config.set_x(1);
  inner_config.set_y(2);
});

But is there a way to avoid having both the plugin name and the type as inputs, which could be mismatched?

What if the factory implemented a ~~constexpr~~ function that returned the associated config proto type? EDIT: I realized a real proto value probably can't be constexpr, but fortunately it still compiles without constexpr.

This is the best I could come up with:

// clang -lstdc++ xxxx.cc
#include <functional>
#include <memory>
#include <string>

// For the real TypedFactory, see https://github.com/envoyproxy/envoy/blob/3adf14af5ca9f34221d59d693b173d20b27c1614/include/envoy/config/typed_config.h
class TypedFactory {  
 public:
  virtual std::string name() = 0;
  // virtual MessagePtr createEmptyConfigProto() = 0;
};

template <typename ConfigProtoT>
class FactoryImprover {
 public:
  virtual ConfigProtoT GetConfigProto() = 0;  // formerly constexpr, but no need for that
};

class AConfigProto {
 public:
  void set_x(int) {}
  void set_y(int) {}
};

// We explicitly declare the relationship between AFactory and AConfigProto:
class AFactory : public TypedFactory, public FactoryImprover<AConfigProto> {
 public:
  std::string name() override { return "com.plugins.a"; }
  // MessagePtr createEmptyConfigProto() override;
  AConfigProto GetConfigProto() override { return AConfigProto(); }
};

struct TypedExtensionConfig {
  // this would be the Envoy proto
};

template <typename FactoryT>
TypedExtensionConfig MakePluginConfig(
    FactoryT factory,
    std::function<void(decltype(factory.GetConfigProto())&)> lambda) {
  TypedExtensionConfig config;

  auto typed_config = factory.GetConfigProto();
  lambda(typed_config);

  std::string name = factory.name();

  // config.set_name(name);
  // config.mutable_typed_config()->PackFrom(typed_config);
  return config;
}

int main(int, char**) {
  TypedExtensionConfig config = MakePluginConfig(AFactory(), [](auto& a) {
    a.set_x(1);
    a.set_y(2);
  });
}

[oschaaf]

Hah, curious to hear what others think about this, but my 2p is that I like this. IMHO the extra complexity introduced by the templating is worth it, as this saves annoying and repetitive bootstrapping code. Also, maybe this could be very useful in TEST_P type tests to check expected behaviour across implementations.

[oschaaf]

(for those who'd like hands on with the PoC @eric846 posted above: I had to #include <string> to get it to build.)