Add proxy protocol config api message.#10845
Add proxy protocol config api message.#10845alyssawilk merged 6 commits intoenvoyproxy:masterfrom wez470:proxy-proto-api
Conversation
Signed-off-by: Weston Carlson <wez470@gmail.com>
alyssawilk
left a comment
There was a problem hiding this comment.
Mind adding the build rules too? We want to make sure this will catch typos. Also I think you need to run
./tools/proto_format/proto_format.sh fix
to get all the other proto changes :-)
|
I think this build rule covers it https://github.com/envoyproxy/envoy/blob/master/api/BUILD#L251? I think I ran proto_format, but will double check after work today edit - updated link to proper location |
Signed-off-by: Weston Carlson <wez470@gmail.com>
|
Definitely forgot to add the shadow file when I committed earlier 😅 |
Signed-off-by: Weston Carlson <wez470@gmail.com>
alyssawilk
left a comment
There was a problem hiding this comment.
I'm still confused at the lack of v4 but then I don't grok the vagaries of the new config system.
@htuch or @mattklein123 for API review.
|
|
||
| // [#protodoc-title: Proxy Protocol] | ||
|
|
||
| message ProxyProtocolConfig { |
There was a problem hiding this comment.
Is this a core type or does it belong somewhere like envoy.config.core? I prefer to keep the type hierarchy for fundamental types.
The reason that there is no v4 is that this does not reference any deprecated fields or other types that have transitively deprecated fields.
There was a problem hiding this comment.
Yeah, I wasn't sure where to put it. Taking another look, the types already in this package probably would be more widely used than the proxy proto config. I can move to envoy.config.core
There was a problem hiding this comment.
+1, agreed on moving it. Thank you.
/wait
Signed-off-by: Weston Carlson <wez470@gmail.com>
Signed-off-by: Weston Carlson <wez470@gmail.com>
|
/azp run envoy-presubmit |
|
Commenter does not have sufficient privileges for PR 10845 in repo envoyproxy/envoy |
|
@htuch @mattklein123 @alyssawilk ready for another look |
Description: This PR creates a common PROXY protocol config API message. Currently it will be used for @alyssawilk's CONNECT work as well as in the transport socket for my upstream proxy proto work. This message could be extended to include TLVs in the future.
Risk Level: Low
Testing: None
Docs Changes: None
Release Notes: None
Discussed in: #10682 (my draft PR to discuss the upstream implementation)
Wasn't 100% sure where the best place to stick this would be but I put it in types for now.