-
Notifications
You must be signed in to change notification settings - Fork 5.1k
forward proxy: per host SNI and SAN verification #7466
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
Conversation
1) Implement auto SAN/SNI setting on a per-host basis. 2) Cleanup some v1 config translation. Part of #1606 Signed-off-by: Matt Klein <mklein@lyft.com>
@PiotrSikora @lizan I want to improve test coverage a bit, but can you give me some early feedback on the TLS code when you get a chance? cc @alyssawilk |
Signed-off-by: Matt Klein <mklein@lyft.com>
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.
I'm happy with the overall plan - having a wrapper class feels a bit heavyweight but I think it allows for maximal code reuse and code reuse makes me happy :-)
test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc
Show resolved
Hide resolved
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
@alyssawilk updated per comments. I regenerated all of the upstream certs because they all need to be signed by the CA. I could probably hack up the script by hand to reuse the existing CA cert, but I'm not sure the effort is worth it. LMK. |
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
@lizan thanks for the great suggestion. This is much better! PTAL, cc @PiotrSikora also. |
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.
Great! LGTM.
Part of #1606
Risk Level: Low
Testing: Integration test
Docs Changes: Done
Release Notes: N/A