Skip to content

Commit e5302e0

Browse files
committed
feat: Add comment strategy support for Foregejo
Added `comment_strategy` configuration to the Foregejo/Gitea provider settings to control how pipeline result comments were handled on pull requests. Supported options included `disable_all` to prevent status comments and `update` to modify a single existing comment per PipelineRun instead of creating new ones on every trigger. Passed the execution context to the status creation methods to support the updated comment functionality and fixed the update marker regex by escaping meta characters. Jira: https://issues.redhat.com/browse/SRVKP-10900 Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>
1 parent e48931a commit e5302e0

File tree

9 files changed

+293
-19
lines changed

9 files changed

+293
-19
lines changed

config/300-repositories.yaml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,17 @@ spec:
451451
forgejo:
452452
description: Forgejo contains Forgejo/Gitea-specific settings.
453453
properties:
454+
comment_strategy:
455+
description: |-
456+
CommentStrategy defines how Forgejo/Gitea comments are handled for pipeline results.
457+
Options:
458+
- 'disable_all': Disables all comments on pull requests
459+
- 'update': Updates a single comment per PipelineRun on every trigger.
460+
enum:
461+
- ""
462+
- disable_all
463+
- update
464+
type: string
454465
user_agent:
455466
description: |-
456467
UserAgent sets the User-Agent header on API requests to the Gitea/Forgejo instance.

docs/content/docs/guide/repositorycrd.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ access to the infrastructure.
128128

129129
## Controlling Pull/Merge Request comment volume
130130

131-
For GitHub (Webhook) and GitLab integrations, you can control the types
131+
For GitHub (Webhook), GitLab, and Gitea/Forgejo integrations, you can control the types
132132
of Pull/Merge request comments that Pipelines as Code emits using
133133
the `spec.<provider>.comment_strategy` setting. This can
134134
help reduce notification volume for repositories that use long-lasting

pkg/apis/pipelinesascode/v1alpha1/types.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,14 @@ type ForgejoSettings struct {
197197
// Defaults to "pipelines-as-code/<version>" when left empty.
198198
// +optional
199199
UserAgent string `json:"user_agent,omitempty"`
200+
201+
// CommentStrategy defines how Forgejo/Gitea comments are handled for pipeline results.
202+
// Options:
203+
// - 'disable_all': Disables all comments on pull requests
204+
// - 'update': Updates a single comment per PipelineRun on every trigger.
205+
// +optional
206+
// +kubebuilder:validation:Enum="";disable_all;update
207+
CommentStrategy string `json:"comment_strategy,omitempty"`
200208
}
201209

