Skip to content

Commit 6bd5498

Browse files
zakiskchmouel
zakisk
authored and
chmouel
committed
fix: Match source and head branch in full path
before this, when source or head branch in cancel-in-progress matching code having full path weren't being matched. e.g. refs/heads/branch Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
1 parent 758c1bc commit 6bd5498

File tree

4 files changed

+78
-101
lines changed

4 files changed

+78
-101
lines changed

‎pkg/pipelineascode/cancel_pipelineruns.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"strconv"
7+
"strings"
78
"sync"
89

910
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
@@ -52,7 +53,7 @@ func (p *PacRun) cancelAllInProgressBelongingToClosedPullRequest(ctx context.Con
5253
// Note: The 'selection.NotIn' operator is used to exclude PipelineRuns that have the
5354
// 'cancel-in-progress' annotation explicitly set to 'false', effectively opting them out of cancellation.
5455
labelsMap = map[string]string{keys.CancelInProgress: "false"}
55-
operator = selection.NotIn
56+
operator = selection.NotIn //codespell:ignore 'NotIn'
5657
} else {
5758
// When the 'cancel-in-progress' setting is disabled globally via the Pipelines-as-Code ConfigMap,
5859
// filter and list only those PipelineRuns that explicitly override the global setting by having the
@@ -153,7 +154,7 @@ func (p *PacRun) cancelInProgressMatchingPipelineRun(ctx context.Context, matchP
153154
// it means we only cancel pipelinerun of the same name that runs to
154155
// the unique branch. Note: HeadBranch is the branch from where the PR
155156
// comes from in git jargon.
156-
if sourceBranch != p.event.HeadBranch {
157+
if strings.TrimPrefix(sourceBranch, "refs/heads/") != strings.TrimPrefix(p.event.HeadBranch, "refs/heads/") {
157158
p.logger.Infof("cancel-in-progress: skipping pipelinerun %v/%v as it is not from the same branch, annotation source-branch: %s event headbranch: %s", pr.GetNamespace(), pr.GetName(), sourceBranch, p.event.HeadBranch)
158159
return false
159160
}

‎pkg/pipelineascode/cancel_pipelineruns_test.go‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,62 @@ func TestCancelInProgressMatchingPipelineRun(t *testing.T) {
10331033
},
10341034
wantLog: "cancel-in-progress for event push is enabled via PipelineRun annotation",
10351035
},
1036+
{
1037+
name: "matching PipelineRun when source branch annotation is having full path refs/heads/",
1038+
event: &info.Event{
1039+
Repository: "foo",
1040+
SHA: "foosha",
1041+
HeadBranch: "head",
1042+
EventType: string(triggertype.Push),
1043+
TriggerTarget: triggertype.Push,
1044+
PullRequestNumber: pullReqNumber,
1045+
},
1046+
pipelineRuns: []*pipelinev1.PipelineRun{
1047+
{
1048+
ObjectMeta: metav1.ObjectMeta{
1049+
Name: "pr-foo-1",
1050+
Namespace: "foo",
1051+
Labels: map[string]string{
1052+
keys.EventType: string(triggertype.Push),
1053+
keys.OriginalPRName: "pr-foo",
1054+
keys.URLRepository: formatting.CleanValueKubernetes("foo"),
1055+
keys.SHA: formatting.CleanValueKubernetes("foosha"),
1056+
},
1057+
Annotations: map[string]string{
1058+
keys.OriginalPRName: "pr-foo",
1059+
keys.Repository: "foo",
1060+
keys.SourceBranch: "refs/heads/head",
1061+
keys.CancelInProgress: "true",
1062+
},
1063+
},
1064+
Spec: pipelinev1.PipelineRunSpec{},
1065+
},
1066+
{
1067+
ObjectMeta: metav1.ObjectMeta{
1068+
Name: "pr-foo-2",
1069+
Namespace: "foo",
1070+
Labels: map[string]string{
1071+
keys.OriginalPRName: "pr-foo",
1072+
keys.URLRepository: formatting.CleanValueKubernetes("foo"),
1073+
keys.SHA: formatting.CleanValueKubernetes("foosha"),
1074+
keys.EventType: string(triggertype.Push),
1075+
},
1076+
Annotations: map[string]string{
1077+
keys.OriginalPRName: "pr-foo",
1078+
keys.Repository: "foo",
1079+
keys.SourceBranch: "head",
1080+
keys.CancelInProgress: "true",
1081+
},
1082+
},
1083+
Spec: pipelinev1.PipelineRunSpec{},
1084+
},
1085+
},
1086+
repo: fooRepo,
1087+
cancelledPipelineRuns: map[string]bool{
1088+
"pr-foo-2": true,
1089+
},
1090+
wantLog: "cancel-in-progress for event push is enabled via PipelineRun annotation",
1091+
},
10361092
{
10371093
name: "skip/cancel in progress with concurrency limit",
10381094
event: &info.Event{
@@ -1466,8 +1522,8 @@ func TestGetLabelSelector(t *testing.T) {
14661522
labelsMap: map[string]string{
14671523
keys.CancelInProgress: "false",
14681524
},
1469-
operator: selection.NotIn,
1470-
want: "pipelinesascode.tekton.dev/cancel-in-progress notin (false)",
1525+
operator: selection.NotIn, //codespell:ignore 'NotIn'
1526+
want: "pipelinesascode.tekton.dev/cancel-in-progress notin (false)", //codespell:ignore 'NotIn'
14711527
},
14721528
}
14731529

‎test/github_pullrequest_test.go‎

Lines changed: 17 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,22 @@ import (
99
"testing"
1010
"time"
1111

12-
"github.com/google/go-github/v70/github"
13-
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
14-
"gotest.tools/v3/assert"
15-
"gotest.tools/v3/assert/cmp"
16-
"gotest.tools/v3/golden"
17-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
18-
"knative.dev/pkg/apis"
19-
2012
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
2113
"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
2214
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
2315
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
16+
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/configmap"
2417
tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github"
2518
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
2619
twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
20+
21+
"github.com/google/go-github/v70/github"
22+
tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
23+
"gotest.tools/v3/assert"
24+
"gotest.tools/v3/assert/cmp"
25+
"gotest.tools/v3/golden"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"knative.dev/pkg/apis"
2728
)
2829

2930
func TestGithubPullRequest(t *testing.T) {
@@ -483,15 +484,12 @@ func TestGithubCancelInProgressSettingFromConfigMapOnPR(t *testing.T) {
483484
ctx, runcnx, _, _, err := tgithub.Setup(ctx, false, false)
484485
assert.NilError(t, err)
485486

486-
patchData := map[string]interface{}{
487-
"data": map[string]string{
488-
"enable-cancel-in-progress-on-pull-requests": "true",
489-
},
487+
patchData := map[string]string{
488+
"enable-cancel-in-progress-on-pull-requests": "true",
490489
}
491490

492-
cm, err := tgithub.PatchPACConfigMap(ctx, runcnx, patchData)
493-
assert.NilError(t, err)
494-
assert.Equal(t, cm.Data["enable-cancel-in-progress-on-pull-requests"], "true")
491+
configMapTearDown := configmap.ChangeGlobalConfig(ctx, t, runcnx, patchData)
492+
defer configMapTearDown()
495493

496494
g := &tgithub.PRTest{
497495
Label: "Github PullRequest",
@@ -524,29 +522,19 @@ func TestGithubCancelInProgressSettingFromConfigMapOnPR(t *testing.T) {
524522

525523
err = twait.UntilPipelineRunHasReason(ctx, g.Cnx.Clients, tektonv1.PipelineRunReasonCancelled, waitOpts)
526524
assert.NilError(t, err)
527-
528-
// patch again settings to false so that it shouldn't disturb other tests
529-
// and fail on error.
530-
defer func() {
531-
err = tgithub.SetCancelInProgressToDefaults(ctx, runcnx)
532-
assert.NilError(t, err)
533-
}()
534525
}
535526

536527
func TestGithubCancelInProgressSettingFromConfigMapOnPush(t *testing.T) {
537528
ctx := context.Background()
538529
ctx, runcnx, _, _, err := tgithub.Setup(ctx, false, false)
539530
assert.NilError(t, err)
540531

541-
patchData := map[string]interface{}{
542-
"data": map[string]string{
543-
"enable-cancel-in-progress-on-push": "true",
544-
},
532+
patchData := map[string]string{
533+
"enable-cancel-in-progress-on-push": "true",
545534
}
546535

547-
cm, err := tgithub.PatchPACConfigMap(ctx, runcnx, patchData)
548-
assert.NilError(t, err)
549-
assert.Equal(t, cm.Data["enable-cancel-in-progress-on-push"], "true")
536+
configMapTearDown := configmap.ChangeGlobalConfig(ctx, t, runcnx, patchData)
537+
defer configMapTearDown()
550538

551539
g := &tgithub.PRTest{
552540
Label: "Github PullRequest",
@@ -580,13 +568,6 @@ func TestGithubCancelInProgressSettingFromConfigMapOnPush(t *testing.T) {
580568

581569
err = twait.UntilPipelineRunHasReason(ctx, g.Cnx.Clients, tektonv1.PipelineRunReasonCancelled, waitOpts)
582570
assert.NilError(t, err)
583-
584-
// patch again settings to false so that it shouldn't disturb other tests
585-
// and fail on error.
586-
defer func() {
587-
err = tgithub.SetCancelInProgressToDefaults(ctx, runcnx)
588-
assert.NilError(t, err)
589-
}()
590571
}
591572

592573
// Local Variables:

‎test/pkg/github/setup.go‎

Lines changed: 0 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ package github
22

33
import (
44
"context"
5-
"encoding/json"
65
"fmt"
7-
"log"
86
"os"
97
"strconv"
108
"strings"
@@ -15,10 +13,6 @@ import (
1513
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx"
1614
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
1715
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/setup"
18-
19-
corev1 "k8s.io/api/core/v1"
20-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
21-
"k8s.io/apimachinery/pkg/types"
2216
)
2317

2418
func Setup(ctx context.Context, onSecondController, viaDirectWebhook bool) (context.Context, *params.Run, options.E2E, *github.Provider, error) {
@@ -112,58 +106,3 @@ func Setup(ctx context.Context, onSecondController, viaDirectWebhook bool) (cont
112106

113107
return ctx, run, e2eoptions, gprovider, nil
114108
}
115-
116-
func PatchPACConfigMap(ctx context.Context, run *params.Run, patch map[string]interface{}) (*corev1.ConfigMap, error) {
117-
ns, _, err := params.GetInstallLocation(ctx, run)
118-
if err != nil {
119-
return nil, fmt.Errorf("failed to get Pipelines as Code namespace: %w", err)
120-
}
121-
122-
patchBytes, err := json.Marshal(patch)
123-
if err != nil {
124-
log.Fatalf("Failed to marshal patch data: %v", err)
125-
}
126-
127-
cm, err := run.Clients.Kube.CoreV1().ConfigMaps(ns).Patch(ctx,
128-
run.Info.Controller.Configmap,
129-
types.StrategicMergePatchType,
130-
patchBytes,
131-
metav1.PatchOptions{},
132-
)
133-
if err != nil {
134-
return nil, fmt.Errorf("failed to patch Pipelines-as-Code config map: %w", err)
135-
}
136-
137-
return cm, nil
138-
}
139-
140-
func SetCancelInProgressToDefaults(ctx context.Context, run *params.Run) error {
141-
ns, _, err := params.GetInstallLocation(ctx, run)
142-
if err != nil {
143-
return fmt.Errorf("failed to get Pipelines as Code namespace: %w", err)
144-
}
145-
146-
patch := map[string]interface{}{
147-
"data": map[string]string{
148-
"enable-cancel-in-progress-on-pull-requests": "false",
149-
"enable-cancel-in-progress-on-push": "false",
150-
},
151-
}
152-
153-
patchBytes, err := json.Marshal(patch)
154-
if err != nil {
155-
log.Fatalf("Failed to marshal patch data: %v", err)
156-
}
157-
158-
_, err = run.Clients.Kube.CoreV1().ConfigMaps(ns).Patch(ctx,
159-
run.Info.Controller.Configmap,
160-
types.StrategicMergePatchType,
161-
patchBytes,
162-
metav1.PatchOptions{},
163-
)
164-
if err != nil {
165-
return fmt.Errorf("failed to patch Pipelines-as-Code config map: %w", err)
166-
}
167-
168-
return nil
169-
}

0 commit comments

Comments
 (0)