-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add support for enabling connect-based ingress TLS per listener. #11163
Conversation
|
||
return &mergedCfg, nil | ||
} | ||
|
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.
This is just a straight move from the ingress file - I missed this ingress-specific function when refactoring into a separate file in the last PR.
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.
Not Quite true anymore - I refactored out the extra lookup since the caller already has listenerCfg
per RB's suggestion. But other than that one minor change this is not new code just moved from the other file!
b775e86
to
985f93b
Compare
985f93b
to
d65bb33
Compare
@rboyer thanks for taking a look! I made the refactor you suggested let me know if there's anything else you can see blocking a merge! |
@@ -0,0 +1,120 @@ | |||
{ |
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.
You should probably rebase and make envoy-regen
since @eculver did an envoy version upgrade since you branched..
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 but needs a rebase
0f63311
to
c891f30
Compare
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/484178. |
This adds support for a case I missed in #10903.
This is technically new functionality not directly related to SDS so I'll add a separate changelog entry but with the addition of nested TLS blocks in more places it makes sense for the fields there to actually function in a reasonable way.
This behaviour was proposed in the RFC. It adds the ability to use the built-in Connect TLS for only a subset of Ingress listeners which might be useful to some people but mostly is to avoid confusing disparity between built-in TLS being all or nothing while SDS is more fine-grained.