202210
func (s *Settings) Merge(newSettings *Settings) {

pkg/provider/gitea/gitea.go

Lines changed: 65 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func (v *Provider) CreateComment(_ context.Context, event *info.Event, commit, u
101101
return err
102102
}
103103

104-
re := regexp.MustCompile(updateMarker)
104+
re := regexp.MustCompile(regexp.QuoteMeta(updateMarker))
105105
for _, comment := range comments {
106106
if re.MatchString(comment.Body) {
107107
// Get the UserID for the PAC user.
@@ -229,7 +229,7 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
229229
return nil
230230
}
231231

232-
func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts providerstatus.StatusOpts) error {
232+
func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOpts providerstatus.StatusOpts) error {
233233
if v.giteaClient == nil {
234234
return fmt.Errorf("cannot set status on gitea no token or url set")
235235
}
@@ -265,10 +265,10 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts
265265
// gitea show weirdly the <br>
266266
statusOpts.Summary = fmt.Sprintf("%s%s %s", v.pacInfo.ApplicationName, onPr, statusOpts.Summary)
267267

268-
return v.createStatusCommit(event, v.pacInfo, statusOpts)
268+
return v.createStatusCommit(ctx, event, v.pacInfo, statusOpts)
269269
}
270270

271-
func (v *Provider) createStatusCommit(event *info.Event, pacopts *info.PacOpts, status providerstatus.StatusOpts) error {
271+
func (v *Provider) createStatusCommit(ctx context.Context, event *info.Event, pacopts *info.PacOpts, status providerstatus.StatusOpts) error {
272272
state := forgejo.StatusState(status.Conclusion)
273273
switch status.Conclusion {
274274
case providerstatus.ConclusionNeutral:
@@ -315,15 +315,45 @@ func (v *Provider) createStatusCommit(event *info.Event, pacopts *info.PacOpts,
315315
if opscomments.IsAnyOpsEventType(eventType.String()) {
316316
eventType = triggertype.PullRequest
317317
}
318-
if status.Text != "" && (eventType == triggertype.PullRequest || event.TriggerTarget == triggertype.PullRequest) {
319-
status.Text = strings.ReplaceAll(strings.TrimSpace(status.Text), "<br>", "\n")
320-
_, _, err := v.Client().CreateIssueComment(event.Organization, event.Repository,
321-
int64(event.PullRequestNumber), forgejo.CreateIssueCommentOption{
322-
Body: fmt.Sprintf("%s\n%s", status.Summary, status.Text),
323-
},
324-
)
325-
if err != nil {
326-
return err
318+
319+
var commentStrategy string
320+
if v.repo != nil && v.repo.Spec.Settings != nil && v.repo.Spec.Settings.Forgejo != nil {
321+
commentStrategy = v.repo.Spec.Settings.Forgejo.CommentStrategy
322+
}
323+
switch commentStrategy {
324+
case provider.DisableAllCommentStrategy:
325+
v.Logger.Warn("Comments related to PipelineRuns status have been disabled for Gitea/Forgejo pull requests")
326+
return nil
327+
case provider.UpdateCommentStrategy:
328+
if eventType == triggertype.PullRequest || event.TriggerTarget == triggertype.PullRequest {
329+
status.Text = strings.ReplaceAll(strings.TrimSpace(status.Text), "<br>", "\n")
330+
statusComment := v.formatPipelineComment(event.SHA, status)
331+
// Creating the prefix that is added to the status comment for a pipeline run.
332+
plrStatusCommentPrefix := fmt.Sprintf(provider.PlrStatusCommentPrefixTemplate, status.OriginalPipelineRunName)
333+
// The entire markdown comment, including the prefix that is added to the pull request for the pipelinerun.
334+
markdownStatusComment := fmt.Sprintf("%s\n%s", plrStatusCommentPrefix, statusComment)
335+
336+
if err := v.CreateComment(ctx, event, markdownStatusComment, plrStatusCommentPrefix); err != nil {
337+
v.eventEmitter.EmitMessage(
338+
v.repo,
339+
zap.ErrorLevel,
340+
"PipelineRunCommentCreationError",
341+
fmt.Sprintf("failed to create comment: %s", err.Error()),
342+
)
343+
return err
344+
}
345+
}
346+
default:
347+
if status.Text != "" && (eventType == triggertype.PullRequest || event.TriggerTarget == triggertype.PullRequest) {
348+
status.Text = strings.ReplaceAll(strings.TrimSpace(status.Text), "<br>", "\n")
349+
_, _, err := v.Client().CreateIssueComment(event.Organization, event.Repository,
350+
int64(event.PullRequestNumber), forgejo.CreateIssueCommentOption{
351+
Body: fmt.Sprintf("%s\n%s", status.Summary, status.Text),
352+
},
353+
)
354+
if err != nil {
355+
return err
356+
}
327357
}
328358
}
329359
return nil
@@ -570,3 +600,25 @@ func (v *Provider) CreateToken(_ context.Context, _ []string, _ *info.Event) (st
570600
func (v *Provider) GetTemplate(commentType provider.CommentType) string {
571601
return provider.GetHTMLTemplate(commentType)
572602
}
603+
604+
func (v *Provider) formatPipelineComment(sha string, status providerstatus.StatusOpts) string {
605+
var emoji string
606+
607+
if status.Status == "in_progress" {
608+
emoji = "🚀"
609+
} else {
610+
switch status.Conclusion {
611+
case providerstatus.ConclusionCancelled:
612+
emoji = "⚠️"
613+
case providerstatus.ConclusionFailure:
614+
emoji = "❌"
615+
case providerstatus.ConclusionSuccess:
616+
emoji = "✅"
617+
default:
618+
emoji = "ℹ️"
619+
}
620+
}
621+
622+
return fmt.Sprintf("%s **%s: %s/%s for %s**\n\n%s\n\n<small>Full log available [here](%s)</small>",
623+
emoji, status.Title, v.pacInfo.ApplicationName, status.OriginalPipelineRunName, sha, status.Text, status.DetailsURL)
624+
}

pkg/provider/gitea/gitea_test.go

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@ import (
2222
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
2323
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
2424
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
25+
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
2526
tgitea "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea/test"
2627
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
2728
"go.uber.org/zap"
2829
zapobserver "go.uber.org/zap/zaptest/observer"
2930
"gotest.tools/v3/assert"
31+
"gotest.tools/v3/golden"
3032
rtesting "knative.dev/pkg/reconciler/testing"
3133
)
3234

@@ -579,7 +581,7 @@ func TestProvider_CreateStatusCommit(t *testing.T) {
579581
giteaClient: fakeclient,
580582
}
581583

582-
if err := v.createStatusCommit(tt.args.event, tt.args.pacopts, tt.args.status); (err != nil) != tt.wantErr {
584+
if err := v.createStatusCommit(context.Background(), tt.args.event, tt.args.pacopts, tt.args.status); (err != nil) != tt.wantErr {
583585
t.Errorf("Provider.createStatusCommit() error = %v, wantErr %v", err, tt.wantErr)
584586
}
585587
})
@@ -653,7 +655,7 @@ func TestProviderCreateStatusCommitRetryOnTransientError(t *testing.T) {
653655
Conclusion: "success",
654656
}
655657

656-
err := v.createStatusCommit(event, pacopts, status)
658+
err := v.createStatusCommit(context.Background(), event, pacopts, status)
657659

658660
if tt.wantErr {
659661
assert.Assert(t, err != nil, "expected an error but got none")
@@ -872,6 +874,110 @@ func TestCreateComment(t *testing.T) {
872874
}
873875
}
874876

877+
func TestCreateStatusUpdateCommentNormalizesBreaks(t *testing.T) {
878+
fakeclient, mux, teardown := tgitea.Setup(t)
879+
defer teardown()
880+
881+
var createCommentBody string
882+
mux.HandleFunc("/repos/org/repo/statuses/", func(rw http.ResponseWriter, _ *http.Request) {
883+
fmt.Fprint(rw, `{"state":"pending"}`)
884+
})
885+
mux.HandleFunc("/repos/org/repo/issues/123/comments", func(rw http.ResponseWriter, r *http.Request) {
886+
switch r.Method {
887+
case http.MethodGet:
888+
fmt.Fprint(rw, `[]`)
889+
case http.MethodPost:
890+
b, err := io.ReadAll(r.Body)
891+
assert.NilError(t, err)
892+
createCommentBody = string(b)
893+
fmt.Fprint(rw, `{}`)
894+
default:
895+
t.Fatalf("unexpected method: %s", r.Method)
896+
}
897+
})
898+
899+
p := &Provider{
900+
giteaClient: fakeclient,
901+
pacInfo: &info.PacOpts{
902+
Settings: settings.Settings{
903+
ApplicationName: settings.PACApplicationNameDefaultValue,
904+
},
905+
},
906+
repo: &v1alpha1.Repository{
907+
Spec: v1alpha1.RepositorySpec{
908+
Settings: &v1alpha1.Settings{
909+
Forgejo: &v1alpha1.ForgejoSettings{CommentStrategy: provider.UpdateCommentStrategy},
910+
},
911+
},
912+
},
913+
}
914+
915+
err := p.CreateStatus(context.Background(), &info.Event{
916+
Organization: "org",
917+
Repository: "repo",
918+
SHA: "abc123",
919+
PullRequestNumber: 123,
920+
EventType: triggertype.PullRequest.String(),
921+
TriggerTarget: triggertype.PullRequest,
922+
}, status.StatusOpts{
923+
Status: "in_progress",
924+
Text: "line1<br>line2",
925+
OriginalPipelineRunName: "demo",
926+
DetailsURL: "https://example.test/log",
927+
})
928+
assert.NilError(t, err)
929+
assert.Assert(t, !strings.Contains(createCommentBody, "<br>"), "comment body should not contain raw <br>: %s", createCommentBody)
930+
assert.Assert(t, strings.Contains(createCommentBody, "line1\\nline2"), "comment body should contain normalized newline: %s", createCommentBody)
931+
932+
golden.Assert(t, createCommentBody, strings.ReplaceAll(fmt.Sprintf("%s.golden", t.Name()), "/", "-"))
933+
}
934+
935+
func TestFormatPipelineCommentEmoji(t *testing.T) {
936+
p := &Provider{
937+
pacInfo: &info.PacOpts{
938+
Settings: settings.Settings{
939+
ApplicationName: settings.PACApplicationNameDefaultValue,
940+
},
941+
},
942+
}
943+
944+
tests := []struct {
945+
name string
946+
status status.StatusOpts
947+
emoji string
948+
}{
949+
{
950+
name: "failure conclusion uses failure emoji",
951+
status: status.StatusOpts{
952+
Conclusion: status.ConclusionFailure,
953+
Title: "Failed",
954+
Text: "details",
955+
OriginalPipelineRunName: "demo",
956+
DetailsURL: "https://example.test/log",
957+
},
958+
emoji: "❌",
959+
},
960+
{
961+
name: "in progress status uses rocket emoji",
962+
status: status.StatusOpts{
963+
Status: "in_progress",
964+
Title: "CI has Started",
965+
Text: "details",
966+
OriginalPipelineRunName: "demo",
967+
DetailsURL: "https://example.test/log",
968+
},
969+
emoji: "🚀",
970+
},
971+
}
972+
973+
for _, tt := range tests {
974+
t.Run(tt.name, func(t *testing.T) {
975+
got := p.formatPipelineComment("abc123", tt.status)
976+
assert.Assert(t, strings.HasPrefix(got, tt.emoji+" "), "expected prefix %q in comment %q", tt.emoji+" ", got)
977+
})
978+
}
979+
}
980+
875981
func TestGetCommitInfo(t *testing.T) {
876982
tests := []struct {
877983
name string
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"body":"\u003c!-- pac-status-demo --\u003e\n🚀 **CI has Started: Pipelines as Code CI/demo for abc123**\n\nline1\nline2\n\n\u003csmall\u003eFull log available [here](https://example.test/log)\u003c/small\u003e"}

pkg/provider/provider.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,13 @@ const (
242242
func IsCommentStrategyUpdate(repo *v1alpha1.Repository) bool {
243243
var commentStrategy string
244244
if repo != nil && repo.Spec.Settings != nil {
245-
if repo.Spec.Settings.Gitlab != nil {
245+
switch {
246+
case repo.Spec.Settings.Gitlab != nil:
246247
commentStrategy = repo.Spec.Settings.Gitlab.CommentStrategy
247-
} else if repo.Spec.Settings.Github != nil {
248+
case repo.Spec.Settings.Github != nil:
248249
commentStrategy = repo.Spec.Settings.Github.CommentStrategy
250+
case repo.Spec.Settings.Forgejo != nil:
251+
commentStrategy = repo.Spec.Settings.Forgejo.CommentStrategy
249252
}
250253
}
251254

pkg/webhook/validation.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ import (
1818

1919
var universalDeserializer = serializer.NewCodecFactory(runtime.NewScheme()).UniversalDeserializer()
2020

21-
var allowedGitlabDisableCommentStrategyOnMr = sets.NewString("", provider.DisableAllCommentStrategy, provider.UpdateCommentStrategy)
21+
var (
22+
allowedGitlabDisableCommentStrategyOnMr = sets.NewString("", provider.DisableAllCommentStrategy, provider.UpdateCommentStrategy)
23+
allowedForgejoCommentStrategyOnPr = sets.NewString("", provider.DisableAllCommentStrategy, provider.UpdateCommentStrategy)
24+
)
2225

2326
// Path implements AdmissionController.
2427
func (ac *reconciler) Path() string {
@@ -68,6 +71,12 @@ func (ac *reconciler) Admit(_ context.Context, request *v1.AdmissionRequest) *v1
6871
}
6972
}
7073

74+
if repo.Spec.Settings != nil && repo.Spec.Settings.Forgejo != nil {
75+
if !allowedForgejoCommentStrategyOnPr.Has(repo.Spec.Settings.Forgejo.CommentStrategy) {
76+
return webhook.MakeErrorStatus("comment strategy '%s' is not supported for Forgejo/Gitea PRs", repo.Spec.Settings.Forgejo.CommentStrategy)
77+
}
78+
}
79+
7180
return &v1.AdmissionResponse{Allowed: true}
7281
}
7382

0 commit comments

Comments
 (0)