feat: proxy-rewrite support config add set and remove header #8336

mscb402 · 2022-11-16T03:38:57Z

Description

Fixes #8239

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

apisix/plugins/proxy-rewrite.lua

tzssangglass · 2022-11-16T11:28:58Z

docs/en/latest/plugins/proxy-rewrite.md

-| headers                     | object        | False    |         |                                                                                                                                        | New Upstream headers. Headers are overwritten if they are already present otherwise, they are added to the present headers. To remove a header, set the header value to an empty string. The values in the header can contain Nginx variables like `$remote_addr` and `$client_addr`.                                                                                                                                                                                        |
+| headers                     | object        | False    |         |                                                                                                                                   |                   |
+| headers.add     | object   | false     |        |                 | Append the new headers. The format is `{"name: value",...}`. The values in the header can contain Nginx variables like $remote_addr and $balancer_ip.                                                                                              |
+| headers.set     | object  | false     |        |                 | Rewriting the headers. The format is  `{"name": "value", ...}`. The values in the header can contain Nginx variables like $remote_addr and $balancer_ip.                                                                                                |


I think use headers.rewrite is more clear.

What happens if headers.set is set, but the actual request does not have a header name that needs to be rewritten?

This headers.set is the same as the response-rewrite plugin. When the header does not exist will add it.

docs/en/latest/plugins/proxy-rewrite.md

t/plugin/proxy-rewrite3.t

tzssangglass · 2022-11-17T02:37:29Z

apisix/plugins/proxy-rewrite.lua

    end

+    local function addHeader(ctx, header_name, header_value)


Suggested change

local function addHeader(ctx, header_name, header_value)

local function add_header(ctx, header_name, header_value)

tzssangglass · 2022-11-17T02:37:48Z

apisix/plugins/proxy-rewrite.lua

    end

+    local function addHeader(ctx, header_name, header_value)
+        local oldHeader = core.request.header(ctx, header_name)


Suggested change

local oldHeader = core.request.header(ctx, header_name)

local old_header = core.request.header(ctx, header_name)

tzssangglass · 2022-11-17T10:21:32Z

t/plugin/proxy-rewrite3.t

+test: sssss
+test: bbb
+--- response_headers
+test: sssss, bbb, 123


why it is

test: sssss test: bbb test: 123

This is for test header combination. First combine two existing headers test: sssss and test: bbb. And then add a new header test: 123 from proxy_rewrite. Finally, it will combine all headers in one.

Is it necessary to merge header with the same name? I'm not sure. cc @spacewander

We need to use https://github.com/openresty/lua-resty-core/blob/master/lib/ngx/req.md#add_header. Merging multiple request headers with , doesn't obey the standard.

done @spacewander

apisix/plugins/proxy-rewrite.lua

t/plugin/proxy-rewrite3.t

spacewander · 2022-11-18T06:45:45Z

apisix/plugins/proxy-rewrite.lua

+                            minItems = 1,
+                            items = {
+                                type = "string",
+                                -- "Set-Cookie"


Set-Cookie is a response header. Better to use another example.

spacewander · 2022-11-18T06:51:21Z

t/plugin/proxy-rewrite3.t

+test: sssss
+test: bbb
+--- response_headers
+test: sssss, bbb, 123


We need to use https://github.com/openresty/lua-resty-core/blob/master/lib/ngx/req.md#add_header. Merging multiple request headers with , doesn't obey the standard.

spacewander · 2022-11-18T06:52:28Z

t/plugin/proxy-rewrite3.t

+        }
+    }
+--- request
+GET /t


Please remove duplicate sections which are set in

apisix/t/plugin/proxy-rewrite3.t

Line 28 in ae400b9

