Skip to content

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

Merged
htuch merged 7 commits intoenvoyproxy:masterfrom
htuch:resource-annotations
Jan 6, 2020
Merged

api/config: use resource type annotations to provide type-to-endpoint.#9566
htuch merged 7 commits intoenvoyproxy:masterfrom
htuch:resource-annotations

Conversation

@htuch
Copy link
Copy Markdown
Member

@htuch htuch commented Jan 6, 2020

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

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>
@htuch htuch requested a review from lizan January 6, 2020 02:58
@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: #9566 was opened by htuch.

see: more, trace.

@htuch htuch requested review from fredlas and mattklein123 January 6, 2020 02:58
@htuch htuch added the api/v3 Major version release @ end of Q3 2019 label Jan 6, 2020
htuch added 4 commits January 6, 2020 11:05
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really fantastic, thanks. A few comments. Also, can we open a doc issue for 1.14 to use these annotations in doc output instead of all the manual insertions we have everywhere now?

/wait-any

htuch added 2 commits January 6, 2020 14:29
Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
@htuch
Copy link
Copy Markdown
Member Author

htuch commented Jan 6, 2020

@mattklein123 #3091 tracks the service method doc generation, I left a comment to make use of the resource type annotations.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@repokitteh-read-only repokitteh-read-only bot removed the api label Jan 6, 2020
@htuch htuch merged commit cceab39 into envoyproxy:master Jan 6, 2020
@htuch htuch deleted the resource-annotations branch January 6, 2020 22:58
htuch pushed a commit that referenced this pull request Jan 7, 2020
This typo breaks spell check triggered by git push, thus preventing me
from cleanly uploading unrelated work.

Signed-off-by: Bence Béky <bnc@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api/v3 Major version release @ end of Q3 2019

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Annotation based mapping between xDS message types and endpoints

3 participants