fix rework

This commit is contained in:
Tore S. Loenoey 2023-11-23 20:35:10 +01:00 committed by Tore Stendal Lønøy
parent 3f6f3775ff
commit 1d53984a88
No known key found for this signature in database
8 changed files with 76 additions and 79 deletions

View file

@ -39,6 +39,10 @@ var globalAuthAnnotations = parser.Annotation{
},
}
type Config struct {
GlobalAuthDefaultEnable bool `json:"global-auth-default-enable,omitempty"`
}
type authReqGlobal struct {
r resolver.Resolver
annotationConfig parser.Annotation
@ -57,7 +61,7 @@ func NewParser(r resolver.Resolver) parser.IngressAnnotation {
func (a authReqGlobal) Parse(ing *networking.Ingress) (interface{}, error) {
enableGlobalAuth, err := parser.GetBoolAnnotation(enableGlobalAuthAnnotation, ing, a.annotationConfig.Annotations)
if err != nil {
enableGlobalAuth = true
enableGlobalAuth = a.r.GetDefaultBackend().GlobalAuthDefaultEnable
}
return enableGlobalAuth, nil

View file

@ -762,7 +762,7 @@ func NewDefault() Configuration {
defNginxStatusIpv4Whitelist = append(defNginxStatusIpv4Whitelist, "127.0.0.1")
defNginxStatusIpv6Whitelist = append(defNginxStatusIpv6Whitelist, "::1")
defProxyDeadlineDuration := time.Duration(5) * time.Second
defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}, false, true}
defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}, false}
cfg := Configuration{
AllowSnippetAnnotations: false,
@ -872,6 +872,7 @@ func NewDefault() Configuration {
DisableProxyInterceptErrors: false,
DenylistSourceRange: []string{},
WhitelistSourceRange: []string{},
GlobalAuthDefaultEnable: true,
SkipAccessLogURLs: []string{},
LimitRate: 0,
LimitRateAfter: 0,
@ -976,5 +977,4 @@ type GlobalExternalAuth struct {
AuthCacheDuration []string `json:"authCacheDuration"`
ProxySetHeaders map[string]string `json:"proxySetHeaders,omitempty"`
AlwaysSetCookie bool `json:"alwaysSetCookie,omitempty"`
DefaultEnable bool `json:"defaultEnable,omitempty"`
}

View file

@ -352,7 +352,7 @@ func ReadConfig(src map[string]string) config.Configuration {
if err != nil {
klog.Warningf("Global auth location denied - %s", fmt.Errorf("cannot convert %s to bool: %v", globalAuthDefaultEnable, err))
}
to.GlobalExternalAuth.DefaultEnable = authDefaultEnable
to.GlobalAuthDefaultEnable = authDefaultEnable
}
// Verify that the configured timeout is parsable as a duration. if not, set the default value

View file

@ -263,36 +263,6 @@ func TestGlobalExternalAlwaysSetCookie(t *testing.T) {
}
}
func TestGlobalExternalDefaultEnable(t *testing.T) {
testCases := map[string]struct {
defaultEnable string
result bool
}{
"true": {
defaultEnable: "true",
result: true,
},
"false": {
defaultEnable: "false",
result: false,
},
"set empty": {
defaultEnable: "",
result: true,
},
"error": {
defaultEnable: "error string",
},
}
for n, tc := range testCases {
cfg := ReadConfig(map[string]string{"global-auth-default-enable": tc.defaultEnable})
if cfg.GlobalExternalAuth.DefaultEnable != tc.result {
t.Errorf("Testing %v. Expected \"%v\" but \"%v\" was returned", n, tc.result, cfg.GlobalExternalAuth.DefaultEnable)
}
}
}
func TestGlobalExternalAuthSigninRedirectParamParsing(t *testing.T) {
testCases := map[string]struct {
param string

View file

@ -541,14 +541,14 @@ func buildLocation(input interface{}, enforceRegex bool) string {
return path
}
func buildAuthLocation(input interface{}, globalExternalAuthURL string, c interface{}) string {
func buildAuthLocation(input interface{}, globalExternalAuthURL string) string {
location, ok := input.(*ingress.Location)
if !ok {
klog.Errorf("expected an '*ingress.Location' type but %T was returned", input)
return ""
}
if (location.ExternalAuth.URL == "") && (!shouldApplyGlobalAuth(input, globalExternalAuthURL, c)) {
if (location.ExternalAuth.URL == "") && (!shouldApplyGlobalAuth(input, globalExternalAuthURL)) {
return ""
}
@ -566,19 +566,13 @@ func buildAuthLocation(input interface{}, globalExternalAuthURL string, c interf
// shouldApplyGlobalAuth returns true only in case when ExternalAuth.URL is not set and
// GlobalExternalAuth is set, or if GlobalExternalAuth.DefaultEnable is true.
func shouldApplyGlobalAuth(input interface{}, globalExternalAuthURL string, c interface{}) bool {
func shouldApplyGlobalAuth(input interface{}, globalExternalAuthURL string) bool {
location, ok := input.(*ingress.Location)
if !ok {
klog.Errorf("expected an '*ingress.Location' type but %T was returned", input)
}
cfg, ok := c.(config.Configuration)
if !ok {
klog.Errorf("expected a 'config.Configuration' type but %T was returned", c)
return false
}
if (location.ExternalAuth.URL == "") && (globalExternalAuthURL != "") && ((cfg.GlobalExternalAuth.DefaultEnable) && (location.EnableGlobalAuth)) {
if (location.ExternalAuth.URL == "") && (globalExternalAuthURL != "") && (location.EnableGlobalAuth) {
return true
}
@ -639,14 +633,9 @@ func buildAuthProxySetHeaders(headers map[string]string) []string {
return res
}
func buildAuthUpstreamName(input interface{}, host string, c interface{}) string {
cfg, ok := c.(config.Configuration)
if !ok {
klog.Errorf("expected a 'config.Configuration' type but %T was returned", c)
return ""
}
func buildAuthUpstreamName(input interface{}, host string) string {
authPath := buildAuthLocation(input, "", cfg)
authPath := buildAuthLocation(input, "")
if authPath == "" || host == "" {
return ""
}

View file

@ -408,10 +408,9 @@ func TestBuildProxyPassAutoHttp(t *testing.T) {
}
func TestBuildAuthLocation(t *testing.T) {
cfg := config.Configuration{}
invalidType := &ingress.Ingress{}
expected := ""
actual := buildAuthLocation(invalidType, "", cfg)
actual := buildAuthLocation(invalidType, "")
if !reflect.DeepEqual(expected, actual) {
t.Errorf("Expected '%v' but returned '%v'", expected, actual)
@ -461,7 +460,7 @@ func TestBuildAuthLocation(t *testing.T) {
loc.ExternalAuth.URL = testCase.authURL
loc.EnableGlobalAuth = testCase.enableglobalExternalAuth
str := buildAuthLocation(loc, testCase.globalAuthURL, cfg)
str := buildAuthLocation(loc, testCase.globalAuthURL)
if str != testCase.expected {
t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, str)
}
@ -471,7 +470,6 @@ func TestBuildAuthLocation(t *testing.T) {
func TestShouldApplyGlobalAuth(t *testing.T) {
authURL := fooAuthHost
globalAuthURL := "foo.com/global-auth"
cfg := config.Configuration{}
loc := &ingress.Location{
ExternalAuth: authreq.Config{
@ -486,38 +484,70 @@ func TestShouldApplyGlobalAuth(t *testing.T) {
authURL string
globalAuthURL string
enableglobalExternalAuth bool
globalAuthDefaultEnable bool
expected bool
}{
{"authURL, globalAuthURL and enabled", authURL, globalAuthURL, true, true, false},
{"authURL, globalAuthURL and disabled", authURL, globalAuthURL, false, true, false},
{"authURL, empty globalAuthURL and enabled", authURL, "", true, true, false},
{"authURL, empty globalAuthURL and disabled", authURL, "", false, true, false},
{"globalAuthURL and enabled", "", globalAuthURL, true, true, true},
{"globalAuthURL and disabled", "", globalAuthURL, false, true, false},
{"all empty and enabled", "", "", true, true, false},
{"all empty and disabled", "", "", false, true, false},
{"authURL, globalAuthURL and enabled, defaultEnable is false", authURL, globalAuthURL, true, false, false},
{"authURL, globalAuthURL and disabled, defaultEnable is false", authURL, globalAuthURL, false, false, false},
{"authURL, empty globalAuthURL and enabled, defaultEnable is false", authURL, "", true, false, false},
{"authURL, empty globalAuthURL and disabled, defaultEnable is false", authURL, "", false, false, false},
{"globalAuthURL and enabled, defaultEnable is false", "", globalAuthURL, true, true, true},
{"globalAuthURL and disabled, defaultEnable is false", "", globalAuthURL, false, false, false},
{"all empty and enabled, defaultEnable is false", "", "", true, false, false},
{"all empty and disabled, defaultEnable is false", "", "", false, false, false},
{"authURL, globalAuthURL and enabled", authURL, globalAuthURL, true, false},
{"authURL, globalAuthURL and disabled", authURL, globalAuthURL, false, false},
{"authURL, empty globalAuthURL and enabled", authURL, "", true, false},
{"authURL, empty globalAuthURL and disabled", authURL, "", false, false},
{"globalAuthURL and enabled", "", globalAuthURL, true, true},
{"globalAuthURL and disabled", "", globalAuthURL, false, false},
{"all empty and enabled", "", "", true, false},
{"all empty and disabled", "", "", false, false},
}
for _, testCase := range testCases {
loc.ExternalAuth.URL = testCase.authURL
loc.EnableGlobalAuth = testCase.enableglobalExternalAuth
result := shouldApplyGlobalAuth(loc, testCase.globalAuthURL, cfg)
result := shouldApplyGlobalAuth(loc, testCase.globalAuthURL)
if result != testCase.expected {
t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, result)
}
}
}
// func TestShouldApplyGlobalAuthWhenEnableDefaultIsFalse(t *testing.T) {
// authURL := fooAuthHost
// globalAuthURL := "foo.com/global-auth"
// loc := &ingress.Location{
// ExternalAuth: authreq.Config{
// URL: authURL,
// },
// Path: "/cat",
// EnableGlobalAuth: true,
// }
// testCases := []struct {
// title string
// authURL string
// globalAuthURL string
// enableglobalExternalAuth bool
// globalAuthDefaultEnable bool
// expected bool
// }{
// {"authURL, globalAuthURL and enabled", authURL, globalAuthURL, true, true, false},
// {"authURL, globalAuthURL and disabled", authURL, globalAuthURL, false, true, false},
// {"authURL, empty globalAuthURL and enabled", authURL, "", true, true, false},
// {"authURL, empty globalAuthURL and disabled", authURL, "", false, true, false},
// {"globalAuthURL and enabled", "", globalAuthURL, true, true, true},
// {"globalAuthURL and disabled", "", globalAuthURL, false, true, false},
// {"all empty and enabled", "", "", true, true, false},
// {"all empty and disabled", "", "", false, true, false},
// }
// for _, testCase := range testCases {
// loc.ExternalAuth.URL = testCase.authURL
// loc.EnableGlobalAuth = testCase.enableglobalExternalAuth
// result := shouldApplyGlobalAuth(loc, testCase.globalAuthURL)
// if result != testCase.expected {
// t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, result)
// }
// }
// }
func TestBuildAuthResponseHeaders(t *testing.T) {
externalAuthResponseHeaders := []string{"h1", "H-With-Caps-And-Dashes"}
tests := []struct {
@ -589,8 +619,7 @@ func TestBuildAuthProxySetHeaders(t *testing.T) {
func TestBuildAuthUpstreamName(t *testing.T) {
invalidType := &ingress.Ingress{}
expected := ""
cfg := config.Configuration{}
actual := buildAuthUpstreamName(invalidType, "", cfg)
actual := buildAuthUpstreamName(invalidType, "")
if !reflect.DeepEqual(expected, actual) {
t.Errorf("Expected '%v' but returned '%v'", expected, actual)
@ -617,7 +646,7 @@ func TestBuildAuthUpstreamName(t *testing.T) {
}
for _, testCase := range testCases {
str := buildAuthUpstreamName(loc, testCase.host, cfg)
str := buildAuthUpstreamName(loc, testCase.host)
if str != testCase.expected {
t.Errorf("%v: expected '%v' but returned '%v'", testCase.title, testCase.expected, str)
}

View file

@ -176,6 +176,11 @@ type Backend struct {
// By default, the NGINX ingress controller uses a list of all endpoints (Pod IP/port) in the NGINX upstream configuration.
// It disables that behavior and instead uses a single upstream in NGINX, the service's Cluster IP and port.
ServiceUpstream bool `json:"service-upstream"`
// By default, the NGINX ingress controller applies global-auth configuration to all Ingress resources,
// if global-auth-url (ConfigMap) is set, and auth-url is not set (Ingess). Default is `true`. By setting this to
// `false`, global-auth is only applied to Ingress resources when global-auth-url (ConfigMap) is set and enable-global-auth is set (Ingress).
GlobalAuthDefaultEnable bool `json:"global-auth-default-enable"`
}
type SecurityConfiguration struct {

View file

@ -636,7 +636,7 @@ http {
{{ range $server := $servers }}
{{ range $location := $server.Locations }}
{{ $applyGlobalAuth := shouldApplyGlobalAuth $location $all.Cfg.GlobalExternalAuth.URL $all.Cfg.GlobalExternalAuth.DefaultEnable }}
{{ $applyGlobalAuth := shouldApplyGlobalAuth $location $all.Cfg.GlobalExternalAuth.URL }}
{{ $applyAuthUpstream := shouldApplyAuthUpstream $location $all.Cfg }}
{{ if and (eq $applyAuthUpstream true) (eq $applyGlobalAuth false) }}
## start auth upstream {{ $server.Hostname }}{{ $location.Path }}
@ -1057,8 +1057,8 @@ stream {
{{ range $location := $server.Locations }}
{{ $path := buildLocation $location $enforceRegex }}
{{ $proxySetHeader := proxySetHeader $location }}
{{ $authPath := buildAuthLocation $location $all.Cfg.GlobalExternalAuth.URL $all.Cfg.GlobalExternalAuth.DefaultEnable }}
{{ $applyGlobalAuth := shouldApplyGlobalAuth $location $all.Cfg.GlobalExternalAuth.URL $all.Cfg.GlobalExternalAuth.DefaultEnable }}
{{ $authPath := buildAuthLocation $location $all.Cfg.GlobalExternalAuth.URL }}
{{ $applyGlobalAuth := shouldApplyGlobalAuth $location $all.Cfg.GlobalExternalAuth.URL }}
{{ $applyAuthUpstream := shouldApplyAuthUpstream $location $all.Cfg }}
{{ $externalAuth := $location.ExternalAuth }}