Merge pull request kubernetes#3017 from diazjf/more-e2e-1

Add e2e tests for CORS

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

@@ -235,37 +235,42 @@ This is a global configuration for the ingress controller. In some cases could b
 
 ### Enable CORS
 
-To enable Cross-Origin Resource Sharing (CORS) in an Ingress rule,
-add the annotation `nginx.ingress.kubernetes.io/enable-cors: "true"`.
-This will add a section in the server location enabling this functionality.
+To enable Cross-Origin Resource Sharing (CORS) in an Ingress rule, add the annotation
+`nginx.ingress.kubernetes.io/enable-cors: "true"`. This will add a section in the server
+location enabling this functionality.
 
 CORS can be controlled with the following annotations:
 
 * `nginx.ingress.kubernetes.io/cors-allow-methods`
-  controls which methods are accepted.
-  This is a multi-valued field, separated by ',' and accepts only letters (upper and lower case).
-  Example: `nginx.ingress.kubernetes.io/cors-allow-methods: "PUT, GET, POST, OPTIONS"`
+  controls which methods are accepted. This is a multi-valued field, separated by ',' and
+  accepts only letters (upper and lower case).
+  - Default: `GET, PUT, POST, DELETE, PATCH, OPTIONS`
+  - Example: `nginx.ingress.kubernetes.io/cors-allow-methods: "PUT, GET, POST, OPTIONS"`
 
 * `nginx.ingress.kubernetes.io/cors-allow-headers`
-  controls which headers are accepted.
-  This is a multi-valued field, separated by ',' and accepts letters, numbers, _ and -.
-  Example: `nginx.ingress.kubernetes.io/cors-allow-headers: "X-Forwarded-For, X-app123-XPTO"`
+  controls which headers are accepted. This is a multi-valued field, separated by ',' and accepts letters,
+  numbers, _ and -.
+  - Default: `DNT,X-CustomHeader,Keep-Alive,User-Agent,X-Requested-With,If-Modified-Since,Cache-Control,Content-Type,Authorization`
+  - Example: `nginx.ingress.kubernetes.io/cors-allow-headers: "X-Forwarded-For, X-app123-XPTO"`
 
 * `nginx.ingress.kubernetes.io/cors-allow-origin`
-  controls what's the accepted Origin for CORS and defaults to '*'.
+  controls what's the accepted Origin for CORS.
   This is a single field value, with the following format: `http(s)://origin-site.com` or `http(s)://origin-site.com:port`
-  Example: `nginx.ingress.kubernetes.io/cors-allow-origin: "https://origin-site.com:4443"`
+  - Default: `*`
+  - Example: `nginx.ingress.kubernetes.io/cors-allow-origin: "https://origin-site.com:4443"`
 
 * `nginx.ingress.kubernetes.io/cors-allow-credentials`
   controls if credentials can be passed during CORS operations.
-  Example: `nginx.ingress.kubernetes.io/cors-allow-credentials: "true"`
+  - Default: `true`
+  - Example: `nginx.ingress.kubernetes.io/cors-allow-credentials: "false"`
 
 * `nginx.ingress.kubernetes.io/cors-max-age`
   controls how long preflight requests can be cached.
+  Default: `1728000`
   Example: `nginx.ingress.kubernetes.io/cors-max-age: 600`
 
 !!! note
-    For more information please see [https://enable-cors.org](https://enable-cors.org/server_nginx.html)
+    For more information please see [https://enable-cors.org](https://enable-cors.org/server_nginx.html) as well as this [example](../../examples/cors/README.md).
 
 ### Server Alias
 

internal/ingress/annotations/cors/main_test.go

@@ -62,41 +62,100 @@ func buildIngress() *extensions.Ingress {
 	}
 }
 
-func TestIngressCorsConfig(t *testing.T) {
+func TestIngressCorsConfigValid(t *testing.T) {
 	ing := buildIngress()
 
 	data := map[string]string{}
+
+	// Valid
 	data[parser.GetAnnotationWithPrefix("enable-cors")] = "true"
 	data[parser.GetAnnotationWithPrefix("cors-allow-headers")] = "DNT,X-CustomHeader, Keep-Alive,User-Agent"
 	data[parser.GetAnnotationWithPrefix("cors-allow-credentials")] = "false"
-	data[parser.GetAnnotationWithPrefix("cors-allow-methods")] = "PUT, GET,OPTIONS, PATCH, $nginx_version"
+	data[parser.GetAnnotationWithPrefix("cors-allow-methods")] = "GET, PATCH"
 	data[parser.GetAnnotationWithPrefix("cors-allow-origin")] = "https://origin123.test.com:4443"
 	data[parser.GetAnnotationWithPrefix("cors-max-age")] = "600"
 	ing.SetAnnotations(data)
 
-	corst, _ := NewParser(&resolver.Mock{}).Parse(ing)
+	corst, err := NewParser(&resolver.Mock{}).Parse(ing)
+	if err != nil {
+		t.Errorf("error parsing annotations: %v", err)
+	}
+
 	nginxCors, ok := corst.(*Config)
 	if !ok {
-		t.Errorf("expected a Config type")
+		t.Errorf("expected a Config type but returned %t", corst)
 	}
 
 	if !nginxCors.CorsEnabled {
-		t.Errorf("expected cors enabled but returned %v", nginxCors.CorsEnabled)
+		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("enable-cors")], nginxCors.CorsEnabled)
+	}
+
+	if nginxCors.CorsAllowCredentials {
+		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("cors-allow-credentials")], nginxCors.CorsAllowCredentials)
 	}
 
 	if nginxCors.CorsAllowHeaders != "DNT,X-CustomHeader, Keep-Alive,User-Agent" {
-		t.Errorf("expected headers not found. Found %v", nginxCors.CorsAllowHeaders)
+		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("cors-allow-headers")], nginxCors.CorsAllowHeaders)
 	}
 
