envoyproxy / envoyproxy/envoy-mobile

Simplify Envoy Config Generation

Open
#2,498 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
566
Forks
85
PR merge metrics
No merged PRs in 30d

Description

The system that Envoy Mobile uses to generate Envoy's config is fairly complicated. It involves snippets of YAML defined as C strings in [library/common/config/config.cc](https://sourcegraph.com/github.com/envoyproxy/envoy-mobile/-/blob/library/common/config/config.cc). In order to avoid duplication, these snippets make use of YAML anchors and aliases. These YAML snippets are conditionally stitched together along with in Swift via [EnvoyConfiguration.m](https://sourcegraph.com/github.com/envoyproxy/envoy-mobile/-/blob/library/objective-c/EnvoyConfiguration.m) and in Java via [EnvoyConfiguration.java](https://sourcegraph.com/github.com/envoyproxy/envoy-mobile/-/blob/library/java/io/envoyproxy/envoymobile/engine/EnvoyConfiguration.java) and in C++ via [library/cc/engine_builder.h](https://sourcegraph.com/github.com/envoyproxy/envoy-mobile/-/blob/library/cc/engine_builder.h).

It seems undesirable to have 3 different builders with different functionality. The Alternate Protocols cache filter is only supported in Java. Custom filters are not supported in the C++ builder.

In addition, the use of YAML anchor and aliases interacts a bit poorly with the builders. Some of the anchors are used by aliases in `config_header` which prevents them from being overridden by builder-supplied values. Of course, other anchor are overrideable and some are currently overrideable in some places but not in others.

I think we should simplify this. My strawman would be that config generation is implemented in a single place in (C or C++). The Java, Swift and C++ Builder would be responsible for calling into this single config generation method with whatever state is required to generate a valid config. (This includes numerous booleans and integers as well as a number of strings, lists and maps). However it is important that this not regress app startup so perhaps another approach would be warranted.

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.