From ecfb22619fdd7d48a78be30586d93e65cc6b6b0b Mon Sep 17 00:00:00 2001 From: mpoqq Date: Wed, 28 Aug 2024 13:25:55 +0200 Subject: [PATCH] fix: tests and json format --- .../gateway/translation/gateway_httproute.go | 12 ++-- .../annotations/plugins/response_rewrite.go | 10 +-- .../plugins/response_rewrite_test.go | 2 +- .../annotations/plugins/rewrite.go | 21 ++++--- .../annotations/plugins/rewrite_test.go | 2 +- pkg/types/apisix/v1/plugin_types.go | 63 ++++++++----------- test/e2e/suite-annotations/rewrite.go | 24 +++---- 7 files changed, 61 insertions(+), 73 deletions(-) diff --git a/pkg/providers/gateway/translation/gateway_httproute.go b/pkg/providers/gateway/translation/gateway_httproute.go index 5b71771ca7..8811a24a13 100644 --- a/pkg/providers/gateway/translation/gateway_httproute.go +++ b/pkg/providers/gateway/translation/gateway_httproute.go @@ -56,25 +56,23 @@ func (t *translator) generatePluginFromHTTPRequestHeaderFilter(plugins apisixv1. return } // TODO: The current apisix plugin does not conform to the specification. - headers := apisixv1.RewriteConfigHeaders{} + plugin := apisixv1.RewriteConfig{} headersToAdd := make([]string, len(reqHeaderModifier.Add)) for i, header := range reqHeaderModifier.Add { headersToAdd[i] = fmt.Sprintf("%s:%s", header.Name, header.Value) } - headers.Add(headersToAdd) + plugin.Headers.SetAddHeaders(headersToAdd) headersToSet := make([]string, len(reqHeaderModifier.Set)) for i, header := range reqHeaderModifier.Set { headersToSet[i] = fmt.Sprintf("%s:%s", header.Name, header.Value) } - headers.Set(headersToSet) + plugin.Headers.SetSetHeaders(headersToSet) - headers.Remove(reqHeaderModifier.Remove) + plugin.Headers.SetRemoveHeaders(reqHeaderModifier.Remove) - plugins["proxy-rewrite"] = apisixv1.RewriteConfig{ - Headers: headers, - } + plugins["proxy-rewrite"] = plugin } func (t *translator) generatePluginFromHTTPRequestMirrorFilter(namespace string, plugins apisixv1.Plugins, reqMirror *gatewayv1beta1.HTTPRequestMirrorFilter) { diff --git a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go index de56053e68..76f8bd3cda 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go @@ -42,12 +42,8 @@ func (c *responseRewrite) Handle(e annotations.Extractor) (interface{}, error) { plugin.StatusCode, _ = strconv.Atoi(e.GetStringAnnotation(annotations.AnnotationsResponseRewriteStatusCode)) plugin.Body = e.GetStringAnnotation(annotations.AnnotationsResponseRewriteBody) plugin.BodyBase64 = e.GetBoolAnnotation(annotations.AnnotationsResponseRewriteBodyBase64) - headers := apisixv1.ResponseRewriteConfigHeaders{ - Headers: make(apisixv1.Headers), - } - headers.Add(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderAdd)) - headers.Set(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderSet)) - headers.Remove(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderRemove)) - plugin.Headers = headers + plugin.Headers.SetAddHeaders(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderAdd)) + plugin.Headers.SetSetHeaders(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderSet)) + plugin.Headers.SetRemoveHeaders(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderRemove)) return &plugin, nil } diff --git a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite_test.go b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite_test.go index 302f7a21d9..b4f1c4d14c 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite_test.go +++ b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite_test.go @@ -41,7 +41,7 @@ func TestResponseRewriteHandler(t *testing.T) { assert.Equal(t, "bar_body", config.Body) assert.Equal(t, false, config.BodyBase64) assert.Equal(t, "response-rewrite", p.PluginName()) - assert.Equal(t, []string{"testkey1:testval1", "testkey2:testval2"}, config.Headers.GetAddedHeaders()) + assert.Equal(t, []string{"testkey1:testval1", "testkey2:testval2"}, config.Headers.GetAddHeaders()) assert.Equal(t, []string{"testkey1", "testkey2"}, config.Headers.GetRemovedHeaders()) assert.Equal(t, map[string]string{ "testkey1": "testval1", diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go index 0cc9e26ca5..8f1bc647e7 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go @@ -39,16 +39,12 @@ func (i *rewrite) Handle(e annotations.Extractor) (interface{}, error) { rewriteTargetRegex := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegex) rewriteTemplate := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegexTemplate) - headers := apisixv1.RewriteConfigHeaders{ - Headers: make(apisixv1.Headers), - } - headers.Add(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderAdd)) - headers.Set(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderSet)) - headers.Remove(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderRemove)) + headersToAdd := e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderAdd) + headersToSet := e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderSet) + headersToRemove := e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderRemove) - if rewriteTarget != "" || rewriteTargetRegex != "" || rewriteTemplate != "" || len(headers.Headers) > 0 { + if rewriteTarget != "" || rewriteTargetRegex != "" || rewriteTemplate != "" || len(headersToAdd) > 0 || len(headersToSet) > 0 || len(headersToRemove) > 0 { plugin.RewriteTarget = rewriteTarget - plugin.Headers = headers if rewriteTargetRegex != "" && rewriteTemplate != "" { _, err := regexp.Compile(rewriteTargetRegex) if err != nil { @@ -56,6 +52,15 @@ func (i *rewrite) Handle(e annotations.Extractor) (interface{}, error) { } plugin.RewriteTargetRegex = []string{rewriteTargetRegex, rewriteTemplate} } + if len(headersToAdd) > 0 { + plugin.Headers.SetAddHeaders(headersToAdd) + } + if len(headersToSet) > 0 { + plugin.Headers.SetSetHeaders(headersToSet) + } + if len(headersToAdd) > 0 { + plugin.Headers.SetRemoveHeaders(headersToRemove) + } return &plugin, nil } return nil, nil diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go index c165413801..cbf017711b 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go @@ -42,7 +42,7 @@ func TestRewriteHandler(t *testing.T) { assert.Equal(t, map[string]string{ "testkey1": "testval1", "testkey2": "testval2", - }, config.Headers.GetAddedHeaders()) + }, config.Headers.GetAddHeaders()) assert.Equal(t, []string{"testkey1", "testkey2"}, config.Headers.GetRemovedHeaders()) assert.Equal(t, map[string]string{ "testsetkey1": "testsetval1", diff --git a/pkg/types/apisix/v1/plugin_types.go b/pkg/types/apisix/v1/plugin_types.go index b1a9895481..72d88e71d6 100644 --- a/pkg/types/apisix/v1/plugin_types.go +++ b/pkg/types/apisix/v1/plugin_types.go @@ -187,14 +187,19 @@ type RequestMirror struct { Host string `json:"host"` } -type Headers map[string]any +type Headers struct { + Set map[string]string `json:"set,omitempty"` + Remove []string `json:"remove,omitempty"` +} type ResponseRewriteConfigHeaders struct { - Headers `json:"headers,omitempty"` + Add []string `json:"add,omitempty"` + Headers } type RewriteConfigHeaders struct { - Headers `json:"headers,omitempty"` + Add map[string]string `json:"add,omitempty"` + Headers } func (p *ResponseRewriteConfigHeaders) DeepCopyInto(out *ResponseRewriteConfigHeaders) { @@ -225,7 +230,7 @@ func (p *RewriteConfigHeaders) DeepCopy() *RewriteConfigHeaders { return out } -func (p *ResponseRewriteConfigHeaders) Add(headersToAdd []string) { +func (p *ResponseRewriteConfigHeaders) SetAddHeaders(headersToAdd []string) { if p == nil { return } @@ -238,22 +243,18 @@ func (p *ResponseRewriteConfigHeaders) Add(headersToAdd []string) { } addedHeader = append(addedHeader, fmt.Sprintf("%s:%s", kv[0], kv[1])) } - (p.Headers)["add"] = addedHeader + p.Add = addedHeader } } -func (p *ResponseRewriteConfigHeaders) GetAddedHeaders() []string { - if p == nil || (p.Headers)["add"] == nil { +func (p *ResponseRewriteConfigHeaders) GetAddHeaders() []string { + if p == nil || p.Add == nil { return nil } - addedheaders, ok := (p.Headers)["add"].([]string) - if ok { - return addedheaders - } - return nil + return p.Add } -func (p *RewriteConfigHeaders) Add(headersToAdd []string) { +func (p *RewriteConfigHeaders) SetAddHeaders(headersToAdd []string) { if p == nil { return } @@ -266,22 +267,18 @@ func (p *RewriteConfigHeaders) Add(headersToAdd []string) { } addedHeader[kv[0]] = kv[1] } - (p.Headers)["add"] = addedHeader + p.Add = addedHeader } } -func (p *RewriteConfigHeaders) GetAddedHeaders() map[string]string { - if p == nil || (p.Headers)["add"] == nil { +func (p *RewriteConfigHeaders) GetAddHeaders() map[string]string { + if p == nil || p.Add == nil { return nil } - addedheaders, ok := (p.Headers)["add"].(map[string]string) - if ok { - return addedheaders - } - return nil + return p.Add } -func (p *Headers) Set(headersToSet []string) { +func (p *Headers) SetSetHeaders(headersToSet []string) { if p == nil { return } @@ -294,39 +291,31 @@ func (p *Headers) Set(headersToSet []string) { } setHeaders[kv[0]] = kv[1] } - (*p)["set"] = setHeaders + p.Set = setHeaders } } func (p *Headers) GetSetHeaders() map[string]string { - if p == nil || (*p)["set"] == nil { + if p == nil || p.Set == nil { return nil } - addedheaders, ok := (*p)["set"].(map[string]string) - if ok { - return addedheaders - } - return nil + return p.Set } -func (p *Headers) Remove(headersToRemove []string) { +func (p *Headers) SetRemoveHeaders(headersToRemove []string) { if p == nil { return } if headersToRemove != nil { removeHeaders := make([]string, 0) removeHeaders = append(removeHeaders, headersToRemove...) - (*p)["remove"] = removeHeaders + p.Remove = removeHeaders } } func (p *Headers) GetRemovedHeaders() []string { - if p == nil || (*p)["remove"] == nil { + if p == nil || p.Remove == nil { return nil } - removedHeaders, ok := (*p)["remove"].([]string) - if ok { - return removedHeaders - } - return nil + return p.Remove } diff --git a/test/e2e/suite-annotations/rewrite.go b/test/e2e/suite-annotations/rewrite.go index 4d38764ae6..765e34fd4b 100644 --- a/test/e2e/suite-annotations/rewrite.go +++ b/test/e2e/suite-annotations/rewrite.go @@ -155,7 +155,7 @@ spec: }) }) -var _ = ginkgo.Describe("suite-annotations: rewrite header annotations", func() { +var _ = ginkgo.FDescribe("suite-annotations: rewrite header annotations", func() { s := scaffold.NewDefaultScaffold() ginkgo.It("enable in ingress networking/v1", func() { @@ -168,8 +168,8 @@ metadata: kubernetes.io/ingress.class: apisix k8s.apisix.apache.org/rewrite-target-regex: "/sample/(.*)" k8s.apisix.apache.org/rewrite-target-regex-template: "/$1" - k8s.apisix.apache.org/rewrite-add-header: "X-Api-Version:v1;X-Api-Engine:Apisix" - k8s.apisix.apache.org/rewrite-set-header: "X-Request-ID:123" + k8s.apisix.apache.org/rewrite-add-header: "X-Api-Version:v1,X-Api-Engine:Apisix" + k8s.apisix.apache.org/rewrite-set-header: "X-Api-Custom:extended" k8s.apisix.apache.org/rewrite-remove-header: "X-Test" name: ingress-v1 spec: @@ -189,27 +189,27 @@ spec: assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") time.Sleep(5 * time.Second) - resp := s.NewAPISIXClient().GET("/sample/get").WithHeader("Host", "httpbin.org").WithHeader("X-Request-ID", "000").WithHeader("X-Test", "Test").Expect() + resp := s.NewAPISIXClient().GET("/sample/get").WithHeader("Host", "httpbin.org").WithHeader("X-Api-Custom", "Basic").WithHeader("X-Test", "Test").Expect() resp.Status(http.StatusOK) resp.Body().Contains("\"X-Api-Version\": \"v1\"") resp.Body().Contains("\"X-Api-Engine\": \"Apisix\"") - resp.Body().Contains("\"X-Request-ID\": \"123\"") + resp.Body().Contains("\"X-Api-Custom\": \"extended\"") resp.Body().NotContains("\"X-Test\"") }) ginkgo.It("enable in ingress networking/v1beta1", func() { backendSvc, backendPort := s.DefaultHTTPBackend() ing := fmt.Sprintf(` -apiVersion: networking.k8s.io/v1 +apiVersion: networking.k8s.io/v1beta1 kind: Ingress metadata: annotations: kubernetes.io/ingress.class: apisix k8s.apisix.apache.org/rewrite-target-regex: "/sample/(.*)" k8s.apisix.apache.org/rewrite-target-regex-template: "/$1" - k8s.apisix.apache.org/rewrite-add-header: "X-Api-Version:v1;X-Api-Engine:Apisix" - k8s.apisix.apache.org/rewrite-set-header: "X-Request-ID:123" + k8s.apisix.apache.org/rewrite-add-header: "X-Api-Version:v1,X-Api-Engine:Apisix" + k8s.apisix.apache.org/rewrite-set-header: "X-Api-Custom:extended" k8s.apisix.apache.org/rewrite-remove-header: "X-Test" - name: ingress-v1 + name: ingress-v1beta1 spec: rules: - host: httpbin.org @@ -220,16 +220,16 @@ spec: backend: serviceName: %s servicePort: %d - `, backendSvc, backendPort[0]) +`, backendSvc, backendPort[0]) err := s.CreateResourceFromString(ing) assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") time.Sleep(5 * time.Second) - resp := s.NewAPISIXClient().GET("/sample/get").WithHeader("Host", "httpbin.org").WithHeader("X-Request-ID", "000").WithHeader("X-Test", "Test").Expect() + resp := s.NewAPISIXClient().GET("/sample/get").WithHeader("Host", "httpbin.org").WithHeader("X-Api-Custom", "Basic").WithHeader("X-Test", "Test").Expect() resp.Status(http.StatusOK) resp.Body().Contains("\"X-Api-Version\": \"v1\"") resp.Body().Contains("\"X-Api-Engine\": \"Apisix\"") - resp.Body().Contains("\"X-Request-ID\": \"123\"") + resp.Body().Contains("\"X-Api-Custom\": \"extended\"") resp.Body().NotContains("\"X-Test\"") }) })