From 26acc37c238d2131dada49df66c390273e6b654b Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Fri, 6 Oct 2023 10:15:55 +0200 Subject: [PATCH] Fallback to other ACL rules if the policy is set On failure we were not falling back to other ACL rules and we were not checking if the other ACL rules were matching (like OWNERS file). Tests is more comprehensive and check that we are falling back properly and as well that the target is matching the OWNERS file. We now properly dump the errors/allowance in the kube events streams. Try to make sharing functions between gitea and GitHub as much as possible. Fix https://issues.redhat.com/browse/SRVKP-3542 Signed-off-by: Chmouel Boudjnah --- hack/dev/kind/install.sh | 2 +- pkg/pipelineascode/match.go | 2 +- pkg/policy/policy.go | 46 ++++-- pkg/policy/policy_test.go | 155 ++++++++---------- pkg/provider/bitbucketcloud/bitbucket.go | 3 +- pkg/provider/bitbucketcloud/bitbucket_test.go | 2 +- .../bitbucketserver/bitbucketserver.go | 3 +- .../bitbucketserver/bitbucketserver_test.go | 2 +- pkg/provider/gitea/acl.go | 43 +++-- pkg/provider/gitea/acl_test.go | 9 +- pkg/provider/gitea/gitea.go | 9 +- pkg/provider/github/acl.go | 41 +++-- pkg/provider/github/acl_test.go | 17 +- pkg/provider/github/github.go | 9 +- pkg/provider/github/github_test.go | 2 +- pkg/provider/gitlab/gitlab.go | 3 +- pkg/provider/gitlab/gitlab_test.go | 8 +- pkg/provider/interface.go | 3 +- pkg/reconciler/reconciler.go | 4 +- pkg/test/provider/testwebvcs.go | 3 +- test/gitea_access_control_test.go | 109 ++++++++++-- test/pkg/bitbucketcloud/setup.go | 2 +- test/pkg/gitea/setup.go | 4 +- test/pkg/gitea/test.go | 2 +- test/pkg/github/setup.go | 3 +- test/pkg/gitlab/setup.go | 2 +- test/testdata/OWNERS | 7 + 27 files changed, 315 insertions(+), 180 deletions(-) create mode 100644 test/testdata/OWNERS diff --git a/hack/dev/kind/install.sh b/hack/dev/kind/install.sh index a40c63808..bf90f3668 100755 --- a/hack/dev/kind/install.sh +++ b/hack/dev/kind/install.sh @@ -10,7 +10,7 @@ # You probably need to install passwordstore https://www.passwordstore.org/ and # add your github secrets : github-application-id github-private-key # webhook.secret in a folder which needs to be specified in -# the PAC_PASS_SECRET_FOLDER variable. or otheriwse you have to create the +# the PAC_PASS_SECRET_FOLDER variable. or otherwise you have to create the # pipelines-as-code-secret manually # # If you need to install old pac as shipped in OSP1.7, you check it out somewhere diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go index fa89ea7bf..9aa8ec60f 100644 --- a/pkg/pipelineascode/match.go +++ b/pkg/pipelineascode/match.go @@ -92,7 +92,7 @@ is that what you want? make sure you use -n when generating the secret, eg: echo // Set the client, we should error out if there is a problem with // token or secret or we won't be able to do much. - err = p.vcx.SetClient(ctx, p.run, p.event, repo.Spec.Settings) + err = p.vcx.SetClient(ctx, p.run, p.event, repo, p.eventEmitter) if err != nil { return repo, err } diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go index b1421c3cc..c8e12e60f 100644 --- a/pkg/policy/policy.go +++ b/pkg/policy/policy.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "github.com/openshift-pipelines/pipelines-as-code/pkg/events" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" "go.uber.org/zap" @@ -19,24 +20,29 @@ const ( ) type Policy struct { - Settings *v1alpha1.Settings - Event *info.Event - VCX provider.Interface - Logger *zap.SugaredLogger + Repository *v1alpha1.Repository + Event *info.Event + VCX provider.Interface + Logger *zap.SugaredLogger + EventEmitter *events.EventEmitter } -func (p *Policy) IsAllowed(ctx context.Context, tType info.TriggerType) (Result, error) { - if p.Settings == nil || p.Settings.Policy == nil { +func (p *Policy) checkAllowed(ctx context.Context, tType info.TriggerType) (Result, error) { + if p.Repository == nil { + return ResultNotSet, nil + } + settings := p.Repository.Spec.Settings + if settings == nil || settings.Policy == nil { return ResultNotSet, nil } var sType []string switch tType { - // NOTE: This make /retest /ok-to-test /test bound to the same policy, which is fine from a security standpoint but maybe we want to refind + // NOTE: This make /retest /ok-to-test /test bound to the same policy, which is fine from a security standpoint but maybe we want to refine this in the future. case info.TriggerTypeOkToTest, info.TriggerTypeRetest: - sType = p.Settings.Policy.OkToTest + sType = settings.Policy.OkToTest case info.TriggerTypePullRequest: - sType = p.Settings.Policy.PullRequest + sType = settings.Policy.PullRequest // NOTE: not supported yet, will imp if it gets requested and reasonable to implement case info.TriggerTypePush, info.TriggerTypeCancel, info.TriggerTypeCheckSuiteRerequested, info.TriggerTypeCheckRunRerequested: return ResultNotSet, nil @@ -51,10 +57,30 @@ func (p *Policy) IsAllowed(ctx context.Context, tType info.TriggerType) (Result, allowed, reason := p.VCX.CheckPolicyAllowing(ctx, p.Event, sType) reasonMsg := fmt.Sprintf("policy check: %s, %s", string(tType), reason) if reason != "" { - p.Logger.Info(reasonMsg) + p.EventEmitter.EmitMessage(p.Repository, zap.InfoLevel, "PolicyCheck", reasonMsg) } if allowed { return ResultAllowed, nil } return ResultDisallowed, fmt.Errorf(reasonMsg) } + +func (p *Policy) IsAllowed(ctx context.Context, tType info.TriggerType) (bool, string) { + var reason string + policyRes, err := p.checkAllowed(ctx, tType) + if err != nil { + return false, err.Error() + } + switch policyRes { + case ResultAllowed: + reason = fmt.Sprintf("policy check: policy is set for sender %s has been allowed to run CI via policy", p.Event.Sender) + p.EventEmitter.EmitMessage(p.Repository, zap.InfoLevel, "PolicySetAllowed", reason) + return true, "" + case ResultDisallowed: + reason = fmt.Sprintf("policy check: policy is set but sender %s is not in the allowed groups trying the next ACL conditions", p.Event.Sender) + p.EventEmitter.EmitMessage(p.Repository, zap.InfoLevel, "PolicySetDisallowed", reason) + case ResultNotSet: + // should we put a warning here? it does fill up quite a bit the log every time! so I am not so sure.. + } + return false, reason +} diff --git a/pkg/policy/policy_test.go b/pkg/policy/policy_test.go index ad8db35e6..879275cca 100644 --- a/pkg/policy/policy_test.go +++ b/pkg/policy/policy_test.go @@ -3,18 +3,32 @@ package policy import ( "testing" - "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" - "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" - testprovider "github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider" "go.uber.org/zap" zapobserver "go.uber.org/zap/zaptest/observer" + "gotest.tools/v3/assert" rtesting "knative.dev/pkg/reconciler/testing" + + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "github.com/openshift-pipelines/pipelines-as-code/pkg/events" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" + testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients" + testprovider "github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider" ) +func newRepoWithPolicy(policy *v1alpha1.Policy) *v1alpha1.Repository { + return &v1alpha1.Repository{ + Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{ + Policy: policy, + }, + }, + } +} + func TestPolicy_IsAllowed(t *testing.T) { type fields struct { - Settings *v1alpha1.Settings - Event *info.Event + repository *v1alpha1.Repository + event *info.Event } type args struct { tType info.TriggerType @@ -24,149 +38,115 @@ func TestPolicy_IsAllowed(t *testing.T) { fields fields args args replyAllowed bool - want Result + want bool wantErr bool + wantReason string }{ { name: "Test Policy.IsAllowed with no settings", fields: fields{ - Settings: nil, - Event: nil, + repository: nil, + event: nil, }, args: args{ tType: info.TriggerTypePush, }, - want: ResultNotSet, + want: false, }, { name: "Test Policy.IsAllowed with unknown event type", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - PullRequest: []string{"pull_request"}, - }, - }, - Event: nil, + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{"pull_request"}}), + event: nil, }, args: args{ tType: "unknown", }, - want: ResultNotSet, + want: false, }, { name: "allowing member not in team for pull request", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - PullRequest: []string{"pull_request"}, - }, - }, - Event: info.NewEvent(), + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{"pull_request"}}), + event: info.NewEvent(), }, args: args{ tType: info.TriggerTypePullRequest, }, replyAllowed: true, - want: ResultAllowed, + want: true, }, { - name: "empty settings policy ignore", + name: "empty settings policy ignore", + wantReason: "policy check: pull_request, policy disallowing", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - PullRequest: []string{}, - }, - }, - Event: info.NewEvent(), + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{""}}), + event: info.NewEvent(), }, args: args{ tType: info.TriggerTypePullRequest, }, - replyAllowed: true, - want: ResultNotSet, + want: false, }, { - name: "disallowing member not in team for pull request", + name: "disallowing member not in team for pull request", + wantReason: "policy check: pull_request, policy disallowing", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - PullRequest: []string{"pull_request"}, - }, - }, - Event: info.NewEvent(), + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{"pull_request"}}), + event: info.NewEvent(), }, args: args{ tType: info.TriggerTypePullRequest, }, - replyAllowed: false, - want: ResultDisallowed, - wantErr: true, + want: false, + wantErr: true, }, { - name: "allowing member not in team for ok-to-test", + name: "allowing member in team for ok-to-test", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - OkToTest: []string{"ok-to-test"}, - }, - }, - Event: info.NewEvent(), + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{"ok-to-test"}}), + event: info.NewEvent(), }, args: args{ tType: info.TriggerTypeOkToTest, }, replyAllowed: true, - want: ResultAllowed, + want: false, }, { name: "disallowing member not in team for ok-to-test", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - OkToTest: []string{"ok-to-test"}, - }, - }, - Event: info.NewEvent(), + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{"ok-to-test"}}), + event: info.NewEvent(), }, args: args{ tType: info.TriggerTypeOkToTest, }, - replyAllowed: false, - want: ResultDisallowed, - wantErr: true, + want: false, + wantErr: true, }, { name: "allowing member not in team for retest", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - OkToTest: []string{"ok-to-test"}, - }, - }, - Event: info.NewEvent(), + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{"ok-to-test"}}), + event: info.NewEvent(), }, args: args{ tType: info.TriggerTypeRetest, }, - replyAllowed: true, - want: ResultAllowed, + want: false, }, { name: "disallowing member not in team for retest", fields: fields{ - Settings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{ - OkToTest: []string{"ok-to-test"}, - }, - }, - Event: info.NewEvent(), + repository: newRepoWithPolicy(&v1alpha1.Policy{PullRequest: []string{"ok-to-test"}}), + event: info.NewEvent(), }, args: args{ tType: info.TriggerTypeRetest, }, - replyAllowed: false, - want: ResultDisallowed, - wantErr: true, + want: false, + wantErr: true, }, } for _, tt := range tests { @@ -174,22 +154,25 @@ func TestPolicy_IsAllowed(t *testing.T) { observer, _ := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() + ctx, _ := rtesting.SetupFakeContext(t) + stdata, _ := testclient.SeedTestData(t, ctx, testclient.Data{}) + vcx := &testprovider.TestProviderImp{PolicyDisallowing: !tt.replyAllowed} - p := &Policy{ - Settings: tt.fields.Settings, - Event: tt.fields.Event, - VCX: vcx, - Logger: logger, + if tt.fields.event == nil { + tt.fields.event = info.NewEvent() } - ctx, _ := rtesting.SetupFakeContext(t) - got, err := p.IsAllowed(ctx, tt.args.tType) - if (err != nil) != tt.wantErr { - t.Errorf("Policy.IsAllowed() error = %v, wantErr %v", err, tt.wantErr) - return + p := &Policy{ + Repository: tt.fields.repository, + Event: tt.fields.event, + VCX: vcx, + Logger: logger, + EventEmitter: events.NewEventEmitter(stdata.Kube, logger), } + got, reason := p.IsAllowed(ctx, tt.args.tType) if got != tt.want { t.Errorf("Policy.IsAllowed() = %v, want %v", got, tt.want) } + assert.Equal(t, tt.wantReason, reason) }) } } diff --git a/pkg/provider/bitbucketcloud/bitbucket.go b/pkg/provider/bitbucketcloud/bitbucket.go index f9d6dc7f1..742cff51d 100644 --- a/pkg/provider/bitbucketcloud/bitbucket.go +++ b/pkg/provider/bitbucketcloud/bitbucket.go @@ -9,6 +9,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/events" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -165,7 +166,7 @@ func (v *Provider) GetFileInsideRepo(_ context.Context, event *info.Event, path, return v.getBlob(event, revision, path) } -func (v *Provider) SetClient(_ context.Context, _ *params.Run, event *info.Event, _ *v1alpha1.Settings) error { +func (v *Provider) SetClient(_ context.Context, _ *params.Run, event *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error { if event.Provider.Token == "" { return fmt.Errorf("no git_provider.secret has been set in the repo crd") } diff --git a/pkg/provider/bitbucketcloud/bitbucket_test.go b/pkg/provider/bitbucketcloud/bitbucket_test.go index bc82dbde6..703b032f8 100644 --- a/pkg/provider/bitbucketcloud/bitbucket_test.go +++ b/pkg/provider/bitbucketcloud/bitbucket_test.go @@ -130,7 +130,7 @@ func TestSetClient(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) v := Provider{} - err := v.SetClient(ctx, nil, tt.event, nil) + err := v.SetClient(ctx, nil, tt.event, nil, nil) if tt.wantErrSubstr != "" { assert.ErrorContains(t, err, tt.wantErrSubstr) return diff --git a/pkg/provider/bitbucketserver/bitbucketserver.go b/pkg/provider/bitbucketserver/bitbucketserver.go index b18f1ef6d..b977a6b17 100644 --- a/pkg/provider/bitbucketserver/bitbucketserver.go +++ b/pkg/provider/bitbucketserver/bitbucketserver.go @@ -10,6 +10,7 @@ import ( "github.com/google/go-github/v53/github" "github.com/mitchellh/mapstructure" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -205,7 +206,7 @@ func (v *Provider) GetFileInsideRepo(_ context.Context, event *info.Event, path, return ret, err } -func (v *Provider) SetClient(ctx context.Context, _ *params.Run, event *info.Event, _ *v1alpha1.Settings) error { +func (v *Provider) SetClient(ctx context.Context, _ *params.Run, event *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error { if event.Provider.User == "" { return fmt.Errorf("no provider.user has been set in the repo crd") } diff --git a/pkg/provider/bitbucketserver/bitbucketserver_test.go b/pkg/provider/bitbucketserver/bitbucketserver_test.go index 86147113b..8db1bebf9 100644 --- a/pkg/provider/bitbucketserver/bitbucketserver_test.go +++ b/pkg/provider/bitbucketserver/bitbucketserver_test.go @@ -282,7 +282,7 @@ func TestSetClient(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) v := &Provider{} - err := v.SetClient(ctx, nil, tt.opts, nil) + err := v.SetClient(ctx, nil, tt.opts, nil, nil) if tt.wantErrSubstr != "" { assert.ErrorContains(t, err, tt.wantErrSubstr) return diff --git a/pkg/provider/gitea/acl.go b/pkg/provider/gitea/acl.go index 9fb7dada0..77c6380bd 100644 --- a/pkg/provider/gitea/acl.go +++ b/pkg/provider/gitea/acl.go @@ -46,36 +46,45 @@ func (v *Provider) CheckPolicyAllowing(_ context.Context, event *info.Event, all func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, pac *info.PacOpts) (bool, error) { aclPolicy := policy.Policy{ - Settings: v.repoSettings, - Event: event, - VCX: v, - Logger: v.Logger, + Repository: v.repo, + EventEmitter: v.eventEmitter, + Event: event, + VCX: v, + Logger: v.Logger, } - // checkIfPolicyIsAllowing + // Try to detect a policy rule allowed it tType, _ := detectTriggerTypeFromPayload("", event.Event) - policyRes, err := aclPolicy.IsAllowed(ctx, tType) - switch policyRes { - case policy.ResultAllowed: + policyAllowed, policyReason := aclPolicy.IsAllowed(ctx, tType) + if policyAllowed { return true, nil - case policy.ResultDisallowed: - return false, err - case policy.ResultNotSet: - // showing as debug so we don't spill useless logs all the time in default info - v.Logger.Debugf("policy check: policy is not set, checking for other conditions for sender: %s", event.Sender) } - // Do most of the checks first, if user is a owner or in a organisation + // Check all the ACL rules allowed, err := v.aclCheckAll(ctx, event) if err != nil { - return false, fmt.Errorf("aclCheckAll: %w", err) + return false, err } if allowed { return true, nil } - // Finally try to parse comments - return v.aclAllowedOkToTestFromAnOwner(ctx, event, pac) + // Try to parse the comment from an owner who has issues a /ok-to-test + ownerAllowed, err := v.aclAllowedOkToTestFromAnOwner(ctx, event, pac) + if err != nil { + return false, err + } + if ownerAllowed { + return true, nil + } + + // error with the policy reason if it was set + if policyReason != "" { + return false, fmt.Errorf(policyReason) + } + + // finally silently return false if no rules allowed this + return false, nil } // allowedOkToTestFromAnOwner Go over comments in a pull request and check diff --git a/pkg/provider/gitea/acl_test.go b/pkg/provider/gitea/acl_test.go index 841285413..e14d5dda3 100644 --- a/pkg/provider/gitea/acl_test.go +++ b/pkg/provider/gitea/acl_test.go @@ -79,10 +79,13 @@ func TestCheckPolicyAllowing(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) observer, _ := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() + repo := &v1alpha1.Repository{Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{}, + }} gprovider := Provider{ - Client: fakeclient, - repoSettings: &v1alpha1.Settings{}, - Logger: logger, + Client: fakeclient, + repo: repo, + Logger: logger, } gotAllowed, gotReason := gprovider.CheckPolicyAllowing(ctx, event, tt.allowedTeams) diff --git a/pkg/provider/gitea/gitea.go b/pkg/provider/gitea/gitea.go index bc64c08f0..f528a73f2 100644 --- a/pkg/provider/gitea/gitea.go +++ b/pkg/provider/gitea/gitea.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/sdk/gitea" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -45,7 +46,8 @@ type Provider struct { giteaInstanceURL string // only exposed for e2e tests Password string - repoSettings *v1alpha1.Settings + repo *v1alpha1.Repository + eventEmitter *events.EventEmitter } // GetTaskURI TODO: Implement ME @@ -80,7 +82,7 @@ func (v *Provider) GetConfig() *info.ProviderConfig { } } -func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event, repoSettings *v1alpha1.Settings) error { +func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event, repo *v1alpha1.Repository, emitter *events.EventEmitter) error { var err error apiURL := runevent.Provider.URL // password is not exposed to CRD, it's only used from the e2e tests @@ -96,7 +98,8 @@ func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Ev return err } v.giteaInstanceURL = runevent.Provider.URL - v.repoSettings = repoSettings + v.eventEmitter = emitter + v.repo = repo return nil } diff --git a/pkg/provider/github/acl.go b/pkg/provider/github/acl.go index 133c2c662..eb3add369 100644 --- a/pkg/provider/github/acl.go +++ b/pkg/provider/github/acl.go @@ -48,26 +48,21 @@ func (v *Provider) CheckPolicyAllowing(ctx context.Context, event *info.Event, a func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, pac *info.PacOpts) (bool, error) { aclPolicy := policy.Policy{ - Settings: v.repoSettings, - Event: event, - VCX: v, - Logger: v.Logger, + Repository: v.repo, + EventEmitter: v.eventEmitter, + Event: event, + VCX: v, + Logger: v.Logger, } - // checkIfPolicyIsAllowing + // Try to detect a policy rule allowed it tType, _ := detectTriggerTypeFromPayload("", event.Event) - policyRes, err := aclPolicy.IsAllowed(ctx, tType) - switch policyRes { - case policy.ResultAllowed: + policyAllowed, policyReason := aclPolicy.IsAllowed(ctx, tType) + if policyAllowed { return true, nil - case policy.ResultDisallowed: - return false, err - case policy.ResultNotSet: - // showing as debug so we don't spill useless logs all the time in default info - v.Logger.Debugf("policy check: policy is not set, checking for other conditions for sender: %s", event.Sender) } - // Do most of the checks first, if user is a owner or in a organisation + // Check all the ACL rules allowed, err := v.aclCheckAll(ctx, event) if err != nil { return false, err @@ -76,8 +71,22 @@ func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, pac *info.P return true, nil } - // Finally try to parse comments - return v.aclAllowedOkToTestFromAnOwner(ctx, event, pac) + // Try to parse the comment from an owner who has issues a /ok-to-test + ownerAllowed, err := v.aclAllowedOkToTestFromAnOwner(ctx, event, pac) + if err != nil { + return false, err + } + if ownerAllowed { + return true, nil + } + + // error with the policy reason if it was set + if policyReason != "" { + return false, fmt.Errorf(policyReason) + } + + // finally silently return false if no rules allowed this + return false, nil } // allowedOkToTestFromAnOwner Go over comments in a pull request and check diff --git a/pkg/provider/github/acl_test.go b/pkg/provider/github/acl_test.go index 183c84147..60275bff0 100644 --- a/pkg/provider/github/acl_test.go +++ b/pkg/provider/github/acl_test.go @@ -84,9 +84,12 @@ func TestCheckPolicyAllowing(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) observer, _ := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() + repo := &v1alpha1.Repository{Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{}, + }} gprovider := Provider{ Client: fakeclient, - repoSettings: &v1alpha1.Settings{}, + repo: repo, Logger: logger, paginedNumber: 1, } @@ -313,9 +316,12 @@ func TestOkToTestComment(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) observer, _ := zapobserver.New(zap.InfoLevel) logger := zap.New(observer).Sugar() + repo := &v1alpha1.Repository{Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{}, + }} gprovider := Provider{ Client: fakeclient, - repoSettings: &v1alpha1.Settings{}, + repo: repo, Logger: logger, paginedNumber: 1, } @@ -580,11 +586,12 @@ func TestIfPullRequestIsForSameRepoWithoutFork(t *testing.T) { fmt.Fprint(rw, string(b)) }) + repo := &v1alpha1.Repository{Spec: v1alpha1.RepositorySpec{ + Settings: &v1alpha1.Settings{}, + }} gprovider := Provider{ Client: fakeclient, - repoSettings: &v1alpha1.Settings{ - Policy: &v1alpha1.Policy{}, - }, + repo: repo, } got, err := gprovider.aclCheckAll(ctx, tt.event) diff --git a/pkg/provider/github/github.go b/pkg/provider/github/github.go index b02885006..f7a7bc705 100644 --- a/pkg/provider/github/github.go +++ b/pkg/provider/github/github.go @@ -15,6 +15,7 @@ import ( "github.com/jonboulle/clockwork" "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/events" "github.com/openshift-pipelines/pipelines-as-code/pkg/params" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -46,7 +47,8 @@ type Provider struct { provenance string Run *params.Run RepositoryIDs []int64 - repoSettings *v1alpha1.Settings + repo *v1alpha1.Repository + eventEmitter *events.EventEmitter paginedNumber int skippedRun } @@ -259,11 +261,12 @@ func (v *Provider) checkWebhookSecretValidity(ctx context.Context, cw clockwork. return nil } -func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.Event, repoSettings *v1alpha1.Settings) error { +func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.Event, repo *v1alpha1.Repository, eventsEmitter *events.EventEmitter) error { client, providerName, apiURL := makeClient(ctx, event.Provider.URL, event.Provider.Token) v.providerName = providerName v.Run = run - v.repoSettings = repoSettings + v.repo = repo + v.eventEmitter = eventsEmitter // check that the Client is not already set, so we don't override our fakeclient // from unittesting. diff --git a/pkg/provider/github/github_test.go b/pkg/provider/github/github_test.go index 6ec20a921..bb32bbb78 100644 --- a/pkg/provider/github/github_test.go +++ b/pkg/provider/github/github_test.go @@ -576,7 +576,7 @@ func TestGithubSetClient(t *testing.T) { t.Run(tt.name, func(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) v := Provider{} - err := v.SetClient(ctx, nil, tt.event, nil) + err := v.SetClient(ctx, nil, tt.event, nil, nil) assert.NilError(t, err) assert.Equal(t, tt.expectedURL, *v.APIURL) assert.Equal(t, "https", v.Client.BaseURL.Scheme) diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index 202fc0259..049ec49df 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -98,7 +99,7 @@ func (v *Provider) GetConfig() *info.ProviderConfig { } } -func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event, _ *v1alpha1.Settings) error { +func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error { var err error if runevent.Provider.Token == "" { return fmt.Errorf("no git_provider.secret has been set in the repo crd") diff --git a/pkg/provider/gitlab/gitlab_test.go b/pkg/provider/gitlab/gitlab_test.go index 13c864591..c84640424 100644 --- a/pkg/provider/gitlab/gitlab_test.go +++ b/pkg/provider/gitlab/gitlab_test.go @@ -197,7 +197,7 @@ func TestGetConfig(t *testing.T) { func TestSetClient(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) v := &Provider{} - assert.Assert(t, v.SetClient(ctx, nil, info.NewEvent(), nil) != nil) + assert.Assert(t, v.SetClient(ctx, nil, info.NewEvent(), nil, nil) != nil) client, _, tearDown := thelp.Setup(t) defer tearDown() @@ -206,7 +206,7 @@ func TestSetClient(t *testing.T) { Provider: &info.Provider{ Token: "hello", }, - }, nil) + }, nil, nil) assert.NilError(t, err) assert.Assert(t, *vv.Token != "") } @@ -218,14 +218,14 @@ func TestSetClientDetectAPIURL(t *testing.T) { defer tearDown() v := &Provider{Client: client} event := info.NewEvent() - err := v.SetClient(ctx, nil, event, nil) + err := v.SetClient(ctx, nil, event, nil, nil) assert.ErrorContains(t, err, "no git_provider.secret has been set") event.Provider.Token = "hello" v.repoURL, event.URL, event.Provider.URL = "", "", "" event.URL = fmt.Sprintf("%s/hello-this-is-me-ze/project", fakehost) - err = v.SetClient(ctx, nil, event, nil) + err = v.SetClient(ctx, nil, event, nil, nil) assert.NilError(t, err) assert.Equal(t, fakehost, v.apiURL) diff --git a/pkg/provider/interface.go b/pkg/provider/interface.go index f49e4aa77..815bced29 100644 --- a/pkg/provider/interface.go +++ b/pkg/provider/interface.go @@ -5,6 +5,7 @@ import ( "net/http" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "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/info" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -33,7 +34,7 @@ type Interface interface { CreateStatus(context.Context, versioned.Interface, *info.Event, *info.PacOpts, StatusOpts) error GetTektonDir(context.Context, *info.Event, string, string) (string, error) // ctx, event, path, provenance GetFileInsideRepo(context.Context, *info.Event, string, string) (string, error) // ctx, event, path, branch - SetClient(context.Context, *params.Run, *info.Event, *v1alpha1.Settings) error + SetClient(context.Context, *params.Run, *info.Event, *v1alpha1.Repository, *events.EventEmitter) error GetCommitInfo(context.Context, *info.Event) error GetConfig() *info.ProviderConfig GetFiles(context.Context, *info.Event) ([]string, error) diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index ee7c6ca13..1a0e2be6b 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -125,7 +125,7 @@ func (r *Reconciler) reportFinalStatus(ctx context.Context, logger *zap.SugaredL } } - err = provider.SetClient(ctx, r.run, event, repo.Spec.Settings) + err = provider.SetClient(ctx, r.run, event, repo, r.eventEmitter) if err != nil { return repo, fmt.Errorf("cannot set client: %w", err) } @@ -190,7 +190,7 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger * } } - err = p.SetClient(ctx, r.run, event, repo.Spec.Settings) + err = p.SetClient(ctx, r.run, event, repo, r.eventEmitter) if err != nil { return fmt.Errorf("cannot set client: %w", err) } diff --git a/pkg/test/provider/testwebvcs.go b/pkg/test/provider/testwebvcs.go index fd6d92995..6944cbe5d 100644 --- a/pkg/test/provider/testwebvcs.go +++ b/pkg/test/provider/testwebvcs.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -55,7 +56,7 @@ func (v *TestProviderImp) GetCommitInfo(_ context.Context, _ *info.Event) error return nil } -func (v *TestProviderImp) SetClient(_ context.Context, _ *params.Run, _ *info.Event, _ *v1alpha1.Settings) error { +func (v *TestProviderImp) SetClient(_ context.Context, _ *params.Run, _ *info.Event, _ *v1alpha1.Repository, _ *events.EventEmitter) error { return nil } diff --git a/test/gitea_access_control_test.go b/test/gitea_access_control_test.go index f1f54df96..78558f9af 100644 --- a/test/gitea_access_control_test.go +++ b/test/gitea_access_control_test.go @@ -8,16 +8,21 @@ import ( "fmt" "os" "regexp" + "strings" "testing" + "time" "gotest.tools/v3/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/configmap" tgitea "github.com/openshift-pipelines/pipelines-as-code/test/pkg/gitea" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" - "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" + "github.com/openshift-pipelines/pipelines-as-code/test/pkg/scm" + twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" ) const okToTestComment = "/ok-to-test" @@ -110,8 +115,6 @@ func TestGiteaPolicyOkToTestRetest(t *testing.T) { topts.ParamsRun.Clients.Log.Infof("Repo CRD %s has been created with Policy: %+v", topts.TargetRefName, topts.Settings.Policy) orgName := "org-" + topts.TargetRefName - adminCNX := topts.GiteaCNX - // create ok-to-test team on org and add user ok-to-test onto it oktotestTeam, err := tgitea.CreateTeam(topts, orgName, "ok-to-test") assert.NilError(t, err) @@ -134,33 +137,37 @@ func TestGiteaPolicyOkToTestRetest(t *testing.T) { topts.ParamsRun.Clients.Log.Infof("Sending a /ok-to-test comment as a user not belonging to an allowed team in Repo CR policy but part of the organization") topts.GiteaCNX = normalUserCnx - tgitea.PostCommentOnPullRequest(t, topts, "/ok-to-test") + tgitea.PostCommentOnPullRequest(t, topts, okToTestComment) topts.Regexp = regexp.MustCompile( fmt.Sprintf(`.*policy check: ok-to-test, user: %s is not a member of any of the allowed teams.*`, normalUserNamePasswd)) - topts.CheckForStatus = "failure" + topts.CheckForStatus = "Skipped" tgitea.WaitForPullRequestCommentMatch(t, topts) - tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, settings.PACApplicationNameDefaultValue, true) - - // make sure we delete the old comment to don't have a false positive - topts.GiteaCNX = adminCNX - assert.NilError(t, tgitea.RemoveCommentMatching(topts, regexp.MustCompile(`.*policy check:`))) topts.ParamsRun.Clients.Log.Infof("Sending a /retest comment as a user not belonging to an allowed team in Repo CR policy but part of the organization") topts.GiteaCNX = normalUserCnx tgitea.PostCommentOnPullRequest(t, topts, "/retest") topts.Regexp = regexp.MustCompile( fmt.Sprintf(`.*policy check: retest, user: %s is not a member of any of the allowed teams.*`, normalUserNamePasswd)) - topts.CheckForStatus = "failure" + topts.CheckForStatus = "Skipped" tgitea.WaitForPullRequestCommentMatch(t, topts) - tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, settings.PACApplicationNameDefaultValue, true) topts.GiteaCNX = okToTestUserCnx topts.ParamsRun.Clients.Log.Infof("Sending a /ok-to-test comment as a user belonging to an allowed team in Repo CR policy") tgitea.PostCommentOnPullRequest(t, topts, "/ok-to-test") topts.Regexp = successRegexp - topts.CheckForStatus = "success" tgitea.WaitForPullRequestCommentMatch(t, topts) - tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, "", true) + + prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{}) + assert.NilError(t, err) + assert.Equal(t, len(prs.Items), 1, "should have only one pipelinerun, but we have: %d", len(prs.Items)) + firstpr := prs.Items[0] + generatename := strings.TrimSuffix(firstpr.GetGenerateName(), "-") + + topts.CheckForStatus = "success" + // NOTE(chmouel): there is two status here, one old one which is the + // failure without the prun and the new one with the prun on success same + // bug we have github checkrun that we need to fix + tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, fmt.Sprintf("%s / %s", settings.PACApplicationNameDefaultValue, generatename), true) } // TestGiteaACLOrgAllowed tests that the policy check works when the user is part of an allowed org @@ -244,7 +251,7 @@ func TestGiteaACLCommentsAllowing(t *testing.T) { tgitea.WaitForPullRequestCommentMatch(t, topts) tgitea.WaitForStatus(t, topts, topts.PullRequest.Head.Sha, "", false) // checking the pod log to make sure /test works - err = wait.RegexpMatchingInPodLog(context.Background(), topts.ParamsRun, topts.TargetNS, + err = twait.RegexpMatchingInPodLog(context.Background(), topts.ParamsRun, topts.TargetNS, "pipelinesascode.tekton.dev/event-type=pull_request", "step-task", *regexp.MustCompile(".*MOTO"), 2) assert.NilError(t, err, "Error while checking the logs of the pods") @@ -353,6 +360,78 @@ func TestGiteaACLCommentsAllowingRememberOkToTestTrue(t *testing.T) { tgitea.WaitForPullRequestCommentMatch(t, topts) } +func TestGiteaPolicyAllowedOwnerFiles(t *testing.T) { + topts := &tgitea.TestOpts{ + OnOrg: true, + NoPullRequestCreation: true, + TargetEvent: options.PullRequestEvent, + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + PullRequest: []string{"normal"}, + }, + }, + } + defer tgitea.TestPR(t, topts)() + targetRef := topts.TargetRefName + orgName := "org-" + topts.TargetRefName + topts.Opts.Organization = orgName + + normalTeam, err := tgitea.CreateTeam(topts, orgName, "normal") + assert.NilError(t, err) + normalUserNamePasswd := fmt.Sprintf("normal-%s", topts.TargetRefName) + _, normalUser, err := tgitea.CreateGiteaUserSecondCnx(topts, normalUserNamePasswd, normalUserNamePasswd) + assert.NilError(t, err) + _, err = topts.GiteaCNX.Client.AddTeamMember(normalTeam.ID, normalUser.UserName) + assert.NilError(t, err) + + // create an allowed user w + allowedUserNamePasswd := fmt.Sprintf("allowed-%s", topts.TargetRefName) + allowedCnx, allowedUser, err := tgitea.CreateGiteaUserSecondCnx(topts, allowedUserNamePasswd, allowedUserNamePasswd) + assert.NilError(t, err) + + prmap := map[string]string{"OWNERS": "testdata/OWNERS"} + entries, err := payload.GetEntries(prmap, topts.TargetNS, topts.DefaultBranch, topts.TargetEvent, map[string]string{ + "Approver": allowedUser.UserName, + }) + assert.NilError(t, err) + + scmOpts := &scm.Opts{ + GitURL: topts.GitCloneURL, + Log: topts.ParamsRun.Clients.Log, + WebURL: topts.GitHTMLURL, + TargetRefName: topts.DefaultBranch, + BaseRefName: topts.DefaultBranch, + } + // push OWNERS file to main + scm.PushFilesToRefGit(t, scmOpts, entries) + scmOpts.TargetRefName = targetRef + + newyamlFiles := map[string]string{".tekton/pr.yaml": "testdata/pipelinerun.yaml"} + newEntries, err := payload.GetEntries(newyamlFiles, topts.TargetNS, topts.DefaultBranch, topts.TargetEvent, map[string]string{}) + assert.NilError(t, err) + scm.PushFilesToRefGit(t, scmOpts, newEntries) + + npr := tgitea.CreateForkPullRequest(t, topts, allowedCnx, "") + waitOpts := twait.Opts{ + RepoName: topts.TargetNS, + Namespace: topts.TargetNS, + MinNumberStatus: 1, + PollTimeout: twait.DefaultTimeout, + TargetSHA: npr.Head.Sha, + } + assert.NilError(t, twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts)) + time.Sleep(5 * time.Second) // “Evil does not sleep. It waits.” - Galadriel + + prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{}) + assert.NilError(t, err) + assert.Equal(t, len(prs.Items), 1, "should have only one pipelinerun, but we have: %d", len(prs.Items)) + + firstpr := prs.Items[0] + topts.CheckForStatus = "success" + generatename := strings.TrimSuffix(firstpr.GetGenerateName(), "-") + tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, fmt.Sprintf("%s / %s", settings.PACApplicationNameDefaultValue, generatename), false) +} + // Local Variables: // compile-command: "go test -tags=e2e -v -run TestGiteaPush ." // End: diff --git a/test/pkg/bitbucketcloud/setup.go b/test/pkg/bitbucketcloud/setup.go index 937b6ba72..c6bb2b7d7 100644 --- a/test/pkg/bitbucketcloud/setup.go +++ b/test/pkg/bitbucketcloud/setup.go @@ -48,7 +48,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, bitbucketcloud.Provid URL: bitbucketCloudAPIURL, User: bitbucketCloudUser, } - if err := bbc.SetClient(ctx, nil, event, nil); err != nil { + if err := bbc.SetClient(ctx, nil, event, nil, nil); err != nil { return nil, options.E2E{}, bitbucketcloud.Provider{}, err } return run, e2eoptions, bbc, nil diff --git a/test/pkg/gitea/setup.go b/test/pkg/gitea/setup.go index 4026c01b5..f5cd188ad 100644 --- a/test/pkg/gitea/setup.go +++ b/test/pkg/gitea/setup.go @@ -32,8 +32,8 @@ func CreateProvider(ctx context.Context, giteaURL, user, password string) (gitea User: user, Token: password, } - if err := gprovider.SetClient(ctx, nil, event, nil); err != nil { - return gitea.Provider{}, err + if err := gprovider.SetClient(ctx, nil, event, nil, nil); err != nil { + return gitea.Provider{}, fmt.Errorf("cannot set client: %w", err) } return gprovider, nil } diff --git a/test/pkg/gitea/test.go b/test/pkg/gitea/test.go index 01332d420..f9b93fed5 100644 --- a/test/pkg/gitea/test.go +++ b/test/pkg/gitea/test.go @@ -236,7 +236,7 @@ func WaitForStatus(t *testing.T, topts *TestOpts, ref, forcontext string, onlyla statuscheck := topts.CheckForStatus if statuscheck != "" && statuscheck != string(cstatus.State) { if statuscheck != cstatus.Description { - t.Errorf("Status on SHA: %s is %s from %s", ref, cstatus.State, cstatus.Context) + t.Fatalf("Status on SHA: %s is %s from %s", ref, cstatus.State, cstatus.Context) } } topts.ParamsRun.Clients.Log.Infof("Status on SHA: %s is %s from %s", ref, cstatus.State, cstatus.Context) diff --git a/test/pkg/github/setup.go b/test/pkg/github/setup.go index 2d0ab146a..91e62bbc0 100644 --- a/test/pkg/github/setup.go +++ b/test/pkg/github/setup.go @@ -93,7 +93,8 @@ func Setup(ctx context.Context, viaDirectWebhook bool) (*params.Run, options.E2E Token: githubToken, URL: githubURL, } - if err := gprovider.SetClient(ctx, nil, event, nil); err != nil { + // TODO: before PR + if err := gprovider.SetClient(ctx, nil, event, nil, nil); err != nil { return nil, options.E2E{}, github.New(), err } diff --git a/test/pkg/gitlab/setup.go b/test/pkg/gitlab/setup.go index 3876718cc..66b321cd0 100644 --- a/test/pkg/gitlab/setup.go +++ b/test/pkg/gitlab/setup.go @@ -60,7 +60,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, gitlab.Provider, erro Token: gitlabToken, URL: gitlabURL, }, - }, nil) + }, nil, nil) if err != nil { return nil, options.E2E{}, gitlab.Provider{}, err } diff --git a/test/testdata/OWNERS b/test/testdata/OWNERS new file mode 100644 index 000000000..c4b67d5f7 --- /dev/null +++ b/test/testdata/OWNERS @@ -0,0 +1,7 @@ +approvers: + - \\ .Approver // + +# vim: ft=yaml +# Local Variables: +# mode: yaml +# End: