Skip to content

Commit 0ce9f18

Browse files
committed
Replace CORS handling logic with NJS scripting (avoid using Nginx 'if' directives)
1 parent b2ecc4f commit 0ce9f18

File tree

5 files changed

+288
-379
lines changed

5 files changed

+288
-379
lines changed

internal/ingress/controller/template/template.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ var funcMap = text_template.FuncMap{
300300
"getenv": os.Getenv,
301301
"contains": strings.Contains,
302302
"split": strings.Split,
303+
"join": strings.Join,
303304
"hasPrefix": strings.HasPrefix,
304305
"hasSuffix": strings.HasSuffix,
305306
"trimSpace": strings.TrimSpace,
@@ -1675,10 +1676,10 @@ func buildOriginRegex(origin string) string {
16751676

16761677
func buildCorsOriginRegex(corsOrigins []string) string {
16771678
if len(corsOrigins) == 1 && corsOrigins[0] == "*" {
1678-
return "set $http_origin *;\nset $cors 'true';"
1679+
return ".*"
16791680
}
16801681

1681-
originsRegex := "if ($http_origin ~* ("
1682+
originsRegex := "("
16821683
for i, origin := range corsOrigins {
16831684
originTrimmed := strings.TrimSpace(origin)
16841685
if originTrimmed != "" {
@@ -1689,6 +1690,6 @@ func buildCorsOriginRegex(corsOrigins []string) string {
16891690
}
16901691
}
16911692
}
1692-
originsRegex += ")$ ) { set $cors 'true'; }"
1693+
originsRegex += ")"
16931694
return originsRegex
16941695
}

internal/ingress/controller/template/template_test.go

+33
Original file line numberDiff line numberDiff line change
@@ -1953,3 +1953,36 @@ func TestCleanConf(t *testing.T) {
19531953
t.Errorf("cleanConf result don't match with expected: %s", diff)
19541954
}
19551955
}
1956+
1957+
func TestBuildCorsOriginRegex(t *testing.T) {
1958+
origins := []string{"http://foo.bar "}
1959+
1960+
result := buildCorsOriginRegex(origins)
1961+
1962+
expected := `((http://foo\.bar))`
1963+
if result != expected {
1964+
t.Errorf("expected '%v' but returned '%v'", expected, result)
1965+
}
1966+
}
1967+
1968+
func TestBuildCorsOriginRegexWithMultipleOrigins(t *testing.T) {
1969+
origins := []string{" http://foo.bar", "http://*.bar"}
1970+
1971+
result := buildCorsOriginRegex(origins)
1972+
1973+
expected := `((http://foo\.bar)|(http://[A-Za-z0-9\-]+\.bar))`
1974+
if result != expected {
1975+
t.Errorf("expected '%v' but returned '%v'", expected, result)
1976+
}
1977+
}
1978+
1979+
func TestBuildCorsOriginRegexWithWildcard(t *testing.T) {
1980+
origins := []string{"*"}
1981+
1982+
result := buildCorsOriginRegex(origins)
1983+
1984+
expected := `.*`
1985+
if result != expected {
1986+
t.Errorf("expected '%v' but returned '%v'", expected, result)
1987+
}
1988+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
function handle_cors(req) {
2+
const originsRegex = new RegExp(`${req.variables.cors_origins_regex}$`, 'i');
3+
4+
if (originsRegex.test(req.headersIn['Origin'])) {
5+
const allowedOrigins = req.variables.cors_allowed_origins.split(',');
6+
7+
req.headersOut['Access-Control-Allow-Origin'] = allowedOrigins.length === 1 && allowedOrigins[0] === '*' ? '*' : req.headersIn['Origin'];
8+
req.headersOut['Access-Control-Allow-Methods'] = req.variables.cors_allow_methods;
9+
req.headersOut['Access-Control-Allow-Headers'] = req.variables.cors_allow_headers;
10+
req.headersOut['Access-Control-Max-Age'] = req.variables.cors_max_age;
11+
if (req.variables.cors_allow_credentials) req.headersOut['Access-Control-Allow-Credentials'] = req.variables.cors_allow_credentials;
12+
if (req.variables.cors_expose_headers) req.headersOut['Access-Control-Expose-Headers'] = req.variables.cors_expose_headers;
13+
14+
if (req.method === 'OPTIONS') {
15+
req.headersOut['Content-Type'] = 'text/plain charset=UTF-8';
16+
req.headersOut['Content-Length'] = '0';
17+
}
18+
}
19+
}
20+
21+
export default {handle_cors};

rootfs/etc/nginx/template/nginx.tmpl

+13-25
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ http {
7474

7575
init_worker_by_lua_file /etc/nginx/lua/ngx_conf_init_worker.lua;
7676

77+
js_import njs_handle_cors from /etc/nginx/js/nginx/ngx_handle_cors.js;
78+
7779
{{/* Enable the real_ip module only if we use either X-Forwarded headers or Proxy Protocol. */}}
7880
{{/* we use the value of the real IP for the geo_ip module */}}
7981
{{ if or (or $cfg.UseForwardedHeaders $cfg.UseProxyProtocol) $cfg.EnableRealIP }}
@@ -837,33 +839,19 @@ stream {
837839
{{/* CORS support from https://michielkalkman.com/snippets/nginx-cors-open-configuration.html */}}
838840
{{ define "CORS" }}
839841
{{ $cors := .CorsConfig }}
840-
# Cors Preflight methods needs additional options and different Return Code
841-
{{ if $cors.CorsAllowOrigin }}
842-
{{ buildCorsOriginRegex $cors.CorsAllowOrigin }}
843-
{{ end }}
844-
if ($request_method = 'OPTIONS') {
845-
set $cors ${cors}options;
846-
}
847842

848-
if ($cors = "true") {
849-
more_set_headers 'Access-Control-Allow-Origin: $http_origin';
850-
{{ if $cors.CorsAllowCredentials }} more_set_headers 'Access-Control-Allow-Credentials: {{ $cors.CorsAllowCredentials }}'; {{ end }}
851-
more_set_headers 'Access-Control-Allow-Methods: {{ $cors.CorsAllowMethods }}';
852-
more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}';
853-
{{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }}
854-
more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}';
855-
}
843+
set $cors_origins_regex '{{ buildCorsOriginRegex $cors.CorsAllowOrigin }}';
844+
set $cors_allowed_origins '{{ join $cors.CorsAllowOrigin "," }}';
845+
set $cors_allow_methods '{{ $cors.CorsAllowMethods }}';
846+
set $cors_allow_headers '{{ $cors.CorsAllowHeaders }}';
847+
set $cors_max_age '{{ $cors.CorsMaxAge }}';
848+
{{ if $cors.CorsAllowCredentials }} set $cors_allow_credentials {{ $cors.CorsAllowCredentials }}; {{ end }}
849+
{{ if not (empty $cors.CorsExposeHeaders) }} set $cors_expose_headers '{{ $cors.CorsExposeHeaders }}'; {{ end }}
850+
851+
js_header_filter njs_handle_cors.handle_cors;
856852

857-
if ($cors = "trueoptions") {
858-
more_set_headers 'Access-Control-Allow-Origin: $http_origin';
859-
{{ if $cors.CorsAllowCredentials }} more_set_headers 'Access-Control-Allow-Credentials: {{ $cors.CorsAllowCredentials }}'; {{ end }}
860-
more_set_headers 'Access-Control-Allow-Methods: {{ $cors.CorsAllowMethods }}';
861-
more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}';
862-
{{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }}
863-
more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}';
864-
more_set_headers 'Content-Type: text/plain charset=UTF-8';
865-
more_set_headers 'Content-Length: 0';
866-
return 204;
853+
if ($request_method = 'OPTIONS') {
854+
return 204;
867855
}
868856
{{ end }}
869857

0 commit comments

Comments
 (0)