fix(auto_cancel): support canceling pull_request:opened and abstract determination logic

api/build/auto_cancel.go

@@ -27,13 +27,8 @@ func AutoCancel(c *gin.Context, b *library.Build, rB *library.Build, r *library.
 		return false, nil
 	}
 
-	// ensure criteria is met before auto canceling (push to same branch, or pull with same action from same head_ref)
-	if (strings.EqualFold(rB.GetEvent(), constants.EventPush) &&
-		strings.EqualFold(b.GetEvent(), constants.EventPush) &&
-		strings.EqualFold(b.GetBranch(), rB.GetBranch())) ||
-		(strings.EqualFold(rB.GetEvent(), constants.EventPull) &&
-			strings.EqualFold(b.GetEventAction(), rB.GetEventAction()) &&
-			strings.EqualFold(b.GetHeadRef(), rB.GetHeadRef())) {
+	// ensure criteria is met
+	if isCancelable(rB, b) {
 		switch {
 		case strings.EqualFold(rB.GetStatus(), constants.StatusPending) && cancelOpts.Pending:
 			// pending build will be handled gracefully by worker once pulled off queue
@@ -185,3 +180,43 @@ func cancelRunning(c *gin.Context, b *library.Build, r *library.Repo) error {
 
 	return nil
 }
+
+// isCancelable is a helper function that determines whether a `target` build should be auto-canceled
+// given a current build that intends to supersede it.
+func isCancelable(target *library.Build, current *library.Build) bool {
+	switch target.GetEvent() {
+	case constants.EventPush:
+		// target is cancelable if current build is also a push event and the branches are the same
+		return strings.EqualFold(current.GetEvent(), constants.EventPush) && strings.EqualFold(current.GetBranch(), target.GetBranch())
+	case constants.EventPull:
+		cancelableAction := strings.EqualFold(target.GetEventAction(), constants.ActionOpened) || strings.EqualFold(target.GetEventAction(), constants.ActionSynchronize)
+
+		// target is cancelable if current build is also a pull event, target is an opened / synchronize action, and the current head ref matches target head ref
+		return strings.EqualFold(current.GetEvent(), constants.EventPull) && cancelableAction && strings.EqualFold(current.GetHeadRef(), target.GetHeadRef())
+	default:
+		return false
+	}
+}
+
+// ShouldAutoCancel is a helper function that determines whether or not a build should be eligible to
+// auto cancel currently running / pending builds.
+func ShouldAutoCancel(opts *pipeline.CancelOptions, b *library.Build, defaultBranch string) bool {
+	// if anything is provided in the auto_cancel metadata, then we start with true
+	runAutoCancel := opts.Running || opts.Pending || opts.DefaultBranch
+
+	switch b.GetEvent() {
+	case constants.EventPush:
+		// pushes to the default branch should only auto cancel if pipeline specifies default_branch: true
+		if !opts.DefaultBranch && strings.EqualFold(b.GetBranch(), defaultBranch) {
+			runAutoCancel = false
+		}
+
+		return runAutoCancel
+
+	case constants.EventPull:
+		// only synchronize actions of the pull_request event are eligible to auto cancel
+		return runAutoCancel && (strings.EqualFold(b.GetEventAction(), constants.ActionSynchronize))
+	default:
+		return false
+	}
+}

api/build/auto_cancel_test.go

@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package build
+
+import (
+	"testing"
+
+	"github.com/go-vela/types/constants"
+	"github.com/go-vela/types/library"
+	"github.com/go-vela/types/pipeline"
+)
+
+func Test_isCancelable(t *testing.T) {
+	// setup types
+	pushEvent := constants.EventPush
+	pullEvent := constants.EventPull
+	tagEvent := constants.EventTag
+
+	branchDev := "dev"
+	branchPatch := "patch-1"
+
+	actionOpened := constants.ActionOpened
+	actionSync := constants.ActionSynchronize
+	actionEdited := constants.ActionEdited
+
+	tests := []struct {
+		name    string
+		target  *library.Build
+		current *library.Build
+		want    bool
+	}{
+		{
+			name: "Wrong Event",
+			target: &library.Build{
+				Event:  &tagEvent,
+				Branch: &branchDev,
+			},
+			current: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchDev,
+			},
+			want: false,
+		},
+		{
+			name: "Cancelable Push",
+			target: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchDev,
+			},
+			current: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchDev,
+			},
+			want: true,
+		},
+		{
+			name: "Push Branch Mismatch",
+			target: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchDev,
+			},
+			current: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchPatch,
+			},
+			want: false,
+		},
+		{
+			name: "Event Mismatch",
+			target: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchDev,
+			},
+			current: &library.Build{
+				Event:   &pullEvent,
+				Branch:  &branchDev,
+				HeadRef: &branchPatch,
+			},
+			want: false,
+		},
+		{
+			name: "Cancelable Pull",
+			target: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				HeadRef:     &branchPatch,
+				EventAction: &actionOpened,
+			},
+			current: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				HeadRef:     &branchPatch,
+				EventAction: &actionSync,
+			},
+			want: true,
+		},
+		{
+			name: "Pull Head Ref Mismatch",
+			target: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				HeadRef:     &branchPatch,
+				EventAction: &actionSync,
+			},
+			current: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				HeadRef:     &branchDev,
+				EventAction: &actionSync,
+			},
+			want: false,
+		},
+		{
+			name: "Pull Ineligible Action",
+			target: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				HeadRef:     &branchPatch,
+				EventAction: &actionEdited,
+			},
+			current: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				HeadRef:     &branchDev,
+				EventAction: &actionSync,
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isCancelable(tt.target, tt.current); got != tt.want {
+				t.Errorf("test %s: isCancelable() = %v, want %v", tt.name, got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_ShouldAutoCancel(t *testing.T) {
+	// setup types
+	pushEvent := constants.EventPush
+	pullEvent := constants.EventPull
+	tagEvent := constants.EventTag
+
+	branchDev := "dev"
+	branchPatch := "patch-1"
+
+	actionOpened := constants.ActionOpened
+	actionSync := constants.ActionSynchronize
+
+	tests := []struct {
+		name   string
+		opts   *pipeline.CancelOptions
+		build  *library.Build
+		branch string
+		want   bool
+	}{
+		{
+			name: "Wrong Event",
+			opts: &pipeline.CancelOptions{
+				Running:       true,
+				Pending:       true,
+				DefaultBranch: true,
+			},
+			build: &library.Build{
+				Event:  &tagEvent,
+				Branch: &branchPatch,
+			},
+			branch: branchDev,
+			want:   false,
+		},
+		{
+			name: "Auto Cancel Disabled",
+			opts: &pipeline.CancelOptions{
+				Running:       false,
+				Pending:       false,
+				DefaultBranch: false,
+			},
+			build: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchPatch,
+			},
+			branch: branchDev,
+			want:   false,
+		},
+		{
+			name: "Eligible Push",
+			opts: &pipeline.CancelOptions{
+				Running:       true,
+				Pending:       true,
+				DefaultBranch: false,
+			},
+			build: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchPatch,
+			},
+			branch: branchDev,
+			want:   true,
+		},
+		{
+			name: "Eligible Push - Default Branch",
+			opts: &pipeline.CancelOptions{
+				Running:       true,
+				Pending:       true,
+				DefaultBranch: true,
+			},
+			build: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchDev,
+			},
+			branch: branchDev,
+			want:   true,
+		},
+		{
+			name: "Push Mismatch - Default Branch",
+			opts: &pipeline.CancelOptions{
+				Running:       true,
+				Pending:       true,
+				DefaultBranch: false,
+			},
+			build: &library.Build{
+				Event:  &pushEvent,
+				Branch: &branchDev,
+			},
+			branch: branchDev,
+			want:   false,
+		},
+		{
+			name: "Eligible Pull",
+			opts: &pipeline.CancelOptions{
+				Running:       true,
+				Pending:       true,
+				DefaultBranch: false,
+			},
+			build: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				EventAction: &actionSync,
+			},
+			branch: branchDev,
+			want:   true,
+		},
+		{
+			name: "Pull Mismatch - Action",
+			opts: &pipeline.CancelOptions{
+				Running:       true,
+				Pending:       true,
+				DefaultBranch: false,
+			},
+			build: &library.Build{
+				Event:       &pullEvent,
+				Branch:      &branchDev,
+				EventAction: &actionOpened,
+			},
+			branch: branchDev,
+			want:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ShouldAutoCancel(tt.opts, tt.build, tt.branch); got != tt.want {
+				t.Errorf("test %s: ShouldAutoCancel() = %v, want %v", tt.name, got, tt.want)
+			}
+		})
+	}
+}