-	if nginxCors.CorsAllowMethods != "GET, PUT, POST, DELETE, PATCH, OPTIONS" {
-		t.Errorf("expected default methods, but got  %v", nginxCors.CorsAllowMethods)
+	if nginxCors.CorsAllowMethods != "GET, PATCH" {
+		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("cors-allow-methods")], nginxCors.CorsAllowMethods)
 	}
 
 	if nginxCors.CorsAllowOrigin != "https://origin123.test.com:4443" {
-		t.Errorf("expected origin https://origin123.test.com:4443, but got  %v", nginxCors.CorsAllowOrigin)
+		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("cors-allow-origin")], nginxCors.CorsAllowOrigin)
 	}
 
 	if nginxCors.CorsMaxAge != 600 {
-		t.Errorf("expected max age 600, but got  %v", nginxCors.CorsMaxAge)
+		t.Errorf("expected %v but returned %v", data[parser.GetAnnotationWithPrefix("cors-max-age")], nginxCors.CorsMaxAge)
+	}
+}
+
+func TestIngressCorsConfigInvalid(t *testing.T) {
+	ing := buildIngress()
+
+	data := map[string]string{}
+
+	// Valid
+	data[parser.GetAnnotationWithPrefix("enable-cors")] = "yes"
+	data[parser.GetAnnotationWithPrefix("cors-allow-headers")] = "@alright, #ingress"
+	data[parser.GetAnnotationWithPrefix("cors-allow-credentials")] = "no"
+	data[parser.GetAnnotationWithPrefix("cors-allow-methods")] = "GET, PATCH, $nginx"
+	data[parser.GetAnnotationWithPrefix("cors-allow-origin")] = "origin123.test.com:4443"
+	data[parser.GetAnnotationWithPrefix("cors-max-age")] = "abcd"
+	ing.SetAnnotations(data)
+
+	corst, err := NewParser(&resolver.Mock{}).Parse(ing)
+	if err != nil {
+		t.Errorf("error parsing annotations: %v", err)
+	}
+
+	nginxCors, ok := corst.(*Config)
+	if !ok {
+		t.Errorf("expected a Config type but returned %t", corst)
+	}
+
+	if nginxCors.CorsEnabled {
+		t.Errorf("expected %v but returned %v", false, nginxCors.CorsEnabled)
+	}
+
+	if !nginxCors.CorsAllowCredentials {
+		t.Errorf("expected %v but returned %v", true, nginxCors.CorsAllowCredentials)
+	}
+
+	if nginxCors.CorsAllowHeaders != defaultCorsHeaders {
+		t.Errorf("expected %v but returned %v", defaultCorsHeaders, nginxCors.CorsAllowHeaders)
+	}
+
+	if nginxCors.CorsAllowMethods != defaultCorsMethods {
+		t.Errorf("expected %v but returned %v", defaultCorsHeaders, nginxCors.CorsAllowMethods)
+	}
+
+	if nginxCors.CorsAllowOrigin != "*" {
+		t.Errorf("expected %v but returned %v", "*", nginxCors.CorsAllowOrigin)
+	}
+
+	if nginxCors.CorsMaxAge != defaultCorsMaxAge {
+		t.Errorf("expected %v but returned %v", defaultCorsMaxAge, nginxCors.CorsMaxAge)
 	}
 }

test/e2e/annotations/clientbodybuffersize.go

@@ -28,7 +28,7 @@ import (
 )
 
 var _ = framework.IngressNginxDescribe("Annotations - Client-Body-Buffer-Size", func() {
-	f := framework.NewDefaultFramework("proxy")
+	f := framework.NewDefaultFramework("clientbodybuffersize")
 
 	BeforeEach(func() {
 		err := f.NewEchoDeploymentWithReplicas(2)