caddyserver / ingress

WIP Caddy 2 ingress controller for Kubernetes
Apache License 2.0
613 stars 69 forks source link

Specifying a service's port name instead of number does not work #211

Open lion7 opened 4 months ago

lion7 commented 4 months ago

I just installed a Helm chart (the Element OnPrem server to be precise) which installs the following Ingress:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: first-element-deployment-synapse
  namespace: element-onprem
spec:
  rules:
  - host: ***MASKED***
    http:
      paths:
      - backend:
          service:
            name: first-element-deployment-synapse-haproxy
            port:
              name: haproxy-http
        path: /_matrix
        pathType: Prefix
      - backend:
          service:
            name: first-element-deployment-synapse-haproxy
            port:
              name: haproxy-http
        path: /_synapse
        pathType: Prefix

Note that the service port is referenced by name and not by number. This breaks when using Caddy Ingress, which I figured would be due to the following line:

https://github.com/caddyserver/ingress/blob/908d854c2b9ef7f399a4417423a1a330e0b9c12a/internal/caddy/ingress/reverseproxy.go#L34

Basically it just returns port 0 here because the port number should be resolved by name instead of directly using the number.

@mavimo Would you be open to a PR to fix this?

mavimo commented 4 months ago

@lion7 I'll look into it soon, in the meantime if you have time to make a PR will be awesome! 🤩

lion7 commented 4 months ago

I tried to fix it yesterday, but it proves to be a challenge since I don't have a k8s client available in reverseproxy.go. The ingress object itself is passed along via the Store thing without further, so I kind of got stuck.

I would need a k8s client to retrieve the Service itself from k8s and figure out the port number associated with the provided name. Perhaps you have some ideas that don't involve altering half of the codebase?

Xinayder commented 3 weeks ago

I tried to have a go at this, but came across the same issue as @lion7. I was able to make a helper function that returns the targetPort port number from a service with a named port, but the issue so far is how to call this helper function from https://github.com/caddyserver/ingress/blob/908d854c2b9ef7f399a4417423a1a330e0b9c12a/internal/caddy/ingress/reverseproxy.go#L25-L60