diff --git a/docs/content/en/docs/configuration/keys.md b/docs/content/en/docs/configuration/keys.md index 67545f476..b307c2de7 100644 --- a/docs/content/en/docs/configuration/keys.md +++ b/docs/content/en/docs/configuration/keys.md @@ -809,7 +809,7 @@ Configures External Authentication options. * ``: can be `http`, `https`, `service` or `svc`. * ``: the IP or hostname if `http` or `https`, or the name of a service if `service`. `svc` is an alias to `service`. Note that the hostname is resolved to a list of IP when the ingress is parsed and will not be dynamically updated later if the DNS record changes. -* ``: the port number, must be provided if a service is used and can be omitted if using `http` or `https`. +* ``: the port number, must be provided if a service is used and can be omitted if using `http` or `https`. If the service uses named ports, use the service's `port.targetPort` field value instead. * ``: optional, the fully qualified path to the authentication service. `http` and `https` protocols are straightforward: use them to connect to an IP or hostname without any further configuration. `http` adds the HTTP `Host` header if a hostname is used, and `https` adds also the sni extension. Note that `https` connects in an insecure way and currently cannot be customized. Do NOT use neither `http` nor `https` if haproxy -> authentication service communication has untrusted networks. diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go index 1d105bd87..56b7d5115 100644 --- a/pkg/converters/ingress/annotations/backend.go +++ b/pkg/converters/ingress/annotations/backend.go @@ -182,7 +182,11 @@ func (c *updater) buildBackendAuthExternal(d *backData) { } backend = c.haproxy.Backends().FindBackend(namespace, name, urlPort) if backend == nil { - // warn already logged when ingress parser tried to acquire the backend + // warn was already logged in the ingress if a service couldn't be found, + // but we still need to add a warning here because, in the current code base, + // a valid named service can lead to a broken configuration. See ingress' + // counterpart code. + c.logger.Warn("skipping auth-url on %v: service '%s:%s' was not found", url.Source, name, urlPort) continue } default: diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go index b95f8ef64..1fdf67f13 100644 --- a/pkg/converters/ingress/annotations/backend_test.go +++ b/pkg/converters/ingress/annotations/backend_test.go @@ -337,7 +337,7 @@ func TestAuthExternal(t *testing.T) { { url: "svc://noservice:80", expBack: hatypes.AuthExternal{AlwaysDeny: true}, - // svc not found, warn is issued in the ingress parsing + logging: `WARN skipping auth-url on ingress 'default/ing1': service 'noservice:80' was not found`, }, // 15 { diff --git a/pkg/converters/ingress/ingress.go b/pkg/converters/ingress/ingress.go index f4645a396..948e57734 100644 --- a/pkg/converters/ingress/ingress.go +++ b/pkg/converters/ingress/ingress.go @@ -435,6 +435,11 @@ func (c *converter) syncIngressHTTP(source *annotations.Source, ing *networking. } // pre-building the auth-url backend // TODO move to updater.buildBackendAuthExternal() + // TODO addBackend() might change the portName on named port configurations to enforce consistency, + // however updater's FindBackend() won't do it, leading to a silently broken configuration. + // See https://github.com/jcmoraisjr/haproxy-ingress/issues/981 + // Moving this logic to updater will fix this behavior, in the mean time we'll add a few more + // tips in the doc. if url := annBack[ingtypes.BackAuthURL]; url != "" { urlProto, urlHost, urlPort, _, _ := ingutils.ParseURL(url) if (urlProto == "service" || urlProto == "svc") && urlHost != "" && urlPort != "" {