From 3eea3f47c1113e406761ab59e8f8643b1dc2b17f Mon Sep 17 00:00:00 2001 From: mpoqq Date: Mon, 26 Aug 2024 16:56:57 +0200 Subject: [PATCH 1/7] feat: Add Rewrite Header Annotations --- docs/en/latest/concepts/annotations.md | 27 +++++++ .../annotations/plugins/rewrite.go | 9 ++- .../annotations/plugins/rewrite_test.go | 48 +++++++++++ .../ingress/translation/annotations/types.go | 3 + test/e2e/suite-annotations/rewrite.go | 81 +++++++++++++++++++ 5 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go diff --git a/docs/en/latest/concepts/annotations.md b/docs/en/latest/concepts/annotations.md index a3292b1ec1..38fe267ab4 100644 --- a/docs/en/latest/concepts/annotations.md +++ b/docs/en/latest/concepts/annotations.md @@ -157,6 +157,33 @@ spec: number: 80 ``` +### Rewrite Headers + +### Add header + +This annotation configures to append the new headers in the upstream request. + +```yaml +k8s.apisix.apache.org/rewrite-add-header: "testkey1:testval1,testkey2:testval2" +``` + +### Set header + +This annotation configures to rewrite the new headers in the upstream request. + +```yaml +k8s.apisix.apache.org/rewrite-set-header: "testkey1:testval1,testkey2:testval2" +``` + +### Remove header + +This annotation configures to remove headers in the upstream request. + +```yaml +k8s.apisix.apache.org/rewrite-remove-header: "testkey1,testkey2" +``` + + ## HTTP to HTTPS This annotation is used to redirect HTTP requests to HTTPS with a `301` status code and with the same URI as the original request. diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go index 406f87f05a..54b8947c63 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go @@ -38,8 +38,15 @@ func (i *rewrite) Handle(e annotations.Extractor) (interface{}, error) { rewriteTarget := e.GetStringAnnotation(annotations.AnnotationsRewriteTarget) rewriteTargetRegex := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegex) rewriteTemplate := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegexTemplate) - if rewriteTarget != "" || rewriteTargetRegex != "" || rewriteTemplate != "" { + + headers := make(apisixv1.Headers) + headers.Add(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderAdd)) + headers.Set(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderSet)) + headers.Remove(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderRemove)) + + if rewriteTarget != "" || rewriteTargetRegex != "" || rewriteTemplate != "" || len(headers) > 0 { plugin.RewriteTarget = rewriteTarget + plugin.Headers = headers if rewriteTargetRegex != "" && rewriteTemplate != "" { _, err := regexp.Compile(rewriteTargetRegex) if err != nil { diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go new file mode 100644 index 0000000000..ab0b1021c4 --- /dev/null +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite_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" + + "github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations" + apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1" +) + +func TestRewriteHandler(t *testing.T) { + anno := map[string]string{ + annotations.AnnotationsRewriteTarget: "/sample", + annotations.AnnotationsRewriteTargetRegex: "/sample/(.*)", + annotations.AnnotationsRewriteTargetRegexTemplate: "/$1", + annotations.AnnotationsRewriteHeaderAdd: "testkey1:testval1,testkey2:testval2", + annotations.AnnotationsRewriteHeaderRemove: "testkey1,testkey2", + annotations.AnnotationsRewriteHeaderSet: "testkey1:testval1,testkey2:testval2", + } + p := NewRewriteHandler() + out, err := p.Handle(annotations.NewExtractor(anno)) + assert.Nil(t, err, "checking given error") + config := out.(*apisixv1.RewriteConfig) + assert.Equal(t, "/sample", config.RewriteTarget) + assert.Equal(t, []string{"/sample/(.*)", "/$1"}, config.RewriteTargetRegex) + assert.Equal(t, "proxy-rewrite", p.PluginName()) + assert.Equal(t, []string{"testkey1:testval1", "testkey2:testval2"}, config.Headers.GetAddedHeaders()) + assert.Equal(t, []string{"testkey1", "testkey2"}, config.Headers.GetRemovedHeaders()) + assert.Equal(t, map[string]string{ + "testkey1": "testval1", + "testkey2": "testval2", + }, config.Headers.GetSetHeaders()) +} diff --git a/pkg/providers/ingress/translation/annotations/types.go b/pkg/providers/ingress/translation/annotations/types.go index aac0a0baf4..9a5ba5d166 100644 --- a/pkg/providers/ingress/translation/annotations/types.go +++ b/pkg/providers/ingress/translation/annotations/types.go @@ -57,6 +57,9 @@ const ( AnnotationsRewriteTarget = AnnotationsPrefix + "rewrite-target" AnnotationsRewriteTargetRegex = AnnotationsPrefix + "rewrite-target-regex" AnnotationsRewriteTargetRegexTemplate = AnnotationsPrefix + "rewrite-target-regex-template" + AnnotationsRewriteHeaderAdd = AnnotationsPrefix + "rewrite-add-header" + AnnotationsRewriteHeaderSet = AnnotationsPrefix + "rewrite-set-header" + AnnotationsRewriteHeaderRemove = AnnotationsPrefix + "rewrite-remove-header" // response-rewrite plugin AnnotationsEnableResponseRewrite = AnnotationsPrefix + "enable-response-rewrite" diff --git a/test/e2e/suite-annotations/rewrite.go b/test/e2e/suite-annotations/rewrite.go index d3795600a5..48779f1c06 100644 --- a/test/e2e/suite-annotations/rewrite.go +++ b/test/e2e/suite-annotations/rewrite.go @@ -154,3 +154,84 @@ spec: _ = s.NewAPISIXClient().GET("/sample/get").WithHeader("Host", "httpbin.org").Expect().Status(http.StatusOK) }) }) + +var _ = ginkgo.Describe("suite-annotations: rewrite header annotations", func() { + s := scaffold.NewDefaultScaffold() + + ginkgo.It("enable in ingress networking/v1", func() { + backendSvc, backendPort := s.DefaultHTTPBackend() + ing := fmt.Sprintf(` +apiVersion: networking.k8s.io/v1 +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-remove-header: "X-Test" + name: ingress-v1 +spec: + rules: + - host: httpbin.org + http: + paths: + - path: /sample + pathType: Prefix + backend: + service: + name: %s + port: + number: %d +`, backendSvc, backendPort[0]) + err := s.CreateResourceFromString(ing) + assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") + time.Sleep(5 * time.Second) + + resp := s.NewAPISIXClient().GET("/sample/ip").WithHeader("Host", "httpbin.org").WithHeader("X-Request-ID", "000").WithHeader("X-Test", "Test").Expect() + resp.Status(http.StatusOK) + resp.Header("X-Api-Version").IsEqual("v1") + resp.Header("X-Api-Engine").IsEqual("Apisix") + resp.Header("X-Request-ID").IsEqual("123") + resp.Header("X-Test").IsEmpty() + }) + ginkgo.It("enable in ingress networking/v1beta1", func() { + backendSvc, backendPort := s.DefaultHTTPBackend() + ing := fmt.Sprintf(` +apiVersion: networking.k8s.io/v1 +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-remove-header: "X-Test" + name: ingress-v1 +spec: + rules: + - host: httpbin.org + http: + paths: + - path: /sample + pathType: Prefix + backend: + service: + name: %s + port: + number: %d + `, backendSvc, backendPort[0]) + err := s.CreateResourceFromString(ing) + assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") + time.Sleep(5 * time.Second) + + resp := s.NewAPISIXClient().GET("/sample/ip").WithHeader("Host", "httpbin.org").WithHeader("X-Request-ID", "000").WithHeader("X-Test", "Test").Expect() + resp.Status(http.StatusOK) + resp.Header("X-Api-Version").IsEqual("v1") + resp.Header("X-Api-Engine").IsEqual("Apisix") + resp.Header("X-Request-ID").IsEqual("123") + resp.Header("X-Test").IsEmpty() + }) +}) From 757b872bfe5debd223458a28a9675cf719bdcc54 Mon Sep 17 00:00:00 2001 From: mpoqq Date: Tue, 27 Aug 2024 16:11:05 +0200 Subject: [PATCH 2/7] fix: RewriteConfigHeader struct --- docs/en/latest/concepts/annotations.md | 1 - .../gateway/translation/gateway_httproute.go | 21 +++-- .../annotations/plugins/response_rewrite.go | 2 +- .../annotations/plugins/rewrite.go | 4 +- pkg/types/apisix/v1/plugin_types.go | 84 +++++++++++++++---- test/e2e/suite-annotations/rewrite.go | 6 +- 6 files changed, 85 insertions(+), 33 deletions(-) diff --git a/docs/en/latest/concepts/annotations.md b/docs/en/latest/concepts/annotations.md index 38fe267ab4..ca479cfb00 100644 --- a/docs/en/latest/concepts/annotations.md +++ b/docs/en/latest/concepts/annotations.md @@ -183,7 +183,6 @@ This annotation configures to remove headers in the upstream request. k8s.apisix.apache.org/rewrite-remove-header: "testkey1,testkey2" ``` - ## HTTP to HTTPS This annotation is used to redirect HTTP requests to HTTPS with a `301` status code and with the same URI as the original request. diff --git a/pkg/providers/gateway/translation/gateway_httproute.go b/pkg/providers/gateway/translation/gateway_httproute.go index f697c55c67..5b71771ca7 100644 --- a/pkg/providers/gateway/translation/gateway_httproute.go +++ b/pkg/providers/gateway/translation/gateway_httproute.go @@ -55,17 +55,22 @@ func (t *translator) generatePluginFromHTTPRequestHeaderFilter(plugins apisixv1. if reqHeaderModifier == nil { return } - headers := map[string]any{} // TODO: The current apisix plugin does not conform to the specification. - for _, header := range reqHeaderModifier.Add { - headers[string(header.Name)] = header.Value - } - for _, header := range reqHeaderModifier.Set { - headers[string(header.Name)] = header.Value + headers := apisixv1.RewriteConfigHeaders{} + + headersToAdd := make([]string, len(reqHeaderModifier.Add)) + for i, header := range reqHeaderModifier.Add { + headersToAdd[i] = fmt.Sprintf("%s:%s", header.Name, header.Value) } - for _, header := range reqHeaderModifier.Remove { - headers[header] = "" + headers.Add(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) + + headers.Remove(reqHeaderModifier.Remove) plugins["proxy-rewrite"] = apisixv1.RewriteConfig{ Headers: headers, diff --git a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go index f03a1d9491..66124104a1 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go @@ -42,7 +42,7 @@ 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 := make(apisixv1.Headers) + headers := apisixv1.ResponseRewriteConfigHeaders{} headers.Add(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderAdd)) headers.Set(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderSet)) headers.Remove(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderRemove)) diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go index 54b8947c63..d14f7e1988 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go @@ -39,12 +39,12 @@ func (i *rewrite) Handle(e annotations.Extractor) (interface{}, error) { rewriteTargetRegex := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegex) rewriteTemplate := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegexTemplate) - headers := make(apisixv1.Headers) + headers := apisixv1.RewriteConfigHeaders{} headers.Add(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderAdd)) headers.Set(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderSet)) headers.Remove(e.GetStringsAnnotation(annotations.AnnotationsRewriteHeaderRemove)) - if rewriteTarget != "" || rewriteTargetRegex != "" || rewriteTemplate != "" || len(headers) > 0 { + if rewriteTarget != "" || rewriteTargetRegex != "" || rewriteTemplate != "" || len(headers.Headers) > 0 { plugin.RewriteTarget = rewriteTarget plugin.Headers = headers if rewriteTargetRegex != "" && rewriteTemplate != "" { diff --git a/pkg/types/apisix/v1/plugin_types.go b/pkg/types/apisix/v1/plugin_types.go index 51c1f6382d..9b5d791b96 100644 --- a/pkg/types/apisix/v1/plugin_types.go +++ b/pkg/types/apisix/v1/plugin_types.go @@ -137,20 +137,20 @@ type WolfRBACConsumerConfig struct { // RewriteConfig is the rule config for proxy-rewrite plugin. // +k8s:deepcopy-gen=true type RewriteConfig struct { - RewriteTarget string `json:"uri,omitempty"` - RewriteTargetRegex []string `json:"regex_uri,omitempty"` - Headers Headers `json:"headers,omitempty"` + RewriteTarget string `json:"uri,omitempty"` + RewriteTargetRegex []string `json:"regex_uri,omitempty"` + Headers RewriteConfigHeaders `json:"headers,omitempty"` } // ResponseRewriteConfig is the rule config for response-rewrite plugin. // +k8s:deepcopy-gen=true type ResponseRewriteConfig struct { - StatusCode int `json:"status_code,omitempty"` - Body string `json:"body,omitempty"` - BodyBase64 bool `json:"body_base64,omitempty"` - Headers Headers `json:"headers,omitempty"` - LuaRestyExpr []expr.Expr `json:"vars,omitempty"` - Filters []map[string]string `json:"filters,omitempty"` + StatusCode int `json:"status_code,omitempty"` + Body string `json:"body,omitempty"` + BodyBase64 bool `json:"body_base64,omitempty"` + Headers ResponseRewriteConfigHeaders `json:"headers,omitempty"` + LuaRestyExpr []expr.Expr `json:"vars,omitempty"` + Filters []map[string]string `json:"filters,omitempty"` } // RedirectConfig is the rule config for redirect plugin. @@ -189,21 +189,43 @@ type RequestMirror struct { type Headers map[string]any -func (p *Headers) DeepCopyInto(out *Headers) { +type ResponseRewriteConfigHeaders struct { + Headers +} + +type RewriteConfigHeaders struct { + Headers +} + +func (p *ResponseRewriteConfigHeaders) DeepCopyInto(out *ResponseRewriteConfigHeaders) { + b, _ := json.Marshal(&p) + _ = json.Unmarshal(b, out) +} + +func (p *ResponseRewriteConfigHeaders) DeepCopy() *ResponseRewriteConfigHeaders { + if p == nil { + return nil + } + out := new(ResponseRewriteConfigHeaders) + p.DeepCopyInto(out) + return out +} + +func (p *RewriteConfigHeaders) DeepCopyInto(out *RewriteConfigHeaders) { b, _ := json.Marshal(&p) _ = json.Unmarshal(b, out) } -func (p *Headers) DeepCopy() *Headers { +func (p *RewriteConfigHeaders) DeepCopy() *RewriteConfigHeaders { if p == nil { return nil } - out := new(Headers) + out := new(RewriteConfigHeaders) p.DeepCopyInto(out) return out } -func (p *Headers) Add(headersToAdd []string) { +func (p *ResponseRewriteConfigHeaders) Add(headersToAdd []string) { if p == nil { return } @@ -216,15 +238,43 @@ func (p *Headers) Add(headersToAdd []string) { } addedHeader = append(addedHeader, fmt.Sprintf("%s:%s", kv[0], kv[1])) } - (*p)["add"] = addedHeader + (p.Headers)["add"] = addedHeader + } +} + +func (p *ResponseRewriteConfigHeaders) GetAddedHeaders() []string { + if p == nil || (p.Headers)["add"] == nil { + return nil + } + addedheaders, ok := (p.Headers)["add"].([]string) + if ok { + return addedheaders + } + return nil +} + +func (p *RewriteConfigHeaders) Add(headersToAdd []string) { + if p == nil { + return + } + if headersToAdd != nil { + addedHeader := make(map[string]string, 0) + for _, h := range headersToAdd { + kv := strings.Split(h, ":") + if len(kv) < 2 { + continue + } + addedHeader[kv[0]] = kv[1] + } + (p.Headers)["add"] = addedHeader } } -func (p *Headers) GetAddedHeaders() []string { - if p == nil || (*p)["add"] == nil { +func (p *RewriteConfigHeaders) GetAddedHeaders() map[string]string { + if p == nil || (p.Headers)["add"] == nil { return nil } - addedheaders, ok := (*p)["add"].([]string) + addedheaders, ok := (p.Headers)["add"].(map[string]string) if ok { return addedheaders } diff --git a/test/e2e/suite-annotations/rewrite.go b/test/e2e/suite-annotations/rewrite.go index 48779f1c06..3f55e3b3b4 100644 --- a/test/e2e/suite-annotations/rewrite.go +++ b/test/e2e/suite-annotations/rewrite.go @@ -218,10 +218,8 @@ spec: - path: /sample pathType: Prefix backend: - service: - name: %s - port: - number: %d + serviceName: %s + servicePort: %d `, backendSvc, backendPort[0]) err := s.CreateResourceFromString(ing) assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") From 41bd484f20e88b633cfa6d351992959155986686 Mon Sep 17 00:00:00 2001 From: mpoqq Date: Wed, 28 Aug 2024 08:58:54 +0200 Subject: [PATCH 3/7] fix: e2e tests --- .../annotations/plugins/response_rewrite.go | 4 +++- .../annotations/plugins/rewrite.go | 4 +++- .../annotations/plugins/rewrite_test.go | 9 ++++++--- pkg/types/apisix/v1/plugin_types.go | 4 ++-- test/e2e/suite-annotations/rewrite.go | 20 +++++++++---------- 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go index 66124104a1..de56053e68 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go @@ -42,7 +42,9 @@ 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 := apisixv1.ResponseRewriteConfigHeaders{ + Headers: make(apisixv1.Headers), + } headers.Add(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderAdd)) headers.Set(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderSet)) headers.Remove(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderRemove)) diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go index d14f7e1988..0cc9e26ca5 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go @@ -39,7 +39,9 @@ func (i *rewrite) Handle(e annotations.Extractor) (interface{}, error) { rewriteTargetRegex := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegex) rewriteTemplate := e.GetStringAnnotation(annotations.AnnotationsRewriteTargetRegexTemplate) - headers := apisixv1.RewriteConfigHeaders{} + 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)) diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go index ab0b1021c4..c165413801 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go @@ -30,7 +30,7 @@ func TestRewriteHandler(t *testing.T) { annotations.AnnotationsRewriteTargetRegexTemplate: "/$1", annotations.AnnotationsRewriteHeaderAdd: "testkey1:testval1,testkey2:testval2", annotations.AnnotationsRewriteHeaderRemove: "testkey1,testkey2", - annotations.AnnotationsRewriteHeaderSet: "testkey1:testval1,testkey2:testval2", + annotations.AnnotationsRewriteHeaderSet: "testsetkey1:testsetval1,testsetkey2:testsetval2", } p := NewRewriteHandler() out, err := p.Handle(annotations.NewExtractor(anno)) @@ -39,10 +39,13 @@ func TestRewriteHandler(t *testing.T) { assert.Equal(t, "/sample", config.RewriteTarget) assert.Equal(t, []string{"/sample/(.*)", "/$1"}, config.RewriteTargetRegex) assert.Equal(t, "proxy-rewrite", p.PluginName()) - assert.Equal(t, []string{"testkey1:testval1", "testkey2:testval2"}, config.Headers.GetAddedHeaders()) - assert.Equal(t, []string{"testkey1", "testkey2"}, config.Headers.GetRemovedHeaders()) assert.Equal(t, map[string]string{ "testkey1": "testval1", "testkey2": "testval2", + }, config.Headers.GetAddedHeaders()) + assert.Equal(t, []string{"testkey1", "testkey2"}, config.Headers.GetRemovedHeaders()) + assert.Equal(t, map[string]string{ + "testsetkey1": "testsetval1", + "testsetkey2": "testsetval2", }, config.Headers.GetSetHeaders()) } diff --git a/pkg/types/apisix/v1/plugin_types.go b/pkg/types/apisix/v1/plugin_types.go index 9b5d791b96..b1a9895481 100644 --- a/pkg/types/apisix/v1/plugin_types.go +++ b/pkg/types/apisix/v1/plugin_types.go @@ -190,11 +190,11 @@ type RequestMirror struct { type Headers map[string]any type ResponseRewriteConfigHeaders struct { - Headers + Headers `json:"headers,omitempty"` } type RewriteConfigHeaders struct { - Headers + Headers `json:"headers,omitempty"` } func (p *ResponseRewriteConfigHeaders) DeepCopyInto(out *ResponseRewriteConfigHeaders) { diff --git a/test/e2e/suite-annotations/rewrite.go b/test/e2e/suite-annotations/rewrite.go index 3f55e3b3b4..4d38764ae6 100644 --- a/test/e2e/suite-annotations/rewrite.go +++ b/test/e2e/suite-annotations/rewrite.go @@ -189,12 +189,12 @@ spec: assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") time.Sleep(5 * time.Second) - resp := s.NewAPISIXClient().GET("/sample/ip").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-Request-ID", "000").WithHeader("X-Test", "Test").Expect() resp.Status(http.StatusOK) - resp.Header("X-Api-Version").IsEqual("v1") - resp.Header("X-Api-Engine").IsEqual("Apisix") - resp.Header("X-Request-ID").IsEqual("123") - resp.Header("X-Test").IsEmpty() + resp.Body().Contains("\"X-Api-Version\": \"v1\"") + resp.Body().Contains("\"X-Api-Engine\": \"Apisix\"") + resp.Body().Contains("\"X-Request-ID\": \"123\"") + resp.Body().NotContains("\"X-Test\"") }) ginkgo.It("enable in ingress networking/v1beta1", func() { backendSvc, backendPort := s.DefaultHTTPBackend() @@ -225,11 +225,11 @@ spec: assert.Nil(ginkgo.GinkgoT(), err, "creating ingress") time.Sleep(5 * time.Second) - resp := s.NewAPISIXClient().GET("/sample/ip").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-Request-ID", "000").WithHeader("X-Test", "Test").Expect() resp.Status(http.StatusOK) - resp.Header("X-Api-Version").IsEqual("v1") - resp.Header("X-Api-Engine").IsEqual("Apisix") - resp.Header("X-Request-ID").IsEqual("123") - resp.Header("X-Test").IsEmpty() + resp.Body().Contains("\"X-Api-Version\": \"v1\"") + resp.Body().Contains("\"X-Api-Engine\": \"Apisix\"") + resp.Body().Contains("\"X-Request-ID\": \"123\"") + resp.Body().NotContains("\"X-Test\"") }) }) From ecfb22619fdd7d48a78be30586d93e65cc6b6b0b Mon Sep 17 00:00:00 2001 From: mpoqq Date: Wed, 28 Aug 2024 13:25:55 +0200 Subject: [PATCH 4/7] 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\"") }) }) From 0fa66cc67a745e4b2e5674989dfde77201a92cdb Mon Sep 17 00:00:00 2001 From: mpoqq Date: Wed, 28 Aug 2024 15:43:38 +0200 Subject: [PATCH 5/7] fix: remove focus test --- test/e2e/suite-annotations/rewrite.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/suite-annotations/rewrite.go b/test/e2e/suite-annotations/rewrite.go index 765e34fd4b..d47a5cec50 100644 --- a/test/e2e/suite-annotations/rewrite.go +++ b/test/e2e/suite-annotations/rewrite.go @@ -155,7 +155,7 @@ spec: }) }) -var _ = ginkgo.FDescribe("suite-annotations: rewrite header annotations", func() { +var _ = ginkgo.Describe("suite-annotations: rewrite header annotations", func() { s := scaffold.NewDefaultScaffold() ginkgo.It("enable in ingress networking/v1", func() { From a4a1b689dfe6d65b3aefab67d13c28fd0d68d7c2 Mon Sep 17 00:00:00 2001 From: mpoqq Date: Mon, 2 Sep 2024 10:00:30 +0200 Subject: [PATCH 6/7] fix: remove empty header from json --- .../annotations/plugins/response_rewrite.go | 16 ++++++++++++--- .../plugins/response_rewrite_test.go | 20 ++++++++++++------- .../annotations/plugins/rewrite.go | 14 ++++++------- .../annotations/plugins/rewrite_test.go | 10 ++++++++++ pkg/types/apisix/v1/plugin_types.go | 18 ++++++++--------- pkg/types/apisix/v1/zz_generated.deepcopy.go | 10 ++++++++-- 6 files changed, 59 insertions(+), 29 deletions(-) diff --git a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go index 76f8bd3cda..8329043dc3 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite.go @@ -42,8 +42,18 @@ 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) - plugin.Headers.SetAddHeaders(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderAdd)) - plugin.Headers.SetSetHeaders(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderSet)) - plugin.Headers.SetRemoveHeaders(e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderRemove)) + + headersToAdd := e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderAdd) + headersToSet := e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderSet) + headersToRemove := e.GetStringsAnnotation(annotations.AnnotationsResponseRewriteHeaderRemove) + + if len(headersToAdd) > 0 || len(headersToSet) > 0 || len(headersToRemove) > 0 { + headers := apisixv1.ResponseRewriteConfigHeaders{} + headers.SetAddHeaders(headersToAdd) + headers.SetSetHeaders(headersToSet) + headers.SetRemoveHeaders(headersToRemove) + plugin.Headers = &headers + } + 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 b4f1c4d14c..8f8d21afa9 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/response_rewrite_test.go +++ b/pkg/providers/ingress/translation/annotations/plugins/response_rewrite_test.go @@ -25,13 +25,10 @@ import ( func TestResponseRewriteHandler(t *testing.T) { anno := map[string]string{ - annotations.AnnotationsEnableResponseRewrite: "true", - annotations.AnnotationsResponseRewriteStatusCode: "200", - annotations.AnnotationsResponseRewriteBody: "bar_body", - annotations.AnnotationsResponseRewriteBodyBase64: "false", - annotations.AnnotationsResponseRewriteHeaderAdd: "testkey1:testval1,testkey2:testval2", - annotations.AnnotationsResponseRewriteHeaderRemove: "testkey1,testkey2", - annotations.AnnotationsResponseRewriteHeaderSet: "testkey1:testval1,testkey2:testval2", + annotations.AnnotationsEnableResponseRewrite: "true", + annotations.AnnotationsResponseRewriteStatusCode: "200", + annotations.AnnotationsResponseRewriteBody: "bar_body", + annotations.AnnotationsResponseRewriteBodyBase64: "false", } p := NewResponseRewriteHandler() out, err := p.Handle(annotations.NewExtractor(anno)) @@ -41,12 +38,21 @@ 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.Nil(t, config.Headers) + + anno[annotations.AnnotationsResponseRewriteHeaderAdd] = "testkey1:testval1,testkey2:testval2" + anno[annotations.AnnotationsResponseRewriteHeaderRemove] = "testkey1,testkey2" + anno[annotations.AnnotationsResponseRewriteHeaderSet] = "testkey1:testval1,testkey2:testval2" + out, err = p.Handle(annotations.NewExtractor(anno)) + assert.Nil(t, err, "checking given error") + config = out.(*apisixv1.ResponseRewriteConfig) 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", "testkey2": "testval2", }, config.Headers.GetSetHeaders()) + anno[annotations.AnnotationsEnableResponseRewrite] = "false" out, err = p.Handle(annotations.NewExtractor(anno)) assert.Nil(t, err, "checking given error") diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go index 8f1bc647e7..d3656a4981 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite.go @@ -52,14 +52,12 @@ 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) + if len(headersToAdd) > 0 || len(headersToSet) > 0 || len(headersToRemove) > 0 { + headers := apisixv1.RewriteConfigHeaders{} + headers.SetAddHeaders(headersToAdd) + headers.SetSetHeaders(headersToSet) + headers.SetRemoveHeaders(headersToRemove) + plugin.Headers = &headers } return &plugin, nil } diff --git a/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go b/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go index cbf017711b..d8bda67c16 100644 --- a/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go +++ b/pkg/providers/ingress/translation/annotations/plugins/rewrite_test.go @@ -48,4 +48,14 @@ func TestRewriteHandler(t *testing.T) { "testsetkey1": "testsetval1", "testsetkey2": "testsetval2", }, config.Headers.GetSetHeaders()) + + anno = map[string]string{ + annotations.AnnotationsRewriteTarget: "/sample", + } + out, err = p.Handle(annotations.NewExtractor(anno)) + assert.Nil(t, err, "checking given error") + config = out.(*apisixv1.RewriteConfig) + assert.Equal(t, "/sample", config.RewriteTarget) + assert.Nil(t, config.RewriteTargetRegex) + assert.Nil(t, config.Headers) } diff --git a/pkg/types/apisix/v1/plugin_types.go b/pkg/types/apisix/v1/plugin_types.go index 72d88e71d6..3f09c08168 100644 --- a/pkg/types/apisix/v1/plugin_types.go +++ b/pkg/types/apisix/v1/plugin_types.go @@ -137,20 +137,20 @@ type WolfRBACConsumerConfig struct { // RewriteConfig is the rule config for proxy-rewrite plugin. // +k8s:deepcopy-gen=true type RewriteConfig struct { - RewriteTarget string `json:"uri,omitempty"` - RewriteTargetRegex []string `json:"regex_uri,omitempty"` - Headers RewriteConfigHeaders `json:"headers,omitempty"` + RewriteTarget string `json:"uri,omitempty"` + RewriteTargetRegex []string `json:"regex_uri,omitempty"` + Headers *RewriteConfigHeaders `json:"headers,omitempty"` } // ResponseRewriteConfig is the rule config for response-rewrite plugin. // +k8s:deepcopy-gen=true type ResponseRewriteConfig struct { - StatusCode int `json:"status_code,omitempty"` - Body string `json:"body,omitempty"` - BodyBase64 bool `json:"body_base64,omitempty"` - Headers ResponseRewriteConfigHeaders `json:"headers,omitempty"` - LuaRestyExpr []expr.Expr `json:"vars,omitempty"` - Filters []map[string]string `json:"filters,omitempty"` + StatusCode int `json:"status_code,omitempty"` + Body string `json:"body,omitempty"` + BodyBase64 bool `json:"body_base64,omitempty"` + Headers *ResponseRewriteConfigHeaders `json:"headers,omitempty"` + LuaRestyExpr []expr.Expr `json:"vars,omitempty"` + Filters []map[string]string `json:"filters,omitempty"` } // RedirectConfig is the rule config for redirect plugin. diff --git a/pkg/types/apisix/v1/zz_generated.deepcopy.go b/pkg/types/apisix/v1/zz_generated.deepcopy.go index 6910b5f421..7583af53d4 100644 --- a/pkg/types/apisix/v1/zz_generated.deepcopy.go +++ b/pkg/types/apisix/v1/zz_generated.deepcopy.go @@ -384,7 +384,10 @@ func (in *RequestMirror) DeepCopy() *RequestMirror { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResponseRewriteConfig) DeepCopyInto(out *ResponseRewriteConfig) { *out = *in - in.Headers.DeepCopyInto(&out.Headers) + if in.Headers != nil { + in, out := &in.Headers, &out.Headers + *out = (*in).DeepCopy() + } if in.LuaRestyExpr != nil { in, out := &in.LuaRestyExpr, &out.LuaRestyExpr *out = make([]expr.Expr, len(*in)) @@ -426,7 +429,10 @@ func (in *RewriteConfig) DeepCopyInto(out *RewriteConfig) { *out = make([]string, len(*in)) copy(*out, *in) } - in.Headers.DeepCopyInto(&out.Headers) + if in.Headers != nil { + in, out := &in.Headers, &out.Headers + *out = (*in).DeepCopy() + } return } From a64b1b5eafa04287b81ff4b2a4e1a486ddcca5fb Mon Sep 17 00:00:00 2001 From: mpoqq Date: Mon, 2 Sep 2024 15:22:12 +0200 Subject: [PATCH 7/7] fix: gateway router headers --- .../gateway/translation/gateway_httproute.go | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/pkg/providers/gateway/translation/gateway_httproute.go b/pkg/providers/gateway/translation/gateway_httproute.go index 8811a24a13..4cf6917096 100644 --- a/pkg/providers/gateway/translation/gateway_httproute.go +++ b/pkg/providers/gateway/translation/gateway_httproute.go @@ -58,19 +58,23 @@ func (t *translator) generatePluginFromHTTPRequestHeaderFilter(plugins apisixv1. // TODO: The current apisix plugin does not conform to the specification. 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) - } - plugin.Headers.SetAddHeaders(headersToAdd) + if len(reqHeaderModifier.Add) > 0 || len(reqHeaderModifier.Set) > 0 || len(reqHeaderModifier.Remove) > 0 { + headers := apisixv1.RewriteConfigHeaders{} - headersToSet := make([]string, len(reqHeaderModifier.Set)) - for i, header := range reqHeaderModifier.Set { - headersToSet[i] = fmt.Sprintf("%s:%s", header.Name, header.Value) - } - plugin.Headers.SetSetHeaders(headersToSet) + headers.Add = make(map[string]string, len(reqHeaderModifier.Add)) + for _, header := range reqHeaderModifier.Add { + headers.Add[string(header.Name)] = header.Value + } - plugin.Headers.SetRemoveHeaders(reqHeaderModifier.Remove) + headers.Set = make(map[string]string, len(reqHeaderModifier.Set)) + for _, header := range reqHeaderModifier.Set { + headers.Set[string(header.Name)] = header.Value + } + + headers.Remove = reqHeaderModifier.Remove + + plugin.Headers = &headers + } plugins["proxy-rewrite"] = plugin }