network ext_authz filter: support include_tls_session option from http variant #33105
Conversation
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
…p variant Signed-off-by: Adam Anderson <[email protected]>
AdamEAnderson
force-pushed
the
network-authz-tls-details
branch
from
781b062
to
f1c4b79
Compare
|
CI currently failing due to flakes/failures not related to this PR. I will merge from main and rerun everything once I have a review to get things working again. |
| @@ -242,6 +242,15 @@ void CheckRequestUtils::createTcpCheck( | |||
| include_peer_certificate); | |||
| setAttrContextPeer(*attrs->mutable_destination(), cb->connection(), server_name, true, | |||
| include_peer_certificate); | |||
| if (include_tls_session) { | |||
| if (cb->connection().ssl() != nullptr) { | |||
There was a problem hiding this comment.
Please extract this into a helper function called from both http and network paths, so that if more tls session fields are added later, they are added for both the network and http variants. (Yes, it will be a 4 line function for now).
There was a problem hiding this comment.
yep, that makes sense
Signed-off-by: Adam Anderson <[email protected]>
|
/lgtm api |
|
Can you please merge main? There was a build break yesterday that you need the fix for. |
|
/wait |
…rk-authz-tls-details Signed-off-by: Adam Anderson <[email protected]>
@ggreenway merged main and most stuff works, but we're failing per-extension code coverage for extensions I haven't touched |
Looking into it; when it's fixed, you'll need to merge main one more time. |
|
The coverage issue should be fixed in main now; please merge main once more and hopefully CI will now pass. /wait |
…rk-authz-tls-details Signed-off-by: Adam Anderson <[email protected]>
…p variant (envoyproxy#33105) Signed-off-by: Adam Anderson <[email protected]>
|
I created envoyproxy/go-control-plane#936 to have this added to the |
Commit Message: network ext_authz filter: support include_tls_session option from http variant
Additional Description: the http ext_authz filter supports including tls session details in the check request. This PR ports that functionality over to the network ext_authz filter using the exact same mechanisms and the same field in the check request.
Risk Level: low
Testing: unit tests included
Docs Changes: docs are included in the protodoc for api changes. Slightly altered the protodoc for the check request attribute to link to both filters' config.
Release Notes: included as a new feature
Platform Specific Features: n/a