feat: support cors annotations for ingress (#2618)

Signed-off-by: Ashing Zheng <[email protected]>

Author: ronething

internal/adc/translator/annotations/plugins/cors.go

@@ -0,0 +1,45 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package plugins
+
+import (
+	adctypes "github.com/apache/apisix-ingress-controller/api/adc"
+	"github.com/apache/apisix-ingress-controller/internal/adc/translator/annotations"
+)
+
+type cors struct{}
+
+// NewCorsHandler creates a handler to convert annotations about
+// CORS to APISIX cors plugin.
+func NewCorsHandler() PluginAnnotationsHandler {
+	return &cors{}
+}
+
+func (c *cors) PluginName() string {
+	return "cors"
+}
+
+func (c *cors) Handle(e annotations.Extractor) (any, error) {
+	if !e.GetBoolAnnotation(annotations.AnnotationsEnableCors) {
+		return nil, nil
+	}
+
+	return &adctypes.CorsConfig{
+		AllowOrigins: e.GetStringAnnotation(annotations.AnnotationsCorsAllowOrigin),
+		AllowMethods: e.GetStringAnnotation(annotations.AnnotationsCorsAllowMethods),
+		AllowHeaders: e.GetStringAnnotation(annotations.AnnotationsCorsAllowHeaders),
+	}, nil
+}

internal/adc/translator/annotations/plugins/cors_test.go

@@ -0,0 +1,48 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements.  See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License.  You may obtain a copy of the License at
+//
+//	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package plugins
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	adctypes "github.com/apache/apisix-ingress-controller/api/adc"
+	"github.com/apache/apisix-ingress-controller/internal/adc/translator/annotations"
+)
+
+func TestCorsHandler(t *testing.T) {
+	anno := map[string]string{
+		annotations.AnnotationsEnableCors:       "true",
+		annotations.AnnotationsCorsAllowHeaders: "abc,def",
+		annotations.AnnotationsCorsAllowOrigin:  "https://a.com",
+		annotations.AnnotationsCorsAllowMethods: "GET,HEAD",
+	}
+	p := NewCorsHandler()
+	out, err := p.Handle(annotations.NewExtractor(anno))
+	assert.Nil(t, err, "checking given error")
+	config := out.(*adctypes.CorsConfig)
+	assert.Equal(t, "abc,def", config.AllowHeaders)
+	assert.Equal(t, "https://a.com", config.AllowOrigins)
+	assert.Equal(t, "GET,HEAD", config.AllowMethods)
+
+	assert.Equal(t, "cors", p.PluginName())
+
+	anno[annotations.AnnotationsEnableCors] = "false"
+	out, err = p.Handle(annotations.NewExtractor(anno))
+	assert.Nil(t, err, "checking given error")
+	assert.Nil(t, out, "checking given output")
+}

internal/adc/translator/annotations/plugins/plugins.go

@@ -37,6 +37,7 @@ var (
 
 	handlers = []PluginAnnotationsHandler{
 		NewRedirectHandler(),
+		NewCorsHandler(),
 	}
 )
 

internal/adc/translator/annotations_test.go

