Skip to content

Commit

Permalink
Do not error when no matching pipeline can be found
Browse files Browse the repository at this point in the history
Returning an error does not seem appropriate because there can be
situations in which no pipeline should match which are "expected". For
example, one might configure to act on PR comments of "/deploy", but for
all other PR comment (events), nothing should happen.

The log will still report that there was no matching pipeline so that
users can figure out what is going on.

The issue was introduced in #685. Since that has not been released yet,
the bugfix will not be recorded in the changelog.
  • Loading branch information
michaelsauter committed May 12, 2023
1 parent 5915f74 commit dc8e614
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 21 deletions.
2 changes: 2 additions & 0 deletions docs/ods-configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,8 @@ The following selection criteria may be specified:
`exceptTags`:: List of tags to which the triggering event may not refer. Patterns as supported by link:https://pkg.go.dev/path#Match[`path.Match`] may be used to match the excluded tags. Omitting the criterion will lead to none of the tags referred to in the webhook event to be excluded.
`prComment`:: Define a prefix a comment has to start with. Might be used to implement functionality like slash commands. If omitted, comments won't be considered in the pipeline selection process.

CAUTION: link:https://pkg.go.dev/path#Match[`path.Match`] does not match e.g. `feature/foo` when the pattern is just `\*`. If you want to match strings with slashes, specify the pattern `*/\*` as well. For example, to match all branches, write `branches: ["*", "\*/*"]`.

Currently, the Bitbucket events `repo:refs_changed` (fired on push to a Bitbucket repository) and any Pull Request related events (event types with prefix `pr:`) are supported (for a full list of events, please refer to the link:https://confluence.atlassian.com/bitbucketserver/event-payload-938025882.html[Atlassian Bitbucket Documentation]). Only the first trigger matching all conditions will be selected. If no trigger section is specified, the pipeline will always match.

==== Passing parameters
Expand Down
21 changes: 10 additions & 11 deletions internal/manager/receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package manager

import (
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -157,33 +156,33 @@ func (s *BitbucketWebhookReceiver) Handle(w http.ResponseWriter, r *http.Request
)
if err != nil {
return nil, httpjson.NewInternalProblem(
fmt.Sprintf("could not download ODS config for repo %s", pInfo.Repository), err,
fmt.Sprintf("could not fetch ODS config for repo %s", pInfo.Repository), err,
)
}

s.Logger.Infof("%+v", pInfo)

cfg, err := identifyPipelineConfig(pInfo, *odsConfig, component)
if err != nil {
cfg := identifyPipelineConfig(pInfo, *odsConfig, component)
if cfg == nil {
return nil, httpjson.NewStatusProblem(
http.StatusBadRequest, "could not identify pipeline to run", err,
http.StatusAccepted, "Could not identify any pipeline to run as no trigger matched", nil,
)
}
s.TriggeredPipelines <- *cfg

s.TriggeredPipelines <- *cfg
return pInfo, nil
}

// identifyPipelineConfig finds the first configuration matching the triggering event
func identifyPipelineConfig(pInfo PipelineInfo, odsConfig config.ODS, component string) (*PipelineConfig, error) {
func identifyPipelineConfig(pInfo PipelineInfo, odsConfig config.ODS, component string) *PipelineConfig {
for _, p := range odsConfig.Pipelines {
if len(p.Triggers) == 0 {
return &PipelineConfig{
PipelineInfo: pInfo,
PVC: makePVCName(component),
PipelineSpec: p,
// no params available
}, nil
}
}
for _, t := range p.Triggers {
if triggerMatches(pInfo, t) {
Expand All @@ -192,11 +191,11 @@ func identifyPipelineConfig(pInfo PipelineInfo, odsConfig config.ODS, component
PVC: makePVCName(component),
PipelineSpec: p,
Params: t.Params,
}, nil
}
}
}
}
return nil, errors.New("no trigger definition matched webhook event")
return nil
}

func triggerMatches(pInfo PipelineInfo, trigger config.Trigger) bool {
Expand Down Expand Up @@ -315,7 +314,7 @@ func fetchODSConfig(bitbucketClient bitbucket.RawClientInterface, project, repos
getErr = err
}
if getErr != nil {
return nil, fmt.Errorf("could not download ODS config for repo %s: %w", repository, getErr)
return nil, fmt.Errorf("could not find ODS config for repo %s: %w", repository, getErr)
}

if body == nil {
Expand Down
58 changes: 48 additions & 10 deletions internal/manager/receive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,17 @@ func TestWebhookHandling(t *testing.T) {
wantBody: `{"title":"Accepted","detail":"Commit 0e183aa3bc3c6deb8f40b93fb2fc4354533cf62f should be skipped"}`,
wantPipelineConfig: false,
},
"repo:refs_changed (branch push) without matching trigger is no-op": {
requestBodyFixture: "manager/payload.json",
bitbucketClient: &bitbucket.TestClient{
Files: map[string][]byte{
"ods.yaml": readTestdataFile(t, "fixtures/manager/non-matching-trigger-ods.yaml"),
},
},
wantStatus: http.StatusAccepted,
wantBody: `{"title":"Accepted","detail":"Could not identify any pipeline to run as no trigger matched"}`,
wantPipelineConfig: false,
},
"repo:refs_changed (branch push) triggers pipeline": {
requestBodyFixture: "manager/payload.json",
bitbucketClient: &bitbucket.TestClient{
Expand Down Expand Up @@ -693,22 +704,49 @@ func TestIdentifyPipelineConfig(t *testing.T) {
}
}
}
got, err := identifyPipelineConfig(tc.pInfo, tc.odsConfig, "component")
if tc.wantPipelineIndex > -1 && err != nil {
t.Fatal(err)
got := identifyPipelineConfig(tc.pInfo, tc.odsConfig, "component")
if tc.wantPipelineIndex > -1 && got == nil {
t.Fatal("wanted a matching pipeline but got none")
}
if tc.wantPipelineIndex < 0 && err == nil {
t.Fatal("expected no matching pipeline, but got one")
if tc.wantPipelineIndex < 0 && got != nil {
t.Fatal("wanted no matching pipeline, but got one")
}
if tc.wantPipelineIndex < 0 && err != nil {
return // no matching pipeline, as expected by the test case.
if tc.wantPipelineIndex < 0 && got == nil {
return // no matching pipeline, as wanted by the test case.
}
// Check if the
if len(got.PipelineSpec.Tasks) < 1 {
t.Fatal("did not match expected pipeline")
t.Fatal("did not match wanted pipeline")
}
if tc.wantTriggerIndex > -1 && len(got.Params) < 1 {
t.Fatal("did not match expected trigger")
t.Fatal("did not match wanted trigger")
}
})
}
}

func TestAnyPatternMatches(t *testing.T) {
tests := []struct {
input string
pattern []string
want bool
}{
{"master", []string{"*"}, true},
// TODO: The following is probably expected to work by users but does not work right now.
// {"feature/foo", []string{"*"}, true},
{"master", []string{"main", "*"}, true},
{"feature/foo", []string{"feature/*"}, true},
{"feature/foo", []string{"*/*"}, true},
{"production", []string{}, true},
{"feature/foo", []string{"feature"}, false},
{"production", []string{"main", "develop"}, false},
{"production", []string{"*/*"}, false},
{"production", []string{"p"}, false},
}
for _, tc := range tests {
t.Run(fmt.Sprintf("%v matches %s", tc.pattern, tc.input), func(t *testing.T) {
got := anyPatternMatches(tc.input, tc.pattern)
if got != tc.want {
t.Fatalf("want %v, got %v", tc.want, got)
}
})
}
Expand Down
11 changes: 11 additions & 0 deletions test/testdata/fixtures/manager/non-matching-trigger-ods.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pipelines:
- triggers:
- branches: ["should-never-match"]
tasks:
- name: build
taskRef:
kind: Task
name: ods-build-go
workspaces:
- name: source
workspace: shared-workspace

0 comments on commit dc8e614

Please sign in to comment.