envoyproxy / envoyproxy/nighthawk

Templatized utilities to improve proto and factory experience

Open
#468 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
414
Forks
95
Avg merge
2d 9h
Merged PRs (30d)
11

Description

# 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(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
TypedExtensionConfig MakePluginConfig(
absl::string_view plugin_name,
std::function 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
#include
#include

// 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
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 {
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
TypedExtensionConfig MakePluginConfig(
FactoryT factory,
std::function 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);
});
}

```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.