diff --git a/config/302-pac-configmap.yaml b/config/302-pac-configmap.yaml index a561cdc895..4c4b31e73e 100644 --- a/config/302-pac-configmap.yaml +++ b/config/302-pac-configmap.yaml @@ -237,6 +237,11 @@ data: # step of a task using StepActions. Set it to false to only report the tasks. status-show-steps: "true" + # Enable or disable the feature to support edited comments events. + # When enabled, edited comments will be processed and PipelineRuns will be triggered on them. + # Default: false + support-edited-comments: "false" + kind: ConfigMap metadata: name: pipelines-as-code diff --git a/docs/content/docs/api/configmap.md b/docs/content/docs/api/configmap.md index 41bf9634ee..f281d497c4 100644 --- a/docs/content/docs/api/configmap.md +++ b/docs/content/docs/api/configmap.md @@ -399,6 +399,22 @@ require-ok-to-test-sha: "false" {{< /param >}} +{{< tech_preview "Support for edited comments" >}} + +{{< param name="support-edited-comments" type="boolean" default="false" id="param-support-edited-comments" >}} + +Enables processing of edited comment events. When enabled, editing a comment that contains a GitOps command (such as `/test` or `/retest`) triggers the command as if it were a new comment. This is useful when you need to fix a typo in a command or change a PipelineRun name without posting a new comment. + +When disabled (the default), only newly created comments trigger PipelineRuns. Edited comments are ignored. + +Currently supported on GitHub only. + +```yaml +support-edited-comments: "false" +``` + +{{< /param >}} + {{< param name="skip-push-event-for-pr-commits" type="boolean" default="true" id="param-skip-push-event-for-pr-commits" >}} Prevents duplicate PipelineRuns when a commit appears in both a push event and a pull request. When a push event arrives from a commit that belongs to an open pull request, Pipelines-as-Code skips the push event. diff --git a/docs/content/docs/guides/gitops-commands/_index.md b/docs/content/docs/guides/gitops-commands/_index.md index 6d7f1dd982..01a41d7c6f 100644 --- a/docs/content/docs/guides/gitops-commands/_index.md +++ b/docs/content/docs/guides/gitops-commands/_index.md @@ -74,3 +74,24 @@ Pipeline execution appears to be unstable due to external factors. Retesting thi {{< callout type="info" >}} GitOps commands such as `/test` and others do not work on closed pull requests or merge requests. {{< /callout >}} + +### Edited Comments + +{{< tech_preview "Support for edited comments" >}} +{{< support_matrix github_app="true" github_webhook="true" forgejo="false" gitlab="false" bitbucket_cloud="false" bitbucket_datacenter="false" >}} + +By default, Pipelines-as-Code only processes **newly created** comments. If you edit an existing comment to add or change a GitOps command, it is ignored. + +When the `support-edited-comments` setting is enabled in the [Pipelines-as-Code ConfigMap]({{< relref "/docs/api/configmap" >}}), edited comments are processed just like new ones. This means you can edit a comment to fix a typo in a command or change a PipelineRun name, and Pipelines-as-Code triggers the updated command. + +To enable this feature, set the following in the `pipelines-as-code` ConfigMap: + +```yaml +support-edited-comments: "true" +``` + +Pipelines-as-Code fetches pull request details from the GitHub API before checking a comment for GitOps commands. Enabling edited comment support also makes this API request for each processed comment edit, including edits to ordinary comments or commands intended for other CI systems. Each request counts toward the API rate limit of the token used by Pipelines-as-Code. + +{{< callout type="warning" >}} +Enabling edited comment support means that editing any comment on a pull request that contains a GitOps command (such as `/test` or `/retest`) will re-trigger the associated PipelineRuns. Be cautious when editing older comments that contain commands. +{{< /callout >}} diff --git a/pkg/adapter/adapter.go b/pkg/adapter/adapter.go index aae9bb39ef..215cf62628 100644 --- a/pkg/adapter/adapter.go +++ b/pkg/adapter/adapter.go @@ -233,7 +233,7 @@ func (l listener) handleEvent(ctx context.Context) http.HandlerFunc { if isIncoming { gitProvider, logger, err = l.processIncoming(event, targettedRepo) } else { - gitProvider, logger, err = l.detectProvider(request, string(payload)) + gitProvider, logger, err = l.detectProvider(request, string(payload), &pacInfo) } // figure out which provider request coming from @@ -335,7 +335,7 @@ func (l listener) processRes(processEvent bool, provider provider.Interface, log return nil, logger, fmt.Errorf("skipping non supported event") } -func (l listener) detectProvider(req *http.Request, reqBody string) (provider.Interface, *zap.SugaredLogger, error) { +func (l listener) detectProvider(req *http.Request, reqBody string, pacInfo *info.PacOpts) (provider.Interface, *zap.SugaredLogger, error) { log := *l.logger // payload validation @@ -346,6 +346,7 @@ func (l listener) detectProvider(req *http.Request, reqBody string) (provider.In gitHub := github.New() gitHub.Run = l.run + gitHub.SetPacInfo(pacInfo) isGH, processReq, logger, reason, err := gitHub.Detect(req, reqBody, &log) if isGH { return l.processRes(processReq, gitHub, logger, reason, err) diff --git a/pkg/adapter/adapter_test.go b/pkg/adapter/adapter_test.go index 8e577492a1..cc97a96a92 100644 --- a/pkg/adapter/adapter_test.go +++ b/pkg/adapter/adapter_test.go @@ -269,7 +269,7 @@ func TestWhichProvider(t *testing.T) { Header: tt.header, } - _, _, err = l.detectProvider(req, string(jeez)) + _, _, err = l.detectProvider(req, string(jeez), &info.PacOpts{}) if tt.wantErrString != "" { assert.ErrorContains(t, err, tt.wantErrString) return @@ -359,9 +359,10 @@ func TestDetectProviderAdditionalProviders(t *testing.T) { assert.NilError(t, err) body = string(payload) } + pacInfo := &info.PacOpts{} req := &http.Request{Header: tt.header} - gotProvider, gotLogger, err := l.detectProvider(req, body) + gotProvider, gotLogger, err := l.detectProvider(req, body, pacInfo) assert.NilError(t, err) assert.Assert(t, gotProvider != nil) assert.Assert(t, gotLogger != nil) diff --git a/pkg/params/settings/config.go b/pkg/params/settings/config.go index bef46cb092..546e1a2a99 100644 --- a/pkg/params/settings/config.go +++ b/pkg/params/settings/config.go @@ -108,6 +108,9 @@ type Settings struct { RememberOKToTest bool `json:"remember-ok-to-test"` RequireOkToTestSHA bool `json:"require-ok-to-test-sha"` + // SupportEditedComments enables or disables the feature to support edited comments events. + SupportEditedComments bool `default:"false" json:"support-edited-comments"` + // Retry Git provider API requests on rate limits and transient errors. // Disabled by default. EnableAPIRetry bool `default:"false" json:"enable-api-retry"` diff --git a/pkg/params/settings/config_test.go b/pkg/params/settings/config_test.go index 7257bbbcb1..2283c76dfd 100644 --- a/pkg/params/settings/config_test.go +++ b/pkg/params/settings/config_test.go @@ -42,6 +42,7 @@ func TestSyncConfig(t *testing.T) { EnableCancelInProgressOnPullRequests: false, EnableCancelInProgressOnPush: false, SkipPushEventForPRCommits: true, + SupportEditedComments: false, StatusShowSteps: true, CustomConsoleName: "", CustomConsoleURL: "", @@ -80,6 +81,7 @@ func TestSyncConfig(t *testing.T) { "custom-console-url-namespace": "https://custom-console-namespace", "remember-ok-to-test": "false", "skip-push-event-for-pr-commits": "true", + "support-edited-comments": "false", "require-ok-to-test-sha": "true", }, expectedStruct: Settings{ @@ -105,6 +107,7 @@ func TestSyncConfig(t *testing.T) { EnableCancelInProgressOnPullRequests: false, EnableCancelInProgressOnPush: false, SkipPushEventForPRCommits: true, + SupportEditedComments: false, StatusShowSteps: true, CustomConsoleName: "custom-console", CustomConsoleURL: "https://custom-console", diff --git a/pkg/provider/github/detect.go b/pkg/provider/github/detect.go index 7005a40b39..85fd6c06cd 100644 --- a/pkg/provider/github/detect.go +++ b/pkg/provider/github/detect.go @@ -76,7 +76,8 @@ func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any } return "", fmt.Sprintf("pull_request: unsupported action \"%s\"", event.GetAction()) case *github.IssueCommentEvent: - if event.GetAction() != "created" { + isEdiCommentEvent := event.GetAction() == "edited" && v.pacInfo.SupportEditedComments + if event.GetAction() != "created" && !isEdiCommentEvent { return "", fmt.Sprintf("issue_comment: unsupported action \"%s\"", event.GetAction()) } @@ -104,7 +105,8 @@ func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any } return "", fmt.Sprintf("check_run: unsupported action \"%s\"", event.GetAction()) case *github.CommitCommentEvent: - if event.GetAction() != "created" { + isEdiCommentEvent := event.GetAction() == "edited" && v.pacInfo.SupportEditedComments + if event.GetAction() != "created" && !isEdiCommentEvent { return "", fmt.Sprintf("commit_comment: unsupported action \"%s\"", event.GetAction()) } diff --git a/pkg/provider/github/detect_test.go b/pkg/provider/github/detect_test.go index 79696f5d88..57e1e50d19 100644 --- a/pkg/provider/github/detect_test.go +++ b/pkg/provider/github/detect_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/google/go-github/v91/github" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/test/logger" "gotest.tools/v3/assert" ) @@ -14,13 +15,14 @@ import ( func TestProviderDetect(t *testing.T) { idd := int64(123) tests := []struct { - name string - wantErrString string - isGH bool - processReq bool - event any - eventType string - wantReason string + name string + wantErrString string + isGH bool + processReq bool + event any + supportEditedComments bool + eventType string + wantReason string }{ { name: "not a github Event", @@ -267,6 +269,47 @@ func TestProviderDetect(t *testing.T) { isGH: true, processReq: true, }, + { + name: "issue comment Event with edited action", + event: github.IssueCommentEvent{ + Action: new("edited"), + Issue: &github.Issue{ + PullRequestLinks: &github.PullRequestLinks{ + URL: new("url"), + }, + State: new("open"), + }, + Installation: &github.Installation{ + ID: &idd, + }, + Comment: &github.IssueComment{Body: new("I am edited")}, + }, + eventType: "issue_comment", + supportEditedComments: true, + isGH: true, + processReq: true, + }, + { + name: "issue comment Event with edited action and support edited comments is disabled", + event: github.IssueCommentEvent{ + Action: new("edited"), + Issue: &github.Issue{ + PullRequestLinks: &github.PullRequestLinks{ + URL: new("url"), + }, + State: new("open"), + }, + Installation: &github.Installation{ + ID: &idd, + }, + Comment: &github.IssueComment{Body: new("I am edited")}, + }, + eventType: "issue_comment", + supportEditedComments: false, + isGH: true, + processReq: false, + wantReason: "issue_comment: unsupported action \"edited\"", + }, { name: "commit comment event with cancel comment", event: github.CommitCommentEvent{ @@ -280,6 +323,33 @@ func TestProviderDetect(t *testing.T) { isGH: true, processReq: true, }, + { + name: "commit comment Event with edited action", + event: github.CommitCommentEvent{ + Action: new("edited"), + Installation: &github.Installation{ + ID: &idd, + }, + }, + eventType: "commit_comment", + supportEditedComments: true, + isGH: true, + processReq: true, + }, + { + name: "commit comment Event with edited action and support edited comments is disabled", + event: github.CommitCommentEvent{ + Action: new("edited"), + Installation: &github.Installation{ + ID: &idd, + }, + }, + eventType: "commit_comment", + supportEditedComments: false, + isGH: true, + processReq: false, + wantReason: "commit_comment: unsupported action \"edited\"", + }, { name: "commit comment Event with retest", event: github.CommitCommentEvent{ @@ -323,7 +393,11 @@ func TestProviderDetect(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gprovider := Provider{} + gprovider := Provider{ + pacInfo: &info.PacOpts{ + SupportEditedComments: tt.supportEditedComments, + }, + } logger, logCatcher := logger.GetLogger() jeez, err := json.Marshal(tt.event) if err != nil { diff --git a/test/github_pullrequest_test_comment_test.go b/test/github_pullrequest_test_comment_test.go index c3d973347d..d650f9d11a 100644 --- a/test/github_pullrequest_test_comment_test.go +++ b/test/github_pullrequest_test_comment_test.go @@ -1,5 +1,3 @@ -//go:build e2e - package test import ( @@ -7,15 +5,19 @@ import ( "fmt" "regexp" "testing" + "time" "github.com/google/go-github/v91/github" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" "github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments" + cctx "github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/configmap" tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github" twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "gotest.tools/v3/assert" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) @@ -51,6 +53,125 @@ func TestGithubGHEPullRequestTest(t *testing.T) { assert.Equal(t, corev1.ConditionTrue, cond.Status) } +func TestGithubGHEPullRequestTestEditedComment(t *testing.T) { + ctx := context.TODO() + g := &tgithub.PRTest{ + Label: "Github test edited comment", + YamlFiles: []string{"testdata/pipelinerun.yaml"}, + GHE: true, + } + g.RunPullRequest(ctx, t) + defer g.TearDown(ctx, t) + + patchData := map[string]string{ + "support-edited-comments": "true", + } + + ctx, err := cctx.GetControllerCtxInfo(ctx, g.Cnx) + assert.NilError(t, err) + + configMapTearDown := configmap.ChangeGlobalConfig(ctx, t, g.Cnx, tgithub.ControllerConfigMapName(g.GHE), patchData) + defer configMapTearDown() + + g.Cnx.Clients.Log.Infof("Creating wrong comment /tost in PullRequest") + comment, _, err := g.Provider.Client().Issues.CreateComment(ctx, + g.Options.Organization, + g.Options.Repo, g.PRNumber, + github.IssueCommentRequest{Body: "/tost"}) // deliberately wrong to test edited comment + assert.NilError(t, err) + + for i := 0; i < 10; i++ { + plrs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", keys.EventType, opscomments.NoOpsCommentEventType.String()), + }) + assert.NilError(t, err) + assert.Equal(t, len(plrs.Items), 0, "no pipelinerun should be created but got %d", len(plrs.Items)) + time.Sleep(1 * time.Second) + g.Cnx.Clients.Log.Infof("Waiting if no PipelineRun is created") + } + + g.Cnx.Clients.Log.Infof("Editing comment %d", comment.GetID()) + _, _, err = g.Provider.Client().Issues.UpdateComment(ctx, + g.Options.Organization, + g.Options.Repo, comment.GetID(), + github.IssueCommentRequest{Body: "/test"}) + assert.NilError(t, err) + + g.Cnx.Clients.Log.Infof("Waiting for PipelineRun to succeed") + waitOpts := twait.Opts{ + Namespace: g.TargetNamespace, + MinNumberStatus: 2, + PollTimeout: twait.DefaultTimeout, + TargetSHA: []string{g.SHA}, + } + prs, err := twait.UntilPipelineRunHasReason(ctx, g.Cnx.Clients, tektonv1.PipelineRunReasonSuccessful, waitOpts) + assert.NilError(t, err) + g.Cnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") + cond := prs[len(prs)-1].Status.GetCondition(apis.ConditionSucceeded) + assert.Assert(t, cond != nil) + assert.Equal(t, corev1.ConditionTrue, cond.Status) +} + +func TestGithubGHEPullRequestOnCommentEditedComment(t *testing.T) { + ctx := context.TODO() + g := &tgithub.PRTest{ + Label: "Github test on comment edited comment", + YamlFiles: []string{"testdata/pipelinerun-on-comment-annotation.yaml"}, + GHE: true, + NoStatusCheck: true, + } + g.RunPullRequest(ctx, t) + defer g.TearDown(ctx, t) + + patchData := map[string]string{ + "support-edited-comments": "true", + } + + ctx, err := cctx.GetControllerCtxInfo(ctx, g.Cnx) + assert.NilError(t, err) + + configMapTearDown := configmap.ChangeGlobalConfig(ctx, t, g.Cnx, "ghe-configmap", patchData) + defer configMapTearDown() + + g.Cnx.Clients.Log.Infof("Creating wrong comment /bye-world in PullRequest") + comment, _, err := g.Provider.Client().Issues.CreateComment(ctx, + g.Options.Organization, + g.Options.Repo, g.PRNumber, + github.IssueCommentRequest{Body: "/bye-world"}) // deliberately wrong to test edited comment + assert.NilError(t, err) + + for i := 0; i < 10; i++ { + plrs, err := g.Cnx.Clients.Tekton.TektonV1().PipelineRuns(g.TargetNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", keys.SHA, g.SHA), + }) + assert.NilError(t, err) + assert.Equal(t, len(plrs.Items), 0, "no pipelinerun should be created but got %d", len(plrs.Items)) + time.Sleep(1 * time.Second) + g.Cnx.Clients.Log.Infof("Waiting if no PipelineRun is created") + } + + g.Cnx.Clients.Log.Infof("Editing comment %d", comment.GetID()) + _, _, err = g.Provider.Client().Issues.UpdateComment(ctx, + g.Options.Organization, + g.Options.Repo, comment.GetID(), + github.IssueCommentRequest{Body: "/hello-world"}) + assert.NilError(t, err) + + g.Cnx.Clients.Log.Infof("Waiting for PipelineRun to succeed") + waitOpts := twait.Opts{ + Namespace: g.TargetNamespace, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: []string{g.SHA}, + } + prs, err := twait.UntilPipelineRunHasReason(ctx, g.Cnx.Clients, tektonv1.PipelineRunReasonSuccessful, waitOpts) + assert.NilError(t, err) + g.Cnx.Clients.Log.Infof("Check if we have the pipelinerun set as succeeded") + cond := prs[len(prs)-1].Status.GetCondition(apis.ConditionSucceeded) + assert.Assert(t, cond != nil) + assert.Equal(t, corev1.ConditionTrue, cond.Status) +} + func TestGithubGHEOnCommentAnnotation(t *testing.T) { g := &tgithub.PRTest{ Label: "Github test implicit comment",