From dc15832030913d13f1f5664e09f42b9028a7e5e5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Feb 2017 22:30:31 +0900 Subject: [PATCH 1/4] nghttpx: Refactor API downstream connection to allow more endpoints --- src/shrpx_api_downstream_connection.cc | 100 ++++++++++++++++++++++--- src/shrpx_api_downstream_connection.h | 34 ++++++++- 2 files changed, 122 insertions(+), 12 deletions(-) diff --git a/src/shrpx_api_downstream_connection.cc b/src/shrpx_api_downstream_connection.cc index a7e2e25d..46413652 100644 --- a/src/shrpx_api_downstream_connection.cc +++ b/src/shrpx_api_downstream_connection.cc @@ -33,8 +33,27 @@ namespace shrpx { +namespace { +// List of API endpoints +const APIEndpoint apis[] = { + APIEndpoint{ + StringRef::from_lit("/api/v1beta1/backendconfig"), true, + (1 << API_METHOD_POST) | (1 << API_METHOD_PUT), + &APIDownstreamConnection::handle_backendconfig, + }, +}; +} // namespace + +namespace { +// The method string. This must be same order of APIMethod. +constexpr StringRef API_METHOD_STRING[] = { + StringRef::from_lit("GET"), StringRef::from_lit("POST"), + StringRef::from_lit("PUT"), +}; +} // namespace + APIDownstreamConnection::APIDownstreamConnection(Worker *worker) - : worker_(worker), abandoned_(false) {} + : worker_(worker), api_(nullptr), shutdown_read_(false) {} APIDownstreamConnection::~APIDownstreamConnection() {} @@ -64,7 +83,7 @@ enum { int APIDownstreamConnection::send_reply(unsigned int http_status, int api_status) { - abandoned_ = true; + shutdown_read_ = true; auto upstream = downstream_->get_upstream(); @@ -130,23 +149,45 @@ int APIDownstreamConnection::send_reply(unsigned int http_status, int APIDownstreamConnection::push_request_headers() { auto &req = downstream_->request(); - auto &resp = downstream_->response(); auto path = StringRef{std::begin(req.path), std::find(std::begin(req.path), std::end(req.path), '?')}; - if (path != StringRef::from_lit("/api/v1beta1/backendconfig")) { + for (auto &p : apis) { + if (p.path == path) { + api_ = &p; + break; + } + } + + if (!api_) { send_reply(404, API_FAILURE); return 0; } - if (req.method != HTTP_POST && req.method != HTTP_PUT) { - resp.fs.add_header_token(StringRef::from_lit("allow"), - StringRef::from_lit("POST, PUT"), false, -1); - send_reply(405, API_FAILURE); - + switch (req.method) { + case HTTP_GET: + if (!(api_->allowed_methods & (1 << API_METHOD_GET))) { + error_method_not_allowed(); + return 0; + } + break; + case HTTP_POST: + if (!(api_->allowed_methods & (1 << API_METHOD_POST))) { + error_method_not_allowed(); + return 0; + } + break; + case HTTP_PUT: + if (!(api_->allowed_methods & (1 << API_METHOD_PUT))) { + error_method_not_allowed(); + return 0; + } + break; + default: + error_method_not_allowed(); return 0; } @@ -161,9 +202,42 @@ int APIDownstreamConnection::push_request_headers() { return 0; } +int APIDownstreamConnection::error_method_not_allowed() { + auto &resp = downstream_->response(); + + size_t len = 0; + for (uint8_t i = 0; i < API_METHOD_MAX; ++i) { + if (api_->allowed_methods & (1 << i)) { + // The length of method + ", " + len += API_METHOD_STRING[i].size() + 2; + } + } + + assert(len > 0); + + auto &balloc = downstream_->get_block_allocator(); + + auto iov = make_byte_ref(balloc, len + 1); + auto p = iov.base; + for (uint8_t i = 0; i < API_METHOD_MAX; ++i) { + if (api_->allowed_methods & (1 << i)) { + auto &s = API_METHOD_STRING[i]; + p = std::copy(std::begin(s), std::end(s), p); + p = std::copy_n(", ", 2, p); + } + } + + p -= 2; + *p = '\0'; + + resp.fs.add_header_token(StringRef::from_lit("allow"), StringRef{iov.base, p}, + false, -1); + return send_reply(405, API_FAILURE); +} + int APIDownstreamConnection::push_upload_data_chunk(const uint8_t *data, size_t datalen) { - if (abandoned_) { + if (shutdown_read_ || !api_->require_body) { return 0; } @@ -187,10 +261,14 @@ int APIDownstreamConnection::push_upload_data_chunk(const uint8_t *data, } int APIDownstreamConnection::end_upload_data() { - if (abandoned_) { + if (shutdown_read_) { return 0; } + return api_->handler(*this); +} + +int APIDownstreamConnection::handle_backendconfig() { auto output = downstream_->get_request_buf(); std::array iov; diff --git a/src/shrpx_api_downstream_connection.h b/src/shrpx_api_downstream_connection.h index 5c1dd4eb..2a0c4b1e 100644 --- a/src/shrpx_api_downstream_connection.h +++ b/src/shrpx_api_downstream_connection.h @@ -26,11 +26,36 @@ #define SHRPX_API_DOWNSTREAM_CONNECTION_H #include "shrpx_downstream_connection.h" +#include "template.h" + +using namespace nghttp2; namespace shrpx { class Worker; +// If new method is added, don't forget to update API_METHOD_STRING as +// well. +enum APIMethod { + API_METHOD_GET, + API_METHOD_POST, + API_METHOD_PUT, + API_METHOD_MAX, +}; + +class APIDownstreamConnection; + +struct APIEndpoint { + // Endpoint path. It must start with "/api/". + StringRef path; + // true if we evaluate request body. + bool require_body; + // Allowed methods. This is bitwise OR of one or more of (1 << + // APIMethod value). + uint8_t allowed_methods; + std::function handler; +}; + class APIDownstreamConnection : public DownstreamConnection { public: APIDownstreamConnection(Worker *worker); @@ -59,10 +84,17 @@ public: virtual DownstreamAddr *get_addr() const; int send_reply(unsigned int http_status, int api_status); + int error_method_not_allowed(); + + // Handles backendconfig API request. + int handle_backendconfig(); private: Worker *worker_; - bool abandoned_; + // This points to the requested APIEndpoint struct. + const APIEndpoint *api_; + // true if we stop reading request body. + bool shutdown_read_; }; } // namespace shrpx From 450ffaa6f0d33d5926da398a6e4c2f212c66f2c4 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 15 Feb 2017 23:23:48 +0900 Subject: [PATCH 2/4] nghttpx: Add configrevision API endpoint This commit adds configuration revision, which is considered opaque string, and changes after reloading configuration with SIGHUP. This revision is returned as a response to configrevision API endpoint. This allows external application to know whether nghttpx has finished reloading new configuration or not. Note that this revision does not change on backendconfig API calls. --- src/shrpx.cc | 1 + src/shrpx_api_downstream_connection.cc | 62 ++++++++++++++++++++++---- src/shrpx_api_downstream_connection.h | 5 ++- src/shrpx_config.h | 8 ++++ 4 files changed, 67 insertions(+), 9 deletions(-) diff --git a/src/shrpx.cc b/src/shrpx.cc index fac5ae34..cbde333a 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -2942,6 +2942,7 @@ void reload_config(WorkerProcess *wp) { new_config->daemon = cur_config->daemon; // loop is reused, and ev_loop_flags gets ignored new_config->ev_loop_flags = cur_config->ev_loop_flags; + new_config->config_revision = cur_config->config_revision + 1; rv = process_options(new_config.get(), suconfig.cmdcfgs); if (rv != 0) { diff --git a/src/shrpx_api_downstream_connection.cc b/src/shrpx_api_downstream_connection.cc index 46413652..2be26bea 100644 --- a/src/shrpx_api_downstream_connection.cc +++ b/src/shrpx_api_downstream_connection.cc @@ -41,6 +41,10 @@ const APIEndpoint apis[] = { (1 << API_METHOD_POST) | (1 << API_METHOD_PUT), &APIDownstreamConnection::handle_backendconfig, }, + APIEndpoint{ + StringRef::from_lit("/api/v1beta1/configrevision"), true, + (1 << API_METHOD_GET), &APIDownstreamConnection::handle_configrevision, + }, }; } // namespace @@ -82,7 +86,7 @@ enum { }; int APIDownstreamConnection::send_reply(unsigned int http_status, - int api_status) { + int api_status, const StringRef &data) { shutdown_read_ = true; auto upstream = downstream_->get_upstream(); @@ -112,7 +116,8 @@ int APIDownstreamConnection::send_reply(unsigned int http_status, // 3 is the number of digits in http_status, assuming it is 3 digits // number. - auto buflen = M1.size() + M2.size() + M3.size() + api_status_str.size() + 3; + auto buflen = M1.size() + M2.size() + M3.size() + data.size() + + api_status_str.size() + 3; auto buf = make_byte_ref(balloc, buflen); auto p = buf.base; @@ -121,6 +126,7 @@ int APIDownstreamConnection::send_reply(unsigned int http_status, p = std::copy(std::begin(api_status_str), std::end(api_status_str), p); p = std::copy(std::begin(M2), std::end(M2), p); p = util::utos(p, http_status); + p = std::copy(std::begin(data), std::end(data), p); p = std::copy(std::begin(M3), std::end(M3), p); buf.len = p - buf.base; @@ -147,6 +153,32 @@ int APIDownstreamConnection::send_reply(unsigned int http_status, return 0; } +namespace { +const APIEndpoint *lookup_api(const StringRef &path) { + switch (path.size()) { + case 26: + switch (path[25]) { + case 'g': + if (util::streq_l("/api/v1beta1/backendconfi", std::begin(path), 25)) { + return &apis[0]; + } + break; + } + break; + case 27: + switch (path[26]) { + case 'n': + if (util::streq_l("/api/v1beta1/configrevisio", std::begin(path), 26)) { + return &apis[1]; + } + break; + } + break; + } + return nullptr; +} +} // namespace + int APIDownstreamConnection::push_request_headers() { auto &req = downstream_->request(); @@ -154,12 +186,7 @@ int APIDownstreamConnection::push_request_headers() { StringRef{std::begin(req.path), std::find(std::begin(req.path), std::end(req.path), '?')}; - for (auto &p : apis) { - if (p.path == path) { - api_ = &p; - break; - } - } + api_ = lookup_api(path); if (!api_) { send_reply(404, API_FAILURE); @@ -365,6 +392,25 @@ int APIDownstreamConnection::handle_backendconfig() { return 0; } +int APIDownstreamConnection::handle_configrevision() { + auto config = get_config(); + auto &balloc = downstream_->get_block_allocator(); + + // Construct the following string: + // , + // "data":{ + // "configRevision": N + // } + auto data = concat_string_ref( + balloc, StringRef::from_lit(R"(,"data":{"configRevision":)"), + util::make_string_ref_uint(balloc, config->config_revision), + StringRef::from_lit("}")); + + send_reply(200, API_SUCCESS, data); + + return 0; +} + void APIDownstreamConnection::pause_read(IOCtrlReason reason) {} int APIDownstreamConnection::resume_read(IOCtrlReason reason, size_t consumed) { diff --git a/src/shrpx_api_downstream_connection.h b/src/shrpx_api_downstream_connection.h index 2a0c4b1e..356ee53d 100644 --- a/src/shrpx_api_downstream_connection.h +++ b/src/shrpx_api_downstream_connection.h @@ -83,11 +83,14 @@ public: get_downstream_addr_group() const; virtual DownstreamAddr *get_addr() const; - int send_reply(unsigned int http_status, int api_status); + int send_reply(unsigned int http_status, int api_status, + const StringRef &data = StringRef{}); int error_method_not_allowed(); // Handles backendconfig API request. int handle_backendconfig(); + // Handles configrevision API request. + int handle_configrevision(); private: Worker *worker_; diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 8efdb40d..79293441 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -850,6 +850,7 @@ struct Config { conn{}, api{}, dns{}, + config_revision{0}, num_worker{0}, padding{0}, rlimit_nofile{0}, @@ -883,6 +884,13 @@ struct Config { StringRef conf_path; StringRef user; StringRef mruby_file; + // The revision of configuration which is opaque string, and changes + // on each configuration reloading. This does not change on + // backendconfig API call. This value is returned in health check + // as "nghttpx-conf-rev" response header field. The external + // program can check this value to know whether reloading has + // completed or not. + uint64_t config_revision; size_t num_worker; size_t padding; size_t rlimit_nofile; From 5618e1bbc92c9fe69445977c329ddbdc33b593aa Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Feb 2017 23:19:53 +0900 Subject: [PATCH 3/4] integration: Add configrevision API tests --- integration-tests/nghttpx_http1_test.go | 37 ++++++++++++++++++++++++ integration-tests/nghttpx_http2_test.go | 38 +++++++++++++++++++++++++ integration-tests/server_tester.go | 5 ++-- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/integration-tests/nghttpx_http1_test.go b/integration-tests/nghttpx_http1_test.go index c99e9364..cbddc13a 100644 --- a/integration-tests/nghttpx_http1_test.go +++ b/integration-tests/nghttpx_http1_test.go @@ -974,6 +974,43 @@ backend=127.0.0.1,3011 } } +// TestH1APIConfigrevision tests configrevision API. +func TestH1APIConfigrevision(t *testing.T) { + st := newServerTesterConnectPort([]string{"-f127.0.0.1,3010;api;no-tls"}, t, func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("request should not be forwarded") + }, 3010) + defer st.Close() + + res, err := st.http1(requestParam{ + name: "TestH1APIConfigrevision", + path: "/api/v1beta1/configrevision", + method: "GET", + }) + if err != nil { + t.Fatalf("Error st.http1() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("res.status: %v; want = %v", got, want) + } + + var apiResp APIResponse + d := json.NewDecoder(bytes.NewBuffer(res.body)) + d.UseNumber() + err = d.Decode(&apiResp) + if err != nil { + t.Fatalf("Error unmarshalling API response: %v", err) + } + if got, want := apiResp.Status, "Success"; got != want { + t.Errorf("apiResp.Status: %v; want %v", got, want) + } + if got, want := apiResp.Code, 200; got != want { + t.Errorf("apiResp.Status: %v; want %v", got, want) + } + if got, want := apiResp.Data["configRevision"], json.Number("0"); got != want { + t.Errorf(`apiResp.Data["configRevision"]: %v %t; want %v`, got, got, want) + } +} + // TestH1APINotFound exercise backendconfig API endpoint routine when // API endpoint is not found. func TestH1APINotFound(t *testing.T) { diff --git a/integration-tests/nghttpx_http2_test.go b/integration-tests/nghttpx_http2_test.go index c2920802..a4c46c8f 100644 --- a/integration-tests/nghttpx_http2_test.go +++ b/integration-tests/nghttpx_http2_test.go @@ -1,6 +1,7 @@ package nghttp2 import ( + "bytes" "crypto/tls" "encoding/json" "fmt" @@ -2071,6 +2072,43 @@ backend=127.0.0.1,3011 } } +// TestH2APIConfigrevision tests configrevision API. +func TestH2APIConfigrevision(t *testing.T) { + st := newServerTesterConnectPort([]string{"-f127.0.0.1,3010;api;no-tls"}, t, func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("request should not be forwarded") + }, 3010) + defer st.Close() + + res, err := st.http2(requestParam{ + name: "TestH2APIConfigrevision", + path: "/api/v1beta1/configrevision", + method: "GET", + }) + if err != nil { + t.Fatalf("Error st.http2() = %v", err) + } + if got, want := res.status, 200; got != want { + t.Errorf("res.status: %v; want = %v", got, want) + } + + var apiResp APIResponse + d := json.NewDecoder(bytes.NewBuffer(res.body)) + d.UseNumber() + err = d.Decode(&apiResp) + if err != nil { + t.Fatalf("Error unmarshalling API response: %v", err) + } + if got, want := apiResp.Status, "Success"; got != want { + t.Errorf("apiResp.Status: %v; want %v", got, want) + } + if got, want := apiResp.Code, 200; got != want { + t.Errorf("apiResp.Status: %v; want %v", got, want) + } + if got, want := apiResp.Data["configRevision"], json.Number("0"); got != want { + t.Errorf(`apiResp.Data["configRevision"]: %v %t; want %v`, got, got, want) + } +} + // TestH2APINotFound exercise backendconfig API endpoint routine when // API endpoint is not found. func TestH2APINotFound(t *testing.T) { diff --git a/integration-tests/server_tester.go b/integration-tests/server_tester.go index 75185907..03271add 100644 --- a/integration-tests/server_tester.go +++ b/integration-tests/server_tester.go @@ -801,6 +801,7 @@ func cloneHeader(h http.Header) http.Header { func noopHandler(w http.ResponseWriter, r *http.Request) {} type APIResponse struct { - Status string `json:"status,omitempty"` - Code int `json:"code,omitempty"` + Status string `json:"status,omitempty"` + Code int `json:"code,omitempty"` + Data map[string]interface{} `json:"data,omitempty"` } From 1f55e5d34deee1dd123a3d76a74371fb45b78a72 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 19 Feb 2017 23:27:40 +0900 Subject: [PATCH 4/4] nghttpx: Document configrevision API --- doc/nghttpx.h2r | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/doc/nghttpx.h2r b/doc/nghttpx.h2r index be308a47..353d8d88 100644 --- a/doc/nghttpx.h2r +++ b/doc/nghttpx.h2r @@ -559,6 +559,9 @@ status code HTTP status code +Additionally, depending on the API endpoint, ``data`` key may be +present, and its value contains the API endpoint specific data. + We wrote "normally", since nghttpx may return ordinal HTML response in some cases where the error has occurred before reaching API endpoint (e.g., header field is too large). @@ -590,6 +593,23 @@ The one limitation is that only numeric IP address is allowd in is used while non numeric hostname is allowed in command-line or configuration file is read using :option:`--conf`. +GET /api/v1beta1/configrevision +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +This API returns configuration revision of the current nghttpx. The +configuration revision is opaque string, and it changes after each +reloading by SIGHUP. With this API, an external application knows +that whether nghttpx has finished reloading its configuration by +comparing the configuration revisions between before and after +reloading. + +This API returns response including ``data`` key. Its value is JSON +object, and it contains at least the following key: + +configRevision + The configuration revision of the current nghttpx + + SEE ALSO --------