Support cors-allow-origin with multiple origins (#7614)

* Add Initial support for multiple cors origins in nginx

- bump cluster version for `make dev-env`
- add buildOriginRegex function in nginx.tmpl
- add e2e 4 e2e tests for cors.go
- refers to feature request #5496

* add tests + use search to identify '*' origin

* add tests + use search to identify '*' origin

Signed-off-by: Christopher Larivière <lariviere.c@gmail.com>

* fix "should enable cors test" looking at improper values

* Modify tests and add some logic for origin validation

- add origin validation in cors ingress annotations
- add extra tests to validate regex
- properly escape regex using "QuoteMeta"
- fix some copy/paste errors

* add TrimSpace and length validation before adding a new origin

* modify documentation for cors and remove dangling comment

* add support for optional port mapping on origin

* support single-level wildcard subdomains + tests

* Remove automatic `*` fonctionality from incorrect origins

- use []string instead of basic string to avoid reparsing in template.go
- fix typo in docs
- modify template to properly enable only if the whole block is enabled
- modify cors parsing
- test properly by validating that the value returned is the proper
  origin
- update unit tests and annotation tests

* Re-add `*` when no cors origins are supplied + fix tests

- fix e2e tests to allow for `*`
- re-add `*` to cors parsing if trimmed cors-allow-origin is empty
(supplied but empty) and if it wasn't supplied at all.

* remove unecessary logic for building cors origin + remove comments

- add some edge cases in e2e tests
- rework logic for building cors origin

there was no need for logic in template.go for buildCorsOriginRegex
if there is a `*` it ill be short-circuited by first if.

if it's a wildcard domain or any domain (without a wildcard), it MUST
match the main/cors.go regex format.

if there's a star in a wildcard domain, it must be replaced with
`[A-Za-z0-9]+`

* add missing check in e2e tests

internal/ingress/annotations/annotations_test.go

@@ -215,15 +215,15 @@ func TestCors(t *testing.T) {
 		corsenabled bool
 		methods     string
 		headers     string
-		origin      string
+		origin      []string
 		credentials bool
 		expose      string
 	}{
-		{map[string]string{annotationCorsEnabled: "true"}, true, defaultCorsMethods, defaultCorsHeaders, "*", true, ""},
-		{map[string]string{annotationCorsEnabled: "true", annotationCorsAllowMethods: "POST, GET, OPTIONS", annotationCorsAllowHeaders: "$nginx_version", annotationCorsAllowCredentials: "false", annotationCorsExposeHeaders: "X-CustomResponseHeader"}, true, "POST, GET, OPTIONS", defaultCorsHeaders, "*", false, "X-CustomResponseHeader"},
-		{map[string]string{annotationCorsEnabled: "true", annotationCorsAllowCredentials: "false"}, true, defaultCorsMethods, defaultCorsHeaders, "*", false, ""},
-		{map[string]string{}, false, defaultCorsMethods, defaultCorsHeaders, "*", true, ""},
-		{nil, false, defaultCorsMethods, defaultCorsHeaders, "*", true, ""},
+		{map[string]string{annotationCorsEnabled: "true"}, true, defaultCorsMethods, defaultCorsHeaders, []string{"*"}, true, ""},
+		{map[string]string{annotationCorsEnabled: "true", annotationCorsAllowMethods: "POST, GET, OPTIONS", annotationCorsAllowHeaders: "$nginx_version", annotationCorsAllowCredentials: "false", annotationCorsExposeHeaders: "X-CustomResponseHeader"}, true, "POST, GET, OPTIONS", defaultCorsHeaders, []string{"*"}, false, "X-CustomResponseHeader"},
+		{map[string]string{annotationCorsEnabled: "true", annotationCorsAllowCredentials: "false"}, true, defaultCorsMethods, defaultCorsHeaders, []string{"*"}, false, ""},
+		{map[string]string{}, false, defaultCorsMethods, defaultCorsHeaders, []string{"*"}, true, ""},
+		{nil, false, defaultCorsMethods, defaultCorsHeaders, []string{"*"}, true, ""},
 	}
 
 	for _, foo := range fooAnns {
@@ -243,12 +243,18 @@ func TestCors(t *testing.T) {
 			t.Errorf("Returned %v but expected %v for Cors Methods", r.CorsAllowMethods, foo.methods)
 		}
 
-		if r.CorsAllowOrigin != foo.origin {
-			t.Errorf("Returned %v but expected %v for Cors Methods", r.CorsAllowOrigin, foo.origin)
+		if len(r.CorsAllowOrigin) != len(foo.origin) {
+			t.Errorf("Lengths of Cors Origins are not equal. Expected %v - Actual %v", r.CorsAllowOrigin, foo.origin)
+		}
+
+		for i, v := range r.CorsAllowOrigin {
+			if v != foo.origin[i] {
+				t.Errorf("Values of Cors Origins are not equal. Expected %v - Actual %v", r.CorsAllowOrigin, foo.origin)
+			}
 		}
 
 		if r.CorsAllowCredentials != foo.credentials {
-			t.Errorf("Returned %v but expected %v for Cors Methods", r.CorsAllowCredentials, foo.credentials)
+			t.Errorf("Returned %v but expected %v for Cors Credentials", r.CorsAllowCredentials, foo.credentials)
 		}
 
 	}

internal/ingress/annotations/cors/main.go

@@ -18,8 +18,10 @@ package cors
 
 import (
 	"regexp"
+	"strings"
 
 	networking "k8s.io/api/networking/v1"
+	"k8s.io/klog/v2"
 
 	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
 	"k8s.io/ingress-nginx/internal/ingress/resolver"
@@ -36,7 +38,7 @@ var (
 	// Regex are defined here to prevent information leak, if user tries to set anything not valid
 	// that could cause the Response to contain some internal value/variable (like returning $pid, $upstream_addr, etc)
 	// Origin must contain a http/s Origin (including or not the port) or the value '*'
-	corsOriginRegex = regexp.MustCompile(`^(https?://[A-Za-z0-9\-\.]*(:[0-9]+)?|\*)?$`)
+	corsOriginRegex = regexp.MustCompile(`^(https?://(\*\.)?[A-Za-z0-9\-\.]*(:[0-9]+)?|\*)?$`)
 	// Method must contain valid methods list (PUT, GET, POST, BLA)
 	// May contain or not spaces between each verb
 	corsMethodsRegex = regexp.MustCompile(`^([A-Za-z]+,?\s?)+$`)
@@ -54,13 +56,13 @@ type cors struct {
 
 // Config contains the Cors configuration to be used in the Ingress
 type Config struct {
-	CorsEnabled          bool   `json:"corsEnabled"`
-	CorsAllowOrigin      string `json:"corsAllowOrigin"`
-	CorsAllowMethods     string `json:"corsAllowMethods"`
-	CorsAllowHeaders     string `json:"corsAllowHeaders"`
-	CorsAllowCredentials bool   `json:"corsAllowCredentials"`
-	CorsExposeHeaders    string `json:"corsExposeHeaders"`
-	CorsMaxAge           int    `json:"corsMaxAge"`
+	CorsEnabled          bool     `json:"corsEnabled"`
+	CorsAllowOrigin      []string `json:"corsAllowOrigin"`
+	CorsAllowMethods     string   `json:"corsAllowMethods"`
+	CorsAllowHeaders     string   `json:"corsAllowHeaders"`
+	CorsAllowCredentials bool     `json:"corsAllowCredentials"`
+	CorsExposeHeaders    string   `json:"corsExposeHeaders"`
+	CorsMaxAge           int      `json:"corsMaxAge"`
 }
 
 // NewParser creates a new CORS annotation parser
@@ -91,13 +93,20 @@ func (c1 *Config) Equal(c2 *Config) bool {
 	if c1.CorsAllowMethods != c2.CorsAllowMethods {
 		return false
 	}
-	if c1.CorsAllowOrigin != c2.CorsAllowOrigin {
-		return false
-	}
 	if c1.CorsEnabled != c2.CorsEnabled {
 		return false
 	}
 
+	if len(c1.CorsAllowOrigin) != len(c2.CorsAllowOrigin) {
+		return false
+	}
+
+	for i, v := range c1.CorsAllowOrigin {
+		if v != c2.CorsAllowOrigin[i] {
+			return false
+		}
+	}
+
 	return true
 }
 
@@ -112,9 +121,23 @@ func (c cors) Parse(ing *networking.Ingress) (interface{}, error) {
 		config.CorsEnabled = false
 	}
 
-	config.CorsAllowOrigin, err = parser.GetStringAnnotation("cors-allow-origin", ing)
-	if err != nil || !corsOriginRegex.MatchString(config.CorsAllowOrigin) {
-		config.CorsAllowOrigin = "*"
+	unparsedOrigins, err := parser.GetStringAnnotation("cors-allow-origin", ing)
+	if err == nil {
+		config.CorsAllowOrigin = strings.Split(unparsedOrigins, ",")
+		for i, origin := range config.CorsAllowOrigin {
+			origin = strings.TrimSpace(origin)
+			if origin == "*" {
+				config.CorsAllowOrigin = []string{"*"}
+				break
+			}
+			if !corsOriginRegex.MatchString(origin) {
+				klog.Errorf("Error parsing cors-allow-origin parameters. Supplied incorrect origin: %s. Skipping.", origin)
+				config.CorsAllowOrigin = append(config.CorsAllowOrigin[:i], config.CorsAllowOrigin[i+1:]...)
+			}
+			klog.Infof("Current config.corsAllowOrigin %v", config.CorsAllowOrigin)
+		}
+	} else {
+		config.CorsAllowOrigin = []string{"*"}
 	}
 
 	config.CorsAllowHeaders, err = parser.GetStringAnnotation("cors-allow-headers", ing)
@@ -143,5 +166,4 @@ func (c cors) Parse(ing *networking.Ingress) (interface{}, error) {
 	}
 
 	return config, nil
-
 }

internal/ingress/annotations/cors/main_test.go

@@ -110,7 +110,7 @@ func TestIngressCorsConfigValid(t *testing.T) {
 		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("cors-allow-methods")], nginxCors.CorsAllowMethods)
 	}
 
-	if nginxCors.CorsAllowOrigin != "https://origin123.test.com:4443" {
+	if nginxCors.CorsAllowOrigin[0] != "https://origin123.test.com:4443" {
 		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("cors-allow-origin")], nginxCors.CorsAllowOrigin)
 	}
 
@@ -164,10 +164,6 @@ func TestIngressCorsConfigInvalid(t *testing.T) {
 		t.Errorf("expected %v but returned %v", defaultCorsHeaders, nginxCors.CorsAllowMethods)
 	}
 
-	if nginxCors.CorsAllowOrigin != "*" {
-		t.Errorf("expected %v but returned %v", "*", nginxCors.CorsAllowOrigin)
-	}
-
 	if nginxCors.CorsExposeHeaders != "" {
 		t.Errorf("expected %v but returned %v", "", nginxCors.CorsExposeHeaders)
 	}

internal/ingress/controller/template/template.go

@@ -276,6 +276,7 @@ var (
 		"shouldLoadAuthDigestModule":         shouldLoadAuthDigestModule,
 		"shouldLoadInfluxDBModule":           shouldLoadInfluxDBModule,
 		"buildServerName":                    buildServerName,
+		"buildCorsOriginRegex":               buildCorsOriginRegex,
 	}
 )
 
@@ -1676,3 +1677,29 @@ func convertGoSliceIntoLuaTable(goSliceInterface interface{}, emptyStringAsNil b
 		return "", fmt.Errorf("could not process type: %s", kind)
 	}
 }
+
+func buildOriginRegex(origin string) string {
+	origin = regexp.QuoteMeta(origin)
+	origin = strings.Replace(origin, "\\*", "[A-Za-z0-9]+", 1)
+	return fmt.Sprintf("(%s)", origin)
+}
+
+func buildCorsOriginRegex(corsOrigins []string) string {
+	if len(corsOrigins) == 1 && corsOrigins[0] == "*" {
+		return "set $http_origin *;\nset $cors 'true';"
+	}
+
+	var originsRegex string = "if ($http_origin ~* ("
+	for i, origin := range corsOrigins {
+		originTrimmed := strings.TrimSpace(origin)
+		if len(originTrimmed) > 0 {
+			builtOrigin := buildOriginRegex(originTrimmed)
+			originsRegex += builtOrigin
+			if i != len(corsOrigins)-1 {
+				originsRegex = originsRegex + "|"
+			}
+		}
+	}
+	originsRegex = originsRegex + ")$ ) { set $cors 'true'; }"
+	return originsRegex
+}