request help: apisix 1.4 can not patch upstream nodes configure

[DHB-liuhong]

Issue description

apisix 1.4 can not patch upstream nodes configure

Environment

[root@ceph68 ~]# curl -i http://127.0.0.1:9080/apisix/admin/routes/1
HTTP/1.1 200 OK
Date: Fri, 10 Jul 2020 02:31:30 GMT
Content-Type: text/plain; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Access-Control-Expose-Headers: *
Access-Control-Max-Age: 3600
Server: APISIX web server

{"node":{"value":{"priority":0,"methods":["PUT","GET","POST","DELETE","PATCH","HEAD","OPTIONS","CONNECT","TRACE"],"uri":"\/*","plugins":{"prometheus":{}},"upstream":{"nodes":{"192.168.3.39:8080":10,"192.168.3.39:8081":10,"192.168.3.39:8082":10,"192.168.3.39:8083":10,"192.168.3.39:8084":10},"hash_on":"vars","type":"roundrobin"}},"createdIndex":40,"key":"\/apisix\/routes\/1","modifiedIndex":40},"action":"get"}

[root@ceph68 ~]# curl -i -X PATCH http://127.0.0.1:9080/apisix/admin/routes/1/upstream/nodes -d '{ "192.168.3.40:8080": 10,"192.168.3.40:8081": 10,"192.168.3.40:8082": 10,"192.168.3.40:8083": 10,"192.168.3.40:8084": 10 }'
HTTP/1.1 400 Bad Request
Date: Fri, 10 Jul 2020 02:31:33 GMT
Content-Type: text/plain; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
Access-Control-Allow-Origin: *
Access-Control-Allow-Credentials: true
Access-Control-Expose-Headers: *
Access-Control-Max-Age: 3600
Server: APISIX web server

{"error_msg":"invalid configuration: additional properties forbidden, found 192.168.3.40:8082"}

• apisix version (cmd: apisix version): 1.4
• OS: centos 7.6

[DHB-liuhong]

please, can you tell me What tools do you use for debugging. thanks.

[membphis]

@nic-chen do you have time to look at this issue?

[membphis]

please, can you tell me What tools do you use for debugging. thanks.

I am a log debugger, mainly use core.log

[nic-chen]

@nic-chen do you have time to look at this issue?

OK

[nic-chen]

@DHB-liuhong

The PATCH method has modified the calling format, in your example, it should be like this:

curl -i -X PATCH http://127.0.0.1:9080/apisix/admin/routes/1 -d '{"upstream":{"nodes":{"192.168.3.39:8080":10,"192.168.3.39:8081":10,"192.168.3.39:8082":10,"192.168.3.39:8083":10,"192.168.3.39:8084":10}}}'

the detail see:
#1609

[moonming]

@DHB-liuhong

The PATCH method has modified the calling format, in your example, it should be like this:

curl -i -X PATCH http://127.0.0.1:9080/apisix/admin/routes/1 -d '{"upstream":{"nodes":{"192.168.3.39:8080":10,"192.168.3.39:8081":10,"192.168.3.39:8082":10,"192.168.3.39:8083":10,"192.168.3.39:8084":10}}}'

the detail see:
#1609

I think @DHB-liuhong is the right way.
Why change to the #1609? @membphis @nic-chen

[membphis]

I can accept both styles. style 1 is better for humans. style 2 is better for robots.

{
    "sub_path": {xxxx}
    ... ...
}

# style 1: sub path
curl -X PATCH http://xxxxx/routes/1/sub_path -d {xxxx}

# style 2: path
curl -X PATCH http://xxxxx/routes/1 -d {sub_path: {xxxx}}

[juzhiyuan]

Why not use standard RESTful API? I prefer this style:

PATCH http://127.0.0.1:9080/apisix/admin/routes/1 -d {a: b}

[moonming]

I think resource name should mapped to a URL path, and resource is contained in the HTTP request body. for example: PATCH 127.0.0.1/apisix/admin/routes/1/uri -d {'index.html'} Thanks, Ming Wen Twitter: _WenMing 琚致远 <notifications@github.com> 于2020年7月12日周日 下午11:35写道：

…

Why not use standard RESTful API? I prefer this style: PATCH http://127.0.0.1:9080/apisix/admin/routes/1 -d {a: b} — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1823 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGJZBK3I32UMMWYKATSG7VTR3HJ3JANCNFSM4OWFETUQ> .

[DHB-liuhong]

I personally think this new type is not friendly to use. why should we set the attribute value to null to remove.
eg:"methods": ["GET", null, null, null, null, null, null, null, null]
thanks

[membphis]

for example: PATCH 127.0.0.1/apisix/admin/routes/1/uri -d {'index.html'}

@moonming I think you provide a wrong example, the correct one should be like this:

PATCH 127.0.0.1/apisix/admin/routes/1/uri -d '"index.html"'

{
    "uri": "index.html"
}

[membphis]

I personally think this new type is not friendly to use. why should we set the attribute value to null to remove.
eg:"methods": ["GET", null, null, null, null, null, null, null, null]
thanks

agree, we need to easier way for this case

[membphis]

Welcome to discuss on this mailing list.

https://lists.apache.org/thread.html/r616d11a2d3c9cfcd20a9ab09715ed62f65ddc8dc682697bba87d6f57%40%3Cdev.apisix.apache.org%3E