From bc27062405c5b565eec75f86187665f047c73072 Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Sat, 4 May 2024 21:55:21 -0300 Subject: [PATCH] allows to configure auth-url globally Adds the option to configure auth-url globally. Global configurations don't rely on a namespace name, so the only change in the configuration is that a service url needs to add the namespace of the service when configured globally. --- pkg/converters/ingress/annotations/backend.go | 30 +++++++++------- .../ingress/annotations/backend_test.go | 36 +++++++++++++++++-- pkg/converters/ingress/annotations/host.go | 2 +- pkg/converters/ingress/annotations/mapper.go | 3 ++ 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/pkg/converters/ingress/annotations/backend.go b/pkg/converters/ingress/annotations/backend.go index bad3f6967..a06214f35 100644 --- a/pkg/converters/ingress/annotations/backend.go +++ b/pkg/converters/ingress/annotations/backend.go @@ -121,13 +121,13 @@ func (c *updater) setAuthExternal(config ConfigValueGetter, auth *hatypes.AuthEx external := c.haproxy.Global().External if external.IsExternal && !external.HasLua { - c.logger.Warn("external authentication on %v needs Lua json module, install lua-json4 and enable 'external-has-lua' global config", url.Source) + c.logger.Warn("external authentication on %s needs Lua json module, install lua-json4 and enable 'external-has-lua' global config", url.Source.String()) return } urlProto, urlHost, urlPort, urlPath, err := ingutils.ParseURL(url.Value) if err != nil { - c.logger.Warn("ignoring URL on %v: %v", url.Source, err) + c.logger.Warn("ignoring URL on %s: %v", url.Source.String(), err) return } @@ -142,7 +142,7 @@ func (c *updater) setAuthExternal(config ConfigValueGetter, auth *hatypes.AuthEx } else { var err error if ipList, err = lookupHost(urlHost); err != nil { - c.logger.Warn("ignoring auth URL with an invalid domain on %v: %v", url.Source, err) + c.logger.Warn("ignoring auth URL with an invalid domain on %s: %v", url.Source.String(), err) return } hostname = urlHost @@ -163,15 +163,21 @@ func (c *updater) setAuthExternal(config ConfigValueGetter, auth *hatypes.AuthEx } case "service", "svc": if urlPort == "" { - c.logger.Warn("skipping auth-url on %v: missing service port: %s", url.Source, url.Value) + c.logger.Warn("skipping auth-url on %s: missing service port: %s", url.Source.String(), url.Value) return } ssvc := strings.Split(urlHost, "/") - namespace := url.Source.Namespace + var namespace string name := ssvc[0] if len(ssvc) == 2 { namespace = ssvc[0] name = ssvc[1] + } else if url.Source != nil { + namespace = url.Source.Namespace + } + if namespace == "" { + c.logger.Warn("skipping auth-url on %s: a globally configured auth-url is missing the namespace", url.Source.String()) + return } backend = c.haproxy.Backends().FindBackend(namespace, name, urlPort) if backend == nil { @@ -179,11 +185,11 @@ func (c *updater) setAuthExternal(config ConfigValueGetter, auth *hatypes.AuthEx // 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) + c.logger.Warn("skipping auth-url on %s: service '%s:%s' was not found", url.Source.String(), name, urlPort) return } default: - c.logger.Warn("ignoring auth URL with an invalid protocol on %v: %s", url.Source, urlProto) + c.logger.Warn("ignoring auth URL with an invalid protocol on %s: %s", url.Source.String(), urlProto) return } // TODO track @@ -195,7 +201,7 @@ func (c *updater) setAuthExternal(config ConfigValueGetter, auth *hatypes.AuthEx authBackendName, err = c.haproxy.Frontend().AcquireAuthBackendName(backend.BackendID()) if err != nil { // TODO remove backend if not used elsewhere - c.logger.Warn("ignoring auth URL on %v: %v", url.Source, err) + c.logger.Warn("ignoring auth URL on %s: %v", url.Source.String(), err) return } } @@ -203,14 +209,14 @@ func (c *updater) setAuthExternal(config ConfigValueGetter, auth *hatypes.AuthEx m := config.Get(ingtypes.BackAuthMethod) method := m.Value if !validMethodRegex.MatchString(method) { - c.logger.Warn("invalid request method '%s' on %s, using GET instead", method, m.Source) + c.logger.Warn("invalid request method '%s' on %s, using GET instead", method, m.Source.String()) method = "GET" } s := config.Get(ingtypes.BackAuthSignin) signin := s.Value if signin != "" && !validURLRegex.MatchString(signin) { - c.logger.Warn("ignoring invalid sign-in URL in %v: %s", s.Source, signin) + c.logger.Warn("ignoring invalid sign-in URL on %s: %s", s.Source.String(), signin) signin = "" } @@ -232,7 +238,7 @@ func (c *updater) setAuthExternal(config ConfigValueGetter, auth *hatypes.AuthEx if signin != "" { if !reflect.DeepEqual(hdrFail, []string{"*"}) { - c.logger.Warn("ignoring '%s' on %v due to signin (redirect) configuration", ingtypes.BackAuthHeadersFail, s.Source) + c.logger.Warn("ignoring '%s' on %s due to signin (redirect) configuration", ingtypes.BackAuthHeadersFail, s.Source.String()) } // `-` instructs auth-request to not terminate the transaction, // so HAProxy has the chance to configure the redirect. @@ -258,7 +264,7 @@ func (c *updater) buildBackendAuthExternal(d *backData) { config := d.mapper.GetConfig(path.Link) isBackend := config.Get(ingtypes.BackAuthExternalPlacement).ToLower() == "backend" url := config.Get(ingtypes.BackAuthURL) - if isBackend && url.Source != nil && url.Value != "" { + if isBackend && url.Value != "" { c.setAuthExternal(config, &path.AuthExternal, url) } } diff --git a/pkg/converters/ingress/annotations/backend_test.go b/pkg/converters/ingress/annotations/backend_test.go index 571ed6763..af4ea3cbe 100644 --- a/pkg/converters/ingress/annotations/backend_test.go +++ b/pkg/converters/ingress/annotations/backend_test.go @@ -196,6 +196,7 @@ func TestAffinity(t *testing.T) { func TestAuthExternal(t *testing.T) { testCase := []struct { + global bool url string signin string method string @@ -290,7 +291,7 @@ func TestAuthExternal(t *testing.T) { AuthPath: "/app", }, expIP: []string{"10.0.0.200:8080"}, - logging: `WARN ignoring invalid sign-in URL in ingress 'default/ing1': http://invalid'`, + logging: `WARN ignoring invalid sign-in URL on ingress 'default/ing1': http://invalid'`, }, // 10 { @@ -478,8 +479,35 @@ func TestAuthExternal(t *testing.T) { }, expIP: []string{"10.0.0.2:80"}, }, + // 28 + { + global: true, + url: "http://app1.local", + expBack: hatypes.AuthExternal{ + AuthBackendName: "_auth_4001", + AuthPath: "/", + }, + expIP: []string{"10.0.0.2:80"}, + }, + // 29 + { + global: true, + url: "svc://authservice:80/auth", + expBack: hatypes.AuthExternal{AlwaysDeny: true}, + logging: `WARN skipping auth-url on : a globally configured auth-url is missing the namespace`, + }, + // 30 + { + global: true, + url: "svc://default/authservice:80/auth", + expBack: hatypes.AuthExternal{ + AuthBackendName: "_auth_4001", + AuthPath: "/auth", + }, + expIP: []string{"10.0.0.11:8080"}, + }, } - source := &Source{ + defaultSource := &Source{ Namespace: "default", Name: "ing1", Type: "ingress", @@ -530,6 +558,10 @@ func TestAuthExternal(t *testing.T) { ingtypes.BackAuthHeadersFail: "*", ingtypes.BackAuthMethod: "GET", } + var source *Source + if !test.global { + source = defaultSource + } d := c.createBackendMappingData("default/app", source, defaults, ann, []string{"/"}) u.buildBackendAuthExternal(d) back := d.backend.Paths[0].AuthExternal diff --git a/pkg/converters/ingress/annotations/host.go b/pkg/converters/ingress/annotations/host.go index c273d98c1..2f5f35ec0 100644 --- a/pkg/converters/ingress/annotations/host.go +++ b/pkg/converters/ingress/annotations/host.go @@ -25,7 +25,7 @@ import ( func (c *updater) buildHostAuthExternal(d *hostData) { isFrontend := d.mapper.Get(ingtypes.BackAuthExternalPlacement).ToLower() == "frontend" url := d.mapper.Get(ingtypes.BackAuthURL) - if isFrontend && url.Source != nil && url.Value != "" { + if isFrontend && url.Value != "" { for _, path := range d.host.Paths { path.AuthExt = &types.AuthExternal{} c.setAuthExternal(d.mapper, path.AuthExt, url) diff --git a/pkg/converters/ingress/annotations/mapper.go b/pkg/converters/ingress/annotations/mapper.go index c320b9df6..48d5b0913 100644 --- a/pkg/converters/ingress/annotations/mapper.go +++ b/pkg/converters/ingress/annotations/mapper.go @@ -240,5 +240,8 @@ func (m *PathConfig) String() string { // String ... func (s *Source) String() string { + if s == nil { + return "" + } return fmt.Sprintf("%s '%s'", s.Type, s.FullName()) }