diff --git a/go.mod b/go.mod index 8d87e848ae..d09a3d2a79 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/google/cel-go v0.28.0 github.com/google/go-cmp v0.7.0 github.com/google/go-github/scrape v0.0.0-20260403152401-96a365122246 + github.com/google/go-github/v84 v84.0.0 github.com/google/go-github/v85 v85.0.0 github.com/hako/durafmt v0.0.0-20210608085754-5c1018a4e16b github.com/jenkins-x/go-scm v1.15.17 @@ -78,7 +79,6 @@ require ( github.com/go-openapi/swag/typeutils v0.25.5 // indirect github.com/go-openapi/swag/yamlutils v0.25.5 // indirect github.com/go-viper/mapstructure/v2 v2.5.0 // indirect - github.com/google/go-github/v84 v84.0.0 // indirect github.com/oklog/ulid/v2 v2.1.1 // indirect github.com/prometheus/otlptranslator v1.0.0 // indirect github.com/rickb777/plural v1.4.10 // indirect diff --git a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go index 97a39460b8..0637d611e5 100644 --- a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go +++ b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go @@ -41,6 +41,7 @@ type Provider struct { apiURL string provenance string projectKey string + previousHeadCommit string repo *v1alpha1.Repository triggerEvent string cachedChangedFiles *changedfiles.ChangedFiles @@ -422,7 +423,13 @@ func (v *Provider) fetchChangedFiles(ctx context.Context, runevent *info.Event) case triggertype.Push: opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit} for { - changes, _, err := v.Client().Git.ListChanges(ctx, orgAndRepo, runevent.SHA, opts) + var changes []*scm.Change + var err error + if v.previousHeadCommit != "" { + changes, _, err = v.getMergeCommitChanges(ctx, runevent.Organization, runevent.Repository, v.previousHeadCommit, runevent.SHA, opts) + } else { + changes, _, err = v.Client().Git.ListChanges(ctx, orgAndRepo, runevent.SHA, opts) + } if err != nil { return changedfiles.ChangedFiles{}, fmt.Errorf("failed to list changes for commit %s: %w", runevent.SHA, err) } diff --git a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go index a671bd9e16..20ec221e98 100644 --- a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go +++ b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go @@ -21,6 +21,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" bbtest "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/test" + bbtypes "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/types" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status" "go.opentelemetry.io/otel" @@ -707,40 +708,62 @@ func TestGetFiles(t *testing.T) { PullRequestNumber: 1, } - pushFiles := []*bbtest.DiffStat{ + pushFiles := []*bbtypes.DiffStat{ { - Path: bbtest.DiffPath{ToString: "added.md"}, + Path: bbtypes.DiffPath{ToString: "added.md"}, Type: "ADD", }, { - Path: bbtest.DiffPath{ToString: "modified.txt"}, + Path: bbtypes.DiffPath{ToString: "modified.txt"}, Type: "MODIFY", }, { - Path: bbtest.DiffPath{ToString: "renamed.yaml"}, + Path: bbtypes.DiffPath{ToString: "renamed.yaml"}, Type: "MOVE", }, { - Path: bbtest.DiffPath{ToString: "deleted.go"}, + Path: bbtypes.DiffPath{ToString: "deleted.go"}, Type: "DELETE", }, } - pullRequestFiles := []*bbtest.DiffStat{ + pullRequestFiles := []*bbtypes.DiffStat{ { - Path: bbtest.DiffPath{ToString: "added.go"}, + Path: bbtypes.DiffPath{ToString: "added.go"}, Type: "ADD", }, { - Path: bbtest.DiffPath{ToString: "modified.yaml"}, + Path: bbtypes.DiffPath{ToString: "modified.yaml"}, Type: "MODIFY", }, { - Path: bbtest.DiffPath{ToString: "renamed.txt"}, + Path: bbtypes.DiffPath{ToString: "renamed.txt"}, Type: "MOVE", }, { - Path: bbtest.DiffPath{ToString: "deleted.md"}, + Path: bbtypes.DiffPath{ToString: "deleted.md"}, + Type: "DELETE", + }, + } + + mergeCommitPushEvent := &info.Event{ + SHA: "MERGESHA456", + Organization: "pac", + Repository: "test", + TriggerTarget: triggertype.Push, + } + + mergeCommitFiles := []*bbtypes.DiffStat{ + { + Path: bbtypes.DiffPath{ToString: "merge-added.go"}, + Type: "ADD", + }, + { + Path: bbtypes.DiffPath{ToString: "merge-modified.txt"}, + Type: "MODIFY", + }, + { + Path: bbtypes.DiffPath{ToString: "merge-deleted.md"}, Type: "DELETE", }, } @@ -748,7 +771,8 @@ func TestGetFiles(t *testing.T) { tests := []struct { name string event *info.Event - changeFiles []*bbtest.DiffStat + changeFiles []*bbtypes.DiffStat + previousHeadCommit string wantAddedFilesCount int wantDeletedFilesCount int wantModifiedFilesCount int @@ -794,6 +818,23 @@ func TestGetFiles(t *testing.T) { wantError: true, errMsg: "failed to list changes for pull request: No message available", }, + { + name: "good/merge commit push event", + event: mergeCommitPushEvent, + changeFiles: mergeCommitFiles, + previousHeadCommit: "PREVIOUSHEAD789", + wantAddedFilesCount: 1, + wantDeletedFilesCount: 1, + wantModifiedFilesCount: 1, + wantRenamedFilesCount: 0, + }, + { + name: "bad/merge commit push event api error", + event: mergeCommitPushEvent, + previousHeadCommit: "PREVIOUSHEAD789", + wantError: true, + errMsg: "failed to list changes for commit MERGESHA456: failed to get merge commit changes: status code: 401", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -801,11 +842,25 @@ func TestGetFiles(t *testing.T) { client, mux, tearDown, tURL := bbtest.SetupBBDataCenterClient() defer tearDown() - stats := &bbtest.DiffStats{ + stats := &bbtypes.DiffStats{ + Pagination: bbtypes.Pagination{ + LastPage: true, + }, Values: tt.changeFiles, } - if tt.event.TriggerTarget == triggertype.Push { + if tt.event.TriggerTarget == triggertype.Push && tt.previousHeadCommit != "" { + mux.HandleFunc("/projects/pac/repos/test/changes", func(w http.ResponseWriter, r *http.Request) { + if tt.wantError { + w.WriteHeader(http.StatusUnauthorized) + return + } + assert.Equal(t, r.URL.Query().Get("since"), tt.previousHeadCommit) + assert.Equal(t, r.URL.Query().Get("until"), tt.event.SHA) + b, _ := json.Marshal(stats) + fmt.Fprint(w, string(b)) + }) + } else if tt.event.TriggerTarget == triggertype.Push { mux.HandleFunc("/projects/pac/repos/test/commits/IAMSHA123/changes", func(w http.ResponseWriter, _ *http.Request) { if tt.wantError { w.WriteHeader(http.StatusUnauthorized) @@ -831,7 +886,7 @@ func TestGetFiles(t *testing.T) { provider := sdkmetric.NewMeterProvider(sdkmetric.WithReader(reader)) otel.SetMeterProvider(provider) - v := &Provider{client: client, baseURL: tURL, triggerEvent: string(tt.event.TriggerTarget)} + v := &Provider{client: client, baseURL: tURL, triggerEvent: string(tt.event.TriggerTarget), previousHeadCommit: tt.previousHeadCommit} changedFiles, err := v.GetFiles(ctx, tt.event) if tt.wantError { assert.Equal(t, err.Error(), tt.errMsg) @@ -855,33 +910,37 @@ func TestGetFiles(t *testing.T) { } } - var rm metricdata.ResourceMetrics - err = reader.Collect(ctx, &rm) - assert.NilError(t, err, "error collecting metrics") - - assert.Equal(t, len(rm.ScopeMetrics), 1) - assert.Equal(t, len(rm.ScopeMetrics[0].Metrics), 1) - assert.Equal(t, rm.ScopeMetrics[0].Metrics[0].Name, "pipelines_as_code_git_provider_api_request_count") - count, ok := rm.ScopeMetrics[0].Metrics[0].Data.(metricdata.Sum[int64]) - assert.Assert(t, ok) - assert.Equal(t, count.DataPoints[0].Value, int64(1)) - - _, _ = v.GetFiles(ctx, tt.event) - // recollect the metrics af - err = reader.Collect(ctx, &rm) - assert.NilError(t, err, "error collecting metrics") - - assert.Equal(t, len(rm.ScopeMetrics), 1) - assert.Equal(t, len(rm.ScopeMetrics[0].Metrics), 1) - assert.Equal(t, rm.ScopeMetrics[0].Metrics[0].Name, "pipelines_as_code_git_provider_api_request_count") - count, ok = rm.ScopeMetrics[0].Metrics[0].Data.(metricdata.Sum[int64]) - assert.Assert(t, ok) - if tt.wantError { - // no caching on error so we expect 2 metrics - assert.Equal(t, count.DataPoints[0].Value, int64(2)) - } else { - // caching on success so we expect 1 metric + // getMergeCommitChanges uses v.client.Do directly (not v.Client()), + // so no API usage metrics are recorded for that path. + if tt.previousHeadCommit == "" { + var rm metricdata.ResourceMetrics + err = reader.Collect(ctx, &rm) + assert.NilError(t, err, "error collecting metrics") + + assert.Equal(t, len(rm.ScopeMetrics), 1) + assert.Equal(t, len(rm.ScopeMetrics[0].Metrics), 1) + assert.Equal(t, rm.ScopeMetrics[0].Metrics[0].Name, "pipelines_as_code_git_provider_api_request_count") + count, ok := rm.ScopeMetrics[0].Metrics[0].Data.(metricdata.Sum[int64]) + assert.Assert(t, ok) assert.Equal(t, count.DataPoints[0].Value, int64(1)) + + _, _ = v.GetFiles(ctx, tt.event) + // recollect the metrics af + err = reader.Collect(ctx, &rm) + assert.NilError(t, err, "error collecting metrics") + + assert.Equal(t, len(rm.ScopeMetrics), 1) + assert.Equal(t, len(rm.ScopeMetrics[0].Metrics), 1) + assert.Equal(t, rm.ScopeMetrics[0].Metrics[0].Name, "pipelines_as_code_git_provider_api_request_count") + count, ok = rm.ScopeMetrics[0].Metrics[0].Data.(metricdata.Sum[int64]) + assert.Assert(t, ok) + if tt.wantError { + // no caching on error so we expect 2 metrics + assert.Equal(t, count.DataPoints[0].Value, int64(2)) + } else { + // caching on success so we expect 1 metric + assert.Equal(t, count.DataPoints[0].Value, int64(1)) + } } }) } diff --git a/pkg/provider/bitbucketdatacenter/diff_api.go b/pkg/provider/bitbucketdatacenter/diff_api.go new file mode 100644 index 0000000000..2da9d05314 --- /dev/null +++ b/pkg/provider/bitbucketdatacenter/diff_api.go @@ -0,0 +1,89 @@ +package bitbucketdatacenter + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/url" + + "github.com/jenkins-x/go-scm/scm" + "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/types" +) + +// getMergeCommitChanges gets the changes between two commits for a merge commit. +// this endpoint is not implemented in the go-scm package, so we need to use the raw HTTP request. +// but we're using the client from the go-scm package to make the request. +func (v *Provider) getMergeCommitChanges(ctx context.Context, org, repo, fromCommit, toCommit string, opts *scm.ListOptions) ([]*scm.Change, *scm.Response, error) { + params := url.Values{} + params.Set("since", fromCommit) + params.Set("until", toCommit) + params.Set("start", fmt.Sprintf("%d", (opts.Page-1)*opts.Size)) + params.Set("limit", fmt.Sprintf("%d", opts.Size)) + path := fmt.Sprintf("rest/api/1.0/projects/%s/repos/%s/changes?%s", + url.PathEscape(org), + url.PathEscape(repo), + params.Encode()) + + resp, err := v.Client().Do(ctx, &scm.Request{ + Method: "GET", + Path: path, + Header: http.Header{ + "Content-Type": {"application/json"}, + "x-atlassian-token": {"no-check"}, + }, + }) + if err != nil { + return nil, nil, fmt.Errorf("failed to get merge commit changes: %w", err) + } + defer resp.Body.Close() + + if resp.Status > 300 { + var errResp struct { + Errors []struct { + Message string `json:"message"` + } `json:"errors"` + } + errMsg := fmt.Sprintf("status code: %d", resp.Status) + if decErr := json.NewDecoder(resp.Body).Decode(&errResp); decErr == nil && len(errResp.Errors) > 0 { + errMsg = fmt.Sprintf("%s, message: %s", errMsg, errResp.Errors[0].Message) + } + return nil, nil, fmt.Errorf("failed to get merge commit changes: %s", errMsg) + } + + out := new(types.DiffStats) + + err = json.NewDecoder(resp.Body).Decode(out) + if err != nil { + return nil, nil, fmt.Errorf("failed to decode merge commit changes: %w", err) + } + + if !out.LastPage { + resp.Page.First = 1 + resp.Page.Next = opts.Page + 1 + } + + return convertDiffstats(out), resp, nil +} + +func convertDiffstats(from *types.DiffStats) []*scm.Change { + to := make([]*scm.Change, 0, len(from.Values)) + for _, v := range from.Values { + to = append(to, convertDiffstat(v)) + } + return to +} + +func convertDiffstat(from *types.DiffStat) *scm.Change { + to := &scm.Change{ + Path: from.Path.ToString, + Added: from.Type == "ADD", + Modified: from.Type == "MODIFY", + Renamed: from.Type == "MOVE", + Deleted: from.Type == "DELETE", + } + if from.SrcPath != nil { + to.PreviousPath = from.SrcPath.ToString + } + return to +} diff --git a/pkg/provider/bitbucketdatacenter/diff_api_test.go b/pkg/provider/bitbucketdatacenter/diff_api_test.go new file mode 100644 index 0000000000..802006bbc9 --- /dev/null +++ b/pkg/provider/bitbucketdatacenter/diff_api_test.go @@ -0,0 +1,195 @@ +package bitbucketdatacenter + +import ( + "encoding/json" + "fmt" + "net/http" + "testing" + + "github.com/jenkins-x/go-scm/scm" + bbtest "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/test" + bbtypes "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/types" + "gotest.tools/v3/assert" + rtesting "knative.dev/pkg/reconciler/testing" +) + +func TestGetMergeCommitChanges(t *testing.T) { + tests := []struct { + name string + fromCommit string + toCommit string + diffStats *bbtypes.DiffStats + wantChangesCount int + wantAdded int + wantModified int + wantRenamed int + wantDeleted int + wantPreviousPath string + wantErr string + apiStatusCode int + setup func(t *testing.T, mux *http.ServeMux) + }{ + { + name: "good/single page of changes", + fromCommit: "abc123", + toCommit: "def456", + diffStats: &bbtypes.DiffStats{ + Pagination: bbtypes.Pagination{ + Start: 0, + Size: 4, + Limit: 100, + LastPage: true, + }, + Values: []*bbtypes.DiffStat{ + {Path: bbtypes.DiffPath{ToString: "added.go"}, Type: "ADD"}, + {Path: bbtypes.DiffPath{ToString: "modified.txt"}, Type: "MODIFY"}, + {Path: bbtypes.DiffPath{ToString: "renamed.yaml"}, Type: "MOVE"}, + {Path: bbtypes.DiffPath{ToString: "deleted.md"}, Type: "DELETE"}, + }, + }, + wantChangesCount: 4, + wantAdded: 1, + wantModified: 1, + wantRenamed: 1, + wantDeleted: 1, + }, + { + name: "good/empty changes", + fromCommit: "abc123", + toCommit: "def456", + diffStats: &bbtypes.DiffStats{ + Pagination: bbtypes.Pagination{ + Start: 0, + Size: 0, + Limit: 100, + LastPage: true, + }, + Values: []*bbtypes.DiffStat{}, + }, + wantChangesCount: 0, + }, + { + name: "good/change with srcPath for rename", + fromCommit: "abc123", + toCommit: "def456", + diffStats: &bbtypes.DiffStats{ + Pagination: bbtypes.Pagination{ + Start: 0, + Size: 1, + Limit: 100, + LastPage: true, + }, + Values: []*bbtypes.DiffStat{ + { + Path: bbtypes.DiffPath{ToString: "new/path.go"}, + SrcPath: &bbtypes.DiffPath{ToString: "old/path.go"}, + Type: "MOVE", + }, + }, + }, + wantChangesCount: 1, + wantRenamed: 1, + wantPreviousPath: "old/path.go", + }, + { + name: "bad/api returns error status", + fromCommit: "abc123", + toCommit: "def456", + apiStatusCode: http.StatusBadRequest, + wantErr: "failed to get merge commit changes: status code: 400", + }, + { + name: "bad/api returns invalid json", + fromCommit: "abc123", + toCommit: "def456", + setup: func(t *testing.T, mux *http.ServeMux) { + t.Helper() + mux.HandleFunc("/projects/owner/repos/repo/changes", func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, "invalid json{{{") + }) + }, + wantErr: "failed to decode merge commit changes", + }, + { + name: "good/pagination sets next page", + fromCommit: "abc123", + toCommit: "def456", + diffStats: &bbtypes.DiffStats{ + Pagination: bbtypes.Pagination{ + Start: 0, + Size: 100, + Limit: 100, + LastPage: false, + NextPage: 100, + }, + Values: []*bbtypes.DiffStat{ + {Path: bbtypes.DiffPath{ToString: "file.go"}, Type: "ADD"}, + }, + }, + wantChangesCount: 1, + wantAdded: 1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + client, mux, tearDown, _ := bbtest.SetupBBDataCenterClient() + defer tearDown() + + if tt.setup != nil { + tt.setup(t, mux) + } else { + mux.HandleFunc("/projects/owner/repos/repo/changes", func(w http.ResponseWriter, r *http.Request) { + if tt.apiStatusCode != 0 { + w.WriteHeader(tt.apiStatusCode) + return + } + assert.Equal(t, r.URL.Query().Get("since"), tt.fromCommit) + assert.Equal(t, r.URL.Query().Get("until"), tt.toCommit) + b, err := json.Marshal(tt.diffStats) + assert.NilError(t, err) + fmt.Fprint(w, string(b)) + }) + } + + v := &Provider{client: client} + opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit} + changes, resp, err := v.getMergeCommitChanges(ctx, "owner", "repo", tt.fromCommit, tt.toCommit, opts) + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + return + } + assert.NilError(t, err) + assert.Equal(t, tt.wantChangesCount, len(changes)) + + added, modified, renamed, deleted := 0, 0, 0, 0 + for _, c := range changes { + if c.Added { + added++ + } + if c.Modified { + modified++ + } + if c.Renamed { + renamed++ + } + if c.Deleted { + deleted++ + } + } + assert.Equal(t, tt.wantAdded, added) + assert.Equal(t, tt.wantModified, modified) + assert.Equal(t, tt.wantRenamed, renamed) + assert.Equal(t, tt.wantDeleted, deleted) + + if tt.wantPreviousPath != "" { + assert.Equal(t, tt.wantPreviousPath, changes[0].PreviousPath) + } + + if tt.diffStats != nil && !tt.diffStats.LastPage { + assert.Equal(t, 1, resp.Page.First) + assert.Equal(t, 2, resp.Page.Next) + } + }) + } +} diff --git a/pkg/provider/bitbucketdatacenter/parse_payload.go b/pkg/provider/bitbucketdatacenter/parse_payload.go index 975cbd74be..46e67fa923 100644 --- a/pkg/provider/bitbucketdatacenter/parse_payload.go +++ b/pkg/provider/bitbucketdatacenter/parse_payload.go @@ -175,6 +175,22 @@ func (v *Provider) ParsePayload(_ context.Context, _ *params.Run, request *http. return nil, fmt.Errorf("push event contains no commits; cannot proceed") } + // 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). + // + // if we detect a merge commit, we will use the previous HEAD commit to get changes from the previous HEAD to the current HEAD. + if len(e.Commits) > 1 && len(e.Commits[0].Parents) > 1 { + v.previousHeadCommit = e.Commits[0].Parents[0].ID + } + processedEvent.SHA = e.Changes[0].ToHash processedEvent.URL = e.Repository.Links.Self[0].Href processedEvent.BaseBranch = e.Changes[0].RefID diff --git a/pkg/provider/bitbucketdatacenter/test/test_types.go b/pkg/provider/bitbucketdatacenter/test/test_types.go index 9f63400501..ef0eab782c 100644 --- a/pkg/provider/bitbucketdatacenter/test/test_types.go +++ b/pkg/provider/bitbucketdatacenter/test/test_types.go @@ -2,29 +2,6 @@ package test import "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketdatacenter/types" -type Pagination struct { - Start int `json:"start"` - Size int `json:"size"` - Limit int `json:"limit"` - LastPage bool `json:"isLastPage"` - NextPage int `json:"nextPageStart"` -} - -type DiffPath struct { - ToString string `json:"toString"` -} - -type DiffStat struct { - Path DiffPath `json:"path"` - SrcPath *DiffPath `json:"srcPath,omitempty"` // used in lib that's why leaving it here nil - Type string `json:"type"` -} - -type DiffStats struct { - Pagination - Values []*DiffStat -} - type ProjGroup struct { Group Group `json:"group"` Permission string `json:"permission"` diff --git a/pkg/provider/bitbucketdatacenter/types/types.go b/pkg/provider/bitbucketdatacenter/types/types.go index 94c524e002..d9031a09db 100644 --- a/pkg/provider/bitbucketdatacenter/types/types.go +++ b/pkg/provider/bitbucketdatacenter/types/types.go @@ -197,3 +197,43 @@ type ToCommit struct { Commit Parents []Commit `json:"parents"` // Commit also has Parents field, but its Parents has only two fields while actual payload has more. } + +type Pagination struct { + Start int `json:"start"` + Size int `json:"size"` + Limit int `json:"limit"` + LastPage bool `json:"isLastPage"` + NextPage int `json:"nextPageStart"` +} + +type DiffStats struct { + Pagination + Values []*DiffStat `json:"values"` +} + +type DiffPath struct { + Components []string `json:"components"` + Parent string `json:"parent"` + Name string `json:"name"` + Extension string `json:"extension"` + ToString string `json:"toString"` +} + +type DiffStat struct { + ContentID string `json:"contentId"` + FromContentID string `json:"fromContentId"` + Path DiffPath `json:"path"` + SrcPath *DiffPath `json:"srcPath,omitempty"` + PercentUnchanged int `json:"percentUnchanged"` + Type string `json:"type"` + NodeType string `json:"nodeType"` + SrcExecutable bool `json:"srcExecutable"` + Links struct { + Self []struct { + Href string `json:"href"` + } `json:"self"` + } `json:"links"` + Properties struct { + GitChangeType string `json:"gitChangeType"` + } `json:"properties"` +} diff --git a/test/bitbucket_datacenter_pull_request_test.go b/test/bitbucket_datacenter_pull_request_test.go index 7c7ed9f015..56a9670267 100644 --- a/test/bitbucket_datacenter_pull_request_test.go +++ b/test/bitbucket_datacenter_pull_request_test.go @@ -8,12 +8,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) { @@ -34,9 +39,12 @@ 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, @@ -63,9 +71,12 @@ 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, @@ -75,3 +86,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_DATA_CENTER_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]") +} diff --git a/test/bitbucket_datacenter_push_test.go b/test/bitbucket_datacenter_push_test.go index 5e77e2219f..999528fc04 100644 --- a/test/bitbucket_datacenter_push_test.go +++ b/test/bitbucket_datacenter_push_test.go @@ -38,7 +38,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) diff --git a/test/pkg/bitbucketdatacenter/pr.go b/test/pkg/bitbucketdatacenter/pr.go index 1a5b1af15f..0f1b2c0221 100644 --- a/test/pkg/bitbucketdatacenter/pr.go +++ b/test/pkg/bitbucketdatacenter/pr.go @@ -6,9 +6,7 @@ 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" @@ -16,21 +14,24 @@ import ( ) 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,7 +42,7 @@ 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) diff --git a/test/pkg/bitbucketdatacenter/setup.go b/test/pkg/bitbucketdatacenter/setup.go index 4acd1b9075..671a7e695f 100644 --- a/test/pkg/bitbucketdatacenter/setup.go +++ b/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) } diff --git a/test/pkg/options/options.go b/test/pkg/options/options.go index 67f565f8f4..64247ff810 100644 --- a/test/pkg/options/options.go +++ b/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 ControllerURL string Concurrency int