Fix multiple jobs running on PullRequest event

.gitignore

@@ -1,2 +1,3 @@
 dist/
-**/rice-box.go
+**/rice-box.go
+.idea

plugins/github-integration/fixtures/processPullRequestEvent_noRerun.json

@@ -34,7 +34,7 @@
                 "type": "User",
                 "site_admin": false
             },
-            "body": "- [x] /werft with-preview",
+            "body": "- [ ] /werft with-preview",
             "created_at": "2021-07-06T13:03:07Z",
             "updated_at": "2022-06-01T19:37:58Z",
             "closed_at": null,
@@ -354,7 +354,7 @@
         },
         "changes": {
             "body": {
-                "from": "- [x] /werft with-preview"
+                "from": "- [ ] /werft with-preview"
             }
         },
         "repository": {
@@ -481,6 +481,16 @@
         }
     },
     "ListResponse": [
+        {
+            "Metadata": {
+                "Repository": {
+                    "Revision": "we're-skipping-this-one"
+                },
+                "Annotations": [
+                    {"Key": "and-this", "Value": "doesn't-matter"}
+                ]
+            }
+        },
         {
             "Metadata": {
                 "Repository": {

plugins/github-integration/fixtures/processPullRequestEvent_rerun.golden

@@ -1 +1,26 @@
-{"metadata":{"owner":"csweichel","repository":{"host":"github.com","owner":"csweichel","repo":"test-repo","ref":"refs/heads/cannot-init","revision":"eed6dfe4dad7e3519acf52679b9880c5d2a2afbf","defaultBranch":"master"},"trigger":"TRIGGER_MANUAL","annotations":[{"key":"updateGitHubStatus","value":"csweichel/test-repo"},{"key":"with-preview"}]},"spec":{"jobPath":""}}
+{
+  "metadata": {
+    "owner": "csweichel",
+    "repository": {
+      "host": "github.com",
+      "owner": "csweichel",
+      "repo": "test-repo",
+      "ref": "refs/heads/cannot-init",
+      "revision": "eed6dfe4dad7e3519acf52679b9880c5d2a2afbf",
+      "defaultBranch": "master"
+    },
+    "trigger": "TRIGGER_MANUAL",
+    "annotations": [
+      {
+        "key": "updateGitHubStatus",
+        "value": "csweichel/test-repo"
+      },
+      {
+        "key": "with-preview"
+      }
+    ]
+  },
+  "spec": {
+    "jobPath": ""
+  }
+}

plugins/github-integration/fixtures/processPullRequestEvent_rerun.json

@@ -485,10 +485,7 @@
             "Metadata": {
                 "Repository": {
                     "Revision": "eed6dfe4dad7e3519acf52679b9880c5d2a2afbf"
-                },
-                "Annotations": [
-                    {"Key": "with-preview", "Value": ""}   
-                ]
+                }
             }
         }
     ]

plugins/github-integration/main.go

@@ -283,7 +283,7 @@ func (p *githubTriggerPlugin) HandleGithubWebhook(w http.ResponseWriter, r *http
 	case *github.IssueCommentEvent:
 		p.processIssueCommentEvent(r.Context(), event)
 	case *github.PullRequestEvent:
-		p.processPullRequestEvent(r.Context(), event)
+		p.processPullRequestEditedEvent(r.Context(), event)
 	case *github.DeleteEvent:
 		// handled by the push event already
 	default:
@@ -292,8 +292,15 @@ func (p *githubTriggerPlugin) HandleGithubWebhook(w http.ResponseWriter, r *http
 	}
 }
 
-func (p *githubTriggerPlugin) processPullRequestEvent(ctx context.Context, event *github.PullRequestEvent) {
+func (p *githubTriggerPlugin) processPullRequestEditedEvent(ctx context.Context, event *github.PullRequestEvent) {
 	pr := event.PullRequest
+	// Potentially (depending on the webhook configuration) we'll get multiple PR events - when pushing (every push generates a "synchronize" pull_request event if there's a PR open),
+	// adding/removing a label,reviewer, assigning/unassigning, etc. The only time annotations are relevant are when a PRs description is edited (opening a PR is handled by a different flow)
+	// So this is the only one we process at this time, the rest we discard.
+	if event.GetAction() != "edited" {
+		return
+	}
+
 	ref := pr.GetHead().GetRef()
 	if !strings.HasPrefix(ref, "refs/") {
 		// we assume this is a branch
@@ -317,26 +324,27 @@ func (p *githubTriggerPlugin) processPullRequestEvent(ctx context.Context, event
 		return
 	}
 
-	annotations := repo.ParseAnnotations(pr.GetBody())
+	prAnnotations := repo.ParseAnnotations(pr.GetBody())
 
-	var noReRun bool
-	for _, j := range lastJobs.Result {
-		if j.Metadata.Repository.Revision != rev {
+	// We only care about the last job that ran for the same commit as that of the PR Event
+	for _, lastJob := range lastJobs.Result {
+		if lastJob.Metadata.Repository.Revision != rev {
 			continue
 		}
 
-		ja := make(map[string]string, len(j.Metadata.Annotations))
-		for _, a := range j.Metadata.Annotations {
-			ja[a.Key] = a.Value
+		jobAnnotations := make(map[string]string, len(lastJob.Metadata.Annotations))
+		for _, a := range lastJob.Metadata.Annotations {
+			jobAnnotations[a.Key] = a.Value
 		}
 
-		if !mapEQ(annotations, ja) {
-			noReRun = true
-			break
+		// If the annotations didn't change between the last job and the event we're currently processing we have nothing to do
+		if mapEQ(prAnnotations, jobAnnotations) {
+			return
 		}
-	}
-	if noReRun {
-		return
+
+		// If we got here, it means that the annotations changed, so we should continue and launch a job with the new annotations
+		// Also we don't care for the rest of the "previous" jobs, so we discard by breaking out of the loop
+		break
 	}
 
 	var (
@@ -347,7 +355,7 @@ func (p *githubTriggerPlugin) processPullRequestEvent(ctx context.Context, event
 	)
 	if p.userIsAllowedToStartJob(ctx, prDstOwner, prDstRepo, event.GetSender().GetLogin()) {
 		req := p.prepareStartJobRequest(event.Sender, event.Repo.Owner, event.Repo.Owner, event.Repo, event.Repo, ref, rev, v1.JobTrigger_TRIGGER_MANUAL)
-		for k, v := range annotations {
+		for k, v := range prAnnotations {
 			req.Metadata.Annotations = append(req.Metadata.Annotations, &v1.Annotation{
 				Key:   k,
 				Value: v,

plugins/github-integration/main_test.go

@@ -257,7 +257,7 @@ func TestProcessPushEvent(t *testing.T) {
 	}
 }
 
-func TestProcessPullRequestEvent(t *testing.T) {
+func TestProcessPullRequestEditedEvent(t *testing.T) {
 	type Fixture struct {
 		Event        *github.PullRequestEvent
 		ListResponse []*v1.JobStatus
@@ -319,7 +319,7 @@ func TestProcessPullRequestEvent(t *testing.T) {
 			if fixture.Event == nil {
 				t.Fatal("broken fixture: no event")
 			}
-			plg.processPullRequestEvent(context.Background(), fixture.Event)
+			plg.processPullRequestEditedEvent(context.Background(), fixture.Event)
 
 			var expectation *v1.StartJobRequest2
 			goldenFN := strings.TrimSuffix(fn, filepath.Ext(fn)) + ".golden"
@@ -349,7 +349,7 @@ func TestProcessPullRequestEvent(t *testing.T) {
 			}
 
 			if diff := cmp.Diff(expectation, startReq); diff != "" {
-				t.Errorf("processPullRequestEvent() mismatch (-want +got):\n%s", diff)
+				t.Errorf("processPullRequestEditedEvent() mismatch (-want +got):\n%s", diff)
 			}
 		})
 	}