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.

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 TLS Passthrough #2041

Merged
merged 17 commits into from
Dec 16, 2021
Merged

Add support for TLS Passthrough #2041

merged 17 commits into from
Dec 16, 2021

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Nov 30, 2021

What this PR does / why we need it:
Adds support for TLS Passthrough by not doing much of anything. Turns out we already run the route override() function on all routes, regardless of what kind of Ingress-like they came from, so editing the allowed protocols string and slapping a konghq.com/protocols annotation on a TCPIngress is sufficient.

This mostly adds tests:

  • Several invalid TCPIngress unit tests
  • Regular TLS TCPIngress integration with multiple TCPIngresses differentiated by SNI
  • TLS passthrough integration

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #2023
fixes #1179

Special notes for your reviewer:

This is a draft because Kong 2.7 is not yet released, and current versions of Kong do not accept the tls_passthrough protocol. It includes a temporary commit to use a nightly and use a modified version of go-kong that doesn't break on nightly release version strings. Once 2.7 is released, we can remove this.

This will also require a new release of KTF that includes Kong/kubernetes-testing-framework@276b0c6

passthrough_test.txt shows a single test run.

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@rainest rainest requested a review from a team as a code owner November 30, 2021 21:55
@rainest rainest marked this pull request as draft November 30, 2021 21:55
@github-actions
Copy link

Licenses differ between commit 63f2b68 and base:

+++ pr_licenses.csv	2021-11-30 21:57:05.087436178 +0000
@@ -8,7 +8,12 @@
 github.com/bombsimon/logrusr,https://github.com/bombsimon/logrusr/blob/master/LICENCE,MIT
 github.com/cenkalti/backoff/v4,https://github.com/cenkalti/backoff/blob/master/v4/LICENSE,MIT
 github.com/cespare/xxhash/v2,https://github.com/cespare/xxhash/blob/master/v2/LICENSE.txt,MIT
+github.com/containerd/containerd,https://github.com/containerd/containerd/blob/master/LICENSE,Apache-2.0
 github.com/davecgh/go-spew/spew,https://github.com/davecgh/go-spew/blob/master/spew/LICENSE,ISC
+github.com/docker/distribution,https://github.com/docker/distribution/blob/master/LICENSE,Apache-2.0
+github.com/docker/docker,https://github.com/docker/docker/blob/master/LICENSE,Apache-2.0
+github.com/docker/go-connections,https://github.com/docker/go-connections/blob/master/LICENSE,Apache-2.0
+github.com/docker/go-units,https://github.com/docker/go-units/blob/master/LICENSE,Apache-2.0
 github.com/evanphx/json-patch,https://github.com/evanphx/json-patch/blob/master/LICENSE,BSD-3-Clause
 github.com/evanphx/json-patch/v5,https://github.com/evanphx/json-patch/blob/master/v5/LICENSE,BSD-3-Clause
 github.com/fatih/color,https://github.com/fatih/color/blob/master/LICENSE.md,MIT
@@ -39,6 +44,8 @@
 github.com/mitchellh/reflectwalk,https://github.com/mitchellh/reflectwalk/blob/master/LICENSE,MIT
 github.com/modern-go/concurrent,https://github.com/modern-go/concurrent/blob/master/LICENSE,Apache-2.0
 github.com/modern-go/reflect2,https://github.com/modern-go/reflect2/blob/master/LICENSE,Apache-2.0
+github.com/opencontainers/go-digest,https://github.com/opencontainers/go-digest/blob/master/LICENSE,Apache-2.0
+github.com/opencontainers/image-spec/specs-go,https://github.com/opencontainers/image-spec/blob/master/specs-go/LICENSE,Apache-2.0
 github.com/pkg/errors,https://github.com/pkg/errors/blob/master/LICENSE,BSD-2-Clause
 github.com/prometheus/client_golang/prometheus,https://github.com/prometheus/client_golang/blob/master/prometheus/LICENSE,Apache-2.0
 github.com/prometheus/client_model/go,https://github.com/prometheus/client_model/blob/master/go/LICENSE,Apache-2.0```

Travis Raines added 2 commits December 8, 2021 17:09
Add an integration test feature to look up a test Kong instance's
version via the admin API.

Skip the TLS Passthrough test on versions older than 2.7.0.
@rainest
Copy link
Contributor Author

rainest commented Dec 9, 2021

Attached are archived runs of integration tests before I reverted the nightly hack and added the version gate. The version gate skips the new integration tests on current versions:

logs_20332-pg.zip
logs_20332.zip

Enterprise was failing because of #2070 but wasn't of interest because I didn't have an Enterprise nightly to test against.

@rainest rainest temporarily deployed to Configure ci December 10, 2021 18:45 Inactive
@rainest rainest marked this pull request as ready for review December 10, 2021 18:58
@rainest rainest temporarily deployed to Configure ci December 10, 2021 18:59 Inactive
Copy link
Contributor

@mflendrich mflendrich left a comment

Choose a reason for hiding this comment

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

LGTM; I just think that suppressing tests should be opt-in (rather than the default) if obtaining the Kong semver fails. See the attached comment.

test/integration/tcpingress_test.go Show resolved Hide resolved
Run version-dependent tests regardless if the version is unknown.

Add a TEST_KONG_VERSION_OVERRIDE envvar to manually specify the Kong
version.
@rainest rainest temporarily deployed to Configure ci December 14, 2021 23:26 Inactive
@rainest rainest enabled auto-merge (squash) December 14, 2021 23:28
@rainest rainest temporarily deployed to Configure ci December 14, 2021 23:39 Inactive
@rainest rainest temporarily deployed to Configure ci December 15, 2021 21:31 Inactive
@rainest rainest temporarily deployed to Configure ci December 15, 2021 21:45 Inactive
@rainest rainest temporarily deployed to Configure ci December 16, 2021 19:05 Inactive
@rainest rainest temporarily deployed to Configure ci December 16, 2021 19:21 Inactive
mflendrich
mflendrich previously approved these changes Dec 16, 2021
Copy link
Contributor

@mflendrich mflendrich 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! A few minor bits, otherwise good to go 👍

test/integration/tcpingress_test.go Outdated Show resolved Hide resolved
test/integration/utils_test.go Outdated Show resolved Hide resolved
test/integration/utils_test.go Show resolved Hide resolved
test/integration/tcpingress_test.go Show resolved Hide resolved
@mflendrich mflendrich temporarily deployed to Configure ci December 16, 2021 21:36 Inactive
@rainest rainest merged commit a7caead into main Dec 16, 2021
@rainest rainest deleted the feat/tlspass branch December 16, 2021 21:50
@mflendrich mflendrich temporarily deployed to Configure ci December 16, 2021 22:13 Inactive
@mflendrich mflendrich temporarily deployed to Configure ci December 16, 2021 22:27 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support protocol selection for TCPIngress TCPIngress Tests
2 participants