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

"kubernetesIngress" provider does not detect protocol for backend pods correctly #5164

Closed
1 of 2 tasks
seh opened this issue Jul 30, 2019 · 5 comments
Closed
1 of 2 tasks

Comments

@seh
Copy link
Contributor

seh commented Jul 30, 2019

Do you want to request a feature or report a bug?

Bug

Did you try using a 1.7.x configuration for the version 2.0?

  • Yes
  • No

What did you do?

Per the discussion in the Discourse "Traefik v2" topic Kubernetes Ingress annotation doesn't force HTTPS to backend pods, the three ways supported by earlier versions of Traefik to indicate that a Kubernetes Ingress object requires HTTPS communication don't work properly in Traefik version 2 (as of version 2.0.0-beta1). The only method that works as advertised is detecting that a Kubernetes Service port has a name and the name starts with "https."

Suppose you have a Kubernetes Ingress object pointing at a Service for which the backing server pods speak HTTPS. You want Traefik to use HTTPS for its proxy connections to these upstream servers. If your Service port either lacks a name or has a name that doesn't start with "https," you want to fall back to one of the other two methods that used to work in earlier versions of Traefik.

First, the Ingress annotation "ingress.kubernetes.io/protocol" with a value of "https" would allow you to request use of HTTPS regardless of your Service ports; neither the name nor the port value should matter. Traefik no longer honors this annotation.

However, if your Service port—not the target port on the backing pod container, but rather the port on which the Service listens to receive connections—is 443, and if that port is selected by the Ingress rule, then Traefik should use HTTPS. Traefik tries here, but gets the implementation wrong.

What did you expect to see?

An Ingress object annotated with a key of "ingress.kubernetes.io/protocol" and a value of "https" pointing at a Service port that lacks a name—or has a name that doesn't start with "https"—should use HTTPS.

Alternately, an Ingress object pointing at a Service port that's not named but has a value of 443—regardless of the target port used by the selected pods—should use HTTPS.

What did you see instead?

Traefik both ignores the aforementioned annotation and looks at the wrong port to decide whether it sees 443; it looks at the Endpoints object's ports, which are the target pod's ports, and not the ports used by the Service sitting in front of those pods.

Output of traefik version: (What version of Traefik are you using?)

v2 Beta 1

What is your environment & configuration (arguments, toml, provider, platform, ...)?

The "kubernetesIngress" provider is active.

Detailed Problem Analysis

Let's review the first Ingress inspection predicate, as written for the previous version of Traefik:

If the service port defined in the ingress spec is 443 (note that you can still use targetPort to use a different port on your pod)

At present, the documentation for Traefik version 2's "kubernetesIngress" provider makes no such promise, but the code is adapted from the earlier version of Traefik, and is clearly trying to implement this promise.

In file pkg/provider/kubernetes/ingress/kubernetes.go, rather than inspecting the EndpointPort.Port field at line 228, we should instead inspect the ServicePort.Port field, available in function loadService as the expression portSpec.Port. In other words, line 228 could read as follows:

if portSpec.Port == 443 || strings.HasPrefix(portName, "https") {

That's still not addressing taking the "ingress.kubernetes.io/protocol" annotation into account, but it's an improvement.

The test fixtures don't capture this case. The one that exercises this port 443 detection uses a Kubernetes Service whose service port is equivalent to its target port.

Note that this same problem afflicts the "kubernetesCRD" provider in file pkg/provider/kubernetes/crd/kubernetes.go, in the similarly named loadServers function here.

@dduportal
Copy link
Contributor

Hi @seh , thanks for this detailed report. Would you be willing to contribute by submitting a PR (on the branch v2.0?

@seh
Copy link
Contributor Author

seh commented Jul 30, 2019

Yes, I'd be happy to, provided that we agree on the proposed solution. I concede that there could be Services that have Endpoints not backed by pods, and it's possible that a Service's front-end port could be something other than 443, but then it turns out the Endpoint ports are 443. However, I think that goes beyond the heuristic that Traefik intended to apply here.

I'm not the designer, though; I just got here. Hence, we need your opinion as a longstanding project steward as to whether the current implementation is mistaken, or whether the specification was misworded.

@seh
Copy link
Contributor Author

seh commented Jul 30, 2019

I now see the "kind/bug/confirmed" label now, so perhaps that's enough to say that you agree with my interpretation.

@dtomcej
Copy link
Contributor

dtomcej commented Jul 30, 2019

Hi @seh,

We agree that the behavior should be consistent with the 1.7 branch, which checks the service ports, not the endpoints.

@traefiker
Copy link
Contributor

Closed by #5167.

@traefik traefik locked and limited conversation to collaborators Sep 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants