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 328 forks source link

Missed test in the case of conf not found. #471

Open hongzhidao opened 4 years ago

hongzhidao commented 4 years ago

Hi.

  1. This test case is important. Now, it's missed in tests/ directory. (grep Value doesn't exist. nothing)

    curl http://127.1:8000/config/
    {
    "listeners": {},
    "applications": {}
    }
    curl http://127.1:8000/config/upstreams -v
    *   Trying 127.0.0.1:8000...
    * Connected to 127.1 (127.0.0.1) port 8000 (#0)
    > GET /config/upstreams HTTP/1.1
    > Host: 127.1:8000
    > User-Agent: curl/7.71.0-DEV
    > Accept: */*
    >
    * Mark bundle as not supporting multiuse
    < HTTP/1.1 404 Not Found
    < Server: nginx/1.19.2
    < Date: Tue, 25 Aug 2020 03:39:24 GMT
    < Content-Type: application/json
    < Content-Length: 40
    < Connection: keep-alive
    < Access-Control-Allow-Origin: *
    <
    {
    "error": "Value doesn't exist."
    }
  2. What about making get value generic. Reference to Unit configuration section. To address parts of the configuration, query the control socket over HTTP; URI path segments of your requests to the API must be names of its JSON object members or indexes of its array elements. As I understand, the URI is used to find the config object. It represents a chain of property names. It's quite similar to access property of an object in JS. For example. /config/upstreams/r1/servers is the same as config['upstreams']['r1']['servers']. If servers does not exist, the get value can be null with status 200. But config['upstreams']['r2']['servers'] still return 404, if r2 does not exists. This only applies to object and array.

    
    diff --git a/src/nxt_conf.c b/src/nxt_conf.c
    index 1aca0a7..611ff35 100644
    --- a/src/nxt_conf.c
    +++ b/src/nxt_conf.c
    @@ -143,6 +143,10 @@ static u_char *nxt_conf_json_print_object(u_char *p, nxt_conf_value_t *value,
    static size_t nxt_conf_json_escape_length(u_char *p, size_t size);
    static u_char *nxt_conf_json_escape(u_char *dst, u_char *src, size_t size);

+static nxt_conf_value_t nxt_conf_json_null = {

i4ki commented 4 years ago

I believe this is both a breaking change and against the REST model. Eg.: We will be returning "200 OK" for a resource not found...

hongzhidao commented 4 years ago

I believe this is both a breaking change and against the REST model

Agree. Sorry for my carelessness.

  1. The purpose of this change is that I want to get all the values that will be assigned to pass. Now, routes, routes/{all groups}, applications/{all apps/PHP targets} and upstreams/{all ups} are needed.

  2. I found pass can be set empty that it will return 500 for HTTP request. Do you think if it needs to be improved (required)?

i4ki commented 4 years ago

The purpose of this change is that I want to get all the values that will be assigned to pass.

Maybe a "fields query" in the API? Something like:

GET /config?fields=routes,applications,upstreams

Reply:

{
    "routes": {},
    "applications": null,
    "upstreams": null
}

This way you're asking for a group of resources and then the API returns 200 for the object and returns null for each one not found in the server. This also can lower the number of requests the Panel needs to send.

Do you think if it needs to be improved (required)? I'm not sure, but it seems so.

The Unit Panel/Dash is amazing, BTW! Congrats.

VBart commented 4 years ago

I think we just need to introduce support for returning default values from the API. Then "applications", "upstreams", "settings" can have defaults as {}.

hongzhidao commented 4 years ago

Maybe a "fields query" in the API? I think we just need to introduce support for returning default values from the API.

I prefer to keep Unit unchanged. Your suggestions have made the question clear. Currently, I think Nginx is a good proxy to help manage Unit configuration including CORS, jwt auth and get passes values. Such as.

location /passes {
   js_content unit.passes; # get full configuration and filter the expected passes
}
{
    "routes": {},
    "applications": null,
    "upstreams": null
}

Furthermore, it would be interesting that there is an example using NGINX, Unit, NJS. Thanks for your great work again :)

hongzhidao commented 4 years ago

Hi. This question is gone. Now the process is:

client-side    -- fetch data (after login)                              --> server-side
               -- stored the config/certs data as a local JS object     <-- 
               -- just get the local JS object and show in the list pages
               -- edit form or delete then submit (PUT/DELETE)          -->
               -- fetch data after successful operation and update the local JS object. -->

But there is a question left that I'm wondering if it is good to return full configuration together with successfully updated. After updating the configuration, it's common to get the latest data. This would be handy for JS client. Otherwise, we have to re-pull configuration data.