ci-github: Handle branch pushes in handle_push_event()

1 files changed, 127 insertions, 58 deletions

diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 23ac4c7..28af0fa 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -307,14 +307,17 @@ namespace brep
 
       if (cs.action == "requested")
       {
-        return handle_check_suite_request (move (cs), warning_success);
+        // Branch pushes are handled in handle_push_request() so ignore this
+        // event.
+        //
+        return true;
       }
       else if (cs.action == "rerequested")
       {
         // Someone manually requested to re-run all the check runs in this
         // check suite. Treat as a new request.
         //
-        return handle_check_suite_request (move (cs), warning_success);
+        return handle_check_suite_rerequest (move (cs), warning_success);
       }
       else if (cs.action == "completed")
       {
@@ -404,7 +407,7 @@ namespace brep
         throw invalid_request (400, move (m));
       }
 
-      // Store the app-id webhook query parameter in the gh_pull_request
+      // Store the app-id webhook query parameter in the gh_pull_request_event
       // object (see gh_pull_request for an explanation).
       //
       // When we receive the other webhooks we do check that the app ids in
@@ -496,6 +499,9 @@ namespace brep
     }
     else if (event == "push")
     {
+      // Push events are triggered by branch pushes, branch creation, and
+      // branch deletion.
+      //
       gh_push_event ps;
       try
       {
@@ -513,9 +519,19 @@ namespace brep
         throw invalid_request (400, move (m));
       }
 
+      // Store the app-id webhook query parameter in the gh_push_event
+      // object (see gh_push_event for an explanation).
+      //
+      // When we receive the other webhooks we do check that the app ids in
+      // the payload and query match but here we have to assume it is valid.
+      //
+      ps.app_id = app_id;
+
       // Note that the push request event has no action.
       //
-      return handle_push_request (move (ps));
+      //   @@ TMP I said no before but there is a `deleted` member...
+      //
+      return handle_push_request (move (ps), warning_success);
     }
     else
     {
@@ -572,12 +588,14 @@ namespace brep
   }
 
   bool ci_github::
-  handle_check_suite_request (gh_check_suite_event cs, bool warning_success)
+  handle_check_suite_rerequest (gh_check_suite_event cs, bool warning_success)
   {
     HANDLER_DIAG;
 
     l3 ([&]{trace << "check_suite event { " << cs << " }";});
 
+    assert (cs.action == "rerequested");
+
     // While we don't need the installation access token in this request,
     // let's obtain it to flush out any permission issues early. Also, it is
     // valid for an hour so we will most likely make use of it.
@@ -595,15 +613,6 @@ namespace brep
 
     l3 ([&]{trace << "installation_access_token { " << *iat << " }";});
 
-    // While it would have been nice to cancel CIs of PRs with this branch as
-    // base not to waste resources, there are complications: Firstly, we can
-    // only do this for remote PRs (since local PRs will most likely share the
-    // result with branch push). Secondly, we try to do our best even if the
-    // branch protection rule for head behind is not enabled. In this case, it
-    // would be good to complete the CI. So maybe/later. See also the head
-    // case in handle_pull_request(), where we do cancel remote PRs that are
-    // not shared.
-
     // Service id that uniquely identifies the CI tenant.
     //
     string sid (cs.repository.node_id + ':' + cs.check_suite.head_sha);
@@ -627,9 +636,10 @@ namespace brep
     string check_sha;
     service_data::kind_type kind;
 
-    bool re_requested (cs.action == "rerequested");
-    if (re_requested && !cs.check_suite.head_branch)
+    if (!cs.check_suite.head_branch)
     {
+      // Rebuild of remote PR.
+      //
       kind = service_data::remote;
 
       if (optional<tenant_data> d = find (*build_db_, "ci-github", sid))
@@ -657,7 +667,7 @@ namespace brep
     }
     else
     {
-      // Branch push or local PR rebuild.
+      // Rebuild of branch push or local PR.
       //
       kind = service_data::local;
       check_sha = cs.check_suite.head_sha;
@@ -670,20 +680,15 @@ namespace brep
                      cs.installation.id,
                      move (cs.repository.node_id),
                      move (cs.repository.clone_url),
-                     kind, false /* pre_check */, re_requested,
+                     kind, false /* pre_check */, true /* re_requested */,
                      move (check_sha),
                      move (cs.check_suite.head_sha) /* report_sha */);
 
-    // If this check suite is being re-run, replace the existing CI tenant if
-    // it exists; otherwise create a new one, doing nothing if a tenant
-    // already exists (which could've been created by handle_pull_request()).
+    // Replace the existing CI tenant if it exists.
     //
     // Note that GitHub UI does not allow re-running the entire check suite
     // until all the check runs are completed.
     //
-    duplicate_tenant_mode dtm (re_requested
-                               ? duplicate_tenant_mode::replace
-                               : duplicate_tenant_mode::ignore);
 
     // Create an unloaded CI tenant.
     //
@@ -704,7 +709,7 @@ namespace brep
                      tenant_service (sid, "ci-github", sd.json ()),
                      chrono::seconds (30) /* interval */,
                      chrono::seconds (0) /* delay */,
-                     dtm));
+                     duplicate_tenant_mode::replace));
 
     if (!pr)
     {
@@ -712,8 +717,7 @@ namespace brep
            << ": unable to create unloaded CI tenant";
     }
 
-    if (dtm == duplicate_tenant_mode::replace &&
-        pr->second == duplicate_tenant_result::created)
+    if (pr->second == duplicate_tenant_result::created)
     {
       error << "check suite " << cs.check_suite.node_id
             << ": re-requested but tenant_service with id " << sid
@@ -1489,50 +1493,114 @@ namespace brep
   }
 
   bool ci_github::
