access log: add CONNECTION_TERMINATION_DETAILS operator#13305
access log: add CONNECTION_TERMINATION_DETAILS operator#13305mattklein123 merged 6 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
alyssawilk
left a comment
There was a problem hiding this comment.
Cool, I think this will be useful for L4 debug.
I think keeping both L4 and L7 makes sense to me - given ordering I wouldn't want say an HTTP invalid header response (which closes the connection) get overwritten by L4 details (which would include the local close, and maybe if all the data was flushed or not)
| This filter also supports policy in both enforcement and shadow modes. Shadow mode won't effect real | ||
| users, it is used to test that a new set of policies work before rolling out to production. | ||
|
|
||
| When a request is denied, the :ref:`RESPONSE_DETAILS<config_access_log_format_response_details>` |
There was a problem hiding this comment.
How do we know it's the RBAC filter setting this? I.e. don't we both need to know the response details and which network filter sets this?
There was a problem hiding this comment.
The RBAC filter will set it in the format of rbac_access_denied_matched_policy[policy_name], this should tell which filter sets it. Also updated the doc to make this more clear.
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
alyssawilk
left a comment
There was a problem hiding this comment.
Looks great - just 2 minor nits and you're good to go!
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
|
Can you merge main and take a look at CI? Not sure what is going on there, but seems related to a recent commit. /wait |
|
//test/common/http:conn_manager_impl_test is currently failing at head |
|
Yup sorry just realized that. Let's wait until that is fixed and then I can take a final pass. |
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
mattklein123
left a comment
There was a problem hiding this comment.
A few questions/comments. Thank you!
/wait
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
mattklein123
left a comment
There was a problem hiding this comment.
Thanks LGTM with one nit.
/wait
Signed-off-by: Yangmin Zhu <ymzhu@google.com>
Follow up of #12912 (comment) for adding a new operator for network filter instead of using RESPONSE_CODE_DETAILS.
I actually wonder would it be better to also use RESPONSE_DETAILS for HTTP to deprecate the RESPONSE_CODE_DETAILS so that the user can use the same operator for both HTTP and TCP? cc @alyssawilkcc @mandarjog
Signed-off-by: Yangmin Zhu ymzhu@google.com
For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Commit Message:
Additional Description:
Risk Level: Low
Testing: Unit tests and integration tests
Docs Changes: Updated
Release Notes: Updated
Issue: #12873