Skip to content

TLS Passthrough support#402

Merged
danehans merged 30 commits intoenvoyproxy:mainfrom
chauhanshubham:tls-passthrough-shchauhan
Oct 11, 2022
Merged

TLS Passthrough support#402
danehans merged 30 commits intoenvoyproxy:mainfrom
chauhanshubham:tls-passthrough-shchauhan

Conversation

@chauhanshubham
Copy link
Member

@chauhanshubham chauhanshubham commented Sep 21, 2022

This commit adds a tlsroute controller which is further used to
configure tls passthrough in envoy.

Testing Done

  1. Deployed the nginx application as mentioned here: https://istio.io/latest/docs/tasks/traffic-management/ingress/ingress-sni-passthrough/

  2. Applied gateway configs as follows

apiVersion: gateway.networking.k8s.io/v1alpha2
kind: Gateway
metadata:
  name: eg
spec:
  gatewayClassName: eg
  listeners:
    - name: tls
      protocol: TLS
      tls:
        mode: Passthrough
      port: 8080
apiVersion: gateway.networking.k8s.io/v1alpha2
kind: TLSRoute
metadata:
  name: tlsroute
spec:
  parentRefs:
    - name: eg
  hostnames:
    - "nginx.example.com"
  rules:
    - backendRefs:
        - group: ""
          kind: Service
          name: my-nginx
          port: 443
          weight: 1
  1. To test traffic
$ kubectl -n envoy-gateway-system port-forward service/envoy-default-eg 8888:8080
$ curl -v --resolve nginx.example.com:8888:127.0.0.1 https://nginx.example.com:8888 --cacert testing/tls-passthrough/example.com.crt
* Added nginx.example.com:8888:127.0.0.1 to DNS cache
* Hostname nginx.example.com was found in DNS cache
*   Trying 127.0.0.1:8888...
* Connected to nginx.example.com (127.0.0.1) port 8888 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*  CAfile: personal/tls-passthrough/example.com.crt
*  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* ... **collapsed for brevity**
* TLSv1.2 (IN), TLS handshake, Finished (20):
* Server certificate:
*  subject: CN=nginx.example.com; O=some organization
*  start date: Oct  8 05:48:33 2022 GMT
*  expire date: Oct  8 05:48:33 2023 GMT
*  common name: nginx.example.com (matched)
*  issuer: O=example Inc.; CN=example.com
*  SSL certificate verify ok.
> GET / HTTP/1.1
> Host: nginx.example.com:8888
> User-Agent: curl/7.77.0
> Accept: */*
>
... **collapsed for brevity**
<title>Welcome to nginx!</title>
... **collapsed for brevity**
* Connection #0 to host nginx.example.com left intact

Fixes #168
Signed-off-by: Shubham Chauhan shubham@tetrate.io

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2022

Codecov Report

Merging #402 (88d2df3) into main (b8f234b) will decrease coverage by 0.22%.
The diff coverage is 63.64%.

@@            Coverage Diff             @@
##             main     #402      +/-   ##
==========================================
- Coverage   62.72%   62.49%   -0.23%     
==========================================
  Files          42       45       +3     
  Lines        4496     5301     +805     
==========================================
+ Hits         2820     3313     +493     
- Misses       1532     1798     +266     
- Partials      144      190      +46     
Impacted Files Coverage Δ
internal/cmd/server.go 7.75% <0.00%> (-0.14%) ⬇️
internal/ir/infra.go 67.41% <ø> (ø)
internal/ir/zz_generated.deepcopy.go 0.00% <0.00%> (ø)
internal/provider/kubernetes/kubernetes.go 53.48% <0.00%> (-4.02%) ⬇️
internal/status/status.go 0.00% <0.00%> (ø)
internal/gatewayapi/helpers_v1alpha2.go 28.88% <28.88%> (ø)
internal/gatewayapi/helpers.go 67.52% <40.00%> (+0.85%) ⬆️
internal/gatewayapi/runner/runner.go 53.77% <42.85%> (-0.78%) ⬇️
internal/xds/translator/translator.go 72.04% <60.00%> (-2.65%) ⬇️
internal/provider/kubernetes/tlsroute.go 60.56% <60.56%> (ø)
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chauhanshubham chauhanshubham force-pushed the tls-passthrough-shchauhan branch 3 times, most recently from 4d2bca9 to 56f56db Compare September 22, 2022 17:05
@chauhanshubham chauhanshubham force-pushed the tls-passthrough-shchauhan branch 2 times, most recently from edc36cc to 2114d5a Compare September 24, 2022 05:31
@chauhanshubham chauhanshubham marked this pull request as ready for review September 24, 2022 06:06
@chauhanshubham chauhanshubham requested a review from a team as a code owner September 24, 2022 06:06
@chauhanshubham
Copy link
Member Author

chauhanshubham commented Sep 24, 2022

This requires two one more no thing

  1. adding tlsroute status updates to be implemented, similar to httproutes in send resource statuses using watchable #395
  2. update tlsroute controller to listen and validate gateway parent Adds Gateway Watch to HTTPRoute Controller #403
    I can take these up separately if this PR is getting too long

Above tasks were added as part of this PR
Minor pending action items include,

  1. including tlsroute in demo profile
  2. doc updates

Copy link
Contributor

@danehans danehans left a comment

Choose a reason for hiding this comment

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

@chauhanshubham the Kube provider RBAC needs to be updated for TLSRoutes.

@chauhanshubham chauhanshubham force-pushed the tls-passthrough-shchauhan branch from 9ad67d4 to 3f1b654 Compare September 26, 2022 19:27
@chauhanshubham chauhanshubham force-pushed the tls-passthrough-shchauhan branch from 3f1b654 to 075b7db Compare September 27, 2022 16:13
@danehans danehans added this to the 0.3.0-rc1 milestone Sep 27, 2022
@chauhanshubham chauhanshubham marked this pull request as draft October 5, 2022 14:52
@chauhanshubham chauhanshubham force-pushed the tls-passthrough-shchauhan branch 2 times, most recently from 11d0e32 to 4b4f062 Compare October 5, 2022 14:54
Copy link
Contributor

@danehans danehans left a comment

Choose a reason for hiding this comment

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

One nit. Otherwise, can you create tlsroute integration test similar to this?

@arkodg arkodg modified the milestones: 0.3.0-rc.1, 0.2.0 Oct 6, 2022
@chauhanshubham chauhanshubham marked this pull request as ready for review October 7, 2022 14:36
@chauhanshubham chauhanshubham force-pushed the tls-passthrough-shchauhan branch from 4478f40 to 04ac43d Compare October 7, 2022 19:20
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>

testfix

Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
@chauhanshubham chauhanshubham force-pushed the tls-passthrough-shchauhan branch from 5c7b8fb to c09d71c Compare October 10, 2022 17:35
Signed-off-by: Shubham Chauhan <shubham@tetrate.io>
if !found {
r.resources.Namespaces.Delete(request.Namespace)
log.Info("deleted namespace from resource map")
r.resources.Services.Delete(request.NamespacedName)
Copy link
Member Author

@chauhanshubham chauhanshubham Oct 10, 2022

Choose a reason for hiding this comment

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

This attempts to delete a possibly non-existent Service, and not the relevant ones. It is introducing an issue in the tlsroute controller, which was present for httproute controller too. I have filed an issue for this here #536

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for spending the extra cycles to get TLS Passthrough support in for 0.2.0 !

@danehans danehans merged commit b9ed6df into envoyproxy:main Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add support for TLSRoute / TLS listener protocol / TLS passthrough

4 participants