Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add ssl-passthrough implementation support #28751

Merged
merged 6 commits into from
Oct 30, 2023
Merged

Add ssl-passthrough implementation support #28751

merged 6 commits into from
Oct 30, 2023

Conversation

youngnick
Copy link
Contributor

@youngnick youngnick commented Oct 24, 2023 •
edited by joestringer
Loading

This PR adds a new ingress.cilium.io/ssl-passthrough annotation for Ingress objects, and support for handling them, along with some tests to cover this, and associated docs.

Updates #20960

Added new `ingress.cilium.io/ssl-passthrough` annotation for Ingress objects

@youngnick youngnick requested review from a team as code owners October 24, 2023 06:00
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 24, 2023
@youngnick youngnick added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Oct 24, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 24, 2023
@youngnick youngnick force-pushed the ssl-passthrough branch 2 times, most recently from 642f28f to 3ff5b01 Compare October 24, 2023 07:37
qmonnet
qmonnet requested changes Oct 24, 2023
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've one request regarding formatting, please see below.

I'd also recommend folding the whitespace fixes from last commit into the relevant, earlier commits.

Documentation/network/servicemesh/ingress.rst Outdated Show resolved Hide resolved
@youngnick
Copy link
Contributor Author

Fixed a couple more things, I think this is reviewable again now.

qmonnet
qmonnet approved these changes Oct 26, 2023
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

@qmonnet
Copy link
Member

qmonnet commented Oct 26, 2023

/test

@youngnick
Copy link
Contributor Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 27, 2023
@dylandreimerink
Copy link
Member

@meyskens are you happy this this PR? Asking since this PR is mainly cilium/sig-servicemesh focused and we have no official approval

meyskens
meyskens reviewed Oct 29, 2023
View reviewed changes
operator/pkg/ingress/annotations/annotations.go Outdated
@@ -16,6 +16,7 @@ const (
ServiceTypeAnnotation = annotation.IngressPrefix + "/service-type"
InsecureNodePortAnnotation = annotation.IngressPrefix + "/insecure-node-port"
SecureNodePortAnnotation = annotation.IngressPrefix + "/secure-node-port"
SSLPassthroughAnnotation = annotation.IngressPrefix + "/tls-passthrough"
Copy link
Member

@meyskens meyskens Oct 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we still call this SSLPassthroughAnnotation while the value is TLS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, renamed.

@youngnick youngnick force-pushed the ssl-passthrough branch 2 times, most recently from d971665 to db8ed89 Compare October 30, 2023 05:17
@youngnick
Add ssl-passthrough implementation support
a7d6d71 
Signed-off-by: Nick Young <nick@isovalent.com>
@youngnick
Add ingestion support and test cases
f4a8674 
Signed-off-by: Nick Young <nick@isovalent.com>
@youngnick
Add support for actually handling passthrough Ingress
90e4e91 
Signed-off-by: Nick Young <nick@isovalent.com>
@youngnick
Add documentation and use 'enabled' as well
dc16660 
Signed-off-by: Nick Young <nick@isovalent.com>
@youngnick
Fix inadvertent CEC name change in dedicated ingress translator
6640bbb 
Signed-off-by: Nick Young <nick@isovalent.com>
@youngnick
Rename annotation from ssl-passthrough to tls-passthrough
bc4313d 
Signed-off-by: Nick Young <nick@isovalent.com>
@youngnick
Copy link
Contributor Author

/test

@aanm aanm requested a review from meyskens October 30, 2023 08:33
@aanm aanm removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 30, 2023
meyskens
meyskens approved these changes Oct 30, 2023
View reviewed changes
Copy link
Member

@meyskens meyskens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@meyskens meyskens added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 30, 2023
@dylandreimerink dylandreimerink merged commit 5f16fc1 into cilium:main Oct 30, 2023
62 checks passed
@sayboras sayboras added the affects/v1.15 This issue affects v1.15 branch label Feb 27, 2024
@YutaroHayakawa YutaroHayakawa added needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch backport/author The backport will be carried out by the author of the PR. and removed affects/v1.15 This issue affects v1.15 branch labels Feb 28, 2024
@aanm aanm added affects/v1.15 This issue affects v1.15 branch and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch affects/v1.15 This issue affects v1.15 branch backport/author The backport will be carried out by the author of the PR. labels Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meyskens meyskens meyskens approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees
No one assigned
Labels
feature/k8s-ingress ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@youngnick @qmonnet @dylandreimerink @meyskens @aanm @sayboras @YutaroHayakawa