Merge cb71d7ea09 into de1a4c463c

docs/user-guide/nginx-configuration/annotations.md

@@ -416,6 +416,10 @@ CORS can be controlled with the following annotations:
 !!! note
     For more information please see [https://enable-cors.org](https://enable-cors.org/server_nginx.html)
 
+#### Handling the `Vary: Origin` Header
+
+When CORS support is enabled and the allowed origins contains either a wildcard host  or more than one entry, the server will automatically add a `Vary: Origin` header to the response if the backend does not explicitly set one. The `Vary` header is used to indicate that the response may vary depending on the value of the `Origin` header in the request. This is important for caching and ensuring that responses are correctly handled based on the origin of the request.
+
 ### HTTP2 Push Preload.
 
 Enables automatic conversion of preload links specified in the “Link” response header fields into push requests.

internal/ingress/controller/template/template.go

@@ -331,6 +331,7 @@ var funcMap = text_template.FuncMap{
 	"shouldLoadAuthDigestModule":         shouldLoadAuthDigestModule,
 	"buildServerName":                    buildServerName,
 	"buildCorsOriginRegex":               buildCorsOriginRegex,
+	"isCorsVaryHeaderNeeded":             isCorsVaryHeaderNeeded,
 }
 
 // escapeLiteralDollar will replace the $ character with ${literal_dollar}
@@ -1692,3 +1693,10 @@ func buildCorsOriginRegex(corsOrigins []string) string {
 	originsRegex += ")$ ) { set $cors 'true'; }"
 	return originsRegex
 }
+
+func isCorsVaryHeaderNeeded(corsOrigins []string) bool {
+	hasMultipleOrigins := len(corsOrigins) > 1
+	hasAnyOrigin := len(corsOrigins) == 1 && corsOrigins[0] == "*"
+	hasSingleDynamicOrigin := len(corsOrigins) == 1 && strings.Contains(corsOrigins[0], "*")
+	return hasMultipleOrigins || hasAnyOrigin || hasSingleDynamicOrigin
+}

rootfs/etc/nginx/template/nginx.tmpl

@@ -409,6 +409,14 @@ http {
 
     {{ end }}
 
+    # CORS with multiple allowed origin need to serve the 'Vary: Origin' header
+    # See https://github.com/kubernetes/ingress-nginx/issues/8469#issuecomment-1195714565
+    # Only set it's value if it is not already set by the backend
+    map $upstream_http_vary $cors_vary {
+       default              "$upstream_http_vary";
+       ''                   "Origin";
+    }
+
     # Create a variable that contains the literal $ character.
     # This works because the geo module will not resolve variables.
     geo $literal_dollar {
@@ -852,7 +860,8 @@ stream {
         more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}';
         {{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }}
         more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}';
-     }
+        {{ if isCorsVaryHeaderNeeded $cors.CorsAllowOrigin }} more_set_headers 'Vary: $cors_vary'; {{ end }}
+    }
 
      if ($cors = "trueoptions") {
         more_set_headers 'Access-Control-Allow-Origin: $http_origin';
@@ -861,6 +870,7 @@ stream {
         more_set_headers 'Access-Control-Allow-Headers: {{ $cors.CorsAllowHeaders }}';
         {{ if not (empty $cors.CorsExposeHeaders) }} more_set_headers 'Access-Control-Expose-Headers: {{ $cors.CorsExposeHeaders }}'; {{ end }}
         more_set_headers 'Access-Control-Max-Age: {{ $cors.CorsMaxAge }}';
+        {{ if isCorsVaryHeaderNeeded $cors.CorsAllowOrigin }} more_set_headers 'Vary: $cors_vary'; {{ end }}
         more_set_headers 'Content-Type: text/plain charset=UTF-8';
         more_set_headers 'Content-Length: 0';
         return 204;

test/e2e/annotations/cors.go

@@ -31,7 +31,7 @@ const (
 )
 
 var _ = framework.DescribeAnnotation("cors-*", func() {
-	f := framework.NewDefaultFramework("cors")
+	f := framework.NewDefaultFramework("cors", framework.WithHTTPBunEnabled())
 
 	ginkgo.BeforeEach(func() {
 		f.NewEchoDeployment(framework.WithDeploymentReplicas(2))
@@ -96,6 +96,134 @@ var _ = framework.DescribeAnnotation("cors-*", func() {
 			})
 	})
 
+	ginkgo.It("should include vary header - multiple origins", func() {
+		host := corsHost
+		origin := originHost
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-cors":       "true",
+			"nginx.ingress.kubernetes.io/cors-allow-origin": "http://origin.com:8080, http://cors.foo.com",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "more_set_headers 'Vary: $cors_vary';")
+			})
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			WithHeader("Origin", origin).
+			Expect().
+			Headers().
+			ValueEqual("Vary", []string{"Origin"})
+	})
+
+	ginkgo.It("should include vary header - any origin", func() {
+		host := corsHost
+		origin := originHost
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-cors":       "true",
+			"nginx.ingress.kubernetes.io/cors-allow-origin": "*",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "more_set_headers 'Vary: $cors_vary';")
+			})
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			WithHeader("Origin", origin).
+			Expect().
+			Headers().
+			ValueEqual("Vary", []string{"Origin"})
+	})
+
+	ginkgo.It("should include vary header - single dynamic origin", func() {
+		host := corsHost
+		origin := "http://foo.origin.cors.com"
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-cors":       "true",
+			"nginx.ingress.kubernetes.io/cors-allow-origin": "http://*.origin.cors.com",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "more_set_headers 'Vary: $cors_vary';")
+			})
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			WithHeader("Origin", origin).
+			Expect().
+			Headers().
+			ValueEqual("Vary", []string{"Origin"})
+	})
+
+	ginkgo.It("should not include vary header - single static origin", func() {
+		host := corsHost
+		origin := originHost
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-cors":       "true",
+			"nginx.ingress.kubernetes.io/cors-allow-origin": originHost,
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return !strings.Contains(server, "more_set_headers 'Vary: $cors_vary';")
+			})
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			WithHeader("Origin", origin).
+			Expect().
+			Headers().
+			NotContainsKey("Vary")
+	})
+
+	ginkgo.It("should preserve upstream vary headers", func() {
+		host := corsHost
+		origin := originHost
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/enable-cors":       "true",
+			"nginx.ingress.kubernetes.io/cors-allow-origin": "*",
+		}
+
+		f.NewHttpbunDeployment(framework.WithDeploymentName("cors-service"))
+		f.EnsureIngress(framework.NewSingleIngress(
+			host,
+			"/",
+			host,
+			f.Namespace,
+			framework.HTTPBunService,
+			80,
+			annotations))
+
+		f.HTTPTestClient().
+			GET("/response-headers").
+			WithQuery("Vary", "Origin, X-My-Custom-Header").
+			WithHeader("Host", host).
+			WithHeader("Origin", origin).
+			Expect().
+			Headers().
+			ValueEqual("Vary", []string{"Origin, X-My-Custom-Header"})
+	})
+
 	ginkgo.It("should disable cors allow credentials", func() {
 		host := corsHost
 		annotations := map[string]string{