-  handle_push_request (gh_push_event ps)
+  handle_push_request (gh_push_event ps, bool warning_success)
   {
     HANDLER_DIAG;
 
     l3 ([&]{trace << "push event { " << ps << " }";});
 
-    // Do nothing if this is a fast-forwarding push.
+    // The common log entry subject.
     //
-    if (!ps.forced)
+    string sub ((ps.forced ? "forced push " : "push ") + ps.after + " to " +
+                ps.ref);
+
+    // Cancel the CI tenant associated with the overwritten previous head
+    // commit if this is a forced push.
+    //
+    if (ps.forced)
     {
-      l3 ([&]{trace << "ignoring fast-forward push "
-                    << ps.after << " to " << ps.ref;});
-      return true;
+      // Service id that will uniquely identify the CI tenant.
+      //
+      string sid (ps.repository.node_id + ':' + ps.before);
+
+      // Note that it's possible this commit still exists in another branch so
+      // we do refcount-aware cancel.
+      //
+      if (optional<tenant_service> ts = cancel (error, warn,
+                                                verb_ ? &trace : nullptr,
+                                                *build_db_, retry_,
+                                                "ci-github", sid,
+                                                true /* ref_count */))
+      {
+        l3 ([&]{trace << sub << ": attempted to cancel CI of previous"
+                      << " head commit with tenant_service id " << sid
+                      << " (ref_count: " << ts->ref_count << ')';});
+      }
+      else
+      {
+        // It's possible that there was no CI for the previous commit for
+        // various reasons (e.g., CI was not enabled).
+        //
+        l3 ([&]{trace << sub << ": failed to cancel CI of previous head commit"
+                      << " with tenant_service id " << sid;});
+      }
     }
 
-    // Cancel the CI tenant associated with the overwritten previous head
-    // commit.
+    // While we don't need the installation access token in this request,
+    // let's obtain it to flush out any permission issues early. Also, it is
+    // valid for an hour so we will most likely make use of it.
+    //
+    optional<string> jwt (generate_jwt (ps.app_id, trace, error));
+    if (!jwt)
+      throw server_error ();
+
+    optional<gh_installation_access_token> iat (
+        obtain_installation_access_token (ps.installation.id,
+                                          move (*jwt),
+                                          error));
+    if (!iat)
+      throw server_error ();
+
+    l3 ([&]{trace << "installation_access_token { " << *iat << " }";});
 
-    // Service id that will uniquely identify the CI tenant.
+    // While it would have been nice to cancel CIs of PRs with this branch as
+    // base not to waste resources, there are complications: Firstly, we can
+    // only do this for remote PRs (since local PRs will most likely share the
+    // result with branch push). Secondly, we try to do our best even if the
+    // branch protection rule for head behind is not enabled. In this case, it
+    // would be good to complete the CI. So maybe/later. See also the head
+    // case in handle_pull_request(), where we do cancel remote PRs that are
+    // not shared.
+
+    // Service id that uniquely identifies the CI tenant.
     //
-    string sid (ps.repository.node_id + ':' + ps.before);
+    string sid (ps.repository.node_id + ':' + ps.after);
 
-    // Note that it's possible this commit still exists in another branch so
-    // we do refcount-aware cancel.
+    service_data sd (warning_success,
+                     iat->token,
+                     iat->expires_at,
+                     ps.app_id,
+                     ps.installation.id,
+                     move (ps.repository.node_id),
+                     move (ps.repository.clone_url),
+                     service_data::local,
+                     false /* pre_check */,
+                     false /* re_requested */,
+                     ps.after /* check_sha */,
+                     ps.after /* report_sha */);
+
+    // Create an unloaded CI tenant, doing nothing if one already exists
+    // (which could've been created by handle_pull_request()). Note that the
+    // tenant's reference count is incremented in all cases.
     //
-    if (optional<tenant_service> ts = cancel (error, warn,
-                                              verb_ ? &trace : nullptr,
-                                              *build_db_, retry_,
-                                              "ci-github", sid,
-                                              true /* ref_count */))
-    {
-      l3 ([&]{trace << "forced push to " << ps.ref
-                    << ": attempted to cancel CI of previous head commit"
-                    << " with tenant_service id " << sid
-                    << " (ref_count: " << ts->ref_count << ')';});
-    }
-    else
+    // Note: use no delay since we need to (re)create the synthetic conclusion
+    // check run as soon as possible.
+    //
+    // Note that we use the create() API instead of start() since duplicate
+    // management is not available in start().
+    //
+    // After this call we will start getting the build_unloaded()
+    // notifications until (1) we load the tenant, (2) we cancel it, or (3)
+    // it gets archived after some timeout.
+    //
+    if (!create (error, warn, verb_ ? &trace : nullptr,
+                 *build_db_, retry_,
+                 tenant_service (sid, "ci-github", sd.json ()),
+                 chrono::seconds (30) /* interval */,
+                 chrono::seconds (0) /* delay */,
+                 duplicate_tenant_mode::ignore))
     {
-      // It's possible that there was no CI for the previous commit for
-      // various reasons (e.g., CI was not enabled).
-      //
-      l3 ([&]{trace << "forced push to " << ps.ref
-                    << ": failed to cancel CI of previous head commit"
-                    << " with tenant_service id " << sid;});
+      fail << sub << ": unable to create unloaded CI tenant";
     }
 
     return true;
@@ -1651,7 +1719,8 @@ namespace brep
 
       // Create an unloaded CI tenant, doing nothing if one already exists
       // (which could've been created by a head branch push or another PR
-      // sharing the same head commit).
+      // sharing the same head commit). Note that the tenant's reference count
+      // is incremented in all cases.
       //
       // Note: use no delay since we need to (re)create the synthetic
       // conclusion check run as soon as possible.