Add support for more PP fields#12864
Conversation
|
autest filter_body failed |
|
[approve ci autest 0] |
|
Needs #12863 to make the new fields work. |
There was a problem hiding this comment.
Pull request overview
Adds support for PROXY Protocol v2 SSL TLV subtypes (cipher and TLS version) and exposes them as new access log fields, enabling operators to log the upstream TLS details provided by a load balancer.
Changes:
- Add
ProxyProtocol::get_tlv_ssl_cipher()/get_tlv_ssl_version()to extract SSL sub-TLV values. - Add new LogAccess marshalers and register new log fields
pptc/pptv. - Extend Proxy Protocol parser unit tests and update admin logging field documentation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/iocore/net/ProxyProtocol.cc | Implements SSL sub-TLV parsing helpers and getters for TLS cipher/version. |
| include/iocore/net/ProxyProtocol.h | Declares new public getters and a private helper for SSL sub-TLV parsing. |
| src/proxy/logging/LogAccess.cc | Adds marshalers for the new Proxy Protocol TLS cipher/version log fields. |
| include/proxy/logging/LogAccess.h | Declares new LogAccess marshal methods for the new fields. |
| src/proxy/logging/Log.cc | Registers two new logging fields (pptc, pptv) mapped to the new marshalers. |
| src/iocore/net/unit_tests/test_ProxyProtocol.cc | Extends TLV parsing test coverage to include SSL cipher/version subtypes. |
| doc/admin-guide/logging/formatting.en.rst | Documents the new log field symbols. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0b343d0 to
2aa259f
Compare
730640d to
dd36d63
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bryancall
left a comment
There was a problem hiding this comment.
LGTM. Clean implementation — the sub-TLV parser handles bounds checking well and the client flag check before scanning is a nice touch. Tests cover both the happy path and the no-SSL case. Log field naming (pptc/pptv) is consistent with the existing pps/ppd/ppa pattern.
This adds support for
PP2_SUBTYPE_SSL_CIPHERandPP2_SUBTYPE_SSL_VERSION, and log fields for those.