-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tls: separate out cert validation logic from ContextImpl #14757
tls: separate out cert validation logic from ContextImpl #14757
Conversation
Signed-off-by: mathetake <[email protected]>
38907b4
to
7004480
Compare
Overall this looks good, the coverage is missing from some error config part: were them covered before? |
looks like they haven't been covered before as well :https://storage.googleapis.com/envoy-pr/e1ca2ab/coverage/source/extensions/transport_sockets/tls/context_impl.cc.gcov.html but some of them are formatted into multiple lines in this PR plus the file size becomes smaller so I think that is why the coverage goes down in terms of the number of lines covered 😕 |
/retest |
Retrying Azure Pipelines: |
@lizan added the coverage exception. PTAL |
@ggreenway for non-tetrands review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
port of tls: separate out cert validation logic from ContextImpl (envoyproxy#14757)
The first step towards #14614. Ref #14614 (comment)
Note that the change is almost moving codes from
context_impl
tocert_validator/**
with a bit of necessary tweaks.Commit Message: tls: separate out cert validation logic from ContextImpl
Additional Description: In order to create the extension point for certificate validation, separated out the cert validation logics from Tls::ContextImpl, and move them to Tls::DefaultCertValidator which implements Tls::CertValidator interface. Ref: #14614 and #9284
Risk Level: low (almost code moves)
Testing: unittest
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
cc @lizan