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

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

Open
lion7 opened this issue Mar 5, 2024 · 3 comments
Open

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

lion7 opened this issue Mar 5, 2024 · 3 comments

Comments

@lion7
Copy link
Contributor

lion7 commented Mar 5, 2024

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:

clusterHostName := fmt.Sprintf("%v.%v.svc.cluster.local:%d", path.Backend.Service.Name, ing.Namespace, path.Backend.Service.Port.Number)

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
Copy link
Member

mavimo commented Mar 6, 2024

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

@lion7
Copy link
Contributor Author

lion7 commented Mar 6, 2024

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
Copy link
Contributor

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

func (p ReverseProxyPlugin) IngressHandler(input converter.IngressMiddlewareInput) (*caddyhttp.Route, error) {
path := input.Path
ing := input.Ingress
backendProtocol := strings.ToLower(getAnnotation(ing, backendProtocol))
// TODO :-
// when setting the upstream url we should bypass kube-dns and get the ip address of
// the pod for the deployment we are proxying to so that we can proxy to that ip address port.
// this is good for session affinity and increases performance.
clusterHostName := fmt.Sprintf("%v.%v.svc.cluster.local:%d", path.Backend.Service.Name, ing.Namespace, path.Backend.Service.Port.Number)
transport := &reverseproxy.HTTPTransport{}
if backendProtocol == "https" {
transport.TLS = &reverseproxy.TLSConfig{
InsecureSkipVerify: getAnnotationBool(ing, insecureSkipVerify, true),
}
}
handler := reverseproxy.Handler{
TransportRaw: caddyconfig.JSONModuleObject(transport, "protocol", "http", nil),
Upstreams: reverseproxy.UpstreamPool{
{Dial: clusterHostName},
},
}
handlerModule := caddyconfig.JSONModuleObject(
handler,
"handler",
"reverse_proxy",
nil,
)
input.Route.HandlersRaw = append(input.Route.HandlersRaw, handlerModule)
return input.Route, nil
}

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

No branches or pull requests

3 participants