if (!$block->request) {

spacewander · 2022-11-18T06:52:35Z

t/plugin/proxy-rewrite3.t

+GET /t
+--- response_body
+passed
+--- no_error_log


apisix/core/request.lua

spacewander · 2022-11-21T09:21:38Z

t/plugin/proxy-rewrite3.t

 --- response_headers
-X-Forwarded-Host: test.com
+test: sssss, bbb, 123
+--- no_error_log


Need to deal with #8336 (comment) in other test

spacewander · 2022-11-21T09:22:03Z

t/plugin/proxy-rewrite3.t

+
+
+
+=== TEST 13: rewrite X-Forwarded-Host


This test isn't relative to this feature?

Nope, but this change is to remove duplicate sections from your advice. So I changed some test code.

#8336 (comment)

docs/en/latest/plugins/proxy-rewrite.md

spacewander · 2022-11-21T09:27:59Z

apisix/core/request.lua

+        error(err)
+    end
+
+    req_add_header(header_name,header_value)


Please ensure a space is after the ','. Let's deal with similar places in this PR.

Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com>

tzssangglass · 2022-11-21T11:38:21Z

apisix/core/request.lua

@@ -39,6 +44,7 @@ local req_get_uri_args = ngx.req.get_uri_args
 local req_set_uri_args = ngx.req.set_uri_args


+


delete this line

mscb402 added 4 commits November 16, 2022 09:50

feat: proxy-rewrite support add set and remove header

c999d53

chore: add document

faba9ca

chore: fix ci error

f2ed20f

chore: fix lint

cbb8b74

mscb402 marked this pull request as ready for review November 16, 2022 07:53

tokers reviewed Nov 16, 2022

View reviewed changes

apisix/plugins/proxy-rewrite.lua Show resolved Hide resolved

apisix/plugins/proxy-rewrite.lua Outdated Show resolved Hide resolved

tzssangglass reviewed Nov 16, 2022

View reviewed changes

tokers reviewed Nov 17, 2022

View reviewed changes

docs/en/latest/plugins/proxy-rewrite.md Outdated Show resolved Hide resolved

docs/en/latest/plugins/proxy-rewrite.md Show resolved Hide resolved

t/plugin/proxy-rewrite3.t Outdated Show resolved Hide resolved

mscb402 added 3 commits November 17, 2022 09:56

chore: fix document and test case

e3c4d0a

doc: add priority in document

fee2b02

doc: fix doc lint

9496b2a

tzssangglass reviewed Nov 17, 2022

View reviewed changes

chore: rename some function

5342ee7

tzssangglass reviewed Nov 17, 2022

View reviewed changes

tokers reviewed Nov 18, 2022

View reviewed changes

apisix/plugins/proxy-rewrite.lua Outdated Show resolved Hide resolved

apisix/plugins/proxy-rewrite.lua Show resolved Hide resolved

t/plugin/proxy-rewrite3.t Show resolved Hide resolved

mscb402 added 2 commits November 18, 2022 10:15

chore: change to oneOf

7c0b85d

chore: change back to anyof

c11d214

spacewander reviewed Nov 18, 2022

View reviewed changes

mscb402 added 6 commits November 18, 2022 07:47

chore: fix code.

36d3760

chore: format code

8d8a03e

chore: clean code

b236bc1

chore: fix ci error

9a82832

chore: fix ci error

07c4104

fix: ci error

d3973dd

mscb402 requested a review from tzssangglass November 21, 2022 06:49

doc: change doc

4f684db

spacewander reviewed Nov 21, 2022

View reviewed changes

mscb402 and others added 2 commits November 21, 2022 17:34

Update docs/en/latest/plugins/proxy-rewrite.md

84a06d6

Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com>

chore: format code style

3c2fb83

chore: fix test style

05daec5

mscb402 requested review from spacewander and tzssangglass and removed request for tzssangglass and spacewander November 21, 2022 10:02

tzssangglass reviewed Nov 21, 2022

View reviewed changes

chore: remove line

6042fe9

mscb402 requested a review from tzssangglass November 22, 2022 01:06

chore: change antOf to oneOf

ab385e4

mscb402 requested review from spacewander and removed request for tzssangglass November 22, 2022 05:45

tokers approved these changes Nov 22, 2022

View reviewed changes

spacewander approved these changes Nov 22, 2022

View reviewed changes

spacewander merged commit 01b4b49 into apache:master Nov 22, 2022

tzssangglass mentioned this pull request Dec 7, 2022

help request: Parse $request_body and add authorization header when using websockets #8466

Closed

mscb402 deleted the feat_8239 branch December 27, 2022 08:21

soulbird mentioned this pull request Apr 12, 2023

help request: Add a header if exist not overwrite #6710

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: proxy-rewrite support config add set and remove header #8336

feat: proxy-rewrite support config add set and remove header #8336

mscb402 commented Nov 16, 2022

tzssangglass Nov 16, 2022

mscb402 Nov 17, 2022

tzssangglass Nov 17, 2022

mscb402 Nov 17, 2022 •

edited

Loading

tzssangglass Nov 17, 2022

mscb402 Nov 17, 2022

tzssangglass Nov 17, 2022

mscb402 Nov 18, 2022

tzssangglass Nov 18, 2022

spacewander Nov 18, 2022

mscb402 Nov 18, 2022

spacewander Nov 18, 2022

spacewander Nov 18, 2022

spacewander Nov 18, 2022

mscb402 Nov 18, 2022

spacewander Nov 18, 2022

mscb402 Nov 18, 2022

spacewander Nov 21, 2022

mscb402 Nov 21, 2022

spacewander Nov 21, 2022

mscb402 Nov 21, 2022 •

edited

Loading

spacewander Nov 21, 2022

mscb402 Nov 21, 2022

tzssangglass Nov 21, 2022

mscb402 Nov 21, 2022

	local function addHeader(ctx, header_name, header_value)
	local function add_header(ctx, header_name, header_value)

	local oldHeader = core.request.header(ctx, header_name)
	local old_header = core.request.header(ctx, header_name)

		@@ -39,6 +44,7 @@ local req_get_uri_args = ngx.req.get_uri_args
		local req_set_uri_args = ngx.req.set_uri_args

feat: proxy-rewrite support config add set and remove header #8336

feat: proxy-rewrite support config add set and remove header #8336

Conversation

mscb402 commented Nov 16, 2022

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mscb402 Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mscb402 Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mscb402 Nov 17, 2022 •

edited

Loading

mscb402 Nov 21, 2022 •

edited

Loading