util: add PROXY protocol generation functions#10548
util: add PROXY protocol generation functions#10548alyssawilk merged 9 commits intoenvoyproxy:masterfrom wez470:proxy-protocol-header-generation
Conversation
Signed-off-by: Weston Carlson <[email protected]>
|
|
||
| // See https://github.com/haproxy/haproxy/blob/master/doc/proxy-protocol.txt for definitions | ||
|
|
||
| constexpr char PROXY_PROTO_V1_SIGNATURE[] = "PROXY "; |
There was a problem hiding this comment.
These constants overlap with ones created by the proxy proto listener filter. I opted to not refactor the listener filter to use these common ones for now to keep this PR simple. I think I can do that in a future PR. Willing to do it here if requested though.
There was a problem hiding this comment.
How about a TODO so we don't lose track?
|
/retest |
|
🐴 hold your horses - no failures detected, yet. |
|
I saw the "envoy-linux (bazel clang_tidy)" build fail because it failed to fetch a repo. Thought I could re-run that with the retest thing but I guess not. |
Signed-off-by: Weston Carlson <[email protected]>
alyssawilk
left a comment
alyssawilk
left a comment
There was a problem hiding this comment.
This is awesome! Thanks for splitting it out :-)
One other thing would be good here, is having a unit test regression test that the header we generate is one the listener filter code can parse. I think it'd be better to land with this PR, but I'm fine with a TODO as long as you land it before landing any config using this utility.
| namespace Common { | ||
| namespace ProxyProtocol { | ||
|
|
||
| void generate_v1_header(const std::string& src_addr, const std::string& dst_addr, uint32_t src_port, |
There was a problem hiding this comment.
per style guide let's swap these from snake case towards generateV1Header etc.
|
|
||
| // See https://github.com/haproxy/haproxy/blob/master/doc/proxy-protocol.txt for definitions | ||
|
|
||
| constexpr char PROXY_PROTO_V1_SIGNATURE[] = "PROXY "; |
There was a problem hiding this comment.
How about a TODO so we don't lose track?
|
Thanks for the feedback! I'll move the regression tests over to this branch 🙂. |
Signed-off-by: Weston Carlson <[email protected]>
Signed-off-by: Weston Carlson <[email protected]>
Signed-off-by: Weston Carlson <[email protected]>
Signed-off-by: Weston Carlson <[email protected]>
Signed-off-by: Weston Carlson <[email protected]>
Signed-off-by: Weston Carlson <[email protected]>
|
LGTM pending CI cleanup :-) |
|
@mattklein123 or @zuercher @snowp any of you up for second pass? |
Signed-off-by: Weston Carlson <[email protected]>
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Nice, just one small drive by comment post-submit. cc @alyssawilk
| void generateV1Header(const std::string& src_addr, const std::string& dst_addr, uint32_t src_port, | ||
| uint32_t dst_port, Network::Address::IpVersion ip_version, | ||
| Buffer::Instance& out) { | ||
| std::ostringstream stream; |
There was a problem hiding this comment.
Sorry late drive by: my (possibly out of date) understanding is that string streams are pretty slow. We probably would want to move this to an envoy buffer or similar in a later perf pass.
3 participants
Description: Add utility functions for generating v1 and v2 PROXY protocol headers. This is the first step to adding Envoy PROXY proto generation support. These functions can also be used in other places that would also like to generate the header.
Risk Level: Low
Testing: Unit
Docs Changes: None
Release Notes: None
Related to #1031
@alyssawilk