diff options
| -rw-r--r-- | mod/mod-ci-github-gh.cxx | 6 | ||||
| -rw-r--r-- | mod/mod-ci-github-gh.hxx | 8 | ||||
| -rw-r--r-- | mod/mod-ci-github-gq.cxx | 425 | ||||
| -rw-r--r-- | mod/mod-ci-github-gq.hxx | 8 | ||||
| -rw-r--r-- | mod/mod-ci-github-service-data.cxx | 10 | ||||
| -rw-r--r-- | mod/mod-ci-github-service-data.hxx | 12 | ||||
| -rw-r--r-- | mod/mod-ci-github.cxx | 40 | ||||
| -rw-r--r-- | mod/mod-ci-github.hxx | 2 | ||||
| -rw-r--r-- | mod/module.cli | 2 |
9 files changed, 417 insertions, 96 deletions
diff --git a/mod/mod-ci-github-gh.cxx b/mod/mod-ci-github-gh.cxx index 2e886ac..42afe1b 100644 --- a/mod/mod-ci-github-gh.cxx +++ b/mod/mod-ci-github-gh.cxx @@ -211,9 +211,9 @@ namespace brep // one can be null. It's unclear under what circumstances, but it // shouldn't happen unless something is broken. // - string* v (p.next_expect_number_null ()); + optional<uint64_t> v (p.next_expect_number_null<uint64_t> ()); - if (v == nullptr) + if (!v) throw_json (p, "check_suite.app.id is null"); app_id = *v; @@ -310,7 +310,7 @@ namespace brep // while (p.next_expect (event::name, event::end_object)) { - if (c (ai, "id")) app_id = p.next_expect_number (); + if (c (ai, "id")) app_id = p.next_expect_number<uint64_t> (); else p.next_expect_value_skip (); } diff --git a/mod/mod-ci-github-gh.hxx b/mod/mod-ci-github-gh.hxx index 91f5bfe..5f6e5b7 100644 --- a/mod/mod-ci-github-gh.hxx +++ b/mod/mod-ci-github-gh.hxx @@ -66,7 +66,7 @@ namespace brep size_t check_runs_count; optional<string> conclusion; - string app_id; + uint64_t app_id; explicit gh_check_suite_ex (json::parser&); @@ -97,7 +97,7 @@ namespace brep string details_url; gh_check_suite check_suite; - string app_id; + uint64_t app_id; explicit gh_check_run_ex (json::parser&); @@ -129,7 +129,7 @@ namespace brep // simplicity we emulate check_suite and check_run by storing the app-id // webhook query parameter here. // - string app_id; + uint64_t app_id; explicit gh_pull_request (json::parser&); @@ -256,7 +256,7 @@ namespace brep // emulate check_suite and check_run by storing the app-id webhook query // parameter here. // - string app_id; + uint64_t app_id; explicit gh_push_event (json::parser&); diff --git a/mod/mod-ci-github-gq.cxx b/mod/mod-ci-github-gq.cxx index 7abd709..763842c 100644 --- a/mod/mod-ci-github-gq.cxx +++ b/mod/mod-ci-github-gq.cxx @@ -19,6 +19,7 @@ namespace brep static const string& gq_name (const string&); static string gq_name (string&&); static string gq_str (const string&); + static string gq_int (uint64_t); static string gq_bool (bool); static const string& gq_enum (const string&); static string gq_enum (string&&); @@ -187,7 +188,7 @@ namespace brep // } // static vector<gh_check_run> - gq_parse_response_check_runs (json::parser& p) + gq_parse_mutate_check_runs_response (json::parser& p) { using event = json::event; @@ -223,20 +224,199 @@ namespace brep return r; } - // Send a GraphQL mutation request `rq` that creates or updates one or more - // check runs. The requested build state is taken from each check_run - // object. Update the check runs in `crs` with the new data (state, node ID - // if unset, and state_synced). Return false and issue diagnostics if the - // request failed. + // Serialize a query that fetches the most recent check runs on a commit. // + static string + gq_query_get_check_runs (uint64_t ai, // App id + const string& ri, // Repository id + const string& ci, // Commit id + size_t cn) // Check run count + { + + ostringstream os; + + os << "query {" << '\n'; + + // Get the repository node. + // + os << "node(id: " << gq_str (ri) << ") {" << '\n' + << "... on Repository {" << '\n'; + + // Get the commit object. + // + os << " object(oid: " << gq_str (ci) << ") {" << '\n' + << " ... on Commit {" << '\n'; + + // Get the check suites on the commit, filtering by our app id. (Note that + // as a result there should never be more than one check suite; see + // below.) + // + os << " checkSuites(first: 1" << '\n' + << " filterBy: {appId: " << gq_int (ai) << "}) {" << '\n' + << " edges { node {" << '\n'; + + // Get the check suite's last N check runs (last:). + // + // Filter by App id because apparently an App can create check runs in + // another App's check suite. + // + // Also ask for the latest check runs only (checkType: LATEST) otherwise + // we could receive multiple check runs with the same name. Although this + // appears to be the default it's not documented anywhere so best make it + // explicit. + // + // Note that the selection set (fields to be returned) must match that of + // the check run mutations (create/update) generated by + // gq_mutation_{create,update}_check_runs(). + // + os << " checkRuns(last: " << gq_int (cn) << '\n' + << " filterBy: {appId: " << gq_int (ai) << '\n' + << " checkType: LATEST}) {" << '\n' + << " edges { node { node_id: id name status } }" << '\n' + << " }" /* checkRuns */ << '\n' + << " } }" /* node, edges */ << '\n' + << " }" /* checkSuites */ << '\n' + << " }" /* ... on Commit */ << '\n' + << " }" /* object */ << '\n' + << "}" /* ... on Repository */ << '\n' + << "}" /* node */ << '\n'; + + os << '}' /* query */ << '\n'; + + return os.str (); + } + + // Parse a response to a "get check runs for repository/commit" GraphQL + // query as constructed by gq_query_get_check_runs(). + // + // Note that there might be other check suites on this commit but they will + // all have been created by other apps (GitHub never creates more than one + // check suite per app). Therefore our query filters by app id and as a + // result there should never be more than one check suite in the response. + // + // Throw invalid_json_input. + // + // Example response (only the part we need to parse here): + // + // { + // "node": { + // "object":{ + // "checkSuites":{ + // "edges":[ + // {"node":{ + // "checkRuns":{ + // "edges":[ + // {"node":{"id":"CR_kwDOLc8CoM8AAAAImvJPfw", + // "name":"check_run0", + // "status":"QUEUED"}}, + // {"node":{"id":"CR_kwDOLc8CoM8AAAAImvJP_Q", + // "name":"check_run1", + // "status":"QUEUED"}} + // ] + // } + // } + // } + // ] + // } + // } + // } + // } + // + static vector<gh_check_run> + gq_parse_get_check_runs_response (json::parser& p) + { + using event = json::event; + + vector<gh_check_run> r; + + gq_parse_response (p, [&r] (json::parser& p) + { + p.next_expect (event::begin_object); // Outermost { + + p.next_expect_member_object ("node"); // Repository node + p.next_expect_member_object ("object"); // Commmit + p.next_expect_member_object ("checkSuites"); + p.next_expect_member_array ("edges"); // Check suites array + p.next_expect (event::begin_object); // Check suite outer { + p.next_expect_member_object ("node"); + p.next_expect_member_object ("checkRuns"); + p.next_expect_member_array ("edges"); // Check runs array + + // Parse the check run elements of the `edges` array. E.g.: + // + // { + // "node":{ + // "node_id":"CR_kwDOLc8CoM8AAAAIobBFlA", + // "name":"CONCLUSION", + // "status":"IN_PROGRESS" + // } + // } + // + while (p.next_expect (event::begin_object, event::end_array)) + { + p.next_expect_name ("node"); + r.emplace_back (p); // Parse check run: { members... } + p.next_expect (event::end_object); + } + + p.next_expect (event::end_object); // checkRuns + p.next_expect (event::end_object); // Check suite node + p.next_expect (event::end_object); // Check suite outer } + p.next_expect (event::end_array); // Check suites edges + p.next_expect (event::end_object); // checkSuites + p.next_expect (event::end_object); // Commit + p.next_expect (event::end_object); // Repository node + + p.next_expect (event::end_object); // Outermost } + }); + + return r; + } + + // Serialize a GraphQL operation (query/mutation) into a GraphQL request. + // + // This is essentially a JSON object with a "query" string member containing + // the GraphQL operation. For example: + // + // { "query": "mutation { cr0:createCheckRun(... }" } + // + static string + gq_serialize_request (const string& o) + { + string b; + json::buffer_serializer s (b); + + s.begin_object (); + s.member ("query", o); + s.end_object (); + + return b; + } + + // Send a GraphQL mutation request `rq` that creates (create=true) or + // updates (create=false) one or more check runs. The requested build state + // is taken from each check_run object. Update the check runs in `crs` with + // the new data (state, node ID if unset, and state_synced). Return false + // and issue diagnostics if the request failed. + // + struct gq_create_data + { + uint64_t app_id; + reference_wrapper<const string> repository_id; + reference_wrapper<const string> head_sha; + }; + static bool gq_mutate_check_runs (const basic_mark& error, - vector<check_run>& crs, + check_runs::iterator crs_b, + check_runs::iterator crs_e, const string& iat, - string rq) + string rq, + const optional<gq_create_data>& create_data) { - vector<gh_check_run> rcrs; + size_t crs_n (crs_e - crs_b); + const char* what (nullptr); try { // Response type which parses a GraphQL response containing multiple @@ -247,29 +427,104 @@ namespace brep vector<gh_check_run> check_runs; // Received check runs. resp (json::parser& p) - : check_runs (gq_parse_response_check_runs (p)) {} + : check_runs (gq_parse_mutate_check_runs_response (p)) {} resp () = default; } rs; + what = create_data ? "create" : "update"; uint16_t sc (github_post (rs, "graphql", // API Endpoint. strings {"Authorization: Bearer " + iat}, move (rq))); + // Turns out it's not uncommon to not get a reply from GitHub if the + // number of check runs being created in build_queued() is large. The + // symptom is a 502 (Bad gateway) reply from GitHub and the theory being + // that their load balancer drops the connection if the request is not + // handled within a certain time. Note that if the number of check runs + // is under 100, they seem to still be created on GitHub, we just don't + // get the reply (and thus their node ids). So we try to re-query that + // information. + // + optional<uint16_t> sc1; + if (sc == 502 && create_data) + { + what = "re-query"; + + // GraphQL query which fetches the most recently-created check runs. + // + string rq (gq_serialize_request ( + gq_query_get_check_runs (create_data->app_id, + create_data->repository_id, + create_data->head_sha, + crs_n))); + + // Type that parses the result of the above GraphQL query. + // + struct resp + { + vector<gh_check_run> check_runs; // Received check runs. + + resp (json::parser& p) + : check_runs (gq_parse_get_check_runs_response (p)) {} + + resp () = default; + } rs1; + + sc1 = github_post (rs1, + "graphql", // API Endpoint. + strings {"Authorization: Bearer " + iat}, + move (rq)); + + if (*sc1 == 200) + { + if (rs1.check_runs.size () == crs_n) + { + // It's possible GitHub did not create all the checkruns we have + // requested. In which case it may return some unrelated checkruns + // (for example, from before re-request). So we verify we got the + // expected ones. + // + size_t i (0); + for (; i != crs_n; ++i) + { + const check_run& cr (*(crs_b + i)); + const gh_check_run& gcr (rs1.check_runs[i]); + + if (cr.name != gcr.name || + cr.state != gh_from_status (gcr.status)) + break; + } + + if (i == crs_n) + { + rs.check_runs = move (rs1.check_runs); + + // Reduce to as-if the create request succeeded. + // + what = "create"; + sc = 200; + } + } + } + } + if (sc == 200) { - rcrs = move (rs.check_runs); + vector<gh_check_run>& rcrs (rs.check_runs); - if (rcrs.size () == crs.size ()) + if (rcrs.size () == crs_n) { - for (size_t i (0); i != rcrs.size (); ++i) + for (size_t i (0); i != crs_n; ++i) { + check_run& cr (*(crs_b + i)); + // Validate the check run in the response against the build. // const gh_check_run& rcr (rcrs[i]); // Received check run. - build_state st (crs[i].state); // Requested state. + build_state st (cr.state); // Requested state. build_state rst (gh_from_status (rcr.status)); // Received state. // Note that GitHub won't allow us to change a built check run to @@ -283,8 +538,6 @@ namespace brep return false; // Fail because something is clearly very wrong. } - check_run& cr (crs[i]); - if (!cr.node_id) cr.node_id = move (rcr.node_id); @@ -298,57 +551,52 @@ namespace brep error << "unexpected number of check_run objects in response"; } else - error << "failed to mutate check runs: error HTTP response status " - << sc; + { + diag_record dr (error); + + dr << "failed to " << what << " check runs: error HTTP response status " + << sc; + + if (sc1) + { + if (*sc1 != 200) + dr << error << "failed to re-query check runs: error HTTP " + << "response status " << *sc1; + else + dr << error << "unexpected number of check_run objects in " + << "re-query response"; + } + } } catch (const json::invalid_json_input& e) // struct resp (via github_post()) { // Note: e.name is the GitHub API endpoint. // - error << "malformed JSON in response from " << e.name << ", line: " - << e.line << ", column: " << e.column << ", byte offset: " - << e.position << ", error: " << e; + error << "malformed JSON in " << what << " response from " << e.name + << ", line: " << e.line << ", column: " << e.column + << ", byte offset: " << e.position + << ", error: " << e; } catch (const invalid_argument& e) // github_post() { - error << "malformed header(s) in response: " << e; + error << "malformed header(s) in " << what << " response: " << e; } catch (const system_error& e) // github_post() { - error << "unable to mutate check runs (errno=" << e.code () << "): " - << e.what (); + error << "unable to " << what << " check runs (errno=" << e.code () + << "): " << e.what (); } - catch (const runtime_error& e) // gq_parse_response_check_runs() + catch (const runtime_error& e) // gq_parse_{mutate,get}_check_runs_response() { // GitHub response contained error(s) (could be ours or theirs at this // point). // - error << "unable to mutate check runs: " << e; + error << "unable to " << what << " check runs: " << e; } return false; } - // Serialize a GraphQL operation (query/mutation) into a GraphQL request. - // - // This is essentially a JSON object with a "query" string member containing - // the GraphQL operation. For example: - // - // { "query": "mutation { cr0:createCheckRun(... }" } - // - static string - gq_serialize_request (const string& o) - { - string b; - json::buffer_serializer s (b); - - s.begin_object (); - s.member ("query", o); - s.end_object (); - - return b; - } - // Serialize `createCheckRun` mutations for one or more builds to GraphQL. // // The check run parameters (names, build states, details_urls, etc.) are @@ -366,7 +614,8 @@ namespace brep static string gq_mutation_create_check_runs (const string& ri, // Repository ID const string& hs, // Head SHA - const vector<check_run>& crs) + brep::check_runs::iterator crs_b, + brep::check_runs::iterator crs_e) { ostringstream os; @@ -374,9 +623,9 @@ namespace brep // Serialize a `createCheckRun` for each build. // - for (size_t i (0); i != crs.size (); ++i) + for (brep::check_runs::iterator crs_i (crs_b); crs_i != crs_e; ++crs_i) { - const check_run& cr (crs[i]); + const check_run& cr (*crs_i); assert (cr.state != build_state::built); // Not supported. @@ -387,7 +636,7 @@ namespace brep (!cr.description->title.empty () && !cr.description->summary.empty ())); - string al ("cr" + to_string (i)); // Field alias. + string al ("cr" + to_string (crs_i - crs_b)); // Field alias. os << gq_name (al) << ":createCheckRun(input: {" << '\n' << " name: " << gq_str (cr.name) << '\n' @@ -496,7 +745,6 @@ namespace brep return os.str (); } - // Serialize an `updateCheckRun` mutation for one build to GraphQL. // // The `br` argument is required if the check run status is completed @@ -568,8 +816,9 @@ namespace brep bool gq_create_check_runs (const basic_mark& error, - vector<check_run>& crs, + brep::check_runs& crs, const string& iat, + uint64_t ai, const string& rid, const string& hs) { @@ -580,16 +829,47 @@ namespace brep assert (cr.state != build_state::built); #endif - string rq ( - gq_serialize_request (gq_mutation_create_check_runs (rid, hs, crs))); + // Trying to create a large number of check runs at once does not work. + // There are two failure modes: + // + // 1. Between about 40 - 60 we may get 502 (bad gateway) but the check + // runs are still created on GitHub. We handle this case be re-quering + // the check runs (see gq_mutate_check_runs() for details). + // + // 2. Above about 60 GitHub may not create all the check runs (while still + // responding with 502). We handle this here by batching the creation. + // + size_t n (crs.size ()); + size_t b (n / 60 + (n % 60 != 0 ? 1 : 0)); + size_t bn (n / b); + + auto i (crs.begin ()); + for (size_t j (0); j != b; ) + { + auto e (++j != b ? (i + bn): crs.end ()); + + string rq ( + gq_serialize_request ( + gq_mutation_create_check_runs (rid, hs, i, e))); - return gq_mutate_check_runs (error, crs, iat, move (rq)); + if (!gq_mutate_check_runs (error, + i, e, + iat, + move (rq), + gq_create_data {ai, rid, hs})) + return false; + + i += bn; + } + + return true; } bool gq_create_check_run (const basic_mark& error, check_run& cr, const string& iat, + uint64_t ai, const string& rid, const string& hs, const optional<string>& du, @@ -610,10 +890,14 @@ namespace brep move (ti), move (su), nullopt /* conclusion */))); - vector<check_run> crs {move (cr)}; + brep::check_runs crs {move (cr)}; crs[0].state = st; - bool r (gq_mutate_check_runs (error, crs, iat, move (rq))); + bool r (gq_mutate_check_runs (error, + crs.begin (), crs.end (), + iat, + move (rq), + gq_create_data {ai, rid, hs})); cr = move (crs[0]); @@ -624,6 +908,7 @@ namespace brep gq_create_check_run (const basic_mark& error, check_run& cr, const string& iat, + uint64_t ai, const string& rid, const string& hs, const optional<string>& du, @@ -639,10 +924,14 @@ namespace brep move (br.title), move (br.summary), move (br.conclusion)))); - vector<check_run> crs {move (cr)}; + brep::check_runs crs {move (cr)}; crs[0].state = build_state::built; - bool r (gq_mutate_check_runs (error, crs, iat, move (rq))); + bool r (gq_mutate_check_runs (error, + crs.begin (), crs.end (), + iat, + move (rq), + gq_create_data {ai, rid, hs})); cr = move (crs[0]); @@ -678,10 +967,14 @@ namespace brep move (ti), move (su), nullopt /* conclusion */))); - vector<check_run> crs {move (cr)}; + brep::check_runs crs {move (cr)}; crs[0].state = st; - bool r (gq_mutate_check_runs (error, crs, iat, move (rq))); + bool r (gq_mutate_check_runs (error, + crs.begin (), crs.end (), + iat, + move (rq), + nullopt)); cr = move (crs[0]); @@ -705,10 +998,14 @@ namespace brep move (br.title), move (br.summary), move (br.conclusion)))); - vector<check_run> crs {move (cr)}; + brep::check_runs crs {move (cr)}; crs[0].state = build_state::built; - bool r (gq_mutate_check_runs (error, crs, iat, move (rq))); + bool r (gq_mutate_check_runs (error, + crs.begin (), crs.end (), + iat, + move (rq), + nullopt)); cr = move (crs[0]); @@ -944,7 +1241,6 @@ namespace brep // Serialize an int to GraphQL. // -#if 0 static string gq_int (uint64_t v) { @@ -953,7 +1249,6 @@ namespace brep s.value (v); return b; } -#endif // Serialize a boolean to GraphQL. // diff --git a/mod/mod-ci-github-gq.hxx b/mod/mod-ci-github-gq.hxx index 19c4924..5fc75aa 100644 --- a/mod/mod-ci-github-gq.hxx +++ b/mod/mod-ci-github-gq.hxx @@ -22,7 +22,8 @@ namespace brep // Create a new check run on GitHub for each build with the build state, // name, details_url, and output taken from each check_run object. Update // `check_runs` with the new data (node id and state_synced). Return false - // and issue diagnostics if the request failed. + // and issue diagnostics if the request failed. Note that in this case some + // elements in check_runs may still have been updated (due to batching). // // Throw invalid_argument if the passed data is invalid, missing, or // inconsistent. @@ -34,8 +35,9 @@ namespace brep // bool gq_create_check_runs (const basic_mark& error, - vector<check_run>& check_runs, + brep::check_runs& check_runs, const string& installation_access_token, + uint64_t app_id, const string& repository_id, const string& head_sha); @@ -56,6 +58,7 @@ namespace brep gq_create_check_run (const basic_mark& error, check_run& cr, const string& installation_access_token, + uint64_t app_id, const string& repository_id, const string& head_sha, const optional<string>& details_url, @@ -77,6 +80,7 @@ namespace brep gq_create_check_run (const basic_mark& error, check_run& cr, const string& installation_access_token, + uint64_t app_id, const string& repository_id, const string& head_sha, const optional<string>& details_url, diff --git a/mod/mod-ci-github-service-data.cxx b/mod/mod-ci-github-service-data.cxx index c51f791..aa2e619 100644 --- a/mod/mod-ci-github-service-data.cxx +++ b/mod/mod-ci-github-service-data.cxx @@ -54,7 +54,7 @@ namespace brep p.next_expect_name ("installation_access"); installation_access = gh_installation_access_token (p); - app_id = p.next_expect_member_string ("app_id"); + app_id = p.next_expect_member_number<uint64_t> ("app_id"); installation_id = p.next_expect_member_string ("installation_id"); repository_node_id = p.next_expect_member_string ("repository_node_id"); @@ -143,7 +143,7 @@ namespace brep service_data (bool ws, string iat_tok, timestamp iat_ea, - string aid, + uint64_t aid, string iid, string rid, string rcu, @@ -155,7 +155,7 @@ namespace brep : kind (k), pre_check (pc), re_request (rr), warning_success (ws), installation_access (move (iat_tok), iat_ea), - app_id (move (aid)), + app_id (aid), installation_id (move (iid)), repository_node_id (move (rid)), repository_clone_url (move (rcu)), @@ -171,7 +171,7 @@ namespace brep service_data (bool ws, string iat_tok, timestamp iat_ea, - string aid, + uint64_t aid, string iid, string rid, string rcu, @@ -185,7 +185,7 @@ namespace brep : kind (k), pre_check (pc), re_request (rr), warning_success (ws), installation_access (move (iat_tok), iat_ea), - app_id (move (aid)), + app_id (aid), installation_id (move (iid)), repository_node_id (move (rid)), repository_clone_url (move (rcu)), diff --git a/mod/mod-ci-github-service-data.hxx b/mod/mod-ci-github-service-data.hxx index 5d36696..9aa512a 100644 --- a/mod/mod-ci-github-service-data.hxx +++ b/mod/mod-ci-github-service-data.hxx @@ -55,6 +55,8 @@ namespace brep } }; + using check_runs = vector<check_run>; + // We have two kinds of service data that correspond to the following two // typical scenarios (until/unless we add support for merge queues): // @@ -96,8 +98,8 @@ namespace brep // gh_installation_access_token installation_access; - string app_id; - string installation_id; + uint64_t app_id; + string installation_id; // @@ TMP Also actually an integer string repository_node_id; // GitHub-internal opaque repository id. @@ -123,7 +125,7 @@ namespace brep // string report_sha; - vector<check_run> check_runs; + brep::check_runs check_runs; // Flag indicating that all the elements in check_runs are built and this // check suite is completed. @@ -159,7 +161,7 @@ namespace brep service_data (bool warning_success, string iat_token, timestamp iat_expires_at, - string app_id, + uint64_t app_id, string installation_id, string repository_node_id, string repository_clone_url, @@ -174,7 +176,7 @@ namespace brep service_data (bool warning_success, string iat_token, timestamp iat_expires_at, - string app_id, + uint64_t app_id, string installation_id, string repository_node_id, string repository_clone_url, diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx index b4babce..8242fc0 100644 --- a/mod/mod-ci-github.cxx +++ b/mod/mod-ci-github.cxx @@ -17,6 +17,8 @@ #include <mod/mod-ci-github-post.hxx> #include <mod/mod-ci-github-service-data.hxx> +#include <cerrno> +#include <cstdlib> // strtoull() #include <stdexcept> // Resources: @@ -261,7 +263,7 @@ namespace brep // Process the `app-id` and `warning` webhook request query parameters. // - string app_id; + uint64_t app_id; bool warning_success; { const name_values& rps (rq.parameters (1024, true /* url_only */)); @@ -281,7 +283,18 @@ namespace brep badreq ("missing 'app-id' webhook query parameter value"); ai = true; - app_id = *rp.value; + + // Parse the app id value. + // + const char* b (rp.value->c_str ()); + char* e (nullptr); + errno = 0; // We must clear it according to POSIX. + app_id = strtoull (b, &e, 10); + if (errno == ERANGE || e == b || *e != '\0') + { + badreq ("invalid 'app-id' webhook query parameter value: '" + + *rp.value + '\''); + } } else if (rp.name == "warning") { @@ -1296,7 +1309,7 @@ namespace brep // a single request (unless something goes wrong) so store them together // from the outset. // - vector<check_run> check_runs (2); + brep::check_runs check_runs (2); check_run& bcr (check_runs[0]); // Build check run check_run& ccr (check_runs[1]); // Conclusion check run @@ -1517,7 +1530,7 @@ namespace brep conclusion_building_summary}; if (gq_create_check_runs (error, check_runs, iat->token, - repo_node_id, head_sha)) + cr.check_run.app_id, repo_node_id, head_sha)) { assert (bcr.state == build_state::queued); assert (ccr.state == build_state::building); @@ -1996,6 +2009,7 @@ namespace brep if (gq_create_check_run (error, cr, iat->token, + sd.app_id, sd.repository_node_id, sd.report_sha, details_url (tenant_id), @@ -2314,7 +2328,7 @@ namespace brep // The builds for which we will be creating check runs. // vector<reference_wrapper<const build>> bs; - vector<check_run> crs; // Parallel to bs. + brep::check_runs crs; // Parallel to bs. // Exclude the builds for which we won't be creating check runs. // @@ -2400,7 +2414,9 @@ namespace brep if (gq_create_check_runs (error, crs, iat->token, - sd.repository_node_id, sd.report_sha)) + sd.app_id, + sd.repository_node_id, + sd.report_sha)) { for (const check_run& cr: crs) { @@ -2519,7 +2535,10 @@ namespace brep } else { - // Network error during queued notification, ignore. + // Network error during queued notification (state unsynchronized), + // ignore. + // + l3 ([&]{trace << "unsynchronized check run " << bid;}); } } else @@ -2880,6 +2899,7 @@ namespace brep if (gq_create_check_run (error, cr, iat->token, + sd.app_id, sd.repository_node_id, sd.report_sha, details_url (b), @@ -3292,7 +3312,7 @@ namespace brep } optional<string> ci_github:: - generate_jwt (const string& app_id, + generate_jwt (uint64_t app_id, const basic_mark& trace, const basic_mark& error) const { @@ -3301,7 +3321,7 @@ namespace brep { // Look up the private key path for the app id and fail if not found. // - const map<string, dir_path>& pks ( + const map<uint64_t, dir_path>& pks ( options_->ci_github_app_id_private_key ()); auto pk (pks.find (app_id)); @@ -3317,7 +3337,7 @@ namespace brep // jwt = brep::generate_jwt ( *options_, - pk->second, app_id, + pk->second, to_string (app_id), chrono::seconds (options_->ci_github_jwt_validity_period ()), chrono::seconds (60)); diff --git a/mod/mod-ci-github.hxx b/mod/mod-ci-github.hxx index 4cedc94..0c90bb1 100644 --- a/mod/mod-ci-github.hxx +++ b/mod/mod-ci-github.hxx @@ -139,7 +139,7 @@ namespace brep details_url (const string& tenant) const; optional<string> - generate_jwt (const string& app_id, + generate_jwt (uint64_t app_id, const basic_mark& trace, const basic_mark& error) const; diff --git a/mod/module.cli b/mod/module.cli index ba2b986..57a5f31 100644 --- a/mod/module.cli +++ b/mod/module.cli @@ -858,7 +858,7 @@ namespace brep Note: make sure to choose a strong (random) secret." } - std::map<string, dir_path> ci-github-app-id-private-key + std::map<uint64_t, dir_path> ci-github-app-id-private-key { "<id>=<path>", "The private key used during GitHub API authentication for the |