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

auth-url does not handle well service with port names instead of numbers #981

Open
didiercolens opened this issue Feb 3, 2023 · 2 comments

Comments

@didiercolens
Copy link

didiercolens commented Feb 3, 2023

Description of the problem

Assume a service with the following manifest (snippet):

  name: my-authentication
spec:
  ports:
  - name: authentication
    port: 1234
    protocol: TCP
    targetPort: authentication

the corresponding deployment has the following port definition:

spec:
  containers:
  - ports:
      - containerPort: 1234
        name: authentication

if an ingress is configured as follows (the documentation says to use port number):

haproxy-ingress.github.io/auth-url: svc://my-authentication:1234/api/v2/auth

it will silently fail (no error logged anywhere) and always deny all requests

Expected behaviour

Either an error is logged to warn that the backend could not be found or the code always converts ports to port number so it works in both cases.

What works is haproxy-ingress.github.io/auth-url: svc://my-authentication:authentication/api/v2/auth

So an alternate (quick) solution is to update the doc and specify that the port should match the targetPort value in the service.

Steps to reproduce the problem

  1. create a deployment that has a port name
  2. expose that deployment and make sure targetPort uses the port name
  3. set haproxy-ingress.github.io/auth-url to svc url with port number used

Environment information

HAProxy Ingress version: v0.14.0

@mischmi2
Copy link

mischmi2 commented Feb 3, 2023

As an additional piece of this, it took a long time to debug what was happening due to the silent error catch in

if backend == nil {
// warn already logged when ingress parser tried to acquire the backend
continue
}
, that doesn't log when it gets hit. In this case, the port name mismatch caused the backend to not be found.

The buildID function port appears to actually be coming from the service object targetPort. The backend gets stored as my-namespace_my-authentication_authentication instead of my-namespace_my-authentication_1234, and when the ingress object tries to lookup the backend from svc://my-authentication:1234/api/v2/auth it's looking for my-namespace_my-authentication_1234.

func buildID(namespace, name, port string) string {
return namespace + "_" + name + "_" + port
}

@jcmoraisjr
Copy link
Owner

Hi, thanks for the detailed description and digging into the code. You folks are 💯!

Port names are normalizes to create backends, so two distinct references that point to the same backend will end up with the same backend name. This however doesn't happen in the annotation parsing, the port name is copied verbatim from the auth external url, which should be different from the way the backend was created if the container uses named port.

I just created #982 trying to circumvent that behavior, adding log and a few more documentation. There is a proper way to fix that, creating the backend in the annotation parsing, but this needs a small refactor in the code. Please let me know if the changes in the PR sounds good to you all, since you faced the issue and found a work around. This issue will be left open until the refactor is made and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants