search

nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.4k stars 331 forks source link

Support POST and GET for application restart commands in control API #1445

Open javorszky opened 1 month ago

javorszky commented 1 month ago

Currently the control API's message to restart a specific application is a GET request per the documentation: https://unit.nginx.org/configuration/#configuration-proc-mgmt

$ curl -X GET --unix-socket /path/to/control.unit.sock  \
      http://localhost/control/applications/app_name/restart

Semantically a GET request should not change state or have side effect on the server. It's more appropriate to use a POST request for this, see https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods

GET The GET method requests a representation of the specified resource. Requests using GET should only retrieve data and should not contain a request content.

POST The POST method submits an entity to the specified resource, often causing a change in state or side effects on the server.

Unit version 1.34 should add support for POST requests for the same endpoints for restarts without removing support for GET requests.

We should also print a deprecation notice when folks are using GET request to issue a restart command to prepare them for its eventual removal in the following version tracked in #1446 .

hongzhidao commented 1 month ago

I wonder how to use the API with POST method, any examples?

javorszky commented 1 month ago

I wonder how to use the API with POST method, any examples?

It would be the same command, but a POST request

$ curl -X POST --unix-socket /path/to/control.unit.sock  \
      http://localhost/control/applications/app_name/restart

We're already using PUT when updating the configuration from a json file.

javorszky commented 1 month ago

For context, this is the logic that decides what method to let through: https://github.com/nginx/unit/blob/master/src/nxt_controller.c#L2309

javorszky commented 1 month ago

@hongzhidao had an excellent idea on Monday about this:

We could also support an endpoint where it’s not application specific, so instead of sending an empty (no body) POST to /control/applications/app_name/restart, we could send a POST to a new endpoint, like /control/applications with a POST body of 

{
  "command": "restart",
  "applications": ["app1", "app2"]
}

This might be a value add for Later™