config: distinct resource/transport API versions. #9526

htuch · 2019-12-30T23:56:26Z

This PR extends #9468 to support a distinct notion of transport and
resource API version. The intuition here is that the opaque resources
(and their type URLs) can be delivered via either v2 or v3 xDS, and the
DiscoveryRequest etc. messages have their own versioning.

Currently, the v2 and v3 transport protocols are indistinguishable
modulo service endpoint. As v3 evolves, e.g. with #9301, differences
will be introduced. At this point it will be necessary to have enhanced
support in the gRPC mux and HTTP subscription modules to handle the
protocol differences.

This is technically a breaking v2 API change, but since the field it
breaks was only added today, I think it's safe to assume it is not in
use yet.

Some other misc. changes that were needed:

Type URL added to REST DiscoveryRequest.
Fixed SDS type URL determination.

Risk level: Medium (introduction of type URL in REST DiscoveryRequests is a wire addition.
Testing: Integration tests added to validate service endpoint and type
URL selection based on transport/resource version.

Signed-off-by: Harvey Tuch htuch@google.com

This PR extends envoyproxy#9468 to support a distinct notion of transport and resource API version. The intuition here is that the opaque resources (and their type URLs) can be delivered via either v2 or v3 xDS, and the DiscoveryRequest etc. messages have their own versioning. Currently, the v2 and v3 transport protocols are indistinguishable modulo service endpoint. As v3 evolves, e.g. with envoyproxy#9301, differences will be introduced. At this point it will be necessary to have enhanced support in the gRPC mux and HTTP subscription modules to handle the protocol differences. This is technically a breaking v2 API change, but since the field it breaks was only added today, I think it's safe to assume it is not in use yet. Risk level: Low Testing: Integration tests added to validate service endpoint and type URL selection based on transport/resource version. Signed-off-by: Harvey Tuch <htuch@google.com>

repokitteh-read-only · 2019-12-30T23:56:31Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/.

🐱

Caused by: #9526 was opened by htuch.

see: more, trace.

htuch · 2019-12-30T23:58:29Z

@Shikugawa @lizan the api_verion_integration_test is still WiP and I need to version switch the ADS gRPC endpoint URL still, but this PR is basically where I see us heading; distinct resource and transport API versions and an e2e test to validate the basics across resource types, config types and config versions.

Signed-off-by: Harvey Tuch <htuch@google.com>

source/common/config/type_to_endpoint.cc

Signed-off-by: Harvey Tuch <htuch@google.com>

htuch · 2020-01-02T22:22:08Z

@lizan @Shikugawa @mattklein123 this now has tests added for functional coverage and is ready for review.

Shikugawa · 2020-01-03T04:51:22Z

@htuch Great thanks for this work. This is very helpful functionality for my planning implementation for API fallback. I will add some comment.

source/common/router/rds_impl.h

mattklein123

At a high level this looks fine to me with one API question.

api/envoy/api/v2/core/config_source.proto

Signed-off-by: Harvey Tuch <htuch@google.com>

Previously, type_to_endpoint.cc had a lot of hardcoding, which doesn't scale well with multiple API versions. See envoyproxy#9526 for an example of the issues encountered. This patch switches to using explicit resource type annotations on service descriptors, which is great for documentation (previously this was sometimes given in comments, sometimes not), and allows for a reflection driven reverse map from resource type URL to endpoints to be built at runtime. Risk level: Low Testing: New unit tests for type_to_endpoint.cc and golden protoxform tests for the new annotations. Fixes envoyproxy#9454. Signed-off-by: Harvey Tuch <htuch@google.com>

#9566) Previously, type_to_endpoint.cc had a lot of hardcoding, which doesn't scale well with multiple API versions. See #9526 for an example of the issues encountered. This patch switches to using explicit resource type annotations on service descriptors, which is great for documentation (previously this was sometimes given in comments, sometimes not), and allows for a reflection driven reverse map from resource type URL to endpoints to be built at runtime. Risk level: Low Testing: New unit tests for type_to_endpoint.cc and golden protoxform tests for the new annotations. Fixes #9454. Signed-off-by: Harvey Tuch <htuch@google.com>

…. (#9566) Previously, type_to_endpoint.cc had a lot of hardcoding, which doesn't scale well with multiple API versions. See envoyproxy/envoy#9526 for an example of the issues encountered. This patch switches to using explicit resource type annotations on service descriptors, which is great for documentation (previously this was sometimes given in comments, sometimes not), and allows for a reflection driven reverse map from resource type URL to endpoints to be built at runtime. Risk level: Low Testing: New unit tests for type_to_endpoint.cc and golden protoxform tests for the new annotations. Fixes #9454. Signed-off-by: Harvey Tuch <htuch@google.com> Mirrored from https://github.com/envoyproxy/envoy @ cceab393664429a3063d787cf28cade3c8ab01c7

Signed-off-by: Harvey Tuch <htuch@google.com>

htuch · 2020-01-07T02:44:39Z

@mattklein123 @lizan ready for another pass, thanks.

mattklein123

LGTM at a high level. Will defer to @lizan and @Shikugawa for in-depth review.

source/common/config/type_to_endpoint.cc

source/common/router/scoped_rds.cc

Signed-off-by: Harvey Tuch <htuch@google.com>

lizan

LGTM, one nit

source/common/config/protobuf_link_hacks.h

#9526 force linked most method descriptors. However, HDS was missing. This PR force links HDS v2. Risk Level: low Testing: compiled with Envoy Mobile's iOS dist target which, as discussed here, requires the hack currently in place. Signed-off-by: Jose Nino <jnino@lyft.com>

This is only refactoring. Aggregated xDS type urls. In previous implementation, loadTypeUrl is scattered on all of xDS subscription classes. That is poor outlook so refactored all of them. Risk Level: Low Testing: None Relates to issues 9468 #9526 Signed-off-by: shikugawa <rei@tetrate.io>

repokitteh-read-only bot added the api label Dec 30, 2019

htuch requested review from Shikugawa and lizan December 30, 2019 23:56

htuch assigned lizan and Shikugawa Dec 30, 2019

htuch added the api/v3 Major version release @ end of Q3 2019 label Dec 30, 2019

Type-to-endpoint support for mixed resource/transport versions.

b23f4c6

Signed-off-by: Harvey Tuch <htuch@google.com>

htuch commented Dec 31, 2019

View reviewed changes

source/common/config/type_to_endpoint.cc Outdated Show resolved Hide resolved

htuch added 13 commits January 2, 2020 10:43

Merge remote-tracking branch 'upstream/master' into v3-endpoints

bfc449e

REST support.

dc7f074

Signed-off-by: Harvey Tuch <htuch@google.com>

CDS and RTDS tests.

8374c0a

Signed-off-by: Harvey Tuch <htuch@google.com>

Bracing.

19c0232

Signed-off-by: Harvey Tuch <htuch@google.com>

SRDS and reduction of combinatorial explosion.

1df88c4

Signed-off-by: Harvey Tuch <htuch@google.com>

RDS, SDS, EDS

9ef233c

Signed-off-by: Harvey Tuch <htuch@google.com>

wip

0d7a9ba

Signed-off-by: Harvey Tuch <htuch@google.com>

Skip VHDS for now.

87da32a

Signed-off-by: Harvey Tuch <htuch@google.com>

ADS support.

e315260

Signed-off-by: Harvey Tuch <htuch@google.com>

fix_format

ddb5342

Signed-off-by: Harvey Tuch <htuch@google.com>

Merge remote-tracking branch 'upstream/master' into v3-endpoints

221ad9b

fix_format

fac5ffa

Signed-off-by: Harvey Tuch <htuch@google.com>

Spelling.

20cd7fb

Signed-off-by: Harvey Tuch <htuch@google.com>

htuch assigned mattklein123 Jan 2, 2020

Shikugawa reviewed Jan 3, 2020

View reviewed changes

source/common/router/rds_impl.h Outdated Show resolved Hide resolved

mattklein123 reviewed Jan 3, 2020

View reviewed changes

api/envoy/api/v2/core/config_source.proto Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into v3-endpoints

fb9076d

htuch added 3 commits January 3, 2020 13:36

fix_format

7bb6e37

Signed-off-by: Harvey Tuch <htuch@google.com>

Merge remote-tracking branch 'upstream/master' into v3-endpoints

63cec0c

fix_format

c0e1995

Signed-off-by: Harvey Tuch <htuch@google.com>

htuch mentioned this pull request Jan 6, 2020

api/config: use resource type annotations to provide type-to-endpoint. #9566

Merged

htuch added 2 commits January 6, 2020 18:37

Merge remote-tracking branch 'upstream/master' into v3-endpoints

3d3ea3e

Update to v3alpha API boost and other recent PRs.

4255fc0

Signed-off-by: Harvey Tuch <htuch@google.com>

mattklein123 reviewed Jan 7, 2020

View reviewed changes

source/common/config/type_to_endpoint.cc Outdated Show resolved Hide resolved

source/common/router/scoped_rds.cc Show resolved Hide resolved

htuch added 2 commits January 7, 2020 12:50

Remove HTD debug messages.

398e738

Signed-off-by: Harvey Tuch <htuch@google.com>

Merge remote-tracking branch 'upstream/master' into v3-endpoints

bc7328f

lizan reviewed Jan 7, 2020

View reviewed changes

source/common/config/protobuf_link_hacks.h Show resolved Hide resolved

lizan approved these changes Jan 7, 2020

View reviewed changes

repokitteh-read-only bot removed the api label Jan 7, 2020

htuch merged commit df05827 into envoyproxy:master Jan 8, 2020

htuch deleted the v3-endpoints branch January 8, 2020 02:40

This was referenced Jan 8, 2020

Hybrid v2/v3 static/dynamic configuration; should we support this? #8137

Closed

Add e2e tests for v(N - M) -> vNalpha #8136

Closed

xDS client endpoint discovery #8421

Closed

junr03 mentioned this pull request Jan 9, 2020

proto: force link missing v2 protos #9615

Merged

Shikugawa mentioned this pull request Jan 10, 2020

xDS: type url aggregation #9649

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: distinct resource/transport API versions. #9526

config: distinct resource/transport API versions. #9526

htuch commented Dec 30, 2019 •

edited

Loading

repokitteh-read-only bot commented Dec 30, 2019

htuch commented Dec 30, 2019

htuch commented Jan 2, 2020

Shikugawa commented Jan 3, 2020

mattklein123 left a comment

htuch commented Jan 7, 2020

mattklein123 left a comment

lizan left a comment

config: distinct resource/transport API versions. #9526

config: distinct resource/transport API versions. #9526

Conversation

htuch commented Dec 30, 2019 • edited Loading

repokitteh-read-only bot commented Dec 30, 2019

htuch commented Dec 30, 2019

htuch commented Jan 2, 2020

Shikugawa commented Jan 3, 2020

mattklein123 left a comment

Choose a reason for hiding this comment

htuch commented Jan 7, 2020

mattklein123 left a comment

Choose a reason for hiding this comment

lizan left a comment

Choose a reason for hiding this comment

htuch commented Dec 30, 2019 •

edited

Loading