cloudfoundry / diego-release

BOSH Release for Diego
Apache License 2.0
201 stars 212 forks source link

Add Inigo test to ensure we are not using any deprecated Envoy API fields #527

Closed sunjayBhatia closed 4 years ago

sunjayBhatia commented 4 years ago

As a follow up to https://github.com/cloudfoundry/diego-release/pull/526

We thought it would be useful to add an Inigo test that curls the Envoy admin server and ensures the config dump does not include any deprecated fields (see https://github.com/cloudfoundry/diego-release/issues/521 for an example of what it would show).

We didn't do it as part of #526 as it is a little involved and we wanted to keep that PR small, the flow should possible go something like this:

This test would start to fail when we bump Envoy and are still using fields that are newly deprecated so we can fix them ASAP, block us releasing, etc. rather than find out after the fact from issues like #521

cf-gitbot commented 4 years ago

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

mdelillo commented 4 years ago

I pushed a WIP branch here. It has the latest release of go-control-plane and udpa, but the test is failing because hidden_envoy_deprecated_build_version is present in the config.

sunjayBhatia commented 4 years ago

Hmm, the deprecated field thing may be because of the following:

bootstrap config (the base of the envoy config) has a node element: https://github.com/envoyproxy/envoy/blob/79d7d4ee917c4faff78b5ae8aa1b07cf2ff91cfc/api/envoy/config/bootstrap/v3/bootstrap.proto#L109

the Node type has a version: https://github.com/envoyproxy/envoy/blob/79d7d4ee917c4faff78b5ae8aa1b07cf2ff91cfc/api/envoy/config/core/v3/base.proto#L182

However it seems b/c we're on v3 and the v2 version is referenced as the previous version/theres a reserved field, the proto dump generated may be adding this deprecated field: https://github.com/envoyproxy/envoy/blob/79d7d4ee917c4faff78b5ae8aa1b07cf2ff91cfc/api/envoy/config/core/v3/base.proto#L137-L141 and https://github.com/envoyproxy/envoy/blob/79d7d4ee917c4faff78b5ae8aa1b07cf2ff91cfc/api/envoy/api/v2/core/base.proto#L166

I can't see a good way to get rid of that field from the config dump, except maybe specifying our own envoy user agent, which is not really relevant to how we use envoy