@@ -189,6 +189,24 @@ func TestTranslateIngressAnnotations(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "cors plugin",
+			anno: map[string]string{
+				annotations.AnnotationsEnableCors:       "true",
+				annotations.AnnotationsCorsAllowOrigin:  "https://example.com",
+				annotations.AnnotationsCorsAllowHeaders: "header-a,header-b",
+				annotations.AnnotationsCorsAllowMethods: "GET,POST",
+			},
+			expected: &IngressConfig{
+				Plugins: adctypes.Plugins{
+					"cors": &adctypes.CorsConfig{
+						AllowOrigins: "https://example.com",
+						AllowHeaders: "header-a,header-b",
+						AllowMethods: "GET,POST",
+					},
+				},
+			},
+		},
 	}
 
 	for _, tt := range tests {

internal/webhook/v1/ingress_webhook.go

@@ -42,10 +42,6 @@ var unsupportedAnnotations = []string{
 	"k8s.apisix.apache.org/use-regex",
 	"k8s.apisix.apache.org/enable-websocket",
 	"k8s.apisix.apache.org/plugin-config-name",
-	"k8s.apisix.apache.org/enable-cors",
-	"k8s.apisix.apache.org/cors-allow-origin",
-	"k8s.apisix.apache.org/cors-allow-headers",
-	"k8s.apisix.apache.org/cors-allow-methods",
 	"k8s.apisix.apache.org/enable-csrf",
 	"k8s.apisix.apache.org/csrf-key",
 	"k8s.apisix.apache.org/rewrite-target",

internal/webhook/v1/ingress_webhook_test.go

@@ -104,30 +104,6 @@ func TestIngressCustomValidator_ValidateCreate_SupportedAnnotations(t *testing.T
 	assert.Empty(t, warnings)
 }
 
-func TestIngressCustomValidator_ValidateUpdate_UnsupportedAnnotations(t *testing.T) {
-	validator := buildIngressValidator(t)
-	oldObj := &networkingv1.Ingress{}
-	obj := &networkingv1.Ingress{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-ingress",
-			Namespace: "default",
-			Annotations: map[string]string{
-				"k8s.apisix.apache.org/enable-cors":       "true",
-				"k8s.apisix.apache.org/cors-allow-origin": "*",
-			},
-		},
-	}
-
-	warnings, err := validator.ValidateUpdate(context.TODO(), oldObj, obj)
-	assert.NoError(t, err)
-	assert.Len(t, warnings, 2)
-
-	// Check that warnings contain the expected unsupported annotations
-	warningsStr := strings.Join(warnings, " ")
-	assert.Contains(t, warningsStr, "k8s.apisix.apache.org/enable-cors")
-	assert.Contains(t, warningsStr, "k8s.apisix.apache.org/cors-allow-origin")
-}
-
 func TestIngressCustomValidator_ValidateDelete_NoWarnings(t *testing.T) {
 	validator := buildIngressValidator(t)
 	obj := &networkingv1.Ingress{

test/e2e/ingress/annotations.go

@@ -19,6 +19,7 @@ package ingress
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"net/http"
 	"time"
@@ -102,6 +103,31 @@ spec:
             port:
               number: 443
 `
+
+			ingressCORS = `
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: cors
+  annotations:
+    k8s.apisix.apache.org/enable-cors: "true"
+    k8s.apisix.apache.org/cors-allow-origin: "https://allowed.example"
+    k8s.apisix.apache.org/cors-allow-methods: "GET,POST"
+    k8s.apisix.apache.org/cors-allow-headers: "Origin,Authorization"
+spec:
+  ingressClassName: %s
+  rules:
+  - host: cors.example
+    http:
+      paths:
+      - path: /get
+        pathType: Exact
+        backend:
+          service:
+            name: nginx
+            port:
+              number: 80
+`
 		)
 		BeforeEach(func() {
 			s.DeployNginx(framework.NginxOptions{
@@ -167,6 +193,53 @@ spec:
 			Expect(upstreams[0].Timeout.Send).To(Equal(3), "checking Upstream send timeout")
 			Expect(upstreams[0].Timeout.Connect).To(Equal(4), "checking Upstream connect timeout")
 		})
+
+		It("cors annotations", func() {
+			Expect(s.CreateResourceFromString(fmt.Sprintf(ingressCORS, s.Namespace()))).ShouldNot(HaveOccurred(), "creating Ingress")
+
+			s.RequestAssert(&scaffold.RequestAssert{
+				Method: "GET",
+				Path:   "/get",
+				Host:   "cors.example",
+				Headers: map[string]string{
+					"Origin": "https://allowed.example",
+				},
+				Checks: []scaffold.ResponseCheckFunc{
+					scaffold.WithExpectedStatus(http.StatusOK),
+					scaffold.WithExpectedHeaders(map[string]string{
+						"Access-Control-Allow-Origin":  "https://allowed.example",
+						"Access-Control-Allow-Methods": "GET,POST",
+						"Access-Control-Allow-Headers": "Origin,Authorization",
+					}),
+				},
+			})
+
+			s.RequestAssert(&scaffold.RequestAssert{
+				Method: "GET",
+				Path:   "/get",
+				Host:   "cors.example",
+				Headers: map[string]string{
+					"Origin": "https://blocked.example",
+				},
+				Checks: []scaffold.ResponseCheckFunc{
+					scaffold.WithExpectedStatus(http.StatusOK),
+					scaffold.WithExpectedNotHeader("Access-Control-Allow-Origin"),
+				},
+			})
+
+			routes, err := s.DefaultDataplaneResource().Route().List(context.Background())
+			Expect(err).NotTo(HaveOccurred(), "listing Service")
+			Expect(routes).To(HaveLen(1), "checking Route length")
+			Expect(routes[0].Plugins).To(HaveKey("cors"), "checking Route plugins")
+			jsonBytes, err := json.Marshal(routes[0].Plugins["cors"])
+			Expect(err).NotTo(HaveOccurred(), "marshalling cors plugin config")
+			var corsConfig map[string]any
+			err = json.Unmarshal(jsonBytes, &corsConfig)
+			Expect(err).NotTo(HaveOccurred(), "unmarshalling cors plugin config")
+			Expect(corsConfig["allow_origins"]).To(Equal("https://allowed.example"), "checking cors allow origins")
+			Expect(corsConfig["allow_methods"]).To(Equal("GET,POST"), "checking cors allow methods")
+			Expect(corsConfig["allow_headers"]).To(Equal("Origin,Authorization"), "checking cors allow headers")
+		})
 	})
 
 	Context("Plugins", func() {

test/e2e/webhook/ingress.go

@@ -87,74 +87,5 @@ spec:
 			})
 		})
 
-		It("should warn about unsupported annotations on update", func() {
-			By("creating Ingress without unsupported annotations")
-			initialIngressYAML := fmt.Sprintf(`
-apiVersion: networking.k8s.io/v1
-kind: Ingress
-metadata:
-  name: test-webhook-update
-  namespace: %s
-spec:
-  ingressClassName: %s
-  rules:
-  - host: webhook-test-update.example.com
-    http:
-      paths:
-      - path: /
-        pathType: Prefix
-        backend:
-          service:
-            name: httpbin-service-e2e-test
-            port:
-              number: 80
-`, s.Namespace(), s.Namespace())
-
-			output, err := s.CreateResourceFromStringAndGetOutput(initialIngressYAML)
-			Expect(err).ShouldNot(HaveOccurred())
-			Expect(output).ShouldNot(ContainSubstring(`Warning`))
-
-			s.RequestAssert(&scaffold.RequestAssert{
-				Method: "GET",
-				Path:   "/get",
-				Host:   "webhook-test-update.example.com",
-				Check:  scaffold.WithExpectedStatus(http.StatusOK),
-			})
-
-			By("updating Ingress with unsupported annotations")
-			updatedIngressYAML := fmt.Sprintf(`
-apiVersion: networking.k8s.io/v1
-kind: Ingress
-metadata:
-  name: test-webhook-update
-  namespace: %s
-  annotations:
-    k8s.apisix.apache.org/enable-cors: "true"
-spec:
-  ingressClassName: %s
-  rules:
-  - host: webhook-test-update.example.com
-    http:
-      paths:
-      - path: /
-        pathType: Prefix
-        backend:
-          service:
-            name: httpbin-service-e2e-test
-            port:
-              number: 80
-`, s.Namespace(), s.Namespace())
-
-			output, err = s.CreateResourceFromStringAndGetOutput(updatedIngressYAML)
-			Expect(err).ShouldNot(HaveOccurred())
-			Expect(output).To(ContainSubstring(`Warning: Annotation 'k8s.apisix.apache.org/enable-cors' is not supported`))
-
-			s.RequestAssert(&scaffold.RequestAssert{
-				Method: "GET",
-				Path:   "/get",
-				Host:   "webhook-test-update.example.com",
-				Check:  scaffold.WithExpectedStatus(http.StatusOK),
-			})
-		})
 	})
 })