envoyproxy / envoyproxy/envoy

deprecate config using the One True ADS Server(tm)

Open
#13,951 12 comments 1 reaction 0 assignees View on GitHub
area/xds design proposal enhancement help wanted
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 20h
Merged PRs (30d)
428

Description

Today, the way ADS is configured is as follows:
- The bootstrap file has an [`ads_config`](https://github.com/envoyproxy/envoy/blob/d9af0e985a2fef2c921641b12f4786a658ad0923/api/envoy/config/bootstrap/v3/bootstrap.proto#L100) field indicating the One True ADS Server(tm).
- The bootstrap file has its [`lds_resource_locator`](https://github.com/envoyproxy/envoy/blob/d9af0e985a2fef2c921641b12f4786a658ad0923/api/envoy/config/bootstrap/v3/bootstrap.proto#L82) and [`cds_resources_locator`](https://github.com/envoyproxy/envoy/blob/d9af0e985a2fef2c921641b12f4786a658ad0923/api/envoy/config/bootstrap/v3/bootstrap.proto#L91) fields set to a `ConfigSource` with the [`ads`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L185) field set.
- In resources sent from the management server, `ConfigSource` protos use the same [`ads`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L185) field.

This approach seems incompatible with the new xDS naming scheme, which was designed to support federation. As part of the new naming scheme, the `ApiConfigSource.ApiType` enum now has [`AGGREGATED_GRPC`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L68) and [`AGGREGATED_DELTA_GRPC`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L73) values, which allows configuring any `ConfigSource` to use a single ADS stream instead of using a separate stream for each resource type. This way of configuring ADS seems much more in-line with the direction we're heading in and will make it easier to transition to the new federated world.

Therefore, I propose replacing this notion of the One True ADS Server(tm) with use of this new `AGGREGATED_GRPC` enum value. The typical ADS configuration described above would instead be represented as follows:
- The bootstrap file will not populate the [`ads_config`](https://github.com/envoyproxy/envoy/blob/d9af0e985a2fef2c921641b12f4786a658ad0923/api/envoy/config/bootstrap/v3/bootstrap.proto#L100) field.
- The bootstrap file will have its [`lds_resource_locator`](https://github.com/envoyproxy/envoy/blob/d9af0e985a2fef2c921641b12f4786a658ad0923/api/envoy/config/bootstrap/v3/bootstrap.proto#L82) and [`cds_resources_locator`](https://github.com/envoyproxy/envoy/blob/d9af0e985a2fef2c921641b12f4786a658ad0923/api/envoy/config/bootstrap/v3/bootstrap.proto#L91) fields both set to the same server, with `ApiType` set to `AGGREGATED_GRPC` or `AGGREGATED_DELTA_GRPC`. (Envoy will internally de-dup these by hashing the `ConfigSource`s, so the result will be a single ADS stream, not two separate ADS streams.)
- In resources sent from the management server, `ConfigSource` protos will use the [`self`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L198) field.

This will require the following changes:
- Implement support for the [`self`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L198) field. (This field was added in #8201 but is not currently implemented in Envoy.)
- Implement support for the [`AGGREGATED_GRPC`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L68) and [`AGGREGATED_DELTA_GRPC`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L73) values in the `ApiType` enum. (I'm not sure how much of this works right now.)
- Deprecate the bootstrap file's [`ads_config`](https://github.com/envoyproxy/envoy/blob/d9af0e985a2fef2c921641b12f4786a658ad0923/api/envoy/config/bootstrap/v3/bootstrap.proto#L100) field.
- Deprecate the [`ads`](https://github.com/envoyproxy/envoy/blob/e1c138b427ff9bcdf5ebcb2639cc2d22f98ac1a8/api/envoy/config/core/v3/config_source.proto#L185) field.

The only configuration that would be broken by this change would be if someone was (e.g.) using non-aggregated LDS to get a `Listener` resource that contained a `ConfigSource` telling the client to get the `RouteConfiguration` via the One True ADS Server(tm). However, it seems exceedingly unlikely that anyone is actually doing that, and there are work-arounds: they can either have their management server send the exact `ConfigSource` containing the ADS server they want to talk to, or better yet, they can start using the new xDS naming scheme, where they can select the xDS server via an authority in the resource name itself.

CC @mattklein123 @htuch

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.