Merge pull request #1120 from jcmoraisjr/jm-global-auth-url

Allows to configure auth-url globally
jcmoraisjr · Jun 3, 2024 · dba53cc · dba53cc
2 parents 6b81f49 + bc27062
commit dba53cc
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 15 deletions.
diff --git 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,27 +163,33 @@ 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 {
 			// 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)
+			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,22 +201,22 @@ 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
 		}
 	}
 
 	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
@@ -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 <global>: 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
@@ -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)