Skip to content

Commit

Permalink
add warning if auth external svc isnt found
Browse files Browse the repository at this point in the history
Auth External works by parsing the authentication service url and
looking for a pre-configured backend. The pre-configuration part works
by making a normalization in the k8s service port number, so different
inputs for the same service will output the same backend. This might
change the port name of the backend on named port configurations.

A proper fix is to make the backend creation accessible by the updater
code, so we don't need to pre-initialize the backend in the ingress
step, and don't need to guess the backend port name in the updater step.
So, for now, we'll improve logging and documentation.

This should be merged as far as v0.13, which implements the first
version of auth external.
  • Loading branch information
jcmoraisjr committed Feb 8, 2023
1 parent d4e9cde commit cf8a58b
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 3 deletions.
2 changes: 1 addition & 1 deletion docs/content/en/docs/configuration/keys.md
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ Configures External Authentication options.

* `<proto>`: can be `http`, `https`, `service` or `svc`.
* `<name>`: 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.
* `<port>`: the port number, must be provided if a service is used and can be omitted if using `http` or `https`.
* `<port>`: 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.
* `<path>`: 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.
Expand Down
6 changes: 5 additions & 1 deletion pkg/converters/ingress/annotations/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion pkg/converters/ingress/annotations/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
5 changes: 5 additions & 0 deletions pkg/converters/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down

0 comments on commit cf8a58b

Please sign in to comment.