api/webhook/post.go

@@ -177,7 +177,7 @@
 
 	defer func() {
 		// send API call to update the webhook
 		_, err = database.FromContext(c).UpdateHook(ctx, h)
 		if err != nil {
 			logrus.Errorf("unable to update webhook %s/%d: %v", r.GetFullName(), h.GetNumber(), err)
 		}
@@ -682,20 +682,7 @@
 		u,
 	)
 
-	// if anything is provided in the auto_cancel metadata, then we start with true
-	runAutoCancel := p.Metadata.AutoCancel.Running || p.Metadata.AutoCancel.Pending || p.Metadata.AutoCancel.DefaultBranch
-
-	// if the event is a push to the default branch and the AutoCancel.DefaultBranch value is false, bypass auto cancel
-	if strings.EqualFold(b.GetEvent(), constants.EventPush) && strings.EqualFold(b.GetBranch(), repo.GetBranch()) && !p.Metadata.AutoCancel.DefaultBranch {
-		runAutoCancel = false
-	}
-
-	// if event is push or pull_request:synchronize, there is a chance this build could be superceding a stale build
-	//
-	// fetch pending and running builds for this repo in order to validate their merit to continue running.
-	if runAutoCancel &&
-		((strings.EqualFold(b.GetEvent(), constants.EventPull) && strings.EqualFold(b.GetEventAction(), constants.ActionSynchronize)) ||
-			strings.EqualFold(b.GetEvent(), constants.EventPush)) {
+	if build.ShouldAutoCancel(p.Metadata.AutoCancel, b, repo.GetBranch()) {
 		// fetch pending and running builds
 		rBs, err := database.FromContext(c).ListPendingAndRunningBuildsForRepo(c, repo)
 		if err != nil {
@@ -746,7 +733,7 @@
 	case "archived", "unarchived", constants.ActionEdited:
 		logrus.Debugf("repository action %s for %s", h.GetEventAction(), r.GetFullName())
 		// send call to get repository from database
 		dbRepo, err := database.FromContext(c).GetRepoForOrg(ctx, r.GetOrg(), r.GetName())
 		if err != nil {
 			retErr := fmt.Errorf("%s: failed to get repo %s: %w", baseErr, r.GetFullName(), err)
 
@@ -757,7 +744,7 @@
 		}
 
 		// send API call to capture the last hook for the repo
 		lastHook, err := database.FromContext(c).LastHookForRepo(ctx, dbRepo)
 		if err != nil {
 			retErr := fmt.Errorf("unable to get last hook for repo %s: %w", r.GetFullName(), err)