diff --git a/pkg/apis/pipelinesascode/keys/keys.go b/pkg/apis/pipelinesascode/keys/keys.go index 69436ecbc6..b13a9eb1c7 100644 --- a/pkg/apis/pipelinesascode/keys/keys.go +++ b/pkg/apis/pipelinesascode/keys/keys.go @@ -48,6 +48,7 @@ const ( OriginalPRName = pipelinesascode.GroupName + "/original-prname" GitAuthSecret = pipelinesascode.GroupName + "/git-auth-secret" CheckRunID = pipelinesascode.GroupName + "/check-run-id" + GitLabPipelineID = pipelinesascode.GroupName + "/gitlab-pipeline-id" OnEvent = pipelinesascode.GroupName + "/on-event" OnComment = pipelinesascode.GroupName + "/on-comment" OnTargetBranch = pipelinesascode.GroupName + "/on-target-branch" diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index 09da6e4593..5b104b1242 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -11,7 +11,10 @@ import ( "regexp" "strconv" "strings" + "sync" + "github.com/openshift-pipelines/pipelines-as-code/pkg/action" + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/changedfiles" "github.com/openshift-pipelines/pipelines-as-code/pkg/events" @@ -69,6 +72,8 @@ type Provider struct { memberCache map[int64]bool cachedChangedFiles *changedfiles.ChangedFiles pacUserID int64 // user login used by PAC + pipelineID int64 + pipelineIDMu sync.Mutex } var defaultGitlabListOptions = gitlab.ListOptions{ @@ -354,6 +359,27 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp Context: gitlab.Ptr(contextName), } + // Reuse a previously discovered pipeline ID so that all commit statuses + // for the same SHA land in the same GitLab pipeline. + if statusOpts.PipelineRun != nil { + if id, ok := statusOpts.PipelineRun.GetAnnotations()[keys.GitLabPipelineID]; ok { + pid, err := strconv.ParseInt(id, 10, 64) + if err == nil { + opt.PipelineID = gitlab.Ptr(pid) + v.pipelineIDMu.Lock() + v.pipelineID = pid + v.pipelineIDMu.Unlock() + } + } + } + if opt.PipelineID == nil { + v.pipelineIDMu.Lock() + if v.pipelineID != 0 { + opt.PipelineID = gitlab.Ptr(v.pipelineID) + } + v.pipelineIDMu.Unlock() + } + // In case we have access, set the status. Typically, on a Merge Request (MR) // from a fork in an upstream repository, the token needs to have write access // to the fork repository in order to create a status. However, the token set on the @@ -361,15 +387,17 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp // a status comment on it. // This would work on a push or an MR from a branch within the same repo. // Ignoring errors because of the write access issues, - _, _, err := v.Client().Commits.SetCommitStatus(event.SourceProjectID, event.SHA, opt) + commitStatus, _, err := v.Client().Commits.SetCommitStatus(event.SourceProjectID, event.SHA, opt) if err != nil { v.Logger.Debugf("cannot set status with the GitLab token on the source project: %v", err) } else { + v.storePipelineID(ctx, statusOpts, commitStatus.PipelineID) // we managed to set the status on the source repo, all good we are done v.Logger.Debugf("created commit status on source project ID %d", event.TargetProjectID) return nil } - if _, _, err = v.Client().Commits.SetCommitStatus(event.TargetProjectID, event.SHA, opt); err == nil { + if commitStatus, _, err = v.Client().Commits.SetCommitStatus(event.TargetProjectID, event.SHA, opt); err == nil { + v.storePipelineID(ctx, statusOpts, commitStatus.PipelineID) v.Logger.Debugf("created commit status on target project ID %d", event.TargetProjectID) // we managed to set the status on the target repo, all good we are done return nil @@ -860,3 +888,40 @@ func (v *Provider) formatPipelineComment(sha string, status providerstatus.Statu return fmt.Sprintf("%s **%s: %s/%s for %s**\n\n%s\n\nFull log available [here](%s)", emoji, status.Title, v.pacInfo.ApplicationName, status.OriginalPipelineRunName, sha, status.Text, status.DetailsURL) } + +// storePipelineID caches the pipeline ID from a successful SetCommitStatus +// response and patches it onto the PipelineRun annotation for the reconciler. +func (v *Provider) storePipelineID(ctx context.Context, statusOpts providerstatus.StatusOpts, pipelineID int64) { + if pipelineID == 0 { + return + } + v.pipelineIDMu.Lock() + v.pipelineID = pipelineID + v.pipelineIDMu.Unlock() + v.patchPipelineIDAnnotation(ctx, statusOpts, pipelineID) +} + +// patchPipelineIDAnnotation stores the GitLab pipeline ID as a PipelineRun +// annotation so the reconciler can read it back across Provider instances. +func (v *Provider) patchPipelineIDAnnotation(ctx context.Context, statusOpts providerstatus.StatusOpts, pipelineID int64) { + pr := statusOpts.PipelineRun + if pr == nil || (pr.GetName() == "" && pr.GetGenerateName() == "") { + return + } + if existing, ok := pr.GetAnnotations()[keys.GitLabPipelineID]; ok { + if existing != strconv.FormatInt(pipelineID, 10) { + v.Logger.Debugf("pipelinerun %s already has gitlab pipeline ID %s, ignoring new ID %d", pr.GetName(), existing, pipelineID) + } + return + } + mergePatch := map[string]any{ + "metadata": map[string]any{ + "annotations": map[string]string{ + keys.GitLabPipelineID: strconv.FormatInt(pipelineID, 10), + }, + }, + } + if _, err := action.PatchPipelineRun(ctx, v.Logger, "gitlabPipelineID", v.run.Clients.Tekton, pr, mergePatch); err != nil { + v.Logger.Debugf("failed to patch pipelinerun with gitlab pipeline ID: %v", err) + } +} diff --git a/pkg/provider/gitlab/gitlab_test.go b/pkg/provider/gitlab/gitlab_test.go index 9ede25241e..920f9e0c89 100644 --- a/pkg/provider/gitlab/gitlab_test.go +++ b/pkg/provider/gitlab/gitlab_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/events" "github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments" @@ -25,6 +26,8 @@ import ( providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status" testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/test/logger" + tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gitlab "gitlab.com/gitlab-org/api/client-go" "go.opentelemetry.io/otel" @@ -36,6 +39,21 @@ import ( rtesting "knative.dev/pkg/reconciler/testing" ) +func setupPipelineIDStatusHandler(t *testing.T, mux *http.ServeMux, path string, responsePID int64) { + t.Helper() + mux.HandleFunc(path, func(rw http.ResponseWriter, r *http.Request) { + t.Helper() + var reqBody map[string]any + if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + _, hasPID := reqBody["pipeline_id"] + assert.Assert(t, !hasPID, "request should not have pipeline_id for %s", path) + rw.WriteHeader(http.StatusCreated) + fmt.Fprintf(rw, `{"id": 1, "pipeline_id": %d}`, responsePID) + }) +} + func TestCreateStatus(t *testing.T) { type fields struct { targetProjectID int @@ -46,11 +64,14 @@ func TestCreateStatus(t *testing.T) { postStr string } tests := []struct { - name string - fields fields - args args - wantErr bool - wantClient bool + name string + fields fields + args args + wantErr bool + wantClient bool + setup func(t *testing.T, mux *http.ServeMux) + wantCachedPID int64 + checkCachedPID bool }{ { name: "no client has been set", @@ -343,6 +364,238 @@ func TestCreateStatus(t *testing.T) { postStr: "has successfully", }, }, + { + name: "pipeline ID first call discovers and caches", + wantClient: true, + checkCachedPID: true, + wantCachedPID: 9999, + fields: fields{targetProjectID: 500}, + args: args{ + statusOpts: providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-1", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + }, + event: &info.Event{ + TriggerTarget: "pull_request", + SourceProjectID: 500, + TargetProjectID: 500, + SHA: "deadbeef", + }, + }, + setup: func(t *testing.T, mux *http.ServeMux) { + t.Helper() + setupPipelineIDStatusHandler(t, mux, "/projects/500/statuses/deadbeef", 9999) + }, + }, + { + name: "pipeline ID read from annotation", + wantClient: true, + fields: fields{targetProjectID: 500}, + args: args{ + statusOpts: providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-1", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr", + Namespace: "default", + Annotations: map[string]string{keys.GitLabPipelineID: "9999"}, + }, + }, + }, + event: &info.Event{ + TriggerTarget: "pull_request", + SourceProjectID: 500, + TargetProjectID: 500, + SHA: "deadbeef01", + }, + }, + setup: func(t *testing.T, mux *http.ServeMux) { + t.Helper() + mux.HandleFunc("/projects/500/statuses/deadbeef01", func(rw http.ResponseWriter, r *http.Request) { + var reqBody map[string]any + if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + pid, ok := reqBody["pipeline_id"] + assert.Assert(t, ok, "request must include pipeline_id") + pidFloat, ok := pid.(float64) + assert.Assert(t, ok, "pipeline_id must be a number") + assert.Equal(t, int64(pidFloat), int64(9999)) + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{"id": 1, "pipeline_id": 9999}`) + }) + }, + }, + { + name: "pipeline ID zero in response does not cache", + wantClient: true, + checkCachedPID: true, + wantCachedPID: 0, + fields: fields{targetProjectID: 501}, + args: args{ + statusOpts: providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-zero-pid", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr-zero", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + }, + event: &info.Event{ + TriggerTarget: "push", + SourceProjectID: 501, + TargetProjectID: 501, + SHA: "deadbeef02", + }, + }, + setup: func(t *testing.T, mux *http.ServeMux) { + t.Helper() + setupPipelineIDStatusHandler(t, mux, "/projects/501/statuses/deadbeef02", 0) + }, + }, + { + name: "pipeline ID target fallback reads annotation", + wantClient: true, + fields: fields{targetProjectID: 600}, + args: args{ + statusOpts: providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-1", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr", + Namespace: "default", + Annotations: map[string]string{keys.GitLabPipelineID: "1234"}, + }, + }, + }, + event: &info.Event{ + TriggerTarget: "pull_request", + SourceProjectID: 404, + TargetProjectID: 600, + SHA: "aabb1122", + }, + }, + setup: func(t *testing.T, mux *http.ServeMux) { + t.Helper() + mux.HandleFunc("/projects/404/statuses/aabb1122", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusNotFound) + fmt.Fprint(rw, `{"message": "404 Not Found"}`) + }) + mux.HandleFunc("/projects/600/statuses/aabb1122", func(rw http.ResponseWriter, r *http.Request) { + var reqBody map[string]any + if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + pid, ok := reqBody["pipeline_id"] + assert.Assert(t, ok, "target request must include pipeline_id") + pidFloat, ok := pid.(float64) + assert.Assert(t, ok, "pipeline_id must be a number") + assert.Equal(t, int64(pidFloat), int64(1234)) + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{"id": 1, "pipeline_id": 1234}`) + }) + }, + }, + { + name: "pipeline ID target fallback without annotation", + wantClient: true, + fields: fields{targetProjectID: 600}, + args: args{ + statusOpts: providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-1", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + }, + event: &info.Event{ + TriggerTarget: "pull_request", + SourceProjectID: 404, + TargetProjectID: 600, + SHA: "aabb1123", + }, + }, + setup: func(t *testing.T, mux *http.ServeMux) { + t.Helper() + mux.HandleFunc("/projects/404/statuses/aabb1123", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusNotFound) + fmt.Fprint(rw, `{"message": "404 Not Found"}`) + }) + mux.HandleFunc("/projects/600/statuses/aabb1123", func(rw http.ResponseWriter, r *http.Request) { + var reqBody map[string]any + if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + _, hasPID := reqBody["pipeline_id"] + assert.Assert(t, !hasPID, "target request should not have pipeline_id") + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{"id": 1, "pipeline_id": 7777}`) + }) + }, + }, + { + name: "pipeline ID same project fallback preserves annotation", + wantClient: true, + fields: fields{targetProjectID: 600}, + args: args{ + statusOpts: providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-1", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr", + Namespace: "default", + Annotations: map[string]string{keys.GitLabPipelineID: "5555"}, + }, + }, + }, + event: &info.Event{ + TriggerTarget: "pull_request", + SourceProjectID: 600, + TargetProjectID: 600, + SHA: "aabb1124", + }, + }, + setup: func(t *testing.T, mux *http.ServeMux) { + t.Helper() + var callCount int + mux.HandleFunc("/projects/600/statuses/aabb1124", func(rw http.ResponseWriter, r *http.Request) { + callCount++ + if callCount == 1 { + rw.WriteHeader(http.StatusForbidden) + fmt.Fprint(rw, `{"message": "403 Forbidden"}`) + return + } + var reqBody map[string]any + if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + pid, ok := reqBody["pipeline_id"] + assert.Assert(t, ok, "target request must include pipeline_id") + pidFloat, ok := pid.(float64) + assert.Assert(t, ok, "pipeline_id must be a number") + assert.Equal(t, int64(pidFloat), int64(5555)) + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{"id": 1, "pipeline_id": 5555}`) + }) + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -351,13 +604,14 @@ func TestCreateStatus(t *testing.T) { stdata, _ := testclient.SeedTestData(t, ctx, testclient.Data{}) run := ¶ms.Run{ Clients: clients.Clients{ - Kube: stdata.Kube, - Log: logger, + Kube: stdata.Kube, + Tekton: stdata.Pipeline, + Log: logger, }, } v := &Provider{ targetProjectID: int64(tt.fields.targetProjectID), - run: params.New(), + run: run, Logger: logger, pacInfo: &info.PacOpts{ Settings: settings.Settings{ @@ -376,59 +630,61 @@ func TestCreateStatus(t *testing.T) { v.SetGitLabClient(client) defer tearDown() - // Mock commit status endpoints for both source and target projects - if tt.args.event.SourceProjectID != 0 { - // Mock source project commit status endpoint - sourceStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.SourceProjectID, tt.args.event.SHA) - mux.HandleFunc(sourceStatusPath, func(rw http.ResponseWriter, _ *http.Request) { - switch tt.args.event.SourceProjectID { - case 400: - rw.WriteHeader(http.StatusBadRequest) - fmt.Fprint(rw, `{"message": "400 Bad Request"}`) - case 401: - rw.WriteHeader(http.StatusUnauthorized) - fmt.Fprint(rw, `{"message": "401 Unauthorized"}`) - case 403: - rw.WriteHeader(http.StatusForbidden) - fmt.Fprint(rw, `{"message": "403 Forbidden"}`) - case 404: - rw.WriteHeader(http.StatusNotFound) - fmt.Fprint(rw, `{"message": "404 Project Not Found"}`) - case 422: - rw.WriteHeader(http.StatusBadRequest) - fmt.Fprint(rw, `{"message": "Cannot transition status via :run from :running"}`) - default: - rw.WriteHeader(http.StatusCreated) - fmt.Fprint(rw, `{}`) - } - }) - } + if tt.setup != nil { + tt.setup(t, mux) + } else { + // Mock commit status endpoints for both source and target projects + if tt.args.event.SourceProjectID != 0 { + sourceStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.SourceProjectID, tt.args.event.SHA) + mux.HandleFunc(sourceStatusPath, func(rw http.ResponseWriter, _ *http.Request) { + switch tt.args.event.SourceProjectID { + case 400: + rw.WriteHeader(http.StatusBadRequest) + fmt.Fprint(rw, `{"message": "400 Bad Request"}`) + case 401: + rw.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(rw, `{"message": "401 Unauthorized"}`) + case 403: + rw.WriteHeader(http.StatusForbidden) + fmt.Fprint(rw, `{"message": "403 Forbidden"}`) + case 404: + rw.WriteHeader(http.StatusNotFound) + fmt.Fprint(rw, `{"message": "404 Project Not Found"}`) + case 422: + rw.WriteHeader(http.StatusBadRequest) + fmt.Fprint(rw, `{"message": "Cannot transition status via :run from :running"}`) + default: + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{}`) + } + }) + } - if tt.args.event.TargetProjectID != 0 { - // Mock target project commit status endpoint - targetStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.TargetProjectID, tt.args.event.SHA) - mux.HandleFunc(targetStatusPath, func(rw http.ResponseWriter, _ *http.Request) { - switch tt.args.event.TargetProjectID { - case 400, 405: - rw.WriteHeader(http.StatusBadRequest) - fmt.Fprint(rw, `{"message": "400 Bad Request"}`) - case 401: - rw.WriteHeader(http.StatusUnauthorized) - fmt.Fprint(rw, `{"message": "401 Unauthorized"}`) - case 403: - rw.WriteHeader(http.StatusForbidden) - fmt.Fprint(rw, `{"message": "403 Forbidden"}`) - case 404: - rw.WriteHeader(http.StatusNotFound) - fmt.Fprint(rw, `{"message": "404 Project Not Found"}`) - case 422: - rw.WriteHeader(http.StatusBadRequest) - fmt.Fprint(rw, `{"message": "Cannot transition status via :run from :running"}`) - default: - rw.WriteHeader(http.StatusCreated) - fmt.Fprint(rw, `{}`) - } - }) + if tt.args.event.TargetProjectID != 0 { + targetStatusPath := fmt.Sprintf("/projects/%d/statuses/%s", tt.args.event.TargetProjectID, tt.args.event.SHA) + mux.HandleFunc(targetStatusPath, func(rw http.ResponseWriter, _ *http.Request) { + switch tt.args.event.TargetProjectID { + case 400, 405: + rw.WriteHeader(http.StatusBadRequest) + fmt.Fprint(rw, `{"message": "400 Bad Request"}`) + case 401: + rw.WriteHeader(http.StatusUnauthorized) + fmt.Fprint(rw, `{"message": "401 Unauthorized"}`) + case 403: + rw.WriteHeader(http.StatusForbidden) + fmt.Fprint(rw, `{"message": "403 Forbidden"}`) + case 404: + rw.WriteHeader(http.StatusNotFound) + fmt.Fprint(rw, `{"message": "404 Project Not Found"}`) + case 422: + rw.WriteHeader(http.StatusBadRequest) + fmt.Fprint(rw, `{"message": "Cannot transition status via :run from :running"}`) + default: + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{}`) + } + }) + } } thelp.MuxNotePost(t, mux, int(v.targetProjectID), tt.args.event.PullRequestNumber, tt.args.postStr) @@ -437,8 +693,105 @@ func TestCreateStatus(t *testing.T) { if err := v.CreateStatus(ctx, tt.args.event, tt.args.statusOpts); (err != nil) != tt.wantErr { t.Errorf("CreateStatus() error = %v, wantErr %v", err, tt.wantErr) } + if tt.checkCachedPID { + assert.Equal(t, v.pipelineID, tt.wantCachedPID) + } + }) + } +} + +func TestCreateStatusPipelineIDSharedAcrossPipelineRuns(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + log, _ := logger.GetLogger() + stdata, _ := testclient.SeedTestData(t, ctx, testclient.Data{}) + run := ¶ms.Run{ + Clients: clients.Clients{ + Kube: stdata.Kube, + Tekton: stdata.Pipeline, + Log: log, + }, + } + + client, mux, tearDown := thelp.Setup(t) + defer tearDown() + + v := &Provider{ + run: run, + Logger: log, + pacInfo: &info.PacOpts{ + Settings: settings.Settings{ + ApplicationName: settings.PACApplicationNameDefaultValue, + }, + }, + eventEmitter: events.NewEventEmitter(run.Clients.Kube, log), + } + v.SetGitLabClient(client) + + sha := "abc123" + projectID := int64(500) + + mux.HandleFunc(fmt.Sprintf("/projects/%d/statuses/%s", projectID, sha), + func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{"id": 1, "pipeline_id": 9999}`) }) + + event := &info.Event{ + TriggerTarget: "pull_request", + SourceProjectID: projectID, + TargetProjectID: projectID, + SHA: sha, + PullRequestNumber: 1, } + + // First PipelineRun — discovers the pipeline ID + err := v.CreateStatus(ctx, event, providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-lint", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr-lint", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + }) + assert.NilError(t, err) + assert.Equal(t, v.pipelineID, int64(9999)) + + // Second PipelineRun — should pick up the pipeline ID from in-memory field + var secondCallPID int64 + secondClient, secondMuxPtr, secondTearDown := thelp.Setup(t) + defer secondTearDown() + v.SetGitLabClient(secondClient) + + secondMuxPtr.HandleFunc(fmt.Sprintf("/projects/%d/statuses/%s", projectID, sha), + func(rw http.ResponseWriter, r *http.Request) { + var reqBody map[string]any + if err := json.NewDecoder(r.Body).Decode(&reqBody); err != nil { + t.Fatalf("failed to decode request body: %v", err) + } + pid, ok := reqBody["pipeline_id"] + assert.Assert(t, ok, "second call must include pipeline_id from cached v.pipelineID") + pidFloat, ok := pid.(float64) + assert.Assert(t, ok, "pipeline_id must be a number") + secondCallPID = int64(pidFloat) + rw.WriteHeader(http.StatusCreated) + fmt.Fprint(rw, `{"id": 2, "pipeline_id": 9999}`) + }) + err = v.CreateStatus(ctx, event, providerstatus.StatusOpts{ + Conclusion: "success", + OriginalPipelineRunName: "pr-test", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pr-test", + Namespace: "default", + Annotations: map[string]string{}, + }, + }, + }) + assert.NilError(t, err) + assert.Equal(t, secondCallPID, int64(9999)) } func TestGetCommitInfo(t *testing.T) { diff --git a/test/gitlab_pipeline_id_test.go b/test/gitlab_pipeline_id_test.go new file mode 100644 index 0000000000..4e396f76b5 --- /dev/null +++ b/test/gitlab_pipeline_id_test.go @@ -0,0 +1,49 @@ +//go:build e2e + +package test + +import ( + "strconv" + "testing" + + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" + tgitlab "github.com/openshift-pipelines/pipelines-as-code/test/pkg/gitlab" + twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" + "gotest.tools/v3/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGitlabPipelineIDAnnotation(t *testing.T) { + topts := &tgitlab.TestOpts{ + TargetEvent: triggertype.PullRequest.String(), + YAMLFiles: map[string]string{ + ".tekton/pipelinerun.yaml": "testdata/pipelinerun.yaml", + }, + } + ctx, cleanup := tgitlab.TestMR(t, topts) + defer cleanup() + + sopt := twait.SuccessOpt{ + Title: "Committing files from test on " + topts.TargetRefName, + OnEvent: "Merge Request", + TargetNS: topts.TargetNS, + NumberofPRMatch: 1, + SHA: topts.SHA, + } + twait.Succeeded(ctx, t, topts.ParamsRun, topts.Opts, sopt) + + prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(ctx, metav1.ListOptions{}) + assert.NilError(t, err) + assert.Assert(t, len(prs.Items) >= 1, "Expected at least one PipelineRun") + + for _, pr := range prs.Items { + pipelineID, ok := pr.Annotations[keys.GitLabPipelineID] + assert.Assert(t, ok, "PipelineRun %s missing %s annotation", pr.Name, keys.GitLabPipelineID) + assert.Assert(t, pipelineID != "", "PipelineRun %s has empty %s annotation", pr.Name, keys.GitLabPipelineID) + pid, err := strconv.ParseInt(pipelineID, 10, 64) + assert.NilError(t, err, "PipelineRun %s has non-numeric %s annotation: %s", pr.Name, keys.GitLabPipelineID, pipelineID) + assert.Assert(t, pid > 0, "PipelineRun %s has invalid %s value: %d", pr.Name, keys.GitLabPipelineID, pid) + t.Logf("PipelineRun %s has %s: %s", pr.Name, keys.GitLabPipelineID, pipelineID) + } +} diff --git a/test/pkg/gitlab/setup.go b/test/pkg/gitlab/setup.go index 8428a93950..110ef3ac22 100644 --- a/test/pkg/gitlab/setup.go +++ b/test/pkg/gitlab/setup.go @@ -14,7 +14,7 @@ import ( gitlab2 "gitlab.com/gitlab-org/api/client-go" ) -func Setup(ctx context.Context) (*params.Run, options.E2E, gitlab.Provider, error) { +func Setup(ctx context.Context) (*params.Run, options.E2E, *gitlab.Provider, error) { if err := setup.RequireEnvs( "TEST_GITLAB_API_URL", "TEST_GITLAB_TOKEN", @@ -23,7 +23,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, gitlab.Provider, erro "TEST_EL_URL", "TEST_GITLAB_SMEEURL", ); err != nil { - return nil, options.E2E{}, gitlab.Provider{}, err + return nil, options.E2E{}, nil, err } gitlabURL := os.Getenv("TEST_GITLAB_API_URL") gitlabToken := os.Getenv("TEST_GITLAB_TOKEN") @@ -31,7 +31,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, gitlab.Provider, erro run := params.New() if err := run.Clients.NewClients(ctx, &run.Info); err != nil { - return nil, options.E2E{}, gitlab.Provider{}, err + return nil, options.E2E{}, nil, err } e2eoptions := options.E2E{ @@ -39,7 +39,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, gitlab.Provider, erro UserName: "oauth2", Password: gitlabToken, } - glprovider := gitlab.Provider{} + glprovider := &gitlab.Provider{} err := glprovider.SetClient(ctx, run, &info.Event{ Provider: &info.Provider{ Token: gitlabToken, @@ -47,7 +47,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, gitlab.Provider, erro }, }, nil, nil) if err != nil { - return nil, options.E2E{}, gitlab.Provider{}, err + return nil, options.E2E{}, nil, err } return run, e2eoptions, glprovider, nil } @@ -57,12 +57,12 @@ func HasSecondIdentity() bool { return ok && token != "" } -func SetupSecond(ctx context.Context, run *params.Run) (options.E2E, gitlab.Provider, error) { +func SetupSecond(ctx context.Context, run *params.Run) (options.E2E, *gitlab.Provider, error) { if err := setup.RequireEnvs( "TEST_GITLAB_API_URL", "TEST_GITLAB_SECOND_TOKEN", ); err != nil { - return options.E2E{}, gitlab.Provider{}, err + return options.E2E{}, nil, err } gitlabURL := os.Getenv("TEST_GITLAB_API_URL") @@ -72,7 +72,7 @@ func SetupSecond(ctx context.Context, run *params.Run) (options.E2E, gitlab.Prov if run == nil { run = params.New() if err := run.Clients.NewClients(ctx, &run.Info); err != nil { - return options.E2E{}, gitlab.Provider{}, err + return options.E2E{}, nil, err } } @@ -81,7 +81,7 @@ func SetupSecond(ctx context.Context, run *params.Run) (options.E2E, gitlab.Prov UserName: "oauth2", Password: gitlabToken, } - glprovider := gitlab.Provider{} + glprovider := &gitlab.Provider{} err := glprovider.SetClient(ctx, run, &info.Event{ Provider: &info.Provider{ Token: gitlabToken, @@ -89,7 +89,7 @@ func SetupSecond(ctx context.Context, run *params.Run) (options.E2E, gitlab.Prov }, }, nil, nil) if err != nil { - return options.E2E{}, gitlab.Provider{}, err + return options.E2E{}, nil, err } return e2eoptions, glprovider, nil diff --git a/test/pkg/gitlab/test.go b/test/pkg/gitlab/test.go index 2f6b9370eb..200b407fe5 100644 --- a/test/pkg/gitlab/test.go +++ b/test/pkg/gitlab/test.go @@ -32,8 +32,8 @@ type TestOpts struct { GitHTMLURL string SHA string ParamsRun *params.Run - GLProvider gitlab2.Provider - SecondGLProvider gitlab2.Provider + GLProvider *gitlab2.Provider + SecondGLProvider *gitlab2.Provider Opts options.E2E SecondOpts options.E2E YAMLFiles map[string]string