From 534e9b7305159ea46bca38bd89b42bfa22650f28 Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Tue, 11 Aug 2026 12:11:55 +0530 Subject: [PATCH] feat(status): report skipped status for unmatched PipelineRuns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a repository has multiple PipelineRuns in .tekton/ targeting different events, only the ones matching the incoming event run. The rest are silently ignored, leaving gaps in the Git provider's status checks UI. For example, a repository with two PipelineRuns: .tekton/build.yaml — on-event: pull_request, on-target-branch: main .tekton/deploy.yaml — on-event: push, on-target-branch: main When a pull request is opened, build.yaml matches and runs. Without this feature, deploy.yaml produces no status at all — it is impossible to tell from the PR whether it was skipped intentionally or never picked up. With status_check enabled: spec: settings: status_check: enabled: true mode: "per_unmatched_pipelinerun" Pipelines-as-Code now reports a "skipped" status for deploy.yaml on the pull request, making the full picture visible in the provider UI. The matcher now returns both matched and unmatched PipelineRuns. After the matched runs complete, the controller iterates the unmatched list and calls CreateStatus on each with the configured conclusion. The conclusion defaults to `skipped` but can be set to `success` or `neutral` via the `no_match_conclusion` field. Every provider maps that conclusion to its native state: - GitHub App: check run conclusion "skipped" - GitHub Webhook: commit status "success" (API has no skipped) - GitLab: pipeline status "skipped" - Bitbucket Cloud: build status "STOPPED" - Bitbucket Data Center: build status "UNKNOWN" - Gitea/Forgejo: commit status "success" (no skipped state) The setting is inheritable from the global Repository CR via the existing Settings.Merge path. A second mode (`aggregate`) is defined in the CRD but not yet implemented. The whole feature is behind an opt-in flag (enabled: false by default) and marked as tech preview in the documentation. Co-Authored-By: Claude Signed-off-by: Zaki Shaikh --- config/300-repositories.yaml | 30 ++ docs/content/docs/api/repository-spec.md | 3 + docs/content/docs/api/repository.md | 3 + docs/content/docs/api/settings.md | 60 ++++ docs/content/docs/guides/_index.md | 2 +- .../guides/repository-crd/status-checks.md | 105 +++++++ docs/content/docs/guides/statuses.md | 4 + .../operations/global-repository-settings.md | 1 + pkg/apis/pipelinesascode/v1alpha1/types.go | 36 +++ .../pipelinesascode/v1alpha1/types_test.go | 86 ++++++ .../v1alpha1/zz_generated.deepcopy.go | 36 +-- pkg/matcher/annotation_matcher.go | 46 ++- pkg/matcher/annotation_matcher_test.go | 98 ++++++- pkg/pipelineascode/match.go | 72 ++--- pkg/pipelineascode/match_test.go | 45 +-- pkg/pipelineascode/pipelineascode.go | 54 +++- .../pipelineascode_statuscheck_test.go | 182 ++++++++++++ pkg/pipelineascode/pipelineascode_test.go | 24 ++ .../testdata/no-match/.tekton/nomatch.yaml | 3 + pkg/provider/bitbucketcloud/bitbucket.go | 2 +- pkg/provider/bitbucketcloud/bitbucket_test.go | 8 + .../bitbucketdatacenter.go | 4 +- .../bitbucketdatacenter_test.go | 9 + pkg/provider/gitea/gitea.go | 11 +- pkg/provider/gitea/status_test.go | 63 +++- pkg/provider/github/status.go | 23 +- pkg/provider/github/status_test.go | 270 +++++++++++++----- pkg/provider/gitlab/gitlab.go | 7 +- pkg/provider/gitlab/gitlab_test.go | 116 ++++++++ pkg/provider/status/status.go | 1 + test/bitbucket_cloud_pullrequest_test.go | 75 +++++ ...ucket_datacenter_dynamic_variables_test.go | 2 +- test/bitbucket_datacenter_on_comment_test.go | 2 +- .../bitbucket_datacenter_pull_request_test.go | 65 ++++- test/bitbucket_datacenter_push_test.go | 2 +- test/gitea_pull_request_test.go | 69 +++++ test/github_pullrequest_test.go | 188 ++++++++++++ test/gitlab_merge_request_test.go | 67 +++++ test/pkg/bitbucketcloud/crd.go | 3 +- test/pkg/bitbucketdatacenter/crd.go | 6 +- 40 files changed, 1698 insertions(+), 185 deletions(-) create mode 100644 docs/content/docs/guides/repository-crd/status-checks.md create mode 100644 pkg/pipelineascode/pipelineascode_statuscheck_test.go diff --git a/config/300-repositories.yaml b/config/300-repositories.yaml index 0e361d8d81..e74b522520 100644 --- a/config/300-repositories.yaml +++ b/config/300-repositories.yaml @@ -432,6 +432,36 @@ spec: type: string type: array type: object + status_checks: + description: StatusChecks configures the status checks for the repository. + properties: + enabled: + description: Enabled defines if the status checks should be reported. Default is false. + type: boolean + mode: + description: |- + Mode defines how the status checks should be reported when is enabled. + Options: + - 'per_pipelinerun': Report the status check of each PipelineRun separately. + enum: + - "" + - per_pipelinerun + type: string + unmatched_conclusion: + description: |- + UnmatchedConclusion defines the conclusion to report when pipeline run is not matched. Default is 'skipped'. + this will be used only if mode is 'per_pipelinerun'. + Options: + - 'success': Report as success. + - 'neutral': Report as neutral. + - 'skipped': Report as skipped. Default. + enum: + - "" + - success + - neutral + - skipped + type: string + type: object type: object url: description: |- diff --git a/docs/content/docs/api/repository-spec.md b/docs/content/docs/api/repository-spec.md index 67036ef4f8..7369d54719 100644 --- a/docs/content/docs/api/repository-spec.md +++ b/docs/content/docs/api/repository-spec.md @@ -307,4 +307,7 @@ spec: container_logs: enabled: true max_lines: 100 + status_checks: + enabled: true + mode: "per_pipelinerun" ``` diff --git a/docs/content/docs/api/repository.md b/docs/content/docs/api/repository.md index f6e2c0a1f6..c72c7cc235 100644 --- a/docs/content/docs/api/repository.md +++ b/docs/content/docs/api/repository.md @@ -143,6 +143,9 @@ spec: container_logs: enabled: true max_lines: 100 + status_checks: + enabled: true + mode: "per_pipelinerun" ``` ## Related resources diff --git a/docs/content/docs/api/settings.md b/docs/content/docs/api/settings.md index a55edc99bf..4ae8c0f2aa 100644 --- a/docs/content/docs/api/settings.md +++ b/docs/content/docs/api/settings.md @@ -366,6 +366,60 @@ settings: {{< /param-group >}} {{< /param >}} +## Status check settings + +{{< param name="status_checks" type="StatusCheck" >}} +Configures status check reporting for PipelineRuns that did not match the incoming event. See the [Status Checks guide]({{< relref "/docs/guides/repository-crd/status-checks" >}}) for full details and provider behavior. + +{{< param-group label="Show StatusCheck Fields" >}} + +{{< param name="status_checks.enabled" type="boolean" id="param-status-check-enabled" >}} +Enables or disables status check reporting for unmatched PipelineRuns. Default: `false`. + +```yaml +settings: + status_checks: + enabled: true +``` + +{{< /param >}} + +{{< param name="status_checks.mode" type="string" id="param-status-check-mode" >}} +Controls how status checks are reported. Options: + +- `per_pipelinerun` - Report a separate status for each unmatched PipelineRun + +```yaml +settings: + status_checks: + mode: "per_pipelinerun" +``` + +{{< /param >}} + +{{< param name="status_checks.unmatched_conclusion" type="string" id="param-status-check-no-match-conclusion" >}} +The conclusion to report for unmatched PipelineRuns. Only used when `mode` is `per_pipelinerun`. Default: `skipped`. Options: `skipped`, `success`, `neutral`. + +```yaml +settings: + status_checks: + unmatched_conclusion: "skipped" +``` + +{{< /param >}} + +{{< /param-group >}} + +```yaml +settings: + status_checks: + enabled: true + mode: "per_pipelinerun" + unmatched_conclusion: "skipped" +``` + +{{< /param >}} + ## Complete example ```yaml @@ -432,6 +486,12 @@ spec: context_items: commit_content: true pr_content: true + + # Status check reporting + status_checks: + enabled: true + mode: "per_pipelinerun" + unmatched_conclusion: "skipped" ``` ## Settings inheritance diff --git a/docs/content/docs/guides/_index.md b/docs/content/docs/guides/_index.md index 82aab774a5..b14623ad8c 100644 --- a/docs/content/docs/guides/_index.md +++ b/docs/content/docs/guides/_index.md @@ -9,7 +9,7 @@ This section covers the core workflows you need to run CI/CD with Pipelines-as-C {{< cards >}} {{< card link="creating-pipelines" title="Authoring PipelineRuns" subtitle="Create pipelines, CEL variables, GitHub token" >}} - {{< card link="repository-crd" title="Repository CR" subtitle="Configure repos, concurrency, comment settings" >}} + {{< card link="repository-crd" title="Repository CR" subtitle="Configure repos, concurrency, comment settings, status checks" >}} {{< card link="event-matching" title="Event matching" subtitle="on-event, on-target-branch, path, CEL, labels" >}} {{< card link="gitops-commands" title="GitOps commands" subtitle="/retest, /test, /cancel and more" >}} {{< card link="statuses" title="PipelineRun status" subtitle="Status reporting and failure detection" >}} diff --git a/docs/content/docs/guides/repository-crd/status-checks.md b/docs/content/docs/guides/repository-crd/status-checks.md new file mode 100644 index 0000000000..3c6e002f18 --- /dev/null +++ b/docs/content/docs/guides/repository-crd/status-checks.md @@ -0,0 +1,105 @@ +--- +title: Status Checks +weight: 5 +--- + +{{< tech_preview "Status Checks settings in Repository CR" >}} + +This page explains how to report status checks for PipelineRuns that did not match the incoming event. Use this when you want visibility into which PipelineRuns in your `.tekton/` directory were skipped because their annotations (target branch, event type, CEL expression, or path filter) did not match. + +By default, Pipelines-as-Code only reports status for PipelineRuns that matched +and ran. PipelineRuns that did not match are silently ignored. Enabling +`status_checks` makes Pipelines-as-Code report a status for each unmatched +PipelineRun so you can see the full picture in your Git provider's UI. + +## Configuration + +Add the `status_checks` block under `spec.settings` in your Repository CR: + +```yaml +apiVersion: "pipelinesascode.tekton.dev/v1alpha1" +kind: Repository +metadata: + name: my-repo +spec: + url: "https://github.com/owner/repo" + settings: + status_checks: + enabled: true + mode: "per_pipelinerun" +``` + +### Fields + +| Field | Type | Default | Description | +| --- | --- | --- | --- | +| `enabled` | bool | `false` | Enable status check reporting for unmatched PipelineRuns. | +| `mode` | string | | How to report status checks. See [Modes](#modes). | +| `unmatched_conclusion` | string | `skipped` | The conclusion to report for unmatched PipelineRuns. Only used when `mode` is `per_pipelinerun`. Accepted values: `skipped`, `success`, `neutral`. | + +### Modes + +#### `per_pipelinerun` + +Reports a separate status for each PipelineRun that did not match the event. +This is useful when you have multiple PipelineRuns targeting different events +(for example, one for `pull_request` and one for `push`) and you want to see +which ones were skipped on each event. + +```yaml +spec: + settings: + status_checks: + enabled: true + mode: "per_pipelinerun" +``` + +### Customizing the conclusion + +By default, unmatched PipelineRuns are reported with a `skipped` conclusion. +You can change this to `success` or `neutral` using the `unmatched_conclusion` +field: + +```yaml +spec: + settings: + status_checks: + enabled: true + mode: "per_pipelinerun" + unmatched_conclusion: "success" +``` + +## Provider behavior + +The `skipped` conclusion maps to different states depending on your Git provider: + +| Provider | Reported state | Notes | +| --- | --- | --- | +| GitHub App | `skipped` | Shown as a skipped check run. | +| GitHub Webhook | `success` | GitHub commit status API does not support `skipped`. Reported as `success` with a "Skipped" description. | +| GitLab | `skipped` | Shown as a skipped pipeline in the Pipelines tab. | +| Bitbucket Cloud | `STOPPED` | Shown as a stopped build status. | +| Bitbucket Data Center | `UNKNOWN` | Reported with an unknown state. | +| Gitea / Forgejo | `success` | Gitea does not support `skipped`. Reported as `success` with a "Skipped" description. | + +## Example + +Consider a repository with two PipelineRuns: + +- `.tekton/build.yaml` -- targets `pull_request` events on the `main` branch +- `.tekton/deploy.yaml` -- targets `push` events on the `main` branch + +When a pull request is opened, `build.yaml` matches and runs. Without +`status_checks`, `deploy.yaml` is silently ignored. With it enabled: + +```yaml +spec: + settings: + status_checks: + enabled: true + mode: "per_pipelinerun" +``` + +Pipelines-as-Code reports a `skipped` status for `deploy.yaml`, making it +visible in the pull request's status checks that the PipelineRun exists but did +not apply to this event. diff --git a/docs/content/docs/guides/statuses.md b/docs/content/docs/guides/statuses.md index ea493f0a38..1cbda4db09 100644 --- a/docs/content/docs/guides/statuses.md +++ b/docs/content/docs/guides/statuses.md @@ -113,6 +113,10 @@ You can use the `tkn pac describe` command from the [CLI]({{< relref "/docs/cli/ all statuses of PipelineRuns associated with your repository and their metadata. +## Status checks for unmatched PipelineRuns + +By default, Pipelines-as-Code only reports status for PipelineRuns that matched and ran. If you want to see which PipelineRuns were skipped because their annotations did not match the incoming event, enable the `status_checks` setting in your Repository CR. See the [Status Checks guide]({{< relref "/docs/guides/repository-crd/status-checks" >}}) for configuration and provider behavior. + ## Notifications Pipelines-as-Code does not manage notifications directly. Instead, you can add notifications to your PipelineRuns using the [finally feature of diff --git a/docs/content/docs/operations/global-repository-settings.md b/docs/content/docs/operations/global-repository-settings.md index ca1caeb390..f3ec0c5bca 100644 --- a/docs/content/docs/operations/global-repository-settings.md +++ b/docs/content/docs/operations/global-repository-settings.md @@ -32,6 +32,7 @@ You can define the following settings in the global Repository CR: - The `type` must be defined in the namespace repository settings and must match the `type` of the global repository (see below for an example). - [Custom Parameters]({{< relref "/docs/advanced/custom-parameters" >}}). - [Incoming Webhooks Rules]({{< relref "/docs/advanced/incoming-webhooks" >}}). +- [Status Checks]({{< relref "/docs/guides/repository-crd/status-checks" >}}). {{< callout type="info" >}} Global settings are only applied when running via a Git provider event; they are not applied when for example using the `tkn pac` cli. diff --git a/pkg/apis/pipelinesascode/v1alpha1/types.go b/pkg/apis/pipelinesascode/v1alpha1/types.go index 8c205f7d6e..37a592aba3 100644 --- a/pkg/apis/pipelinesascode/v1alpha1/types.go +++ b/pkg/apis/pipelinesascode/v1alpha1/types.go @@ -4,6 +4,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + StatusCheckModePerPipelineRun = "per_pipelinerun" +) + // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object @@ -135,6 +139,10 @@ type Settings struct { // AIAnalysis contains AI/LLM analysis configuration for automated CI/CD pipeline analysis. // +optional AIAnalysis *AIAnalysisConfig `json:"ai,omitempty"` + + // StatusChecks configures the status checks for the repository. + // +optional + StatusChecks *StatusChecks `json:"status_checks,omitempty"` } type GitlabSettings struct { @@ -182,6 +190,30 @@ type ForgejoSettings struct { CommentStrategy string `json:"comment_strategy,omitempty"` } +// StatusChecks configures the status checks for the repository. +type StatusChecks struct { + // Enabled defines if the status checks should be reported. Default is false. + // +optional + Enabled bool `json:"enabled,omitempty"` + + // Mode defines how the status checks should be reported when is enabled. + // Options: + // - 'per_pipelinerun': Report the status check of each PipelineRun separately. + // +optional + // +kubebuilder:validation:Enum="";per_pipelinerun + Mode string `json:"mode,omitempty"` + + // UnmatchedConclusion defines the conclusion to report when pipeline run is not matched. Default is 'skipped'. + // this will be used only if mode is 'per_pipelinerun'. + // Options: + // - 'success': Report as success. + // - 'neutral': Report as neutral. + // - 'skipped': Report as skipped. Default. + // +optional + // +kubebuilder:validation:Enum="";success;neutral;skipped + UnmatchedConclusion string `json:"unmatched_conclusion,omitempty"` +} + func (s *Settings) Merge(newSettings *Settings) { if newSettings.PipelineRunProvenance != "" && s.PipelineRunProvenance == "" { s.PipelineRunProvenance = newSettings.PipelineRunProvenance @@ -211,6 +243,10 @@ func (s *Settings) Merge(newSettings *Settings) { if newSettings.GitOpsCommandPrefix != "" && s.GitOpsCommandPrefix == "" { s.GitOpsCommandPrefix = newSettings.GitOpsCommandPrefix } + + if newSettings.StatusChecks != nil && s.StatusChecks == nil { + s.StatusChecks = newSettings.StatusChecks + } } func (s *GitlabSettings) Merge(newSettings *GitlabSettings) { diff --git a/pkg/apis/pipelinesascode/v1alpha1/types_test.go b/pkg/apis/pipelinesascode/v1alpha1/types_test.go index de8df17e9d..6b0276fc72 100644 --- a/pkg/apis/pipelinesascode/v1alpha1/types_test.go +++ b/pkg/apis/pipelinesascode/v1alpha1/types_test.go @@ -343,6 +343,92 @@ func TestMergeSpecs(t *testing.T) { }, }, }, + { + name: "status check from global", + local: &RepositorySpec{ + Settings: &Settings{}, + GitProvider: &GitProvider{}, + }, + global: RepositorySpec{ + Settings: &Settings{ + StatusChecks: &StatusChecks{ + Enabled: true, + Mode: "per_pipelinerun", + UnmatchedConclusion: "skipped", + }, + }, + GitProvider: &GitProvider{}, + }, + expected: &RepositorySpec{ + Settings: &Settings{ + StatusChecks: &StatusChecks{ + Enabled: true, + Mode: "per_pipelinerun", + UnmatchedConclusion: "skipped", + }, + }, + GitProvider: &GitProvider{}, + }, + }, + { + name: "local status check takes precedence", + local: &RepositorySpec{ + Settings: &Settings{ + StatusChecks: &StatusChecks{ + Enabled: true, + Mode: "per_pipelinerun", + UnmatchedConclusion: "skipped", + }, + }, + GitProvider: &GitProvider{}, + }, + global: RepositorySpec{ + Settings: &Settings{ + StatusChecks: &StatusChecks{ + Enabled: true, + Mode: "per_pipelinerun", + UnmatchedConclusion: "neutral", + }, + }, + GitProvider: &GitProvider{}, + }, + expected: &RepositorySpec{ + Settings: &Settings{ + StatusChecks: &StatusChecks{ + Enabled: true, + Mode: "per_pipelinerun", + UnmatchedConclusion: "skipped", + }, + }, + GitProvider: &GitProvider{}, + }, + }, + { + name: "nil local settings inherits global status check", + local: &RepositorySpec{ + GitProvider: &GitProvider{}, + }, + global: RepositorySpec{ + Settings: &Settings{ + StatusChecks: &StatusChecks{ + Enabled: true, + Mode: "per_pipelinerun", + UnmatchedConclusion: "success", + }, + }, + GitProvider: &GitProvider{}, + }, + expected: &RepositorySpec{ + Settings: &Settings{ + StatusChecks: &StatusChecks{ + Enabled: true, + Mode: "per_pipelinerun", + UnmatchedConclusion: "success", + }, + }, + GitProvider: &GitProvider{}, + }, + }, } for _, tt := range tests { diff --git a/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go index 3483c6b410..511eb44ad7 100644 --- a/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipelinesascode/v1alpha1/zz_generated.deepcopy.go @@ -1,21 +1,5 @@ //go:build !ignore_autogenerated -/* -Copyright Red Hat - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - // Code generated by controller-gen. DO NOT EDIT. package v1alpha1 @@ -410,6 +394,11 @@ func (in *Settings) DeepCopyInto(out *Settings) { *out = new(AIAnalysisConfig) (*in).DeepCopyInto(*out) } + if in.StatusChecks != nil { + in, out := &in.StatusChecks, &out.StatusChecks + *out = new(StatusChecks) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Settings. @@ -422,6 +411,21 @@ func (in *Settings) DeepCopy() *Settings { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *StatusChecks) DeepCopyInto(out *StatusChecks) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StatusChecks. +func (in *StatusChecks) DeepCopy() *StatusChecks { + if in == nil { + return nil + } + out := new(StatusChecks) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskInfos) DeepCopyInto(out *TaskInfos) { *out = *in diff --git a/pkg/matcher/annotation_matcher.go b/pkg/matcher/annotation_matcher.go index f9a7d95086..fdc79eb495 100644 --- a/pkg/matcher/annotation_matcher.go +++ b/pkg/matcher/annotation_matcher.go @@ -208,8 +208,10 @@ func checkPipelineRunAnnotation(prun *tektonv1.PipelineRun, eventEmitter *events } } -func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger, pruns []*tektonv1.PipelineRun, cs *params.Run, event *info.Event, vcx provider.Interface, eventEmitter *events.EventEmitter, repo *apipac.Repository, reportErrors bool) ([]Match, error) { +func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger, pruns []*tektonv1.PipelineRun, cs *params.Run, event *info.Event, vcx provider.Interface, eventEmitter *events.EventEmitter, repo *apipac.Repository, reportErrors bool) ([]Match, []*tektonv1.PipelineRun, error) { matchedPRs := []Match{} + unmatchedPRs := make([]*tektonv1.PipelineRun, 0, len(pruns)) + isAlreadyAddedInUnmatched := false logger.Debugf("MatchPipelinerunByAnnotation: pipelineruns=%d event_type=%s trigger_target=%s report_errors=%t", len(pruns), event.EventType, event.TriggerTarget, reportErrors) infomsg := fmt.Sprintf( "matching pipelineruns to event: URL=%s, target-branch=%s, source-branch=%s, target-event=%s", @@ -240,6 +242,7 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger celValidationErrors := []*pacerrors.PacYamlValidations{} for _, prun := range pruns { + isAlreadyAddedInUnmatched = false logger.Debugf("MatchPipelinerunByAnnotation: evaluating pipelinerun=%s annotations=%d", getName(prun), len(prun.GetObjectMeta().GetAnnotations())) prMatch := Match{ PipelineRun: prun, @@ -302,6 +305,9 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger matchedPRs = append(matchedPRs, prMatch) continue } + logger.Debugf("PipelineRun %s: on-comment annotation did not match, skipping", prName) + unmatchedPRs = append(unmatchedPRs, prun) + isAlreadyAddedInUnmatched = true } // if the event is a comment event, but we don't have any match from the keys.OnComment then skip the other evaluations if event.EventType == opscomments.NoOpsCommentEventType.String() || event.EventType == opscomments.OnCommentEventType.String() { @@ -342,6 +348,10 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger logger.Debugf("PipelineRun %s: CEL result=%v", prName, out) if out != types.True { logger.Infof("CEL expression for PipelineRun %s is not matching, skipping", prName) + if !isAlreadyAddedInUnmatched { + unmatchedPRs = append(unmatchedPRs, prun) + } + // won't make isAlreadyAddedInUnmatched true because loop is continuing from here continue } logger.Infof("CEL expression has been evaluated and matched") @@ -357,10 +367,14 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger matched, targetEvent, targetBranch, err := getTargetBranch(prun, event) if err != nil { - return matchedPRs, err + return matchedPRs, unmatchedPRs, err } if !matched { logger.Debugf("PipelineRun %s: target branch/event did not match", prName) + if !isAlreadyAddedInUnmatched { + unmatchedPRs = append(unmatchedPRs, prun) + } + // won't make isAlreadyAddedInUnmatched true because loop is continuing from here continue } prMatch.Config["target-branch"] = targetBranch @@ -378,10 +392,14 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger // our own path changes. we may split up if needed to refine. matched, err := matchOnAnnotation(key, changedFiles.All, true) if err != nil { - return matchedPRs, err + return matchedPRs, unmatchedPRs, err } if !matched { logger.Debugf("PipelineRun %s: path-change annotation did not match", prName) + if !isAlreadyAddedInUnmatched { + unmatchedPRs = append(unmatchedPRs, prun) + } + // won't make isAlreadyAddedInUnmatched true because loop is continuing from here continue } logger.Infof("matched PipelineRun with name: %s, annotation PathChange: %q", prName, key) @@ -391,10 +409,14 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger if key, ok := prun.GetObjectMeta().GetAnnotations()[keys.OnLabel]; ok { matched, err := matchOnAnnotation(key, event.PullRequestLabel, false) if err != nil { - return matchedPRs, err + return matchedPRs, unmatchedPRs, err } if !matched { - logger.Debugf("PipelineRun %s: label annotation did not match", prName) + logger.Debugf("PipelineRun %s: label annotation did not match, skipping", prName) + if !isAlreadyAddedInUnmatched { + unmatchedPRs = append(unmatchedPRs, prun) + } + // won't make isAlreadyAddedInUnmatched true because loop is continuing from here continue } logger.Infof("matched PipelineRun with name: %s, annotation Label: %q", prName, key) @@ -412,10 +434,14 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger // our own path changes. we may split up if needed to refine. matched, err := matchOnAnnotation(key, changedFiles.All, true) if err != nil { - return matchedPRs, err + return matchedPRs, unmatchedPRs, err } if matched { logger.Infof("Skipping pipelinerun with name: %s, annotation PathChangeIgnore: %q", prName, key) + if !isAlreadyAddedInUnmatched { + unmatchedPRs = append(unmatchedPRs, prun) + } + // won't make isAlreadyAddedInUnmatched true because loop is continuing from here continue } prMatch.Config["path-change-ignore"] = key @@ -439,14 +465,14 @@ func MatchPipelinerunByAnnotation(ctx context.Context, logger *zap.SugaredLogger logger.Debugf("MatchPipelinerunByAnnotation: filtering successful templates for event_type=%s", event.EventType) filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, vcx, matchedPRs) if len(filtered) == 0 { - return nil, NoFailedPipelineToRetestError(provider.GetGitOpsCommentPrefix(repo)) + return nil, unmatchedPRs, NoFailedPipelineToRetestError(provider.GetGitOpsCommentPrefix(repo)) } - return filtered, nil + return filtered, unmatchedPRs, nil } - return matchedPRs, nil + return matchedPRs, unmatchedPRs, nil } - return nil, fmt.Errorf("%s", buildAvailableMatchingAnnotationErr(event, pruns)) + return nil, unmatchedPRs, fmt.Errorf("%s", buildAvailableMatchingAnnotationErr(event, pruns)) } // filterSuccessfulTemplates filters out templates that already have successful PipelineRuns diff --git a/pkg/matcher/annotation_matcher_test.go b/pkg/matcher/annotation_matcher_test.go index bfcb5a0438..4b5156454f 100644 --- a/pkg/matcher/annotation_matcher_test.go +++ b/pkg/matcher/annotation_matcher_test.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "reflect" + "slices" "strings" "testing" "time" @@ -1699,7 +1700,7 @@ func runTest(ctx context.Context, t *testing.T, tt annotationTest, vcx provider. repo = tt.args.data.Repositories[0] } - matches, err := MatchPipelinerunByAnnotation( + matches, _, err := MatchPipelinerunByAnnotation( ctx, logger, tt.args.pruns, client, &tt.args.runevent, vcx, eventEmitter, repo, true, @@ -1747,6 +1748,15 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) { }, } + pipelinePushCel := &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipeline-push-cel", + Annotations: map[string]string{ + keys.OnCelExpression: `event == "push"`, + }, + }, + } + pipelinePush := &tektonv1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Name: "pipeline-push", @@ -1766,6 +1776,17 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) { }, } + pipelineOnLabel := &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipeline-on-label", + Annotations: map[string]string{ + keys.OnLabel: "[bug]", + keys.OnEvent: "[pull_request]", + keys.OnTargetBranch: "[main]", + }, + }, + } + pipelineOther := &tektonv1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Name: "pipeline-other", @@ -1806,7 +1827,7 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) { }, } - observer, log := zapobserver.New(zap.InfoLevel) + observer, log := zapobserver.New(zap.DebugLevel) logger := zap.New(observer).Sugar() pipelinePullRequestForRetest := &tektonv1.PipelineRun{ @@ -1828,8 +1849,8 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) { args args wantErr bool wantPrName string + wantUnmatchedPRs []string wantLog []string - logLevel int repo *v1alpha1.Repository seedData *testclient.Data wantErrNoFailedPipelineToRetest bool @@ -2342,6 +2363,68 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) { }}, }, }, + { + name: "good-match-with-only-one-unmatched-pipeline-run", + args: args{ + pruns: []*tektonv1.PipelineRun{pipelineGood, pipelinePushCel}, + runevent: info.Event{ + URL: "https://hello/moto", + TriggerTarget: "pull_request", + EventType: "pull_request", + HeadBranch: "source", + BaseBranch: "main", + PullRequestNumber: 10, + Request: &info.Request{ + Header: http.Header{}, + }, + }, + }, + wantErr: false, + wantUnmatchedPRs: []string{"pipeline-push-cel"}, + wantLog: []string{"CEL expression for PipelineRun pipeline-push-cel is not matching, skipping"}, + }, + { + name: "good-match-with-only-one-unmatched-pipeline-run-on-comment", + args: args{ + pruns: []*tektonv1.PipelineRun{pipelineOnComment}, + runevent: info.Event{ + URL: "https://hello/moto", + TriggerTarget: "pull_request", + EventType: "no-ops-comment", + HeadBranch: "source", + BaseBranch: "main", + PullRequestNumber: 10, + TriggerComment: "/bye-world", + Request: &info.Request{ + Header: http.Header{}, + }, + }, + }, + wantErr: true, + wantUnmatchedPRs: []string{"pipeline-on-comment"}, + wantLog: []string{"PipelineRun pipeline-on-comment: on-comment annotation did not match, skipping"}, + }, + { + name: "good-match-with-only-one-unmatched-pipeline-run-on-label", + args: args{ + pruns: []*tektonv1.PipelineRun{pipelineOnLabel}, + runevent: info.Event{ + URL: "https://hello/moto", + TriggerTarget: "pull_request", + EventType: "pull_request_labeled", + HeadBranch: "source", + BaseBranch: "main", + PullRequestNumber: 10, + PullRequestLabel: []string{"feature"}, + Request: &info.Request{ + Header: http.Header{}, + }, + }, + }, + wantErr: true, + wantUnmatchedPRs: []string{"pipeline-on-label"}, + wantLog: []string{"PipelineRun pipeline-on-label: label annotation did not match, skipping"}, + }, } for _, tt := range tests { @@ -2359,7 +2442,7 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) { eventEmitter := events.NewEventEmitter(cs.Clients.Kube, logger) repo := tt.repo - matches, err := MatchPipelinerunByAnnotation(ctx, logger, tt.args.pruns, cs, &tt.args.runevent, &ghprovider.Provider{}, eventEmitter, repo, true) + matches, unmatchedPRs, err := MatchPipelinerunByAnnotation(ctx, logger, tt.args.pruns, cs, &tt.args.runevent, &ghprovider.Provider{}, eventEmitter, repo, true) if tt.wantErrNoFailedPipelineToRetest { assert.Assert(t, err != nil, "expected ErrNoFailedPipelineToRetest") assert.Assert(t, errors.Is(err, NoFailedPipelineToRetestError("/pac ")), "expected ErrNoFailedPipelineToRetest, got: %v", err) @@ -2376,6 +2459,13 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) { assert.Assert(t, matches[0].PipelineRun.GetName() == tt.wantPrName, "Pipelinerun hasn't been matched: %+v", matches[0].PipelineRun.GetName(), tt.wantPrName) } + + if len(tt.wantUnmatchedPRs) > 0 { + assert.Assert(t, len(unmatchedPRs) == len(tt.wantUnmatchedPRs), "expected %d unmatched pipelineruns, got %d", len(tt.wantUnmatchedPRs), len(unmatchedPRs)) + for _, unmatchedPR := range unmatchedPRs { + assert.Assert(t, slices.Contains(tt.wantUnmatchedPRs, unmatchedPR.GetName()), "unmatched pipelinerun %s not in expected list", unmatchedPR.GetName()) + } + } if len(tt.wantLog) > 0 { assert.Assert(t, log.Len() > 0, "We didn't get any log message") all := log.TakeAll() diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go index db3df2f451..a1ff1d9c63 100644 --- a/pkg/pipelineascode/match.go +++ b/pkg/pipelineascode/match.go @@ -25,30 +25,30 @@ import ( "go.uber.org/zap" ) -func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, *v1alpha1.Repository, error) { +func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, []*tektonv1.PipelineRun, *v1alpha1.Repository, error) { p.debugf("matchRepoPR: starting repo verification for url=%s", p.event.URL) repo, err := p.verifyRepoAndUser(ctx) if err != nil { - return nil, nil, err + return nil, nil, repo, err } if repo == nil { p.debugf("matchRepoPR: no repository match for url=%s", p.event.URL) - return nil, nil, nil + return nil, nil, repo, nil } if p.event.CancelPipelineRuns { p.debugf("matchRepoPR: cancel pipeline runs requested, skipping match") - return nil, repo, p.cancelPipelineRunsOpsComment(ctx, repo) + return nil, nil, repo, p.cancelPipelineRunsOpsComment(ctx, repo) } p.debugf("matchRepoPR: fetching pipelineruns from repo=%s/%s", repo.GetNamespace(), repo.GetName()) - matchedPRs, err := p.getPipelineRunsFromRepo(ctx, repo) + matchedPRs, unmatchedPRs, err := p.getPipelineRunsFromRepo(ctx, repo) if err != nil { - return nil, repo, err + return nil, nil, repo, err } p.debugf("matchRepoPR: matched=%d repo=%s/%s", len(matchedPRs), repo.GetNamespace(), repo.GetName()) - return matchedPRs, repo, nil + return matchedPRs, unmatchedPRs, repo, nil } // verifyRepoAndUser verifies if the Repo CR exists for the Git Repository, @@ -143,7 +143,7 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e } // getPipelineRunsFromRepo fetches pipelineruns from git repository and prepare them for creation. -func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Repository) ([]matcher.Match, error) { +func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Repository) ([]matcher.Match, []*tektonv1.PipelineRun, error) { provenance := "source" if repo.Spec.Settings != nil && repo.Spec.Settings.PipelineRunProvenance != "" { provenance = repo.Spec.Settings.PipelineRunProvenance @@ -168,10 +168,10 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep }, }, ) - return nil, nil + return nil, nil, nil } - return nil, err + return nil, nil, err } if err != nil { @@ -203,14 +203,14 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep msg = fmt.Sprintf("cannot locate templates in %s/ directory for this repository in %s", tektonDir, p.event.HeadBranch) } p.eventEmitter.EmitMessage(nil, logLevel, reason, msg) - return nil, nil + return nil, nil, nil } // check for condition if need update the pipelinerun with regexp from the // "raw" pipelinerun string if msg, needUpdate := p.checkNeedUpdate(rawTemplates); needUpdate { p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryNeedUpdate", msg) - return nil, fmt.Errorf("%s", msg) + return nil, nil, fmt.Errorf("%s", msg) } // This is for bitbucket @@ -232,11 +232,11 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep if p.event.TargetTestPipelineRun == "" { rtypes, err := resolve.ReadTektonTypes(ctx, p.logger, rawTemplates) if err != nil { - return nil, err + return nil, nil, err } p.debugf("getPipelineRunsFromRepo: pre-parse types: pipelineruns=%d pipelines=%d tasks=%d", len(rtypes.PipelineRuns), len(rtypes.Pipelines), len(rtypes.Tasks)) // Don't fail or do anything if we don't have a match yet, we will do it properly later in this function - _, _ = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, rtypes.PipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, false) + _, _, _ = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, rtypes.PipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, false) } // Replace those {{var}} placeholders user has in her template to the run.Info variable allTemplates := p.makeTemplate(ctx, repo, rawTemplates) @@ -244,7 +244,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep types, err := resolve.ReadTektonTypes(ctx, p.logger, allTemplates) if err != nil { - return nil, err + return nil, nil, err } p.debugf("getPipelineRunsFromRepo: parsed types: pipelineruns=%d pipelines=%d tasks=%d validation_errors=%d", len(types.PipelineRuns), len(types.Pipelines), len(types.Tasks), len(types.ValidationErrors)) @@ -255,7 +255,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep if len(pipelineRuns) == 0 { msg := fmt.Sprintf("cannot locate valid templates in %s/ directory for this repository in %s", tektonDir, p.event.HeadBranch) p.eventEmitter.EmitMessage(nil, zap.InfoLevel, "RepositoryCannotLocatePipelineRun", msg) - return nil, nil + return nil, nil, nil } p.debugf("getPipelineRunsFromRepo: pipelineRuns count=%d", len(pipelineRuns)) pipelineRuns, err = resolve.MetadataResolve(pipelineRuns) @@ -264,26 +264,27 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep // reporting creates a comment, which triggers another webhook, which hits the same error. if p.event.EventType == opscomments.NoOpsCommentEventType.String() { p.logger.Infof("skipping MetadataResolve error for no-ops comment event: %s", err) - return nil, nil + return nil, nil, nil } p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "FailedToResolvePipelineRunMetadata", err.Error()) - return nil, err + return nil, nil, err } p.debugf("getPipelineRunsFromRepo: metadata resolved for pipelineRuns count=%d", len(pipelineRuns)) // Match the PipelineRun with annotation var matchedPRs []matcher.Match + var unmatchedPRs []*tektonv1.PipelineRun if p.event.TargetTestPipelineRun == "" { - if matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, true); err != nil { + if matchedPRs, unmatchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, true); err != nil { prefix := provider.GetGitOpsCommentPrefix(repo) // Check if all pipelines have already succeeded - post comment so user gets feedback if errors.Is(err, matcher.NoFailedPipelineToRetestError(prefix)) { p.logger.Infof("RepositoryAllPipelinesSucceeded: %s", err.Error()) p.eventEmitter.EmitMessage(nil, zap.InfoLevel, "RepositoryAllPipelinesSucceeded", err.Error()) if commentErr := p.vcx.CreateComment(ctx, p.event, err.Error(), ""); commentErr != nil { - return nil, fmt.Errorf("error adding no pipelineruns to rerun comment: %w", commentErr) + return nil, nil, fmt.Errorf("error adding no pipelineruns to rerun comment: %w", commentErr) } - return nil, nil + return nil, unmatchedPRs, nil } // Don't fail when you don't have a match between pipeline and annotations p.eventEmitter.EmitMessage(nil, zap.WarnLevel, "RepositoryNoMatch", err.Error()) @@ -298,7 +299,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep } p.eventEmitter.EmitMessage(nil, zap.InfoLevel, "RepositoryNoMatch", text) } - return nil, nil + return nil, unmatchedPRs, nil } p.debugf("getPipelineRunsFromRepo: initial match count=%d", len(matchedPRs)) } @@ -314,7 +315,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep AccessDenied: true, } if allowed, err := p.checkAccessOrError(ctx, repo, status, "by GitOps comment on push commit"); !allowed { - return nil, err + return nil, nil, err } } @@ -323,7 +324,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep if pipelineRuns == nil { msg := fmt.Sprintf("cannot find pipelinerun %s for matching an incoming event in this repository", p.event.TargetPipelineRun) p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryCannotLocatePipelineRunForIncomingEvent", msg) - return nil, nil + return nil, nil, nil } p.debugf("getPipelineRunsFromRepo: incoming filter result count=%d", len(pipelineRuns)) @@ -333,7 +334,7 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep if targetPR == nil { msg := fmt.Sprintf("cannot find the targeted pipelinerun %s in this repository", p.event.TargetTestPipelineRun) p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryCannotLocatePipelineRun", msg) - return nil, nil + return nil, nil, nil } pipelineRuns = []*tektonv1.PipelineRun{targetPR} p.debugf("getPipelineRunsFromRepo: filtered to target pipelinerun=%s", p.event.TargetTestPipelineRun) @@ -361,13 +362,13 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep }) if err != nil { p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "RepositoryFailedToMatch", fmt.Sprintf("failed to match pipelineRuns: %s", err.Error())) - return nil, err + return nil, nil, err } } err = p.changePipelineRun(ctx, repo, pipelineRuns) if err != nil { - return nil, err + return nil, nil, err } p.debugf("getPipelineRunsFromRepo: updated pipelineRuns count=%d", len(pipelineRuns)) // if we are doing explicit /test command then we only want to run the one that has matched the /test @@ -377,30 +378,35 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep if selectedPr == nil { msg := fmt.Sprintf("cannot find the targeted pipelinerun %s in this repository", p.event.TargetTestPipelineRun) p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryCannotLocatePipelineRun", msg) - return nil, nil + return nil, nil, nil } selectedRepo := p.resolveTargetNamespaceRepo(ctx, repo, selectedPr) if selectedRepo == nil { msg := fmt.Sprintf("skipping pipelinerun %s: target-namespace repo not found", pipelineRunIdentifier(selectedPr)) p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryTargetNamespaceNotFound", msg) - return nil, nil + return nil, nil, nil } p.debugf("getPipelineRunsFromRepo: explicit /test using repo=%s/%s for pipelinerun=%s", selectedRepo.GetNamespace(), selectedRepo.GetName(), pipelineRunIdentifier(selectedPr)) return []matcher.Match{{ PipelineRun: selectedPr, Repo: selectedRepo, - }}, nil + }}, nil, nil } - matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, false) + // unmatchedPRs are filtered out above if RemoteTasks is enabled so we should also check if RemoteTasks is enabled then + // return unmatchedPRs from last call to MatchPipelinerunByAnnotation and if RemoteTasks is disabled then return unmatchedPRs from here + matchedPRs, unmatchedPRsNew, err := matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx, p.eventEmitter, repo, false) + if !p.pacInfo.RemoteTasks { + unmatchedPRs = unmatchedPRsNew + } if err != nil { // Don't fail when you don't have a match between pipeline and annotations p.eventEmitter.EmitMessage(nil, zap.WarnLevel, "RepositoryNoMatch", err.Error()) - return nil, nil + return nil, unmatchedPRs, nil } p.debugf("getPipelineRunsFromRepo: final match count=%d", len(matchedPRs)) - return matchedPRs, nil + return matchedPRs, unmatchedPRs, nil } func getRepositoryRevisionForProvenance(event *info.Event, provenance string) string { diff --git a/pkg/pipelineascode/match_test.go b/pkg/pipelineascode/match_test.go index 7d1b450bb2..bd2e9b6685 100644 --- a/pkg/pipelineascode/match_test.go +++ b/pkg/pipelineascode/match_test.go @@ -317,7 +317,7 @@ spec: nil, ) - matchedPRs, err := p.getPipelineRunsFromRepo(ctx, repositories[0]) + matchedPRs, _, err := p.getPipelineRunsFromRepo(ctx, repositories[0]) assert.NilError(t, err) if tt.wantNoMatch { assert.Equal(t, len(matchedPRs), 0) @@ -453,7 +453,7 @@ spec: p := NewPacs(event, vcx, cs, pacInfo, nil, logger, nil) p.eventEmitter = events.NewEventEmitter(stdata.Kube, logger) - matchedPRs, err := p.getPipelineRunsFromRepo(ctx, repo) + matchedPRs, _, err := p.getPipelineRunsFromRepo(ctx, repo) assert.NilError(t, err) assert.Equal(t, 1, len(matchedPRs)) assert.Equal(t, tt.wantRevision, vcx.fileInsideRepoRevision) @@ -523,14 +523,15 @@ func TestGetPipelineRunsFromRepo(t *testing.T) { } tests := []struct { - name string - repositories *v1alpha1.Repository - tektondir string - expectedNumberOfPruns int - event *info.Event - logSnippet string - wantErr bool - seedData *testclient.Data + name string + repositories *v1alpha1.Repository + tektondir string + expectedNumberOfPruns int + expectedNumberOfUnmatchedPruns int + event *info.Event + logSnippet string + wantErr bool + seedData *testclient.Data }{ { name: "more than one pipelinerun in .tekton dir", @@ -541,9 +542,10 @@ func TestGetPipelineRunsFromRepo(t *testing.T) { }, Spec: v1alpha1.RepositorySpec{}, }, - tektondir: "testdata/pull_request_multiplepipelineruns", - expectedNumberOfPruns: 2, - event: pullRequestEvent, + tektondir: "testdata/pull_request_multiplepipelineruns", + expectedNumberOfPruns: 2, + expectedNumberOfUnmatchedPruns: 1, + event: pullRequestEvent, }, { name: "single pipelinerun in .tekton dir", @@ -582,9 +584,10 @@ func TestGetPipelineRunsFromRepo(t *testing.T) { }, // we have 3 PR in there 2 that has a match on pull request and 1 that is a no-matching // matching those two that is matching here - tektondir: "testdata/no-match", - expectedNumberOfPruns: 2, - event: pullRequestEvent, + tektondir: "testdata/no-match", + expectedNumberOfPruns: 2, + expectedNumberOfUnmatchedPruns: 1, + event: pullRequestEvent, }, { name: "no-match pipelineruns in .tekton dir, only match the no-match", @@ -613,9 +616,10 @@ func TestGetPipelineRunsFromRepo(t *testing.T) { // if `testdata/no_yaml` dir is supplied here p.getPipelineRunsFromRepo func will return after // GetTektonDir so providing `testdat/push_branch` so that it should call MatchPipelineRunsByAnnotation // first and then create a neutral check-run. - tektondir: "testdata/push_branch", - expectedNumberOfPruns: 0, - event: okToTestEvent, + tektondir: "testdata/push_branch", + expectedNumberOfPruns: 0, + expectedNumberOfUnmatchedPruns: 1, + event: okToTestEvent, }, { name: "no .tekton dir in repository", @@ -751,7 +755,7 @@ func TestGetPipelineRunsFromRepo(t *testing.T) { vcx.SetPacInfo(pacInfo) p := NewPacs(tt.event, vcx, cs, pacInfo, k8int, logger, nil) p.eventEmitter = events.NewEventEmitter(stdata.Kube, logger) - matchedPRs, err := p.getPipelineRunsFromRepo(ctx, tt.repositories) + matchedPRs, unmatchedPRs, err := p.getPipelineRunsFromRepo(ctx, tt.repositories) if tt.wantErr { assert.Assert(t, err != nil, "expected an error but got nil") return @@ -765,6 +769,7 @@ func TestGetPipelineRunsFromRepo(t *testing.T) { assert.Assert(t, logCatcher.FilterMessageSnippet(tt.logSnippet).Len() > 0, logCatcher.All()) } assert.Equal(t, len(matchedPRNames), tt.expectedNumberOfPruns) + assert.Equal(t, len(unmatchedPRs), tt.expectedNumberOfUnmatchedPruns) }) } } diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go index 1808c6e722..1524dae943 100644 --- a/pkg/pipelineascode/pipelineascode.go +++ b/pkg/pipelineascode/pipelineascode.go @@ -81,7 +81,7 @@ func (p *PacRun) Run(ctx context.Context) error { return nil } - matchedPRs, repo, err := p.matchRepoPR(ctx) + matchedPRs, unmatchedPRs, repo, err := p.matchRepoPR(ctx) if err != nil { createStatusErr := p.vcx.CreateStatus(ctx, p.event, providerstatus.StatusOpts{ Status: CompletedStatus, @@ -103,6 +103,10 @@ func (p *PacRun) Run(ctx context.Context) error { p.debugf("match results: matched=%d repo=%s/%s", len(matchedPRs), repoNamespace, repoName) if len(matchedPRs) == 0 { p.debugf("no pipelineruns matched; returning without starting any runs") + // check and report if status check is enabled from repo CR settings here so that status reporting will be done early + // if there is no matched PipelineRun otherwise after all the pipelineruns are started, we will report the status check + // for all the unmatched pipelineruns, to not cause delay to matched PipelineRuns start and reporting process. + p.reportStatusCheckFromRepoSettings(ctx, repo, unmatchedPRs, "when there is no matched pipelinerun") return nil } if repo == nil { @@ -204,9 +208,57 @@ func (p *PacRun) Run(ctx context.Context) error { } } wg.Wait() + // report status check for unmatched pipelineruns after all the pipelineruns are started + p.reportStatusCheckFromRepoSettings(ctx, repo, unmatchedPRs, "after all the pipelineruns are started") return nil } +func (p *PacRun) reportStatusCheckFromRepoSettings(ctx context.Context, repo *v1alpha1.Repository, unmatchedPRs []*tektonv1.PipelineRun, whenMsg string) { + p.debugf("checking status check settings from repo CR %s", whenMsg) + // since only per_pipelinerun is supported now, we default to it if mode is not set but enabled is true + if repo != nil && repo.Spec.Settings != nil && repo.Spec.Settings.StatusChecks != nil && + repo.Spec.Settings.StatusChecks.Enabled && + (repo.Spec.Settings.StatusChecks.Mode == v1alpha1.StatusCheckModePerPipelineRun || repo.Spec.Settings.StatusChecks.Mode == "") { + p.debugf("status check is enabled from repo CR settings in mode=%s for %d unmatched pipelineruns", repo.Spec.Settings.StatusChecks.Mode, len(unmatchedPRs)) + } else { + return + } + + conclusion := providerstatus.Conclusion(repo.Spec.Settings.StatusChecks.UnmatchedConclusion) + if conclusion == "" { + conclusion = providerstatus.ConclusionSkipped + } + p.debugf("reporting status check from repo settings for %d unmatched pipelineruns with conclusion=%s", len(unmatchedPRs), conclusion) + + var wg sync.WaitGroup + for _, pr := range unmatchedPRs { + wg.Add(1) + + go func(pr *tektonv1.PipelineRun) { + defer wg.Done() + prName := pr.GetName() + if prName == "" { + prName = pr.GetGenerateName() + } + err := p.vcx.CreateStatus(ctx, p.event, providerstatus.StatusOpts{ + PipelineRunName: prName, + IsUnmatchedReport: true, + PipelineRun: pr, + OriginalPipelineRunName: prName, + DetailsURL: p.run.Clients.ConsoleUI().URL(), + Status: CompletedStatus, + Conclusion: conclusion, + Text: fmt.Sprintf("PipelineRun %s is not matched to event %s", prName, p.event.TriggerTarget.String()), + }) + // we don't return the error here because we want to report all the status checks + if err != nil { + p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "RepositoryStatusCheckReportFailed", fmt.Sprintf("error reporting status check from repo settings: %s", err.Error())) + } + }(pr) + } + wg.Wait() +} + func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.PipelineRun, error) { prName := match.PipelineRun.GetName() if prName == "" { diff --git a/pkg/pipelineascode/pipelineascode_statuscheck_test.go b/pkg/pipelineascode/pipelineascode_statuscheck_test.go new file mode 100644 index 0000000000..991c5d5e60 --- /dev/null +++ b/pkg/pipelineascode/pipelineascode_statuscheck_test.go @@ -0,0 +1,182 @@ +package pipelineascode + +import ( + "context" + "fmt" + "sync" + "testing" + + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "github.com/openshift-pipelines/pipelines-as-code/pkg/consoleui" + "github.com/openshift-pipelines/pipelines-as-code/pkg/events" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" + providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status" + testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" + testprovider "github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider" + tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "go.uber.org/zap" + zapobserver "go.uber.org/zap/zaptest/observer" + "gotest.tools/v3/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + rtesting "knative.dev/pkg/reconciler/testing" +) + +type statusCapturingProvider struct { + testprovider.TestProviderImp + mu sync.Mutex + statuses []providerstatus.StatusOpts +} + +func (s *statusCapturingProvider) CreateStatus(_ context.Context, _ *info.Event, opts providerstatus.StatusOpts) error { + if s.CreateStatusErorring { + return fmt.Errorf("some provider error occurred while reporting status") + } + s.mu.Lock() + s.statuses = append(s.statuses, opts) + s.mu.Unlock() + return nil +} + +func TestReportStatusCheckFromRepoSettings(t *testing.T) { + tests := []struct { + name string + noMatchConclusion string + unmatchedPRs []*tektonv1.PipelineRun + createStatusErorring bool + expectedConclusion providerstatus.Conclusion + expectedStatusCount int + expectedLogSnippet string + }{ + { + name: "default conclusion is skipped when not set", + noMatchConclusion: "", + unmatchedPRs: []*tektonv1.PipelineRun{ + {ObjectMeta: metav1.ObjectMeta{GenerateName: "pr-one-"}}, + }, + expectedConclusion: providerstatus.ConclusionSkipped, + expectedStatusCount: 1, + }, + { + name: "custom conclusion success", + noMatchConclusion: "success", + unmatchedPRs: []*tektonv1.PipelineRun{ + {ObjectMeta: metav1.ObjectMeta{GenerateName: "pr-one-"}}, + }, + expectedConclusion: providerstatus.ConclusionSuccess, + expectedStatusCount: 1, + }, + { + name: "custom conclusion neutral", + noMatchConclusion: "neutral", + unmatchedPRs: []*tektonv1.PipelineRun{ + {ObjectMeta: metav1.ObjectMeta{Name: "pr-with-name"}}, + }, + expectedConclusion: providerstatus.ConclusionNeutral, + expectedStatusCount: 1, + }, + { + name: "multiple unmatched pipelineruns", + noMatchConclusion: "skipped", + unmatchedPRs: []*tektonv1.PipelineRun{ + {ObjectMeta: metav1.ObjectMeta{GenerateName: "pr-one-"}}, + {ObjectMeta: metav1.ObjectMeta{GenerateName: "pr-two-"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "pr-three"}}, + }, + expectedConclusion: providerstatus.ConclusionSkipped, + expectedStatusCount: 3, + }, + { + name: "empty unmatched pipelineruns", + noMatchConclusion: "skipped", + unmatchedPRs: []*tektonv1.PipelineRun{}, + expectedStatusCount: 0, + }, + { + name: "uses name over generateName", + noMatchConclusion: "skipped", + unmatchedPRs: []*tektonv1.PipelineRun{ + {ObjectMeta: metav1.ObjectMeta{Name: "my-named-pr", GenerateName: "should-not-use-"}}, + }, + expectedConclusion: providerstatus.ConclusionSkipped, + expectedStatusCount: 1, + }, + { + name: "create status error emits log message", + noMatchConclusion: "skipped", + createStatusErorring: true, + unmatchedPRs: []*tektonv1.PipelineRun{ + {ObjectMeta: metav1.ObjectMeta{GenerateName: "pr-fail-"}}, + }, + expectedStatusCount: 0, + expectedLogSnippet: "error reporting status check from repo settings", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + observerCore, logCatcher := zapobserver.New(zap.InfoLevel) + logger := zap.New(observerCore).Sugar() + ctx, _ := rtesting.SetupFakeContext(t) + stdata, _ := testclient.SeedTestData(t, ctx, testclient.Data{}) + + vcx := &statusCapturingProvider{} + vcx.CreateStatusErorring = tt.createStatusErorring + + event := &info.Event{ + TriggerTarget: triggertype.PullRequest, + } + + repo := &v1alpha1.Repository{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-repo", + Namespace: "test-ns", + }, + Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{ + StatusChecks: &v1alpha1.StatusChecks{ + Enabled: true, + Mode: v1alpha1.StatusCheckModePerPipelineRun, + UnmatchedConclusion: tt.noMatchConclusion, + }, + }, + }, + } + + run := ¶ms.Run{ + Clients: clients.Clients{}, + } + run.Clients.SetConsoleUI(consoleui.FallBackConsole{}) + + p := &PacRun{ + event: event, + vcx: vcx, + run: run, + logger: logger, + eventEmitter: events.NewEventEmitter(stdata.Kube, logger), + } + + p.reportStatusCheckFromRepoSettings(ctx, repo, tt.unmatchedPRs, "") + + assert.Equal(t, len(vcx.statuses), tt.expectedStatusCount) + + for _, s := range vcx.statuses { + assert.Equal(t, s.Status, CompletedStatus) + assert.Equal(t, s.Conclusion, tt.expectedConclusion) + assert.Assert(t, s.PipelineRun != nil) + assert.Assert(t, s.PipelineRunName != "") + assert.Equal(t, s.PipelineRunName, s.OriginalPipelineRunName) + } + + if tt.name == "uses name over generateName" && len(vcx.statuses) > 0 { + assert.Equal(t, vcx.statuses[0].PipelineRunName, "my-named-pr") + } + + if tt.expectedLogSnippet != "" { + assert.Assert(t, logCatcher.FilterMessageSnippet(tt.expectedLogSnippet).Len() > 0, logCatcher.All()) + } + }) + } +} diff --git a/pkg/pipelineascode/pipelineascode_test.go b/pkg/pipelineascode/pipelineascode_test.go index cc3657527a..a40055bf98 100644 --- a/pkg/pipelineascode/pipelineascode_test.go +++ b/pkg/pipelineascode/pipelineascode_test.go @@ -232,6 +232,30 @@ func TestRun(t *testing.T) { finalStatus: "neutral", finalStatusText: "StatusDurationName", }, + { + name: "pull request/allowed with status check mode per unmatched pipelinerun", + runevent: info.Event{ + Event: &github.PullRequestEvent{ + PullRequest: &github.PullRequest{ + Number: new(666), + }, + }, + SHA: "fromwebhook", + Organization: "owner", + Sender: "owner", + Repository: "repo", + URL: "https://service/documentation", + HeadBranch: "press", + BaseBranch: "main", + EventType: "pull_request", + TriggerTarget: "pull_request", + PullRequestNumber: 666, + InstallationID: 1234, + }, + tektondir: "testdata/no-match", + finalStatus: "skipped", + finalStatusText: "PipelineRun no-match is not matched to event pull_request", + }, { name: "pull request/with webhook", runevent: info.Event{ diff --git a/pkg/pipelineascode/testdata/no-match/.tekton/nomatch.yaml b/pkg/pipelineascode/testdata/no-match/.tekton/nomatch.yaml index 79ae57effc..92e5ec9c8f 100644 --- a/pkg/pipelineascode/testdata/no-match/.tekton/nomatch.yaml +++ b/pkg/pipelineascode/testdata/no-match/.tekton/nomatch.yaml @@ -2,6 +2,9 @@ apiVersion: tekton.dev/v1beta1 kind: PipelineRun metadata: name: no-match + annotations: + pipelinesascode.tekton.dev/on-target-branch: "[non-existent-branch]" + pipelinesascode.tekton.dev/on-event: "[push]" spec: pipelineRef: name: pipeline1 diff --git a/pkg/provider/bitbucketcloud/bitbucket.go b/pkg/provider/bitbucketcloud/bitbucket.go index 878dcac658..11a0542f9f 100644 --- a/pkg/provider/bitbucketcloud/bitbucket.go +++ b/pkg/provider/bitbucketcloud/bitbucket.go @@ -107,7 +107,7 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts switch statusopts.Conclusion { case status.ConclusionSkipped: state = types.StateStopped - statusopts.Title = "➖ Skipping this commit" + statusopts.Title = "➖ Skipping this PipelineRun" case status.ConclusionNeutral: state = types.StateStopped statusopts.Title = "➖ CI has stopped" diff --git a/pkg/provider/bitbucketcloud/bitbucket_test.go b/pkg/provider/bitbucketcloud/bitbucket_test.go index 094e29c013..d7790ae9f3 100644 --- a/pkg/provider/bitbucketcloud/bitbucket_test.go +++ b/pkg/provider/bitbucketcloud/bitbucket_test.go @@ -343,6 +343,14 @@ func TestCreateStatus(t *testing.T) { }, expectedDescSubstr: "started", }, + { + name: "skipped", + status: status.StatusOpts{ + Conclusion: "skipped", + OriginalPipelineRunName: originalPipelineRunName, + }, + expectedDescSubstr: "Skipping", + }, { name: "success", status: status.StatusOpts{ diff --git a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go index b5612c5149..eb9a7e2d7c 100644 --- a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go +++ b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go @@ -97,8 +97,8 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp switch statusOpts.Conclusion { case status.ConclusionSkipped: - state = scm.StateFailure - statusOpts.Title = "➖ Skipping this commit" + state = scm.StateUnknown + statusOpts.Title = "➖ Skipping this PipelineRun" case status.ConclusionNeutral: state = scm.StateFailure statusOpts.Title = "➖ CI has stopped" diff --git a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go index fa5113d0c4..1e3d6e71a9 100644 --- a/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go +++ b/pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go @@ -211,6 +211,15 @@ func TestCreateStatus(t *testing.T) { Text: "Pending approval, waiting for an /ok-to-test", }, + pacOpts: pacopts, + }, + { + name: "good/skipped", + status: status.StatusOpts{ + Conclusion: "skipped", + Text: "Skipping", + }, + pacOpts: pacopts, }, } diff --git a/pkg/provider/gitea/gitea.go b/pkg/provider/gitea/gitea.go index c42c7ad8e0..c8616f5bdf 100644 --- a/pkg/provider/gitea/gitea.go +++ b/pkg/provider/gitea/gitea.go @@ -318,7 +318,10 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp case providerstatus.ConclusionNeutral: statusOpts.Title = "Unknown" statusOpts.Summary = "doesn't know what happened with this commit." - case providerstatus.ConclusionCancelled, providerstatus.ConclusionCompleted, providerstatus.ConclusionSkipped: + case providerstatus.ConclusionSkipped: + statusOpts.Title = "Skipped" + statusOpts.Summary = "has skipped." + case providerstatus.ConclusionCancelled, providerstatus.ConclusionCompleted: } if statusOpts.Status == "in_progress" { @@ -340,7 +343,9 @@ func (v *Provider) createStatusCommit(ctx context.Context, event *info.Event, pa state := forgejo.StatusState(status.Conclusion) switch status.Conclusion { case providerstatus.ConclusionNeutral: - state = forgejo.StatusSuccess // We don't have a choice than setting as success, no pending here.c + state = forgejo.StatusSuccess // We don't have a choice than setting as success, no pending here. + case providerstatus.ConclusionSkipped: + state = forgejo.StatusSuccess // We don't have a choice than setting as success, skipped is neither pending nor failure. case providerstatus.ConclusionPending: if status.Title != "" { state = forgejo.StatusPending @@ -393,7 +398,7 @@ func (v *Provider) createStatusCommit(ctx context.Context, event *info.Event, pa v.Logger.Warn("Comments related to PipelineRuns status have been disabled for Gitea/Forgejo pull requests") return nil case provider.UpdateCommentStrategy: - if eventType == triggertype.PullRequest || event.TriggerTarget == triggertype.PullRequest { + if !status.IsUnmatchedReport && eventType == triggertype.PullRequest || event.TriggerTarget == triggertype.PullRequest { status.Text = strings.ReplaceAll(strings.TrimSpace(status.Text), "
", "\n") statusComment := v.formatPipelineComment(event.SHA, status) // Creating the prefix that is added to the status comment for a pipeline run. diff --git a/pkg/provider/gitea/status_test.go b/pkg/provider/gitea/status_test.go index 49d82b2668..9d88d8b732 100644 --- a/pkg/provider/gitea/status_test.go +++ b/pkg/provider/gitea/status_test.go @@ -163,6 +163,7 @@ func TestProviderCreateStatus(t *testing.T) { } func TestProviderCreateStatusCommit(t *testing.T) { + commentCreationAPICalled := false type args struct { event *info.Event pacopts *info.PacOpts @@ -172,6 +173,7 @@ func TestProviderCreateStatusCommit(t *testing.T) { name string args args wantErr bool + wantCommentCreationAPICalled bool wantCommentJSON, wantStatusJSON string }{ { @@ -252,8 +254,9 @@ func TestProviderCreateStatusCommit(t *testing.T) { SHA: "123456", }, }, - wantStatusJSON: `{"state":"pending","target_url":"","description":"Pipeline run for myapp has been triggered","context":"myapp"}`, - wantCommentJSON: `{"body":"\ntime to get started"}`, + wantCommentCreationAPICalled: true, + wantStatusJSON: `{"state":"pending","target_url":"","description":"Pipeline run for myapp has been triggered","context":"myapp"}`, + wantCommentJSON: `{"body":"\ntime to get started"}`, }, { name: "cancel", @@ -274,8 +277,9 @@ func TestProviderCreateStatusCommit(t *testing.T) { SHA: "123456", }, }, - wantStatusJSON: `{"state":"pending","target_url":"","description":"Pipeline run for myapp has been triggered","context":"myapp"}`, - wantCommentJSON: `{"body":"\ntime to get started"}`, + wantCommentCreationAPICalled: true, + wantStatusJSON: `{"state":"pending","target_url":"","description":"Pipeline run for myapp has been triggered","context":"myapp"}`, + wantCommentJSON: `{"body":"\ntime to get started"}`, }, { name: "retest", @@ -296,12 +300,57 @@ func TestProviderCreateStatusCommit(t *testing.T) { SHA: "123456", }, }, - wantStatusJSON: `{"state":"pending","target_url":"","description":"Pipeline run for myapp has been triggered","context":"myapp"}`, - wantCommentJSON: `{"body":"\ntime to get started"}`, + wantCommentCreationAPICalled: true, + wantStatusJSON: `{"state":"pending","target_url":"","description":"Pipeline run for myapp has been triggered","context":"myapp"}`, + wantCommentJSON: `{"body":"\ntime to get started"}`, + }, + { + name: "skipped", + args: args{ + status: status.StatusOpts{ + Conclusion: status.ConclusionSkipped, + Title: "Skipped", + Text: "has skipped.", + }, + pacopts: &info.PacOpts{Settings: settings.Settings{ + ApplicationName: "myapp", + }}, + event: &info.Event{ + Organization: "myorg", + Repository: "myrepo", + PullRequestNumber: 1, + TriggerTarget: "pull_request", + SHA: "123456", + }, + }, + wantCommentCreationAPICalled: true, + wantStatusJSON: `{"state":"success","target_url":"","description":"Skipped","context":"myapp"}`, + wantCommentJSON: `{"body":"\nhas \u003cb\u003eskipped\u003c/b\u003e."}`, + }, + { + name: "unmatched report", + args: args{ + pacopts: &info.PacOpts{Settings: settings.Settings{ + ApplicationName: "myapp", + }}, + event: &info.Event{ + Organization: "myorg", + Repository: "myrepo", + PullRequestNumber: 1, + TriggerTarget: "pull_request", + SHA: "123456", + }, + status: status.StatusOpts{ + Conclusion: status.ConclusionSkipped, + IsUnmatchedReport: true, + }, + }, + wantStatusJSON: `{"state":"success","target_url":"","description":"","context":"myapp"}`, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + commentCreationAPICalled = false fakeclient, mux, teardown := tgitea.Setup(t) defer teardown() @@ -322,6 +371,7 @@ func TestProviderCreateStatusCommit(t *testing.T) { // Mock the CreateIssueComment API mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/issues/%d/comments", tt.args.event.Organization, tt.args.event.Repository, tt.args.event.PullRequestNumber), func(rw http.ResponseWriter, r *http.Request) { + commentCreationAPICalled = true body, err := io.ReadAll(r.Body) if err != nil { http.Error(rw, "Failed to read request body", http.StatusInternalServerError) @@ -341,6 +391,7 @@ func TestProviderCreateStatusCommit(t *testing.T) { if err := v.createStatusCommit(context.Background(), tt.args.event, tt.args.pacopts, tt.args.status); (err != nil) != tt.wantErr { t.Errorf("Provider.createStatusCommit() error = %v, wantErr %v", err, tt.wantErr) } + assert.Equal(t, tt.wantCommentCreationAPICalled, commentCreationAPICalled) }) } } diff --git a/pkg/provider/github/status.go b/pkg/provider/github/status.go index 92e7748b14..983dda8817 100644 --- a/pkg/provider/github/status.go +++ b/pkg/provider/github/status.go @@ -194,11 +194,14 @@ func (v *Provider) createCheckRunStatus(ctx context.Context, runevent *info.Even Summary: new(status.Summary), Text: new(status.Text), }, - DetailsURL: new(status.DetailsURL), ExternalID: new(status.PipelineRunName), StartedAt: &now, } + if status.DetailsURL != "" { + checkrunoption.DetailsURL = new(status.DetailsURL) + } + if status.Status != "in_progress" && status.Status != "queued" { checkrunoption.Conclusion = new(string(status.Conclusion)) } @@ -317,9 +320,10 @@ func (v *Provider) getOrUpdateCheckRunStatus(ctx context.Context, runevent *info } // Patch the pipelineRun with the checkRunID and logURL only when the pipelineRun is not nil and has a name - // because on validation failed PipelineRun will provide PipelineRun struct but it is not a valid resource - // created in cluster so if its only validation error report then ignore patching the pipelineRun. - if statusOpts.PipelineRun != nil && (statusOpts.PipelineRun.GetName() != "" || statusOpts.PipelineRun.GetGenerateName() != "") { + // and this is not an unmatched report because on validation failed PipelineRun will provide PipelineRun + // struct but it is not a valid resource created in cluster so if its only validation error report then ignore + // patching the pipelineRun. + if !statusOpts.IsUnmatchedReport && statusOpts.PipelineRun != nil && (statusOpts.PipelineRun.GetName() != "" || statusOpts.PipelineRun.GetGenerateName() != "") { if _, err := action.PatchPipelineRun(ctx, v.Logger, "checkRunID and logURL", v.Run.Clients.Tekton, statusOpts.PipelineRun, metadataPatch(checkRunID, statusOpts.DetailsURL)); err != nil { return err } @@ -399,6 +403,8 @@ func (v *Provider) createStatusCommit(ctx context.Context, runevent *info.Event, switch status.Conclusion { case providerstatus.ConclusionNeutral: status.Conclusion = providerstatus.ConclusionSuccess // We don't have a choice other than setting as success, no pending here. + case providerstatus.ConclusionSkipped: + status.Conclusion = providerstatus.ConclusionSuccess // GitHub commit status API doesn't support "skipped", map to success. case providerstatus.ConclusionPending: if status.Title != "" { status.Conclusion = providerstatus.ConclusionPending @@ -439,7 +445,7 @@ func (v *Provider) createStatusCommit(ctx context.Context, runevent *info.Event, return nil case provider.UpdateCommentStrategy: if (status.Status == "completed" || (status.Status == "queued" && status.Title == pendingApproval)) && - status.Text != "" && eventType == triggertype.PullRequest { + status.Text != "" && eventType == triggertype.PullRequest && !status.IsUnmatchedReport { statusComment := v.formatPipelineComment(runevent.SHA, status) // Creating the prefix that is added to the status comment for a pipeline run. plrStatusCommentPrefix := fmt.Sprintf(provider.PlrStatusCommentPrefixTemplate, status.OriginalPipelineRunName) @@ -458,7 +464,7 @@ func (v *Provider) createStatusCommit(ctx context.Context, runevent *info.Event, } default: if (status.Status == "completed" || (status.Status == "queued" && status.Title == pendingApproval)) && - status.Text != "" && eventType == triggertype.PullRequest { + status.Text != "" && eventType == triggertype.PullRequest && !status.IsUnmatchedReport { _, _, err = wrapAPI(v, "create_issue_comment", func() (*github.IssueComment, *github.Response, error) { return v.Client().Issues.CreateComment( ctx, runevent.Organization, runevent.Repository, @@ -513,7 +519,10 @@ func (v *Provider) CreateStatus(ctx context.Context, runevent *info.Event, statu statusOpts.Title = "Unknown" } statusOpts.Summary = "Completed" - case providerstatus.ConclusionCompleted, providerstatus.ConclusionSkipped: + case providerstatus.ConclusionSkipped: + statusOpts.Title = "Skipped" + statusOpts.Summary = "has skipped this PipelineRun." + case providerstatus.ConclusionCompleted: } if statusOpts.Status == "in_progress" { diff --git a/pkg/provider/github/status_test.go b/pkg/provider/github/status_test.go index bf75ed969b..df8ac15409 100644 --- a/pkg/provider/github/status_test.go +++ b/pkg/provider/github/status_test.go @@ -14,6 +14,7 @@ import ( "github.com/google/go-github/v91/github" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys" + pipelinesascode "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" @@ -25,82 +26,145 @@ import ( tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "gotest.tools/v3/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + k8stesting "k8s.io/client-go/testing" rtesting "knative.dev/pkg/reconciler/testing" ) -func TestGithubProviderCreateCheckRun(t *testing.T) { - ctx, _ := rtesting.SetupFakeContext(t) - fakeclient, mux, _, teardown := ghtesthelper.SetupGH() - l, _ := logger.GetLogger() - cnx := Provider{ - ghClient: fakeclient, - Run: params.New(), - pacInfo: &info.PacOpts{ - Settings: settings.Settings{ - ApplicationName: settings.PACApplicationNameDefaultValue, +func TestGetOrUpdateCheckRunStatus(t *testing.T) { + tests := []struct { + name string + statusOpts []providerstatus.StatusOpts + expectPatched bool + }{ + { + name: "create check run with pipeline run name", + expectPatched: true, + statusOpts: []providerstatus.StatusOpts{ + { + PipelineRunName: "pr1", + Status: "hello moto", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pr1", + Namespace: "default", + }, + }, + }, + }, + }, + { + name: "multiple failed PipelineRuns only creates one check run", + expectPatched: true, + statusOpts: []providerstatus.StatusOpts{ + { + PipelineRunName: "", + Title: "Failed", + InstanceCountForCheckRun: 0, + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "failed-pr", + Namespace: "default", + }, + }, + }, + { + PipelineRunName: "", + Title: "Failed", + InstanceCountForCheckRun: 1, + }, + }, + }, + { + name: "matched report patches PipelineRun", + expectPatched: true, + statusOpts: []providerstatus.StatusOpts{ + { + PipelineRunName: "matched-pr", + Status: "completed", + Conclusion: providerstatus.ConclusionSuccess, + Text: "PipelineRun matched", + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "matched-pr", + Namespace: "default", + }, + }, + }, + }, + }, + { + name: "unmatched report skips PipelineRun patch", + statusOpts: []providerstatus.StatusOpts{ + { + PipelineRunName: "unmatched-pr", + Status: "completed", + Conclusion: providerstatus.ConclusionSkipped, + Text: "PipelineRun not matched", + IsUnmatchedReport: true, + PipelineRun: &tektonv1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unmatched-pr", + Namespace: "default", + }, + }, + }, }, }, - Logger: l, } - defer teardown() - mux.HandleFunc("/repos/check/info/check-runs", func(w http.ResponseWriter, _ *http.Request) { - _, _ = fmt.Fprint(w, `{"id": 555}`) - }) - - mux.HandleFunc("/repos/check/info/check-runs/555", func(w http.ResponseWriter, _ *http.Request) { - _, _ = fmt.Fprint(w, `{"id": 555}`) - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, _ := rtesting.SetupFakeContext(t) + fakeclient, mux, _, teardown := ghtesthelper.SetupGH() + defer teardown() - event := &info.Event{ - Organization: "check", - Repository: "info", - SHA: "createCheckRunSHA", - } + l, _ := logger.GetLogger() + run := params.New() + testData := testclient.Data{} + for _, so := range tt.statusOpts { + if so.PipelineRun != nil { + testData.PipelineRuns = append(testData.PipelineRuns, so.PipelineRun) + } + } + stdata, _ := testclient.SeedTestData(t, ctx, testData) + run.Clients.Tekton = stdata.Pipeline - err := cnx.getOrUpdateCheckRunStatus(ctx, event, providerstatus.StatusOpts{ - PipelineRunName: "pr1", - Status: "hello moto", - }) - assert.NilError(t, err) -} + var patched bool + stdata.Pipeline.PrependReactor("patch", "pipelineruns", func(_ k8stesting.Action) (bool, runtime.Object, error) { + patched = true + return false, nil, nil + }) -func TestGetOrUpdateCheckRunStatusForMultipleFailedPipelineRun(t *testing.T) { - ctx, _ := rtesting.SetupFakeContext(t) - fakeclient, mux, _, teardown := ghtesthelper.SetupGH() - l, _ := logger.GetLogger() - cnx := Provider{ - ghClient: fakeclient, - Run: params.New(), - pacInfo: &info.PacOpts{}, - Logger: l, - } - defer teardown() - statusOptionData := []providerstatus.StatusOpts{{ - PipelineRunName: "", - Title: "Failed", - InstanceCountForCheckRun: 0, - }, { - PipelineRunName: "", - Title: "Failed", - InstanceCountForCheckRun: 1, - }} - mux.HandleFunc("/repos/check/info/check-runs", func(w http.ResponseWriter, _ *http.Request) { - _, _ = fmt.Fprint(w, `{"id": 555}`) - }) + cnx := Provider{ + ghClient: fakeclient, + Run: run, + pacInfo: &info.PacOpts{ + Settings: settings.Settings{ + ApplicationName: settings.PACApplicationNameDefaultValue, + }, + }, + Logger: l, + } - mux.HandleFunc("/repos/check/info/check-runs/555", func(w http.ResponseWriter, _ *http.Request) { - _, _ = fmt.Fprint(w, `{"id": 555}`) - }) + mux.HandleFunc("/repos/check/info/check-runs", func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprint(w, `{"id": 555}`) + }) + mux.HandleFunc("/repos/check/info/check-runs/555", func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprint(w, `{"id": 555}`) + }) - event := &info.Event{ - Organization: "check", - Repository: "info", - SHA: "createCheckRunSHA", - } + event := &info.Event{ + Organization: "check", + Repository: "info", + SHA: "createCheckRunSHA", + } - for i := range statusOptionData { - err := cnx.getOrUpdateCheckRunStatus(ctx, event, statusOptionData[i]) - assert.NilError(t, err) + for i := range tt.statusOpts { + err := cnx.getOrUpdateCheckRunStatus(ctx, event, tt.statusOpts[i]) + assert.NilError(t, err) + } + assert.Equal(t, patched, tt.expectPatched) + }) } } @@ -406,6 +470,34 @@ func TestGithubProviderCreateStatus(t *testing.T) { want: &github.CheckRun{ID: &resultid}, wantErr: false, }, + { + name: "skipped via github apps", + args: args{ + runevent: runEvent, + status: "completed", + conclusion: "skipped", + text: "PipelineRun not matched", + detailsURL: "https://cireport.com", + titleSubstr: "Skipped", + githubApps: true, + }, + want: &github.CheckRun{ID: &resultid}, + wantErr: false, + }, + { + name: "skipped via webhook", + args: args{ + runevent: runEvent, + status: "completed", + conclusion: "skipped", + text: "PipelineRun not matched", + detailsURL: "https://cireport.com", + titleSubstr: "Skipped", + githubApps: false, + }, + want: &github.CheckRun{ID: &resultid}, + wantErr: false, + }, { name: "unknown", args: args{ @@ -546,6 +638,7 @@ func TestGithubProviderCreateStatus(t *testing.T) { } func TestGithubProvidercreateStatusCommit(t *testing.T) { + commentCreationAPICalled := false issuenumber := 666 anevent := &info.Event{ Event: &github.PullRequestEvent{PullRequest: &github.PullRequest{Number: new(issuenumber)}}, @@ -556,11 +649,13 @@ func TestGithubProvidercreateStatusCommit(t *testing.T) { PullRequestNumber: issuenumber, } tests := []struct { - name string - event *info.Event - wantErr bool - status providerstatus.StatusOpts - expectedConclusion string + name string + event *info.Event + repo *pipelinesascode.Repository + wantErr bool + wantCommentCreationAPICalled bool + status providerstatus.StatusOpts + expectedConclusion string }{ { name: "completed", @@ -571,7 +666,8 @@ func TestGithubProvidercreateStatusCommit(t *testing.T) { Text: "Finito amigo", Conclusion: "completed", }, - expectedConclusion: "completed", + expectedConclusion: "completed", + wantCommentCreationAPICalled: true, }, { name: "in_progress", @@ -597,9 +693,37 @@ func TestGithubProvidercreateStatusCommit(t *testing.T) { }, expectedConclusion: "success", }, + { + name: "pull_request status skipped", + event: anevent, + status: providerstatus.StatusOpts{ + Conclusion: providerstatus.ConclusionSkipped, + }, + expectedConclusion: "success", + }, + { + name: "unmatched report", + event: anevent, + repo: &pipelinesascode.Repository{ + Spec: pipelinesascode.RepositorySpec{ + Settings: &pipelinesascode.Settings{ + Github: &pipelinesascode.GithubSettings{ + CommentStrategy: "", // keep it empty so that comment strategy will be default and try to create a comment + }, + }, + }, + }, + status: providerstatus.StatusOpts{ + Conclusion: providerstatus.ConclusionSkipped, + IsUnmatchedReport: true, + }, + expectedConclusion: "success", + wantCommentCreationAPICalled: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + commentCreationAPICalled = false fakeclient, mux, _, teardown := ghtesthelper.SetupGH() defer teardown() mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/statuses/%s", @@ -610,6 +734,7 @@ func TestGithubProvidercreateStatusCommit(t *testing.T) { if tt.status.Status == "completed" { mux.HandleFunc(fmt.Sprintf("/repos/%s/%s/issues/%d/comments", tt.event.Organization, tt.event.Repository, issuenumber), func(_ http.ResponseWriter, r *http.Request) { + commentCreationAPICalled = true body, _ := io.ReadAll(r.Body) assert.Equal(t, fmt.Sprintf(`{"body":"%s
%s"}`, tt.status.Summary, tt.status.Text)+"\n", string(body)) }) @@ -628,9 +753,14 @@ func TestGithubProvidercreateStatusCommit(t *testing.T) { Logger: l, } + if tt.repo != nil { + provider.repo = tt.repo + } + if err := provider.createStatusCommit(ctx, tt.event, tt.status); (err != nil) != tt.wantErr { t.Errorf("GetCommitInfo() error = %v, wantErr %v", err, tt.wantErr) } + assert.Equal(t, tt.wantCommentCreationAPICalled, commentCreationAPICalled) }) } } diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index 298e33cc66..f2599ee8f2 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -634,7 +634,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp v.Logger.Warn("Comments related to PipelineRuns status have been disabled for GitLab merge requests") return nil case provider.UpdateCommentStrategy: - if eventType == triggertype.PullRequest || provider.Valid(event.EventType, anyMergeRequestEventType) { + if eventType == triggertype.PullRequest || provider.Valid(event.EventType, anyMergeRequestEventType) && !statusOpts.IsUnmatchedReport { statusComment := v.formatPipelineComment(event.SHA, statusOpts) // Creating the prefix that is added to the status comment for a pipeline run. plrStatusCommentPrefix := fmt.Sprintf(provider.PlrStatusCommentPrefixTemplate, statusOpts.OriginalPipelineRunName) @@ -652,7 +652,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp } } default: - if eventType == triggertype.PullRequest || provider.Valid(event.EventType, anyMergeRequestEventType) { + if eventType == triggertype.PullRequest || provider.Valid(event.EventType, anyMergeRequestEventType) && !statusOpts.IsUnmatchedReport { mopt := &gitlab.CreateMergeRequestNoteOptions{Body: gitlab.Ptr(body)} _, _, err := v.Client().Notes.CreateMergeRequestNote(event.TargetProjectID, int64(event.PullRequestNumber), mopt) return err @@ -1142,6 +1142,9 @@ func (v *Provider) storePipelineID(ctx context.Context, statusOpts providerstatu // 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) { + if statusOpts.IsUnmatchedReport { + return + } pr := statusOpts.PipelineRun if pr == nil || (pr.GetName() == "" && pr.GetGenerateName() == "") { return diff --git a/pkg/provider/gitlab/gitlab_test.go b/pkg/provider/gitlab/gitlab_test.go index a7738bc8cd..3df307b4db 100644 --- a/pkg/provider/gitlab/gitlab_test.go +++ b/pkg/provider/gitlab/gitlab_test.go @@ -701,6 +701,122 @@ func TestCreateStatus(t *testing.T) { } } +func TestCreateStatusUnmatchedReportSkipsMRComment(t *testing.T) { + tests := []struct { + name string + isUnmatchedReport bool + repo *v1alpha1.Repository + wantNoteCreated bool + }{ + { + name: "unmatched report skips MR comment with default comment strategy", + isUnmatchedReport: true, + wantNoteCreated: false, + }, + { + name: "matched report creates MR comment with default comment strategy", + isUnmatchedReport: false, + wantNoteCreated: true, + }, + { + name: "unmatched report skips MR comment with update comment strategy", + isUnmatchedReport: true, + repo: &v1alpha1.Repository{ + Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{ + Gitlab: &v1alpha1.GitlabSettings{ + CommentStrategy: "update", + }, + }, + }, + }, + wantNoteCreated: false, + }, + { + name: "matched report creates MR comment with update comment strategy", + isUnmatchedReport: false, + repo: &v1alpha1.Repository{ + Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{ + Gitlab: &v1alpha1.GitlabSettings{ + CommentStrategy: "update", + }, + }, + }, + }, + wantNoteCreated: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(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{ + targetProjectID: 100, + run: run, + Logger: log, + repo: tt.repo, + pacInfo: &info.PacOpts{ + Settings: settings.Settings{ + ApplicationName: settings.PACApplicationNameDefaultValue, + }, + }, + eventEmitter: events.NewEventEmitter(run.Clients.Kube, log), + } + v.SetGitLabClient(client) + + event := &info.Event{ + TriggerTarget: "pull_request", + EventType: "Merge Request", + SourceProjectID: 400, + TargetProjectID: 400, + SHA: "abc123", + PullRequestNumber: 42, + } + + mux.HandleFunc("/user", func(rw http.ResponseWriter, _ *http.Request) { + fmt.Fprint(rw, `{"id": 100}`) + }) + + // Both source and target return errors so CreateStatus falls through to MR comment path + mux.HandleFunc("/projects/400/statuses/abc123", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusBadRequest) + fmt.Fprint(rw, `{"message": "400 Bad Request"}`) + }) + + noteCreated := false + mux.HandleFunc(fmt.Sprintf("/projects/%d/merge_requests/42/notes", event.TargetProjectID), func(rw http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + fmt.Fprint(rw, `[]`) + return + } + noteCreated = true + fmt.Fprint(rw, `{}`) + }) + + err := v.CreateStatus(ctx, event, providerstatus.StatusOpts{ + Conclusion: providerstatus.ConclusionSkipped, + OriginalPipelineRunName: "test-pr", + IsUnmatchedReport: tt.isUnmatchedReport, + }) + assert.NilError(t, err) + assert.Equal(t, tt.wantNoteCreated, noteCreated, "MR note creation mismatch") + }) + } +} + func TestCreateStatusPipelineIDSharedAcrossPipelineRuns(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) log, _ := logger.GetLogger() diff --git a/pkg/provider/status/status.go b/pkg/provider/status/status.go index a6cf21a401..0bd4fa6f24 100644 --- a/pkg/provider/status/status.go +++ b/pkg/provider/status/status.go @@ -34,4 +34,5 @@ type StatusOpts struct { Title string InstanceCountForCheckRun int AccessDenied bool + IsUnmatchedReport bool // if true, the status is reported for unmatched pipelineruns } diff --git a/test/bitbucket_cloud_pullrequest_test.go b/test/bitbucket_cloud_pullrequest_test.go index cbec9513b4..67e0cd52dd 100644 --- a/test/bitbucket_cloud_pullrequest_test.go +++ b/test/bitbucket_cloud_pullrequest_test.go @@ -11,6 +11,7 @@ import ( "github.com/ktrysmt/go-bitbucket" "github.com/mitchellh/mapstructure" + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketcloud/types" tbb "github.com/openshift-pipelines/pipelines-as-code/test/pkg/bitbucketcloud" @@ -208,6 +209,80 @@ func TestBitbucketCloudPRBuildStatusReported(t *testing.T) { assert.Equal(t, foundStatus, true, "should have found the status for the pipeline run") } +func TestBitbucketCloudPRSkippedStatusReported(t *testing.T) { + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + ctx := context.Background() + + runcnx, opts, bprovider, err := tbb.Setup(ctx) + if err != nil { + t.Skip(err.Error()) + return + } + opts.Settings = &v1alpha1.Settings{ + StatusChecks: &v1alpha1.StatusChecks{ + Enabled: true, + Mode: v1alpha1.StatusCheckModePerPipelineRun, + }, + } + bcrepo := tbb.CreateCRD(ctx, t, bprovider, runcnx, opts, targetNS) + targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + title := "TestPullRequest - " + targetRefName + + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-matching.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}, + ) + assert.NilError(t, err) + + // this is not going to match as it's targeting main branch on push event while we're gonna raise a pull request + skipEntry, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-skipped.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.Push.String(), map[string]string{}, + ) + assert.NilError(t, err) + entries[".tekton/pipelinerun-skipped.yaml"] = skipEntry[".tekton/pipelinerun-skipped.yaml"] + + pr, repobranch := tbb.MakePR(t, bprovider, runcnx, bcrepo, opts, title, targetRefName, entries) + defer tbb.TearDown(ctx, t, runcnx, bprovider, opts, pr.ID, targetRefName, targetNS, false) + + hash, ok := repobranch.Target["hash"].(string) + assert.Assert(t, ok) + + sopt := twait.SuccessOpt{ + TargetNS: targetNS, + OnEvent: triggertype.PullRequest.String(), + NumberofPRMatch: 1, + SHA: hash, + Title: title, + MinNumberStatus: 1, + } + twait.Succeeded(ctx, t, runcnx, opts, sopt) + + resp, err := bprovider.Client().Repositories.Commits.GetCommitStatuses(&bitbucket.CommitsOptions{ + Owner: opts.Organization, + RepoSlug: opts.Repo, + Revision: hash, + }) + assert.NilError(t, err) + + statusesMap, ok := resp.(map[string]any) + assert.Equal(t, ok, true, "cannot convert Bitbucket commit statuses response to map[string]any") + + statuses := []*types.Status{} + + err = mapstructure.Decode(statusesMap["values"], &statuses) + assert.NilError(t, err, fmt.Sprintf("cannot decode Bitbucket commit statuses from response payload: %v", err)) + + foundStatus := false + for _, status := range statuses { + if status.State == "STOPPED" && strings.Contains(status.Description, "Skipping this PipelineRun") { + foundStatus = true + break + } + } + assert.Equal(t, foundStatus, true, "should have found the status for the pipeline run") +} + // Local Variables: // compile-command: "go test -tags=e2e -v -run TestBitbucketCloudPullRequest$ ." // End: diff --git a/test/bitbucket_datacenter_dynamic_variables_test.go b/test/bitbucket_datacenter_dynamic_variables_test.go index 969af2d534..7692ad91ed 100644 --- a/test/bitbucket_datacenter_dynamic_variables_test.go +++ b/test/bitbucket_datacenter_dynamic_variables_test.go @@ -27,7 +27,7 @@ func TestBitbucketDataCenterDynamicVariables(t *testing.T) { ctx, runcnx, opts, client, err := tbbdc.Setup(ctx) assert.NilError(t, err) - repo := tbbdc.CreateCRD(ctx, t, client, runcnx, bitbucketWSOwner, targetNS) + repo := tbbdc.CreateCRD(ctx, t, client, runcnx, opts, bitbucketWSOwner, targetNS) runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name) defer tbbdc.TearDownNs(ctx, t, runcnx, targetNS) diff --git a/test/bitbucket_datacenter_on_comment_test.go b/test/bitbucket_datacenter_on_comment_test.go index d53614d0da..fe48a232d6 100644 --- a/test/bitbucket_datacenter_on_comment_test.go +++ b/test/bitbucket_datacenter_on_comment_test.go @@ -29,7 +29,7 @@ func TestBitbucketDataCenterNonGitopsCommentTriggersPipelineRun(t *testing.T) { ctx, runcnx, opts, client, err := tbbdc.Setup(ctx) assert.NilError(t, err) - repo := tbbdc.CreateCRD(ctx, t, client, runcnx, bitbucketWSOwner, targetNS) + repo := tbbdc.CreateCRD(ctx, t, client, runcnx, opts, bitbucketWSOwner, targetNS) runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name) defer tbbdc.TearDownNs(ctx, t, runcnx, targetNS) diff --git a/test/bitbucket_datacenter_pull_request_test.go b/test/bitbucket_datacenter_pull_request_test.go index 56a9670267..b5d66a3b75 100644 --- a/test/bitbucket_datacenter_pull_request_test.go +++ b/test/bitbucket_datacenter_pull_request_test.go @@ -6,9 +6,11 @@ import ( "context" "fmt" "os" + "strings" "testing" "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/params/triggertype" tbbdc "github.com/openshift-pipelines/pipelines-as-code/test/pkg/bitbucketdatacenter" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" @@ -29,7 +31,7 @@ func TestBitbucketDataCenterPullRequest(t *testing.T) { ctx, runcnx, opts, client, err := tbbdc.Setup(ctx) assert.NilError(t, err) - repo := tbbdc.CreateCRD(ctx, t, client, runcnx, bitbucketWSOwner, targetNS) + repo := tbbdc.CreateCRD(ctx, t, client, runcnx, opts, bitbucketWSOwner, targetNS) runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name) defer tbbdc.TearDownNs(ctx, t, runcnx, targetNS) @@ -63,7 +65,7 @@ func TestBitbucketDataCenterCELPathChangeInPullRequest(t *testing.T) { ctx, runcnx, opts, client, err := tbbdc.Setup(ctx) assert.NilError(t, err) - repo := tbbdc.CreateCRD(ctx, t, client, runcnx, bitbucketWSOwner, targetNS) + repo := tbbdc.CreateCRD(ctx, t, client, runcnx, opts, bitbucketWSOwner, targetNS) runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name) defer tbbdc.TearDownNs(ctx, t, runcnx, targetNS) @@ -101,7 +103,7 @@ func TestBitbucketDataCenterOnPathChangeAnnotationOnPRMerge(t *testing.T) { ctx, runcnx, opts, client, err := tbbdc.Setup(ctx) assert.NilError(t, err) - repo := tbbdc.CreateCRD(ctx, t, client, runcnx, bitbucketWSOwner, targetNS) + repo := tbbdc.CreateCRD(ctx, t, client, runcnx, opts, bitbucketWSOwner, targetNS) runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name) defer tbbdc.TearDownNs(ctx, t, runcnx, targetNS) @@ -147,3 +149,60 @@ func TestBitbucketDataCenterOnPathChangeAnnotationOnPRMerge(t *testing.T) { // check that pipeline run contains on-path-change annotation. assert.Equal(t, pipelineRuns.Items[0].GetAnnotations()[keys.OnPathChange], "[doc/***.md]") } + +func TestBitbucketDataCenterPRSkippedStatusReported(t *testing.T) { + targetNS := 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) + + opts.Settings = &v1alpha1.Settings{ + StatusChecks: &v1alpha1.StatusChecks{ + Enabled: true, + Mode: v1alpha1.StatusCheckModePerPipelineRun, + }, + } + repo := tbbdc.CreateCRD(ctx, t, client, runcnx, opts, bitbucketWSOwner, targetNS) + runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name) + defer tbbdc.TearDownNs(ctx, t, runcnx, targetNS) + + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-matching.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}, + ) + assert.NilError(t, err) + + // this is not going to match as it's targeting main branch on push event while we're gonna raise a pull request + skipEntry, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-skipped.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.Push.String(), map[string]string{}, + ) + assert.NilError(t, err) + entries[".tekton/pipelinerun-skipped.yaml"] = skipEntry[".tekton/pipelinerun-skipped.yaml"] + + pr := tbbdc.CreatePR(ctx, t, client, runcnx, opts, repo, entries, bitbucketWSOwner, targetNS) + runcnx.Clients.Log.Infof("Pull Request with title '%s' is created", pr.Title) + defer tbbdc.TearDown(ctx, t, runcnx, client, pr, bitbucketWSOwner, targetNS) + + successOpts := wait.SuccessOpt{ + TargetNS: targetNS, + OnEvent: triggertype.PullRequest.String(), + NumberofPRMatch: 1, + MinNumberStatus: 1, + } + wait.Succeeded(ctx, t, runcnx, opts, successOpts) + + statuses, _, err := client.Repositories.ListStatus(ctx, bitbucketWSOwner, pr.Sha, &scm.ListOptions{}) + assert.NilError(t, err) + + foundStatus := false + for _, status := range statuses { + if status.State == scm.StateUnknown && strings.Contains(status.Label, "pipelinerun-skipped") { + foundStatus = true + break + } + } + assert.Equal(t, foundStatus, true, "should have found the status for the skipped pipeline run") +} diff --git a/test/bitbucket_datacenter_push_test.go b/test/bitbucket_datacenter_push_test.go index 999528fc04..9b2b46bd61 100644 --- a/test/bitbucket_datacenter_push_test.go +++ b/test/bitbucket_datacenter_push_test.go @@ -26,7 +26,7 @@ func TestBitbucketDataCenterCELPathChangeOnPush(t *testing.T) { ctx, runcnx, opts, client, err := tbbs.Setup(ctx) assert.NilError(t, err) - repo := tbbs.CreateCRD(ctx, t, client, runcnx, bitbucketWSOwner, targetNS) + repo := tbbs.CreateCRD(ctx, t, client, runcnx, opts, bitbucketWSOwner, targetNS) runcnx.Clients.Log.Infof("Repository %s has been created", repo.Name) defer tbbs.TearDownNs(ctx, t, runcnx, targetNS) diff --git a/test/gitea_pull_request_test.go b/test/gitea_pull_request_test.go index 320daf3dc4..8ea7b4c5fb 100644 --- a/test/gitea_pull_request_test.go +++ b/test/gitea_pull_request_test.go @@ -14,6 +14,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" "github.com/openshift-pipelines/pipelines-as-code/pkg/sort" tgitea "github.com/openshift-pipelines/pipelines-as-code/test/pkg/gitea" @@ -151,3 +152,71 @@ func TestGiteaPullRequestPrivateRepository(t *testing.T) { assert.NilError(t, err) tgitea.WaitForSecretDeletion(t, topts, topts.TargetRefName) } + +func TestGiteaPRSkippedStatusReported(t *testing.T) { + topts := &tgitea.TestOpts{ + TargetEvent: triggertype.PullRequest.String(), + NoPullRequestCreation: true, + SkipEventsCheck: true, + Settings: &v1alpha1.Settings{ + StatusChecks: &v1alpha1.StatusChecks{ + Enabled: true, + Mode: v1alpha1.StatusCheckModePerPipelineRun, + }, + }, + } + ctx, f := tgitea.TestPR(t, topts) + defer f() + + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-matching.yaml": "testdata/pipelinerun.yaml"}, + topts.TargetNS, topts.DefaultBranch, triggertype.PullRequest.String(), map[string]string{}, + ) + assert.NilError(t, err) + + // this is not going to match as it's targeting main branch on push event while we're gonna raise a pull request + skipEntry, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-skipped.yaml": "testdata/pipelinerun.yaml"}, + topts.TargetNS, topts.DefaultBranch, triggertype.Push.String(), map[string]string{}, + ) + assert.NilError(t, err) + entries[".tekton/pipelinerun-skipped.yaml"] = skipEntry[".tekton/pipelinerun-skipped.yaml"] + + scmOpts := &scm.Opts{ + GitURL: topts.GitCloneURL, + Log: topts.ParamsRun.Clients.Log, + WebURL: topts.GitHTMLURL, + TargetRefName: topts.TargetRefName, + BaseRefName: topts.DefaultBranch, + } + topts.SHA = scm.PushFilesToRefGit(t, scmOpts, entries) + + pr, _, err := topts.GiteaCNX.Client().CreatePullRequest(topts.Opts.Organization, topts.Opts.Repo, forgejo.CreatePullRequestOption{ + Title: "Test Pull Request - " + topts.TargetRefName, + Head: topts.TargetRefName, + Base: topts.DefaultBranch, + }) + assert.NilError(t, err) + topts.PullRequest = pr + topts.ParamsRun.Clients.Log.Infof("PullRequest %s has been created", pr.HTMLURL) + + sopt := twait.SuccessOpt{ + TargetNS: topts.TargetNS, + OnEvent: triggertype.PullRequest.String(), + NumberofPRMatch: 1, + MinNumberStatus: 1, + } + twait.Succeeded(ctx, t, topts.ParamsRun, topts.Opts, sopt) + + statuses, _, err := topts.GiteaCNX.Client().ListStatuses(topts.Opts.Organization, topts.Opts.Repo, topts.SHA, forgejo.ListStatusesOption{}) + assert.NilError(t, err) + + foundStatus := false + for _, cstatus := range statuses { + if cstatus.State == forgejo.StatusSuccess && cstatus.Description == "Skipped" { + foundStatus = true + break + } + } + assert.Equal(t, foundStatus, true, "should have found the skipped status for the non-matching pipeline run") +} diff --git a/test/github_pullrequest_test.go b/test/github_pullrequest_test.go index fb6968476f..2ff24b575b 100644 --- a/test/github_pullrequest_test.go +++ b/test/github_pullrequest_test.go @@ -5,6 +5,7 @@ package test import ( "context" "fmt" + "os" "regexp" "strings" "testing" @@ -20,6 +21,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/test/pkg/configmap" tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" "github.com/google/go-github/v91/github" @@ -787,6 +789,192 @@ func TestGithubGHEPullRequestCELJoin(t *testing.T) { assert.NilError(t, err) } +func TestGithubGHEPRSkippedStatusReported(t *testing.T) { + ctx := context.Background() + g := &tgithub.PRTest{ + Label: "Github Skipped Status", + GHE: true, + } + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + targetRefName := fmt.Sprintf("refs/heads/%s", targetNS) + + ctx, runcnx, opts, ghcnx, err := tgithub.Setup(ctx, true, false) + assert.NilError(t, err) + g.Cnx = runcnx + g.Options = opts + g.Provider = ghcnx + g.TargetNamespace = targetNS + g.Logger = runcnx.Clients.Log + + repoinfo, _, err := ghcnx.Client().Repositories.Get(ctx, opts.Organization, opts.Repo) + assert.NilError(t, err) + + opts.Settings = &v1alpha1.Settings{ + StatusChecks: &v1alpha1.StatusChecks{ + Enabled: true, + Mode: v1alpha1.StatusCheckModePerPipelineRun, + }, + } + err = tgithub.CreateCRD(ctx, t, repoinfo, runcnx, opts, ghcnx, targetNS) + assert.NilError(t, err) + + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-matching.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}, + ) + assert.NilError(t, err) + + // this is not going to match as it's targeting main branch on push event while we're gonna raise a pull request + skipEntry, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-skipped.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.Push.String(), map[string]string{}, + ) + assert.NilError(t, err) + entries[".tekton/pipelinerun-skipped.yaml"] = skipEntry[".tekton/pipelinerun-skipped.yaml"] + + commitTitle := fmt.Sprintf("Testing skipped status on %s", targetNS) + g.CommitTitle = commitTitle + g.TargetRefName = targetRefName + + sha, _, err := tgithub.PushFilesToRef(ctx, ghcnx.Client(), commitTitle, + repoinfo.GetDefaultBranch(), targetRefName, opts.Organization, opts.Repo, entries) + assert.NilError(t, err) + g.SHA = sha + + number, err := tgithub.PRCreate(ctx, runcnx, ghcnx, opts.Organization, + opts.Repo, targetRefName, repoinfo.GetDefaultBranch(), commitTitle) + assert.NilError(t, err) + g.PRNumber = number + defer g.TearDown(ctx, t) + + sopt := twait.SuccessOpt{ + Title: commitTitle, + OnEvent: triggertype.PullRequest.String(), + TargetNS: targetNS, + NumberofPRMatch: 1, + SHA: sha, + } + twait.Succeeded(ctx, t, runcnx, opts, sopt) + + opt := github.ListOptions{} + res := &github.ListCheckRunsResults{} + resp := &github.Response{} + counter := 0 + for { + res, resp, err = ghcnx.Client().Checks.ListCheckRunsForRef(ctx, opts.Organization, opts.Repo, sha, &github.ListCheckRunsOptions{ + AppID: ghcnx.ApplicationID, + ListOptions: opt, + }) + assert.NilError(t, err) + assert.Equal(t, resp.StatusCode, 200) + if len(res.CheckRuns) >= 2 { + break + } + runcnx.Clients.Log.Infof("Waiting for the check runs to be created (%d/2)", len(res.CheckRuns)) + if counter > 20 { + t.Fatalf("Check runs not created after 20 tries, got %d", len(res.CheckRuns)) + } + time.Sleep(5 * time.Second) + counter++ + } + + foundStatus := false + for _, cr := range res.CheckRuns { + if cr.GetConclusion() == "skipped" && strings.Contains(cr.GetName(), "pipelinerun-skipped") { + foundStatus = true + break + } + } + assert.Equal(t, foundStatus, true, "should have found a check run with skipped conclusion for the non-matching pipeline run") +} + +func TestGithubGHEWebhookPRSkippedStatusReported(t *testing.T) { + ctx := context.Background() + g := &tgithub.PRTest{ + Label: "Github Webhook Skipped Status", + GHE: true, + Webhook: true, + } + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + targetRefName := fmt.Sprintf("refs/heads/%s", targetNS) + + ctx, runcnx, opts, ghcnx, err := tgithub.Setup(ctx, true, true) + assert.NilError(t, err) + g.Cnx = runcnx + g.Provider = ghcnx + g.TargetNamespace = targetNS + g.Logger = runcnx.Clients.Log + + repoName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + smeeURL := os.Getenv("TEST_GITHUB_SECOND_WEBHOOK_SMEE_URL") + webhookSecret := os.Getenv("TEST_EL_WEBHOOK_SECRET") + + repoinfo, err := tgithub.CreateGHERepo(ctx, ghcnx.Client(), opts.Organization, repoName, smeeURL, webhookSecret, runcnx.Clients.Log) + assert.NilError(t, err) + opts.Repo = repoName + opts.Settings = &v1alpha1.Settings{ + StatusChecks: &v1alpha1.StatusChecks{ + Enabled: true, + Mode: v1alpha1.StatusCheckModePerPipelineRun, + }, + } + g.Options = opts + g.DynamicRepoName = repoName + + err = tgithub.CreateCRD(ctx, t, repoinfo, runcnx, opts, ghcnx, targetNS) + assert.NilError(t, err) + + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-matching.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}, + ) + assert.NilError(t, err) + + // this is not going to match as it's targeting main branch on push event while we're gonna raise a pull request + skipEntry, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-skipped.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.Push.String(), map[string]string{}, + ) + assert.NilError(t, err) + entries[".tekton/pipelinerun-skipped.yaml"] = skipEntry[".tekton/pipelinerun-skipped.yaml"] + + commitTitle := fmt.Sprintf("Testing webhook skipped status on %s", targetNS) + g.CommitTitle = commitTitle + g.TargetRefName = targetRefName + + sha, _, err := tgithub.PushFilesToRef(ctx, ghcnx.Client(), commitTitle, + repoinfo.GetDefaultBranch(), targetRefName, opts.Organization, opts.Repo, entries) + assert.NilError(t, err) + g.SHA = sha + + number, err := tgithub.PRCreate(ctx, runcnx, ghcnx, opts.Organization, + opts.Repo, targetRefName, repoinfo.GetDefaultBranch(), commitTitle) + assert.NilError(t, err) + g.PRNumber = number + defer g.TearDown(ctx, t) + + sopt := twait.SuccessOpt{ + Title: commitTitle, + OnEvent: triggertype.PullRequest.String(), + TargetNS: targetNS, + NumberofPRMatch: 1, + SHA: sha, + } + twait.Succeeded(ctx, t, runcnx, opts, sopt) + + statuses, _, err := ghcnx.Client().Repositories.ListStatuses(ctx, opts.Organization, opts.Repo, sha, &github.ListOptions{}) + assert.NilError(t, err) + + foundStatus := false + for _, status := range statuses { + if status.GetState() == "success" && status.GetDescription() == "Skipped" && strings.Contains(status.GetContext(), "pipelinerun-skipped") { + foundStatus = true + break + } + } + assert.Equal(t, foundStatus, true, "should have found a commit status with success state and Skipped description for the non-matching pipeline run") +} + // Local Variables: // compile-command: "go test -tags=e2e -v -info TestGithubPullRequest$ ." // End: diff --git a/test/gitlab_merge_request_test.go b/test/gitlab_merge_request_test.go index b304509a7b..5f6941a0e5 100644 --- a/test/gitlab_merge_request_test.go +++ b/test/gitlab_merge_request_test.go @@ -901,6 +901,73 @@ func TestGitlabMergeRequestCommentStrategyUpdateCELErrorReplacement(t *testing.T celErrorNoteID, updatedNoteID) } +func TestGitlabMRSkippedStatusReported(t *testing.T) { + topts := &tgitlab.TestOpts{ + NoMRCreation: true, + SkipEventsCheck: true, + Settings: &v1alpha1.Settings{ + StatusChecks: &v1alpha1.StatusChecks{ + Enabled: true, + Mode: v1alpha1.StatusCheckModePerPipelineRun, + }, + }, + } + ctx, cleanup := tgitlab.TestMR(t, topts) + defer cleanup() + + entries, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-matching.yaml": "testdata/pipelinerun.yaml"}, + topts.TargetNS, topts.DefaultBranch, triggertype.PullRequest.String(), map[string]string{}, + ) + assert.NilError(t, err) + + // this is not going to match as it's targeting main branch on push event while we're gonna raise a pull request + skipEntry, err := payload.GetEntries( + map[string]string{".tekton/pipelinerun-skipped.yaml": "testdata/pipelinerun.yaml"}, + topts.TargetNS, topts.DefaultBranch, triggertype.Push.String(), map[string]string{}, + ) + assert.NilError(t, err) + entries[".tekton/pipelinerun-skipped.yaml"] = skipEntry[".tekton/pipelinerun-skipped.yaml"] + + scmOpts := &scm.Opts{ + GitURL: topts.GitCloneURL, + Log: topts.ParamsRun.Clients.Log, + WebURL: topts.GitHTMLURL, + TargetRefName: topts.TargetRefName, + BaseRefName: topts.DefaultBranch, + } + topts.SHA = scm.PushFilesToRefGit(t, scmOpts, entries) + + mrTitle := "TestMergeRequest - " + topts.TargetRefName + mrID, err := tgitlab.CreateMR(topts.GLProvider.Client(), topts.ProjectID, topts.TargetRefName, topts.DefaultBranch, mrTitle) + assert.NilError(t, err) + topts.MRNumber = mrID + topts.ParamsRun.Clients.Log.Infof("MergeRequest %s/-/merge_requests/%d has been created", topts.GitHTMLURL, mrID) + + sopt := twait.SuccessOpt{ + TargetNS: topts.TargetNS, + OnEvent: "Merge Request", + NumberofPRMatch: 1, + MinNumberStatus: 1, + } + twait.Succeeded(ctx, t, topts.ParamsRun, topts.Opts, sopt) + + mr, _, err := topts.GLProvider.Client().MergeRequests.GetMergeRequest(topts.ProjectID, int64(topts.MRNumber), nil) + assert.NilError(t, err) + + commitStatuses, _, err := topts.GLProvider.Client().Commits.GetCommitStatuses(topts.ProjectID, mr.SHA, &clientGitlab.GetCommitStatusesOptions{}) + assert.NilError(t, err) + + foundStatus := false + for _, cs := range commitStatuses { + if cs.Status == "skipped" && strings.Contains(cs.Name, "pipelinerun-skipped") { + foundStatus = true + break + } + } + assert.Equal(t, foundStatus, true, "should have found the skipped status for the non-matching pipeline run") +} + // Local Variables: // compile-command: "go test -tags=e2e -v -run ^TestGitlabMergeRequest$" // End: diff --git a/test/pkg/bitbucketcloud/crd.go b/test/pkg/bitbucketcloud/crd.go index 6297a7cc50..d7e79cf8c2 100644 --- a/test/pkg/bitbucketcloud/crd.go +++ b/test/pkg/bitbucketcloud/crd.go @@ -35,7 +35,8 @@ func CreateCRD(ctx context.Context, t *testing.T, bprovider bitbucketcloud.Provi Name: targetNS, }, Spec: v1alpha1.RepositorySpec{ - URL: links.HTML.HRef, + URL: links.HTML.HRef, + Settings: opts.Settings, }, } err = pacrepo.CreateNS(ctx, targetNS, run) diff --git a/test/pkg/bitbucketdatacenter/crd.go b/test/pkg/bitbucketdatacenter/crd.go index bade6a062f..46a2ae2365 100644 --- a/test/pkg/bitbucketdatacenter/crd.go +++ b/test/pkg/bitbucketdatacenter/crd.go @@ -8,6 +8,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" pacrepo "github.com/openshift-pipelines/pipelines-as-code/test/pkg/repository" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/secret" @@ -16,7 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func CreateCRD(ctx context.Context, t *testing.T, client *scm.Client, run *params.Run, orgAndRepo, targetNS string) *scm.Repository { +func CreateCRD(ctx context.Context, t *testing.T, client *scm.Client, run *params.Run, opts options.E2E, orgAndRepo, targetNS string) *scm.Repository { repo, resp, err := client.Repositories.Find(ctx, orgAndRepo) assert.NilError(t, err, "error getting repository: http status code: %d: %v", resp.Status, err) @@ -26,7 +27,8 @@ func CreateCRD(ctx context.Context, t *testing.T, client *scm.Client, run *param Name: targetNS, }, Spec: v1alpha1.RepositorySpec{ - URL: url, + URL: url, + Settings: opts.Settings, }, }