fix: issue in on-path-change on pr merge in bitbucket data center

fixed an issue where push pipeline run was not triggering on
pr merge when pipeline run definition is having on-path-cahnge
annotation. this was happening due to bitbucket data center
api behavior that it creates merge commit on pr merge that
is set as HEAD commit of the branch and when changes for that
commit is retrieved, they're being received empty because
merge commit in bitbucket data center doesn't contain code
changes of its parent.

https://issues.redhat.com/browse/SRVKP-7432

Signed-off-by: Zaki Shaikh <[email protected]>

Author: zakisk

pkg/provider/bitbucketdatacenter/parse_payload.go

@@ -88,7 +88,7 @@ func sanitizeOwner(owner string) string {
 }
 
 // ParsePayload parses the payload from the event.
-func (v *Provider) ParsePayload(_ context.Context, _ *params.Run, request *http.Request,
+func (v *Provider) ParsePayload(_ context.Context, run *params.Run, request *http.Request,
 	payload string,
 ) (*info.Event, error) {
 	processedEvent := info.NewEvent()
@@ -169,6 +169,27 @@ func (v *Provider) ParsePayload(_ context.Context, _ *params.Run, request *http.
 		}
 
 		processedEvent.SHA = e.Changes[0].ToHash
+
+		// In Bitbucket Data Center, when a pull request is merged, it creates two commits in the repository:
+		// 1. A merge commit, which is represented by `changes[0].ToHash`.
+		// 2. The actual commit containing the changes from the source branch.
+		//
+		// However, the merge commit often does not contain any file changes itself,
+		// which can cause issues when determining whether file modifications should trigger PipelineRuns.
+		//
+		// Typically, a regular (non-merge) commit has a single parent, but a merge commit has two parents:
+		// - The first parent is the previous HEAD of the destination branch (the branch into which the PR was merged).
+		// - The second parent is the HEAD of the source branch (the branch being merged).
+		//
+		// To correctly identify the actual commit that contains the changes (i.e., the source branch's HEAD),
+		// we inspect `e.Commits[0]`, and if it has more than one parent, we take the second parent.
+		// This helps ensure we reference the correct commit.
+		if len(e.Commits) > 1 && len(e.Commits[0].Parents) > 1 {
+			processedEvent.SHA = e.Commits[0].Parents[1].ID
+			run.Clients.Log.Infof("Detected a merge commit as HEAD; "+
+				"using second parent commit SHA %s (source branch HEAD) to target push event", e.Commits[0].Parents[1].ID)
+		}
+
 		processedEvent.URL = e.Repository.Links.Self[0].Href
 		processedEvent.BaseBranch = e.Changes[0].RefID
 		processedEvent.HeadBranch = e.Changes[0].RefID

pkg/provider/bitbucketdatacenter/parse_payload_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	bbv1test "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/test"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/types"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/test/logger"
 	"gotest.tools/v3/assert"
 	rtesting "knative.dev/pkg/reconciler/testing"
 )
@@ -570,6 +571,7 @@ func TestParsePayload(t *testing.T) {
 		payloadEvent            any
 		expEvent                *info.Event
 		eventType               string
+		wantSHA                 string
 		wantErrSubstr           string
 		rawStr                  string
 		targetPipelinerun       string
@@ -607,12 +609,14 @@ func TestParsePayload(t *testing.T) {
 			eventType:    "pr:opened",
 			payloadEvent: bbv1test.MakePREvent(ev1, ""),
 			expEvent:     ev1,
+			wantSHA:      "abcd",
 		},
 		{
 			name:         "good/push",
 			eventType:    "repo:refs_changed",
 			payloadEvent: bbv1test.MakePushEvent(ev1, []types.PushRequestEventChange{{ToHash: ev1.SHA, RefID: "base"}}, []types.Commit{{ID: ev1.SHA}}),
 			expEvent:     ev1,
+			wantSHA:      "abcd",
 		},
 		{
 			name:          "bad/changes are empty in push",
@@ -628,37 +632,82 @@ func TestParsePayload(t *testing.T) {
 			expEvent:      ev1,
 			wantErrSubstr: "push event contains no commits; cannot proceed",
 		},
+		{
+			name:      "good/changes are empty in push",
+			eventType: "repo:refs_changed",
+			payloadEvent: bbv1test.MakePushEvent(ev1, []types.PushRequestEventChange{
+				{
+					Ref:    types.Ref{ID: "refs/heads/main"},
+					ToHash: "abcd",
+				},
+			}, []types.Commit{
+				{
+					Parents: []struct {
+						ID        string `json:"id"`
+						DisplayID string `json:"displayId"`
+					}{
+						{
+							ID:        "efghabcd",
+							DisplayID: "efgh",
+						},
+						{
+							ID:        "abcdefgh",
+							DisplayID: "abcd",
+						},
+					},
+				},
+				{
+					ID: "abcdefgh",
+					Parents: []struct {
+						ID        string `json:"id"`
+						DisplayID string `json:"displayId"`
+					}{
+						{
+							ID:        "weroiusf",
+							DisplayID: "wero",
+						},
+					},
+				},
+			}),
+			expEvent: ev1,
+			wantSHA:  "abcdefgh",
+		},
 		{
 			name:         "good/comment ok-to-test",
 			eventType:    "pr:comment:added",
 			payloadEvent: bbv1test.MakePREvent(ev1, "/ok-to-test"),
 			expEvent:     ev1,
+			wantSHA:      "abcd",
 		},
 		{
 			name:         "good/comment test",
 			eventType:    "pr:comment:added",
 			payloadEvent: bbv1test.MakePREvent(ev1, "/test"),
 			expEvent:     ev1,
+			wantSHA:      "abcd",
 		},
 		{
 			name:              "good/comment retest a pr",
 			eventType:         "pr:comment:added",
 			payloadEvent:      bbv1test.MakePREvent(ev1, "/retest dummy"),
 			expEvent:          ev1,
 			targetPipelinerun: "dummy",
+			wantSHA:           "abcd",
 		},
 		{
 			name:                    "good/comment cancel a pr",
 			eventType:               "pr:comment:added",
 			payloadEvent:            bbv1test.MakePREvent(ev1, "/cancel dummy"),
 			expEvent:                ev1,
 			canceltargetPipelinerun: "dummy",
+			wantSHA:                 "abcd",
 		},
 		{
 			name:         "good/comment cancel all",
 			eventType:    "pr:comment:added",
 			payloadEvent: bbv1test.MakePREvent(ev1, "/cancel"),
 			expEvent:     ev1,
+			wantSHA:      "abcd",
 		},
 	}
 	for _, tt := range tests {
@@ -672,6 +721,7 @@ func TestParsePayload(t *testing.T) {
 			run := &params.Run{
 				Info: info.Info{},
 			}
+			run.Clients.Log, _ = logger.GetLogger()
 			_b, err := json.Marshal(tt.payloadEvent)
 			assert.NilError(t, err)
 			payload := string(_b)
@@ -686,6 +736,9 @@ func TestParsePayload(t *testing.T) {
 			}
 			assert.NilError(t, err)
 
+			// assert SHA ID
+			assert.Equal(t, tt.wantSHA, got.SHA)
+
 			assert.Equal(t, got.AccountID, tt.expEvent.AccountID)
 
 			// test that we got slashed

test/bitbucket_datacenter_pull_request_test.go

@@ -9,12 +9,17 @@ import (
 	"os"
 	"testing"
 
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
 	tbbdc "github.com/openshift-pipelines/pipelines-as-code/test/pkg/bitbucketdatacenter"
+	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
+	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
 	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
 
+	"github.com/jenkins-x/go-scm/scm"
 	"github.com/tektoncd/pipeline/pkg/names"
 	"gotest.tools/v3/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestBitbucketDataCenterPullRequest(t *testing.T) {
@@ -35,9 +40,11 @@ func TestBitbucketDataCenterPullRequest(t *testing.T) {
 		files[fmt.Sprintf(".tekton/pipelinerun-%d.yaml", i)] = "testdata/pipelinerun.yaml"
 	}
 
+	files, err = payload.GetEntries(files, targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{})
+	assert.NilError(t, err)
+
 	pr := tbbdc.CreatePR(ctx, t, client, runcnx, opts, repo, files, bitbucketWSOwner, targetNS)
-	runcnx.Clients.Log.Infof("Pull Request with title '%s' is created", pr.Title)
-	defer tbbdc.TearDown(ctx, t, runcnx, client, pr.Number, bitbucketWSOwner, targetNS)
+	defer tbbdc.TearDown(ctx, t, runcnx, client, pr, bitbucketWSOwner, targetNS)
 
 	successOpts := wait.SuccessOpt{
 		TargetNS:        targetNS,
@@ -64,9 +71,11 @@ func TestBitbucketDataCenterCELPathChangeInPullRequest(t *testing.T) {
 		".tekton/pipelinerun.yaml": "testdata/pipelinerun-cel-path-changed.yaml",
 	}
 
+	files, err = payload.GetEntries(files, targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{})
+	assert.NilError(t, err)
+
 	pr := tbbdc.CreatePR(ctx, t, client, runcnx, opts, repo, files, bitbucketWSOwner, targetNS)
-	runcnx.Clients.Log.Infof("Pull Request with title '%s' is created", pr.Title)
-	defer tbbdc.TearDown(ctx, t, runcnx, client, pr.Number, bitbucketWSOwner, targetNS)
+	defer tbbdc.TearDown(ctx, t, runcnx, client, pr, bitbucketWSOwner, targetNS)
 
 	successOpts := wait.SuccessOpt{
 		TargetNS:        targetNS,
@@ -76,3 +85,64 @@ func TestBitbucketDataCenterCELPathChangeInPullRequest(t *testing.T) {
 	}
 	wait.Succeeded(ctx, t, runcnx, opts, successOpts)
 }
+
+func TestBitbucketDataCenterOnPathChangeAnnotationOnPRMerge(t *testing.T) {
+	targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
+	// this would be a temporary base branch for the pull request we're going to raise
+	// we need this because we're going to merge the pull request so that after test
+	// we can delete the temporary base branch and our main branch should not be affected
+	// by this merge because we run the E2E frequently.
+	tempBaseBranch := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
+
+	ctx := context.Background()
+	bitbucketWSOwner := os.Getenv("TEST_BITBUCKET_SERVER_E2E_REPOSITORY")
+
+	ctx, runcnx, opts, client, err := tbbdc.Setup(ctx)
+	assert.NilError(t, err)
+
+	repo := tbbdc.CreateCRD(ctx, t, client, runcnx, bitbucketWSOwner, targetNS)
+	runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name)
+	defer tbbdc.TearDownNs(ctx, t, runcnx, targetNS)
+
+	branch, resp, err := client.Git.CreateRef(ctx, bitbucketWSOwner, tempBaseBranch, repo.Branch)
+	assert.NilError(t, err, "error creating branch: http status code: %d : %v", resp.Status, err)
+	runcnx.Clients.Log.Infof("Base branch %s has been created", branch.Name)
+
+	opts.BaseBranch = branch.Name
+
+	if os.Getenv("TEST_NOCLEANUP") != "true" {
+		defer func() {
+			_, err := client.Git.DeleteRef(ctx, bitbucketWSOwner, tempBaseBranch)
+			assert.NilError(t, err, "error deleting branch: http status code: %d : %v", resp.Status, err)
+		}()
+	}
+
+	files := map[string]string{
+		".tekton/pr.yaml":       "testdata/pipelinerun-on-path-change.yaml",
+		"doc/foo/bar/README.md": "README.md",
+	}
+
+	files, err = payload.GetEntries(files, targetNS, tempBaseBranch, triggertype.Push.String(), map[string]string{})
+	assert.NilError(t, err)
+
+	pr := tbbdc.CreatePR(ctx, t, client, runcnx, opts, repo, files, bitbucketWSOwner, targetNS)
+	defer tbbdc.TearDown(ctx, t, runcnx, client, nil, bitbucketWSOwner, targetNS)
+
+	// merge the pull request so that we can get push event.
+	_, err = client.PullRequests.Merge(ctx, bitbucketWSOwner, pr.Number, &scm.PullRequestMergeOptions{})
+	assert.NilError(t, err)
+
+	successOpts := wait.SuccessOpt{
+		TargetNS:        targetNS,
+		OnEvent:         triggertype.Push.String(),
+		NumberofPRMatch: 1,
+		MinNumberStatus: 1,
+	}
+	wait.Succeeded(ctx, t, runcnx, opts, successOpts)
+
+	pipelineRuns, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{})
+	assert.NilError(t, err)
+	assert.Equal(t, len(pipelineRuns.Items), 1)
+	// check that pipeline run contains on-path-change annotation.
+	assert.Equal(t, pipelineRuns.Items[0].GetAnnotations()[keys.OnPathChange], "[doc/***.md]")
+}

test/bitbucket_datacenter_push_test.go

@@ -39,7 +39,7 @@ func TestBitbucketDataCenterCELPathChangeOnPush(t *testing.T) {
 	branch, resp, err := client.Git.CreateRef(ctx, bitbucketWSOwner, targetNS, mainBranchRef)
 	assert.NilError(t, err, "error creating branch: http status code: %d : %v", resp.Status, err)
 	runcnx.Clients.Log.Infof("Branch %s has been created", branch.Name)
-	defer tbbs.TearDown(ctx, t, runcnx, client, -1, bitbucketWSOwner, branch.Name)
+	defer tbbs.TearDown(ctx, t, runcnx, client, nil, bitbucketWSOwner, branch.Name)
 
 	files, err = payload.GetEntries(files, targetNS, branch.Name, triggertype.Push.String(), map[string]string{})
 	assert.NilError(t, err)

test/pkg/bitbucketdatacenter/pr.go

@@ -6,31 +6,32 @@ import (
 	"testing"
 
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
-	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
 	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
-	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
 	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/scm"
 
 	goscm "github.com/jenkins-x/go-scm/scm"
 	"gotest.tools/v3/assert"
 )
 
 func CreatePR(ctx context.Context, t *testing.T, client *goscm.Client, runcnx *params.Run, opts options.E2E, repo *goscm.Repository, files map[string]string, orgAndRepo, targetNS string) *goscm.PullRequest {
-	mainBranchRef := "refs/heads/main"
-	branch, resp, err := client.Git.CreateRef(ctx, orgAndRepo, targetNS, mainBranchRef)
+	baseBranchRef := repo.Branch
+	if opts.BaseBranch != "" {
+		baseBranchRef = opts.BaseBranch
+	}
+
+	branch, resp, err := client.Git.CreateRef(ctx, orgAndRepo, targetNS, baseBranchRef)
 	assert.NilError(t, err, "error creating branch: http status code: %d : %v", resp.Status, err)
 	runcnx.Clients.Log.Infof("Branch %s has been created", branch.Name)
 
-	files, err = payload.GetEntries(files, targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{})
-	assert.NilError(t, err)
 	gitCloneURL, err := scm.MakeGitCloneURL(repo.Clone, opts.UserName, opts.Password)
 	assert.NilError(t, err)
+
 	scmOpts := &scm.Opts{
 		GitURL:        gitCloneURL,
 		Log:           runcnx.Clients.Log,
 		WebURL:        repo.Clone,
 		TargetRefName: targetNS,
-		BaseRefName:   repo.Branch,
+		BaseRefName:   baseBranchRef,
 		CommitTitle:   fmt.Sprintf("commit %s", targetNS),
 	}
 	scm.PushFilesToRefGit(t, scmOpts, files)
@@ -41,9 +42,10 @@ func CreatePR(ctx context.Context, t *testing.T, client *goscm.Client, runcnx *p
 		Title: title,
 		Body:  "Test PAC on bitbucket data center",
 		Head:  targetNS,
-		Base:  "main",
+		Base:  baseBranchRef,
 	}
 	pr, resp, err := client.PullRequests.Create(ctx, orgAndRepo, prOpts)
 	assert.NilError(t, err, "error creating pull request: http status code: %d : %v", resp.Status, err)
+	runcnx.Clients.Log.Infof("Created Pull Request with Title '%s'. Head branch '%s' ⮕ Base Branch '%s'", pr.Title, pr.Head.Ref, pr.Base.Ref)
 	return pr
 }

test/pkg/bitbucketdatacenter/setup.go

@@ -82,15 +82,16 @@ func TearDownNs(ctx context.Context, t *testing.T, runcnx *params.Run, targetNS
 	repository.NSTearDown(ctx, t, runcnx, targetNS)
 }
 
-func TearDown(ctx context.Context, t *testing.T, runcnx *params.Run, client *scm.Client, prID int, orgAndRepo, ref string) {
+func TearDown(ctx context.Context, t *testing.T, runcnx *params.Run, client *scm.Client, pr *scm.PullRequest, orgAndRepo, ref string) {
 	if os.Getenv("TEST_NOCLEANUP") == "true" {
 		runcnx.Clients.Log.Infof("Not cleaning up and closing PR since TEST_NOCLEANUP is set")
 		return
 	}
 
-	if prID != -1 {
-		runcnx.Clients.Log.Infof("Deleting PR #%d", prID)
-		_, err := client.PullRequests.DeletePullRequest(ctx, orgAndRepo, prID)
+	// in Bitbucket Data Center, merged pull requests cannot be deleted.
+	if pr != nil && !pr.Merged {
+		runcnx.Clients.Log.Infof("Deleting PR #%d", pr.Number)
+		_, err := client.PullRequests.DeletePullRequest(ctx, orgAndRepo, pr.Number)
 		assert.NilError(t, err)
 	}
 

test/pkg/options/options.go

@@ -4,6 +4,7 @@ import "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascod
 
 type E2E struct {
 	Repo, Organization string
+	BaseBranch         string
 	DirectWebhook      bool
 	ProjectID          int
 	ControllerURL      string