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

feat: add optional appProtocol field on Service ports #858

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

baarde
Copy link
Contributor

@baarde baarde commented Jun 2, 2023

What does this PR do?

This PR enables the user to specify an optional value for the .spec.ports[*].appProtocol field.

Motivation

The appProtocol field may be set on a Service as a hint for implementations to offer richer behavior for protocols that they understand. For example, this field is used by the GKE Gateway controller to enable TLS between the load balancer and the backend.

Use case: I want to deploy a Google External HTTP(S) load balancer (so I can use Cloud Armor) and then forward the traffic over HTTPS (so it remains encrypted) to Traefik (so I can use Traefik).

Compatibility

appProtocol is available as an optional feature in Kubernetes 1.18, enabled by default in Kubernetes 1.19, and general available since Kubernetes 1.20.

Setting a value for ports.*.appProtocol breaks the compatibility with older version of Kubernetes. Therefore, no default value is set.

More

  • Yes, I updated the tests accordingly
  • Yes, I ran make test and all the tests passed

@mloiseleur mloiseleur added the kind/enhancement New feature or request label Jun 2, 2023
@mloiseleur mloiseleur changed the title Add appProtocol field feat: add optional appProtocol field on Service ports Jun 2, 2023
Copy link
Contributor

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, @baarde !
LGTM

@mloiseleur
Copy link
Contributor

Current example on GCP is not covering this use case.
Do you think you can add a working example with this setup ?

Copy link
Contributor

@darkweaver87 darkweaver87 left a comment

Choose a reason for hiding this comment

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

LGTM

Since Kubernetes 1.20, the `.spec.ports[*].appProtocol` field may be set on a Service as a hint for implementations to offer richer behavior for protocols that they understand. For example, this field is used by the GKE Gateway controller to enable TLS between the load balancer and the backend.
@traefiker traefiker merged commit 795478d into traefik:master Jun 6, 2023
@baarde
Copy link
Contributor Author

baarde commented Jun 7, 2023

Thank you @mloiseleur.

Current example on GCP is not covering this use case.
Do you think you can add a working example with this setup ?

Here is an example of my setup:

ports:
  websecure:
    appProtocol: HTTPS # Hint for Google L7 load balancer
service:
  type: NodePort
extraObjects:
- apiVersion: gateway.networking.k8s.io/v1beta1
  kind: Gateway
  metadata:
    name: traefik
    annotations:
      networking.gke.io/certmap: "myCertificateMap"
  spec:
    gatewayClassName: gke-l7-global-external-managed
    addresses:
    - type: NamedAddress
      value: "myGlobalIPName"
    listeners:
    - name: https
      protocol: HTTPS
      port: 443
- apiVersion: gateway.networking.k8s.io/v1beta1
  kind: HTTPRoute
  metadata:
    name: traefik
  spec:
    parentRefs:
    - kind: Gateway
      name: traefik
    rules:
    - backendRefs:
      - name: traefik
        port: 443
- apiVersion: networking.gke.io/v1
  kind: HealthCheckPolicy
  metadata:
    name: traefik
  spec:
    default:
      config:
        type: HTTP
        httpHealthCheck:
          port: 9000
          requestPath: /ping
    targetRef:
      group: ""
      kind: Service
      name: traefik

It's very verbose (and I've kept out HTTP-to-HTTPS redirection). Unfortunately, I don't think I can make it more compact: Google External HTTP(S) load balancer insists on performing their own health check instead of relying on the Pod's readiness.

If you think that would be helpful, I'll create a new PR to add it.

Note: To anyone using the GKE Gateway controller, make sure you haven't enabled IPv6 on the Service. It's not supported yet. I lost several hours failing to understand why the NEG controller wasn't attaching the Pod's IPs to my network endpoint group.

Edit: Fixed a typo and an indentation error.

@mloiseleur
Copy link
Contributor

\o Thanks for this.
Yes, I think that would be helpful !

baarde added a commit to baarde/traefik-helm-chart that referenced this pull request Jun 8, 2023
This example makes use of the `ports.*.appProtocol` value introduced in traefik#858.

Notes:
* This example assumes that a certificate map named `myCertificateMap` exists.
* This example assumes that a global static external IP address named `myGlobalIPName` has been registered.
* This is a minimal example: HTTP-to-HTTPS redirection is left as an exercise for the reader.
@baarde baarde deleted the app-protocol branch June 8, 2023 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants