Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not error when no matching pipeline can be found #690

Merged
merged 1 commit into from
May 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: ["*", "\*/*"]`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #691


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
59 changes: 49 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,50 @@ 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) {
match := true
tests := []struct {
input string
pattern []string
want bool
}{
{"master", []string{"*"}, match},
// TODO: The following is probably expected to work by users but does not work right now.
// {"feature/foo", []string{"*"}, true},
{"master", []string{"main", "*"}, match},
{"feature/foo", []string{"feature/*"}, match},
{"feature/foo", []string{"*/*"}, match},
{"production", []string{}, match},
{"feature/foo", []string{"feature"}, !match},
{"production", []string{"main", "develop"}, !match},
{"production", []string{"*/*"}, !match},
{"production", []string{"p"}, !match},
}
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