diff --git a/.tekton/doc.yaml b/.tekton/doc.yaml index 36e4d5b2a..8ff738c11 100644 --- a/.tekton/doc.yaml +++ b/.tekton/doc.yaml @@ -40,7 +40,7 @@ spec: - name: source steps: - name: hugo-gen - image: registry.access.redhat.com/ubi9/python-311 # using this image since it has git and curl and python + image: registry.access.redhat.com/ubi9/python-311 # using this image since it has git and curl and python workingDir: $(workspaces.source.path) env: - name: UPLOADER_PUBLIC_URL diff --git a/config/300-repositories.yaml b/config/300-repositories.yaml index ac809fd4b..9f6eb9f81 100644 --- a/config/300-repositories.yaml +++ b/config/300-repositories.yaml @@ -70,6 +70,20 @@ spec: description: Settings relative to the Repository type: object properties: + policy: + type: object + description: Set policy on actions allowing only some teams + properties: + ok_to_test: + type: array + items: + description: list of teams allowed to run /ok-to-test + type: string + pull_request: + type: array + items: + description: list of teams allowed to have ci run on pull/merge requests. + type: string github_app_token_scope_repos: type: array items: diff --git a/docs/content/docs/guide/policy.md b/docs/content/docs/guide/policy.md new file mode 100644 index 000000000..d128d410f --- /dev/null +++ b/docs/content/docs/guide/policy.md @@ -0,0 +1,63 @@ +--- +title: Policy on actions +weight: 9 +--- +# Policy on Pipeline as Code actions + +Pipelines as Code has the concepts of Policy to let you control an action allowed +to be executed by a set of users belonging to a Team on an Organisation as +defined on GitHub or other Git Providers (only GitHub and Gitea is supported at +the moment). + +## List of actions supported + +* `pull_request` - This action is triggering the CI on Pipelines as Code, + specifying a team will only allow the members of the team to trigger the CI + and will not allow other members regadless if they are Owners or Collaborators + of the repository or the Organization. The OWNERS file is still taken into + account and will as well allow the members of the OWNERS file to trigger the + CI. +* `ok_to_test` - This action will let a user belonging to the allowed team to + issue a `/ok-to-test` comment on a Pull Request to trigger the CI on + Pipelines as Code, this let running the CI on Pull Request contributed by a + non collaborator of the repository or the organisation. This apply to the + `/test` and `/retest` commands as well. This take precendence on the + `pull_request` action. + +## Configuring the Policy on the Repository CR + +To configure the Policy on the Repository CR you need to add the following to the setting of the Repository CR: + +```yaml +apiVersion: "pipelinesascode.tekton.dev/v1alpha1" +kind: Repository +metadata: + name: repository1 +spec: + url: "https://github.com/org/repo" + settings: + policy: + ok_to_test: + - ci-admins + pull_request: + - ci-users +``` + +Users in `ci-admins` team will be able to let other users run the CI on the pull +request and users in `ci-users` team will be able to run the CI on their own +pull request. + +## Configuring teams on GitHub + +You will need to configure the GitHub Apps on your organisation to use this +feature. + +See the documentation on GitHub to configure the teams: + + + +## Configuring teams on Gitea + +Teams on Gitea are configured on the Organization level. No documentation is +available but you can look at the GitHub documentation to get an idea of how to +configure it. diff --git a/pkg/apis/pipelinesascode/v1alpha1/types.go b/pkg/apis/pipelinesascode/v1alpha1/types.go index eae9cf2de..d1703f468 100644 --- a/pkg/apis/pipelinesascode/v1alpha1/types.go +++ b/pkg/apis/pipelinesascode/v1alpha1/types.go @@ -70,7 +70,7 @@ type TaskInfos struct { // RepositorySpec is the spec of a repo type RepositorySpec struct { - ConcurrencyLimit *int `json:"concurrency_limit,omitempty"` + ConcurrencyLimit *int `json:"concurrency_limit,omitempty"` // move it to settings in further version of the spec URL string `json:"url"` GitProvider *GitProvider `json:"git_provider,omitempty"` Incomings *[]Incoming `json:"incoming,omitempty"` @@ -81,6 +81,12 @@ type RepositorySpec struct { type Settings struct { GithubAppTokenScopeRepos []string `json:"github_app_token_scope_repos,omitempty"` PipelineRunProvenance string `json:"pipelinerun_provenance,omitempty"` + Policy *Policy `json:"policy,omitempty"` +} + +type Policy struct { + OkToTest []string `json:"ok_to_test,omitempty"` + PullRequest []string `json:"pull_request,omitempty"` } type Params struct { diff --git a/pkg/params/info/events.go b/pkg/params/info/events.go index bc7a611bf..b0fc0cc1b 100644 --- a/pkg/params/info/events.go +++ b/pkg/params/info/events.go @@ -19,6 +19,8 @@ type Event struct { // TriggerTarget stable field across providers, ie: on Gitlab, Github and // others it would be always be pull_request we can rely on to know if it's // a push or a pull_request + // we need to merge this with the TriggerType type by doing a review of + // every instance using this and adapt it. TriggerTarget string // Target PipelineRun, the target PipelineRun user request. Used in incoming webhook @@ -89,3 +91,15 @@ func NewEvent() *Event { Request: &Request{}, } } + +type TriggerType string + +const ( + TriggerTypeOkToTest TriggerType = "ok-to-test" + TriggerTypeRetest TriggerType = "retest" + TriggerTypePush TriggerType = "push" + TriggerTypePullRequest TriggerType = "pull_request" + TriggerTypeCancel TriggerType = "cancel" + TriggerTypeCheckSuiteRerequested TriggerType = "check-suite-rerequested" + TriggerTypeCheckRunRerequested TriggerType = "check-run-rerequested" +) diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go index 397fe9c39..2da4773e6 100644 --- a/pkg/pipelineascode/match.go +++ b/pkg/pipelineascode/match.go @@ -98,7 +98,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) + err = p.vcx.SetClient(ctx, p.run, p.event, repo.Spec.Settings) if err != nil { return repo, err } diff --git a/pkg/policy/policy.go b/pkg/policy/policy.go new file mode 100644 index 000000000..b1421c3cc --- /dev/null +++ b/pkg/policy/policy.go @@ -0,0 +1,60 @@ +package policy + +import ( + "context" + "fmt" + + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" + "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" + "go.uber.org/zap" +) + +type Result int + +const ( + ResultNotSet Result = 0 + ResultAllowed Result = 1 + ResultDisallowed Result = 2 +) + +type Policy struct { + Settings *v1alpha1.Settings + Event *info.Event + VCX provider.Interface + Logger *zap.SugaredLogger +} + +func (p *Policy) IsAllowed(ctx context.Context, tType info.TriggerType) (Result, error) { + if p.Settings == nil || p.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 + case info.TriggerTypeOkToTest, info.TriggerTypeRetest: + sType = p.Settings.Policy.OkToTest + case info.TriggerTypePullRequest: + sType = p.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 + default: + return ResultNotSet, nil + } + + if len(sType) == 0 { + return ResultNotSet, nil + } + + 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) + } + if allowed { + return ResultAllowed, nil + } + return ResultDisallowed, fmt.Errorf(reasonMsg) +} diff --git a/pkg/policy/policy_test.go b/pkg/policy/policy_test.go new file mode 100644 index 000000000..ad8db35e6 --- /dev/null +++ b/pkg/policy/policy_test.go @@ -0,0 +1,195 @@ +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" + rtesting "knative.dev/pkg/reconciler/testing" +) + +func TestPolicy_IsAllowed(t *testing.T) { + type fields struct { + Settings *v1alpha1.Settings + Event *info.Event + } + type args struct { + tType info.TriggerType + } + tests := []struct { + name string + fields fields + args args + replyAllowed bool + want Result + wantErr bool + }{ + { + name: "Test Policy.IsAllowed with no settings", + fields: fields{ + Settings: nil, + Event: nil, + }, + args: args{ + tType: info.TriggerTypePush, + }, + want: ResultNotSet, + }, + { + name: "Test Policy.IsAllowed with unknown event type", + fields: fields{ + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + PullRequest: []string{"pull_request"}, + }, + }, + Event: nil, + }, + args: args{ + tType: "unknown", + }, + want: ResultNotSet, + }, + { + name: "allowing member not in team for pull request", + fields: fields{ + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + PullRequest: []string{"pull_request"}, + }, + }, + Event: info.NewEvent(), + }, + args: args{ + tType: info.TriggerTypePullRequest, + }, + replyAllowed: true, + want: ResultAllowed, + }, + { + name: "empty settings policy ignore", + fields: fields{ + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + PullRequest: []string{}, + }, + }, + Event: info.NewEvent(), + }, + args: args{ + tType: info.TriggerTypePullRequest, + }, + replyAllowed: true, + want: ResultNotSet, + }, + { + name: "disallowing member not in team for pull request", + fields: fields{ + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + PullRequest: []string{"pull_request"}, + }, + }, + Event: info.NewEvent(), + }, + args: args{ + tType: info.TriggerTypePullRequest, + }, + replyAllowed: false, + want: ResultDisallowed, + wantErr: true, + }, + { + name: "allowing member not in team for ok-to-test", + fields: fields{ + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + OkToTest: []string{"ok-to-test"}, + }, + }, + Event: info.NewEvent(), + }, + args: args{ + tType: info.TriggerTypeOkToTest, + }, + replyAllowed: true, + want: ResultAllowed, + }, + { + 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(), + }, + args: args{ + tType: info.TriggerTypeOkToTest, + }, + replyAllowed: false, + want: ResultDisallowed, + 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(), + }, + args: args{ + tType: info.TriggerTypeRetest, + }, + replyAllowed: true, + want: ResultAllowed, + }, + { + name: "disallowing member not in team for retest", + fields: fields{ + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + OkToTest: []string{"ok-to-test"}, + }, + }, + Event: info.NewEvent(), + }, + args: args{ + tType: info.TriggerTypeRetest, + }, + replyAllowed: false, + want: ResultDisallowed, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() + + vcx := &testprovider.TestProviderImp{PolicyDisallowing: !tt.replyAllowed} + p := &Policy{ + Settings: tt.fields.Settings, + Event: tt.fields.Event, + VCX: vcx, + Logger: logger, + } + 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 + } + if got != tt.want { + t.Errorf("Policy.IsAllowed() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/provider/bitbucketcloud/bitbucket.go b/pkg/provider/bitbucketcloud/bitbucket.go index fd6c29d48..f9d6dc7f1 100644 --- a/pkg/provider/bitbucketcloud/bitbucket.go +++ b/pkg/provider/bitbucketcloud/bitbucket.go @@ -8,6 +8,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" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -16,6 +17,8 @@ import ( "go.uber.org/zap" ) +var _ provider.Interface = (*Provider)(nil) + type Provider struct { Client *bitbucket.Client Logger *zap.SugaredLogger @@ -24,6 +27,11 @@ type Provider struct { provenance string } +// CheckPolicyAllowing TODO: Implement ME +func (v *Provider) CheckPolicyAllowing(_ context.Context, _ *info.Event, _ []string) (bool, string) { + return false, "" +} + // GetTaskURI TODO: Implement ME func (v *Provider) GetTaskURI(_ context.Context, _ *params.Run, _ *info.Event, _ string) (bool, string, error) { return false, "", nil @@ -157,7 +165,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) error { +func (v *Provider) SetClient(_ context.Context, _ *params.Run, event *info.Event, _ *v1alpha1.Settings) 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 f77db19f2..bc82dbde6 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) + err := v.SetClient(ctx, nil, tt.event, 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 d8659930e..4190a28ad 100644 --- a/pkg/provider/bitbucketserver/bitbucketserver.go +++ b/pkg/provider/bitbucketserver/bitbucketserver.go @@ -9,6 +9,7 @@ import ( bbv1 "github.com/gfleury/go-bitbucket-v1" "github.com/google/go-github/v52/github" "github.com/mitchellh/mapstructure" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -20,6 +21,8 @@ const taskStatusTemplate = ` {{range $taskrun := .TaskRunList }}* **{{ formatCondition $taskrun.PipelineRunTaskRunStatus.Status.Conditions }}** {{ $taskrun.ConsoleLogURL }} *{{ formatDuration $taskrun.Status.StartTime $taskrun.Status.CompletionTime }}* {{ end }}` +var _ provider.Interface = (*Provider)(nil) + type Provider struct { Client *bbv1.APIClient Logger *zap.SugaredLogger @@ -31,6 +34,10 @@ type Provider struct { projectKey string } +func (v *Provider) CheckPolicyAllowing(_ context.Context, _ *info.Event, _ []string) (bool, string) { + return false, "" +} + // GetTaskURI TODO: Implement ME func (v *Provider) GetTaskURI(_ context.Context, _ *params.Run, _ *info.Event, _ string) (bool, string, error) { return false, "", nil @@ -198,7 +205,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) error { +func (v *Provider) SetClient(ctx context.Context, _ *params.Run, event *info.Event, _ *v1alpha1.Settings) 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 5dc260a83..86147113b 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) + err := v.SetClient(ctx, nil, tt.opts, 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 75493401a..01e4a68bc 100644 --- a/pkg/provider/gitea/acl.go +++ b/pkg/provider/gitea/acl.go @@ -3,15 +3,68 @@ package gitea import ( "context" "fmt" + "net/http" "strings" "code.gitea.io/sdk/gitea" "github.com/openshift-pipelines/pipelines-as-code/pkg/acl" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" + "github.com/openshift-pipelines/pipelines-as-code/pkg/policy" giteaStructs "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea/structs" ) +func (v *Provider) CheckPolicyAllowing(_ context.Context, event *info.Event, allowedTeams []string) (bool, string) { + if event.Organization == event.Repository { + return true, "" + } + // TODO: caching + orgTeams, resp, err := v.Client.ListOrgTeams(event.Organization, gitea.ListTeamsOptions{}) + if resp.StatusCode == http.StatusNotFound { + // we explicitly disallow the policy when there is no team on org + return false, fmt.Sprintf("no teams on org %s", event.Organization) + } + if err != nil { + // probably a 500 or another api error, no need to try again and again with other teams + return false, fmt.Sprintf("error while getting org team, error: %s", err.Error()) + } + for _, allowedTeam := range allowedTeams { + for _, orgTeam := range orgTeams { + if orgTeam.Name == allowedTeam { + teamMember, _, err := v.Client.GetTeamMember(orgTeam.ID, event.Sender) + if err != nil { + v.Logger.Infof("error while getting team member: %s, error: %s", event.Sender, err.Error()) + continue + } + if teamMember.ID != 0 { + return true, fmt.Sprintf("allowing user: %s as a member of the team: %s", event.Sender, orgTeam.Name) + } + } + } + } + return false, fmt.Sprintf("user: %s is not a member of any of the allowed teams: %v", event.Sender, allowedTeams) +} + func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, error) { + aclPolicy := policy.Policy{ + Settings: v.repoSettings, + Event: event, + VCX: v, + Logger: v.Logger, + } + + // checkIfPolicyIsAllowing + tType, _ := detectTriggerTypeFromPayload("", event.Event) + policyRes, err := aclPolicy.IsAllowed(ctx, tType) + switch policyRes { + case policy.ResultAllowed: + 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 allowed, err := v.aclCheckAll(ctx, event) if err != nil { @@ -93,8 +146,6 @@ func (v *Provider) aclCheckAll(ctx context.Context, rev *info.Event) (bool, erro return acl.UserInOwnerFile(ownerContent, rev.Sender) } -// checkSenderOrgMembership Get sender user's organization. We can -// only get the one that the user sets as public 🤷 func (v *Provider) checkSenderRepoMembership(_ context.Context, runevent *info.Event) (bool, error) { ret, _, err := v.Client.IsCollaborator(runevent.Organization, runevent.Repository, runevent.Sender) return ret, err diff --git a/pkg/provider/gitea/acl_test.go b/pkg/provider/gitea/acl_test.go index 859a706f8..376170f6f 100644 --- a/pkg/provider/gitea/acl_test.go +++ b/pkg/provider/gitea/acl_test.go @@ -11,6 +11,8 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" giteaStructs "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea/structs" tgitea "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea/test" + "go.uber.org/zap" + zapobserver "go.uber.org/zap/zaptest/observer" "gotest.tools/v3/assert" rtesting "knative.dev/pkg/reconciler/testing" ) @@ -95,6 +97,8 @@ func TestOkToTestComment(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() tt.runevent.TriggerTarget = "ok-to-test-comment" fakeclient, mux, teardown := tgitea.Setup(t) defer teardown() @@ -111,6 +115,7 @@ func TestOkToTestComment(t *testing.T) { ctx, _ := rtesting.SetupFakeContext(t) gprovider := Provider{ Client: fakeclient, + Logger: logger, } isAllowed, err := gprovider.IsAllowed(ctx, &tt.runevent) @@ -175,10 +180,13 @@ func TestAclCheckAll(t *testing.T) { t.Run(tt.name, func(t *testing.T) { fakeclient, mux, teardown := tgitea.Setup(t) defer teardown() + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() ctx, _ := rtesting.SetupFakeContext(t) gprovider := Provider{ Client: fakeclient, + Logger: logger, } if tt.allowedRules.collabo { diff --git a/pkg/provider/gitea/detect.go b/pkg/provider/gitea/detect.go index 08211d62f..67f93770a 100644 --- a/pkg/provider/gitea/detect.go +++ b/pkg/provider/gitea/detect.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" giteaStructs "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea/structs" "go.uber.org/zap" @@ -14,9 +15,9 @@ import ( // returns (if is a Gitea event, whether to process or reject, logger with event metadata,, error if any occurred) func (v *Provider) Detect(req *http.Request, payload string, logger *zap.SugaredLogger) (bool, bool, *zap.SugaredLogger, string, error) { isGitea := false - event := req.Header.Get("X-Gitea-Event-Type") - if event == "" { - return false, false, logger, "no gitea event", nil + eventType := req.Header.Get("X-Gitea-Event-Type") + if eventType == "" { + return false, false, logger, "not a gitea event", nil } isGitea = true @@ -27,41 +28,51 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared return isGitea, processEvent, logger, reason, err } - eventInt, err := parseWebhook(whEventType(event), []byte(payload)) + eventInt, err := parseWebhook(whEventType(eventType), []byte(payload)) if err != nil { return setLoggerAndProceed(false, "", err) } _ = json.Unmarshal([]byte(payload), &eventInt) + eType, errReason := detectTriggerTypeFromPayload(eventType, eventInt) + if eType != "" { + return setLoggerAndProceed(true, "", nil) + } + + return setLoggerAndProceed(false, errReason, nil) +} + +// detectTriggerTypeFromPayload will detect the event type from the payload, +// filtering out the events that are not supported. +func detectTriggerTypeFromPayload(ghEventType string, eventInt any) (info.TriggerType, string) { + switch event := eventInt.(type) { + case *giteaStructs.PushPayload: + if event.Pusher != nil { + return info.TriggerTypePush, "" + } + return "", "invalid payload: no pusher in event" + case *giteaStructs.PullRequestPayload: + if provider.Valid(string(event.Action), []string{"opened", "synchronize", "synchronized", "reopened"}) { + return info.TriggerTypePullRequest, "" + } + return "", fmt.Sprintf("pull_request: unsupported action \"%s\"", event.Action) - switch gitEvent := eventInt.(type) { case *giteaStructs.IssueCommentPayload: - if gitEvent.Action == "created" && - gitEvent.Issue.PullRequest != nil && - gitEvent.Issue.State == "open" { - if provider.IsTestRetestComment(gitEvent.Comment.Body) { - return setLoggerAndProceed(true, "", nil) + if event.Action == "created" && + event.Issue.PullRequest != nil && + event.Issue.State == "open" { + if provider.IsTestRetestComment(event.Comment.Body) { + return info.TriggerTypeRetest, "" } - if provider.IsOkToTestComment(gitEvent.Comment.Body) { - return setLoggerAndProceed(true, "", nil) + if provider.IsOkToTestComment(event.Comment.Body) { + return info.TriggerTypeOkToTest, "" } - if provider.IsCancelComment(gitEvent.Comment.Body) { - return setLoggerAndProceed(true, "", nil) + if provider.IsCancelComment(event.Comment.Body) { + return info.TriggerTypeCancel, "" } - return setLoggerAndProceed(false, "", nil) - } - return setLoggerAndProceed(false, "not a issue comment we care about", nil) - case *giteaStructs.PullRequestPayload: - if provider.Valid(string(gitEvent.Action), []string{"opened", "synchronize", "synchronized", "reopened"}) { - return setLoggerAndProceed(true, "", nil) - } - return setLoggerAndProceed(false, fmt.Sprintf("not a merge event we care about: \"%s\"", - string(gitEvent.Action)), nil) - case *giteaStructs.PushPayload: - if gitEvent.Pusher != nil { - return setLoggerAndProceed(true, "", nil) + // this ignores the comment if it is not a PAC gitops comment and not return an error + return "", "" } - return setLoggerAndProceed(false, "push: no pusher in event", nil) - default: - return setLoggerAndProceed(false, "", fmt.Errorf("gitea: event \"%s\" is not supported", event)) + return "", "skip: not a PAC gitops comment" } + return "", fmt.Sprintf("gitea: event \"%v\" is not supported", ghEventType) } diff --git a/pkg/provider/gitea/detect_test.go b/pkg/provider/gitea/detect_test.go index 28d78e00c..9fa552eb2 100644 --- a/pkg/provider/gitea/detect_test.go +++ b/pkg/provider/gitea/detect_test.go @@ -25,7 +25,7 @@ func TestProviderDetect(t *testing.T) { }{ { name: "bad/test not a gitea request", - wantReason: "no gitea event", + wantReason: "not a gitea event", args: args{ req: &http.Request{ Header: http.Header{ @@ -57,7 +57,7 @@ func TestProviderDetect(t *testing.T) { }, payload: `{"foo": "bar"}`, }, - wantReason: "push: no pusher in event", + wantReason: "invalid payload: no pusher in event", isGitea: true, }, { @@ -70,7 +70,7 @@ func TestProviderDetect(t *testing.T) { }, payload: `{"action": "foo"}`, }, - wantReason: `not a merge event we care about: "foo"`, + wantReason: `pull_request: unsupported action "foo"`, isGitea: true, }, { @@ -83,7 +83,7 @@ func TestProviderDetect(t *testing.T) { }, payload: `{"action": "foo"}`, }, - wantReason: `not a issue comment we care about`, + wantReason: `skip: not a PAC gitops comment`, isGitea: true, }, { diff --git a/pkg/provider/gitea/gitea.go b/pkg/provider/gitea/gitea.go index 8c45acf28..6e0b37631 100644 --- a/pkg/provider/gitea/gitea.go +++ b/pkg/provider/gitea/gitea.go @@ -9,6 +9,7 @@ import ( "strings" "code.gitea.io/sdk/gitea" + "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/info" @@ -34,13 +35,17 @@ const ( ` ) +// validate the struct to interface +var _ provider.Interface = (*Provider)(nil) + type Provider struct { Client *gitea.Client Logger *zap.SugaredLogger Token *string giteaInstanceURL string // only exposed for e2e tests - Password string + Password string + repoSettings *v1alpha1.Settings } // GetTaskURI TODO: Implement ME @@ -75,7 +80,7 @@ func (v *Provider) GetConfig() *info.ProviderConfig { } } -func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event) error { +func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event, repoSettings *v1alpha1.Settings) error { var err error apiURL := runevent.Provider.URL // password is not exposed to CRD, it's only used from the e2e tests @@ -91,6 +96,7 @@ func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Ev return err } v.giteaInstanceURL = runevent.Provider.URL + v.repoSettings = repoSettings return nil } diff --git a/pkg/provider/github/acl.go b/pkg/provider/github/acl.go index 4a775e1ef..605c30224 100644 --- a/pkg/provider/github/acl.go +++ b/pkg/provider/github/acl.go @@ -9,9 +9,55 @@ import ( "github.com/google/go-github/v52/github" "github.com/openshift-pipelines/pipelines-as-code/pkg/acl" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" + "github.com/openshift-pipelines/pipelines-as-code/pkg/policy" ) +// CheckPolicyAllowing check that policy is +func (v *Provider) CheckPolicyAllowing(ctx context.Context, event *info.Event, allowedTeams []string) (bool, string) { + for _, team := range allowedTeams { + // TODO: caching + members, resp, err := v.Client.Teams.ListTeamMembersBySlug(ctx, event.Organization, team, &github.TeamListTeamMembersOptions{}) + if resp.StatusCode == http.StatusNotFound { + // we explicitly disallow the policy when the team is not found + // maybe we should ignore it instead? i'd rather keep this explicit + // and conservative since being security related. + return false, fmt.Sprintf("team: %s is not found on the organization: %s", team, event.Organization) + } + if err != nil { + // probably a 500 or another api error, no need to try again and again with other teams + return false, fmt.Sprintf("error while getting team membership for user: %s in team: %s, error: %s", event.Sender, team, err.Error()) + } + for _, member := range members { + if member.GetLogin() == event.Sender { + return true, fmt.Sprintf("allowing user: %s as a member of the team: %s", event.Sender, team) + } + } + } + + return false, fmt.Sprintf("user: %s is not a member of any of the allowed teams: %v", event.Sender, allowedTeams) +} + func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, error) { + aclPolicy := policy.Policy{ + Settings: v.repoSettings, + Event: event, + VCX: v, + Logger: v.Logger, + } + + // checkIfPolicyIsAllowing + tType, _ := detectTriggerTypeFromPayload("", event.Event) + policyRes, err := aclPolicy.IsAllowed(ctx, tType) + switch policyRes { + case policy.ResultAllowed: + 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 allowed, err := v.aclCheckAll(ctx, event) if err != nil { diff --git a/pkg/provider/github/acl_test.go b/pkg/provider/github/acl_test.go index 136b814d3..f64778874 100644 --- a/pkg/provider/github/acl_test.go +++ b/pkg/provider/github/acl_test.go @@ -8,11 +8,84 @@ import ( "testing" "github.com/google/go-github/v52/github" + "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" ghtesthelper "github.com/openshift-pipelines/pipelines-as-code/pkg/test/github" + "go.uber.org/zap" + zapobserver "go.uber.org/zap/zaptest/observer" + "gotest.tools/v3/assert" rtesting "knative.dev/pkg/reconciler/testing" ) +func TestCheckPolicyAllowing(t *testing.T) { + tests := []struct { + name string + allowedTeams []string + reply string + otherReply string + wantAllowed bool + wantReason string + }{ + { + name: "user is a member of the allowed team", + allowedTeams: []string{"allowedTeam"}, + wantAllowed: true, + wantReason: "allowing user: allowedUser as a member of the team: allowedTeam", + reply: `[{"login": "allowedUser"}]`, + }, + { + name: "user is not a member of the allowed team", + allowedTeams: []string{"otherteam"}, + wantAllowed: false, + wantReason: "user: allowedUser is not a member of any of the allowed teams: [otherteam]", + otherReply: `[{"login": "myuser"}]`, + }, + { + name: "team is not found", + allowedTeams: []string{"nothere"}, + wantAllowed: false, + wantReason: "team: nothere is not found on the organization: myorg", + }, + { + name: "error while getting team membership", + allowedTeams: []string{"allowedTeam"}, + wantAllowed: false, + wantReason: `error while getting team membership for user: allowedUser in team: allowedTeam, error: invalid character 't' in literal true (expecting 'r')`, + reply: `tttttt`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeclient, mux, _, teardown := ghtesthelper.SetupGH() + defer teardown() + + event := &info.Event{ + Organization: "myorg", + Sender: "allowedUser", + } + mux.HandleFunc("/orgs/myorg/teams/allowedTeam/members", func(rw http.ResponseWriter, r *http.Request) { + fmt.Fprint(rw, tt.reply) + }) + mux.HandleFunc("/orgs/myorg/teams/otherteam/members", func(rw http.ResponseWriter, r *http.Request) { + fmt.Fprint(rw, tt.otherReply) + }) + ctx, _ := rtesting.SetupFakeContext(t) + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() + gprovider := Provider{ + Client: fakeclient, + repoSettings: &v1alpha1.Settings{}, + Logger: logger, + } + + gotAllowed, gotReason := gprovider.CheckPolicyAllowing(ctx, event, tt.allowedTeams) + assert.Equal(t, tt.wantAllowed, gotAllowed) + assert.Equal(t, tt.wantReason, gotReason) + }) + } +} + func TestOkToTestComment(t *testing.T) { tests := []struct { name string @@ -119,8 +192,12 @@ func TestOkToTestComment(t *testing.T) { fmt.Fprint(rw, "[]") }) ctx, _ := rtesting.SetupFakeContext(t) + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() gprovider := Provider{ - Client: fakeclient, + Client: fakeclient, + repoSettings: &v1alpha1.Settings{}, + Logger: logger, } got, err := gprovider.IsAllowed(ctx, &tt.runevent) @@ -182,9 +259,12 @@ func TestAclCheckAll(t *testing.T) { rw.WriteHeader(204) }) + observer, _ := zapobserver.New(zap.InfoLevel) + logger := zap.New(observer).Sugar() ctx, _ := rtesting.SetupFakeContext(t) gprovider := Provider{ Client: fakeclient, + Logger: logger, } tests := []struct { @@ -369,6 +449,9 @@ func TestIfPullRequestIsForSameRepoWithoutFork(t *testing.T) { gprovider := Provider{ Client: fakeclient, + repoSettings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{}, + }, } got, err := gprovider.aclCheckAll(ctx, tt.event) diff --git a/pkg/provider/github/detect.go b/pkg/provider/github/detect.go index 76b79db92..e020df8bc 100644 --- a/pkg/provider/github/detect.go +++ b/pkg/provider/github/detect.go @@ -6,6 +6,7 @@ import ( "net/http" "github.com/google/go-github/v52/github" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" "go.uber.org/zap" ) @@ -39,7 +40,7 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared } _ = json.Unmarshal([]byte(payload), &eventInt) - eType, errReason := detectEventTypeFromPayload(eventType, eventInt) + eType, errReason := detectTriggerTypeFromPayload(eventType, eventInt) if eType != "" { return setLoggerAndProceed(true, "", nil) } @@ -47,19 +48,19 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared return setLoggerAndProceed(false, errReason, nil) } -// detectEventTypeFromPayload will detect the event type from the payload, +// detectTriggerTypeFromPayload will detect the event type from the payload, // filtering out the events that are not supported. // first arg will get the event type and the second one will get an error string explaining why it's not supported. -func detectEventTypeFromPayload(eventType string, eventInt any) (string, string) { +func detectTriggerTypeFromPayload(ghEventType string, eventInt any) (info.TriggerType, string) { switch event := eventInt.(type) { case *github.PushEvent: if event.GetPusher() != nil { - return "push", "" + return info.TriggerTypePush, "" } return "", "no pusher in payload" case *github.PullRequestEvent: if provider.Valid(event.GetAction(), []string{"opened", "synchronize", "synchronized", "reopened"}) { - return "pull_request", "" + return info.TriggerTypePullRequest, "" } return "", fmt.Sprintf("pull_request: unsupported action \"%s\"", event.GetAction()) case *github.IssueCommentEvent: @@ -67,26 +68,26 @@ func detectEventTypeFromPayload(eventType string, eventInt any) (string, string) event.GetIssue().IsPullRequest() && event.GetIssue().GetState() == "open" { if provider.IsTestRetestComment(event.GetComment().GetBody()) { - return "comment-retest", "" + return info.TriggerTypeRetest, "" } if provider.IsOkToTestComment(event.GetComment().GetBody()) { - return "comment-ok-to-test", "" + return info.TriggerTypeOkToTest, "" } if provider.IsCancelComment(event.GetComment().GetBody()) { - return "comment-cancel", "" + return info.TriggerTypeCancel, "" } } return "", "comment: not a PAC gitops pull request comment" case *github.CheckSuiteEvent: if event.GetAction() == "rerequested" && event.GetCheckSuite() != nil { - return "check-suite-rerequested", "" + return info.TriggerTypeCheckSuiteRerequested, "" } return "", fmt.Sprintf("check_suite: unsupported action \"%s\"", event.GetAction()) case *github.CheckRunEvent: if event.GetAction() == "rerequested" && event.GetCheckRun() != nil { - return "check-run-rerequested", "" + return info.TriggerTypeCheckRunRerequested, "" } return "", fmt.Sprintf("check_run: unsupported action \"%s\"", event.GetAction()) } - return "", fmt.Sprintf("github: event \"%v\" is not supported", eventType) + return "", fmt.Sprintf("github: event \"%v\" is not supported", ghEventType) } diff --git a/pkg/provider/github/github.go b/pkg/provider/github/github.go index 25a8eab93..10aed34da 100644 --- a/pkg/provider/github/github.go +++ b/pkg/provider/github/github.go @@ -13,6 +13,7 @@ import ( "github.com/google/go-github/v52/github" "github.com/jonboulle/clockwork" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -31,6 +32,8 @@ const ( publicRawURLHost = "raw.githubusercontent.com" ) +var _ provider.Interface = (*Provider)(nil) + type Provider struct { Client *github.Client Logger *zap.SugaredLogger @@ -40,6 +43,7 @@ type Provider struct { provenance string Run *params.Run RepositoryIDs []int64 + repoSettings *v1alpha1.Settings skippedRun } @@ -230,10 +234,11 @@ func (v *Provider) checkWebhookSecretValidity(ctx context.Context, cw clockwork. return nil } -func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.Event) error { +func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.Event, repoSettings *v1alpha1.Settings) error { client, providerName, apiURL := makeClient(ctx, event.Provider.URL, event.Provider.Token) v.providerName = providerName v.Run = run + v.repoSettings = repoSettings // 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 c6b22d352..3e4193d8b 100644 --- a/pkg/provider/github/github_test.go +++ b/pkg/provider/github/github_test.go @@ -488,7 +488,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) + err := v.SetClient(ctx, nil, tt.event, 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/github/parse_payload.go b/pkg/provider/github/parse_payload.go index 0136ab86b..8c9f046ab 100644 --- a/pkg/provider/github/parse_payload.go +++ b/pkg/provider/github/parse_payload.go @@ -162,6 +162,7 @@ func (v *Provider) ParsePayload(ctx context.Context, run *params.Run, request *h return nil, err } + processedEvent.Event = eventInt processedEvent.InstallationID = installationIDFrompayload processedEvent.GHEURL = event.Provider.URL processedEvent.Provider.URL = event.Provider.URL @@ -197,7 +198,6 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt var err error processedEvent = info.NewEvent() - processedEvent.Event = eventInt switch gitEvent := eventInt.(type) { case *github.CheckRunEvent: diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index d43e3b259..cf7c96953 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -35,6 +36,8 @@ const ( ` ) +var _ provider.Interface = (*Provider)(nil) + type Provider struct { Client *gitlab.Client Logger *zap.SugaredLogger @@ -52,6 +55,11 @@ func (v *Provider) GetTaskURI(_ context.Context, _ *params.Run, _ *info.Event, _ return false, "", nil } +// CheckPolicyAllowing TODO: Implement ME +func (v *Provider) CheckPolicyAllowing(_ context.Context, _ *info.Event, _ []string) (bool, string) { + return false, "" +} + func (v *Provider) SetLogger(logger *zap.SugaredLogger) { v.Logger = logger } @@ -90,7 +98,7 @@ func (v *Provider) GetConfig() *info.ProviderConfig { } } -func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event) error { +func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event, _ *v1alpha1.Settings) 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 d7a921890..13c864591 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) + assert.Assert(t, v.SetClient(ctx, nil, info.NewEvent(), nil) != nil) client, _, tearDown := thelp.Setup(t) defer tearDown() @@ -206,7 +206,7 @@ func TestSetClient(t *testing.T) { Provider: &info.Provider{ Token: "hello", }, - }) + }, 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) + err := v.SetClient(ctx, nil, event, 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) + err = v.SetClient(ctx, nil, event, nil) assert.NilError(t, err) assert.Equal(t, fakehost, v.apiURL) diff --git a/pkg/provider/interface.go b/pkg/provider/interface.go index 89b23f301..f63d02e8e 100644 --- a/pkg/provider/interface.go +++ b/pkg/provider/interface.go @@ -4,6 +4,7 @@ import ( "context" "net/http" + "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/info" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -32,12 +33,13 @@ 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) error + SetClient(context.Context, *params.Run, *info.Event, *v1alpha1.Settings) error GetCommitInfo(context.Context, *info.Event) error GetConfig() *info.ProviderConfig GetFiles(context.Context, *info.Event) ([]string, error) GetTaskURI(ctx context.Context, params *params.Run, event *info.Event, uri string) (bool, string, error) CreateToken(context.Context, []string, *params.Run, *info.Event) (string, error) + CheckPolicyAllowing(context.Context, *info.Event, []string) (bool, string) } const DefaultProviderAPIUser = "git" diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index 82e9928c5..3f2ff876b 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -116,7 +116,7 @@ func (r *Reconciler) reportFinalStatus(ctx context.Context, logger *zap.SugaredL } } - err = provider.SetClient(ctx, r.run, event) + err = provider.SetClient(ctx, r.run, event, repo.Spec.Settings) if err != nil { return repo, fmt.Errorf("cannot set client: %w", err) } @@ -181,7 +181,7 @@ func (r *Reconciler) updatePipelineRunToInProgress(ctx context.Context, logger * } } - err = p.SetClient(ctx, r.run, event) + err = p.SetClient(ctx, r.run, event, repo.Spec.Settings) 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 0ab28525a..54e7a91d6 100644 --- a/pkg/test/provider/testwebvcs.go +++ b/pkg/test/provider/testwebvcs.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" + "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/info" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" @@ -21,6 +22,14 @@ type TestProviderImp struct { CreateStatusErorring bool FilesInsideRepo map[string]string WantProviderRemoteTask bool + PolicyDisallowing bool +} + +func (v *TestProviderImp) CheckPolicyAllowing(_ context.Context, _ *info.Event, _ []string) (bool, string) { + if v.PolicyDisallowing { + return false, "policy disallowing" + } + return true, "" } func (v *TestProviderImp) SetLogger(_ *zap.SugaredLogger) { @@ -46,7 +55,7 @@ func (v *TestProviderImp) GetCommitInfo(_ context.Context, _ *info.Event) error return nil } -func (v *TestProviderImp) SetClient(_ context.Context, _ *params.Run, _ *info.Event) error { +func (v *TestProviderImp) SetClient(_ context.Context, _ *params.Run, _ *info.Event, _ *v1alpha1.Settings) error { return nil } diff --git a/test/gitea_access_control_test.go b/test/gitea_access_control_test.go new file mode 100644 index 000000000..59ebbbeab --- /dev/null +++ b/test/gitea_access_control_test.go @@ -0,0 +1,309 @@ +//go:build e2e +// +build e2e + +package test + +import ( + "context" + "fmt" + "os" + "regexp" + "testing" + + "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/settings" + 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/payload" + pacrepo "github.com/openshift-pipelines/pipelines-as-code/test/pkg/repository" + twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/names" + "gopkg.in/yaml.v2" + "gotest.tools/v3/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// TestGiteaPolicyOkToTestRetest test the policy on pull request scenario +// create a CRD which a policy allowing only users in the team pull_requester to allow a PR +// we create a org +// we create a team normal on org and add the user normal-$RANDOM onto it +// we create a pull request form a fork +// we test that it was denied +// we create a pull request from a fork with the user pull_requester which is in the allowed pull_requester team +// we test that it was allowed succeeded +// +// this test paths is mostly to test the logic in the pkg/policy package, there +// is not much gitea specifics compared to github +func TestGiteaPolicyPullRequest(t *testing.T) { + topts := &tgitea.TestOpts{ + OnOrg: true, + SkipEventsCheck: true, + TargetEvent: options.PullRequestEvent, + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + PullRequest: []string{"pull_requester"}, + }, + }, + YAMLFiles: map[string]string{".tekton/pr.yaml": "testdata/pipelinerun.yaml"}, + } + tgitea.TestPR(t, topts) + topts.ParamsRun.Clients.Log.Infof("Repo CRD %s has been created with Policy: %+v", topts.TargetRefName, topts.Settings.Policy) + + orgName := "org-" + topts.TargetRefName + topts.Opts.Organization = orgName + + // create normal team on org and add user normal onto it + normalTeam, err := tgitea.CreateTeam(topts, orgName, "normal") + assert.NilError(t, err) + normalUserNamePasswd := fmt.Sprintf("normal-%s", topts.TargetRefName) + normalUserCnx, normalUser, err := tgitea.CreateGiteaUserSecondCnx(topts, normalUserNamePasswd, normalUserNamePasswd) + assert.NilError(t, err) + _, err = topts.GiteaCNX.Client.AddTeamMember(normalTeam.ID, normalUser.UserName) + assert.NilError(t, err) + topts.ParamsRun.Clients.Log.Infof("User %s has been added to team %s", normalUser.UserName, normalTeam.Name) + tgitea.CreateForkPullRequest(t, topts, normalUserCnx, "", "echo Hello from user "+topts.TargetRefName) + topts.CheckForStatus = "failure" + topts.Regexp = regexp.MustCompile( + fmt.Sprintf(`.*policy check: pull_request, user: %s is not a member of any of the allowed teams.*`, normalUserNamePasswd)) + tgitea.WaitForPullRequestCommentMatch(t, topts) + tgitea.WaitForStatus(t, topts, topts.TargetRefName, settings.PACApplicationNameDefaultValue) + + pullRequesterTeam, err := tgitea.CreateTeam(topts, orgName, "pull_requester") + assert.NilError(t, err) + pullRequesterUserNamePasswd := fmt.Sprintf("pullRequester-%s", topts.TargetRefName) + pullRequesterUserCnx, pullRequesterUser, err := tgitea.CreateGiteaUserSecondCnx(topts, pullRequesterUserNamePasswd, pullRequesterUserNamePasswd) + assert.NilError(t, err) + _, err = topts.GiteaCNX.Client.AddTeamMember(pullRequesterTeam.ID, pullRequesterUser.UserName) + assert.NilError(t, err) + topts.ParamsRun.Clients.Log.Infof("User %s has been added to team %s", pullRequesterUser.UserName, pullRequesterTeam.Name) + tgitea.CreateForkPullRequest(t, topts, pullRequesterUserCnx, "", "echo Hello from user "+topts.TargetRefName) + topts.Regexp = successRegexp + tgitea.WaitForPullRequestCommentMatch(t, topts) +} + +// TestGiteaPolicyOkToTestRetest test the /ok-to-test policy scenarios +// create a CRD which a policy allowing only users in the team /ok-to-test to allow a PR +// we create a org +// we create a team ok-to-test on org and add the user ok-to-test-$RANDOM onto it +// we create a team normal on org and add the user normal-$RANDOM onto it +// we issue a /ok-to-test as user normal and check it was denied +// we delete the old pac comment to make the pac reliable checking it was denied. +// we issue a /retest as user normal and check it was denied +// we issue a /ok-to-test as user ok-to-test and check it was succeeded +// +// this test paths is mostly to test the logic in the pkg/policy package, there +// is not much gitea specifics compared to github +func TestGiteaPolicyOkToTestRetest(t *testing.T) { + topts := &tgitea.TestOpts{ + Regexp: regexp.MustCompile(fmt.Sprintf(`.*User %s is not allowed to run CI on this repo`, os.Getenv("TEST_GITEA_USERNAME"))), + OnOrg: true, + SkipEventsCheck: true, + TargetEvent: options.PullRequestEvent, + Settings: &v1alpha1.Settings{ + Policy: &v1alpha1.Policy{ + OkToTest: []string{"ok-to-test"}, + }, + }, + YAMLFiles: map[string]string{".tekton/pr.yaml": "testdata/pipelinerun.yaml"}, + } + tgitea.TestPR(t, topts) + 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) + okToTestUserNamePasswd := fmt.Sprintf("ok-to-test-%s", topts.TargetRefName) + okToTestUserCnx, okToTestUser, err := tgitea.CreateGiteaUserSecondCnx(topts, okToTestUserNamePasswd, okToTestUserNamePasswd) + assert.NilError(t, err) + _, err = topts.GiteaCNX.Client.AddTeamMember(oktotestTeam.ID, okToTestUser.UserName) + assert.NilError(t, err) + topts.ParamsRun.Clients.Log.Infof("User %s has been added to team %s", okToTestUser.UserName, oktotestTeam.Name) + + // create normal team on org and add user normal onto it + normalTeam, err := tgitea.CreateTeam(topts, orgName, "normal") + assert.NilError(t, err) + normalUserNamePasswd := fmt.Sprintf("normal-%s", topts.TargetRefName) + normalUserCnx, normalUser, err := tgitea.CreateGiteaUserSecondCnx(topts, normalUserNamePasswd, normalUserNamePasswd) + assert.NilError(t, err) + _, err = topts.GiteaCNX.Client.AddTeamMember(normalTeam.ID, normalUser.UserName) + assert.NilError(t, err) + topts.ParamsRun.Clients.Log.Infof("User %s has been added to team %s", normalUser.UserName, normalTeam.Name) + + 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") + 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" + tgitea.WaitForPullRequestCommentMatch(t, topts) + tgitea.WaitForStatus(t, topts, topts.TargetRefName, settings.PACApplicationNameDefaultValue) + + // 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: ok-to-test, user: %s is not a member of any of the allowed teams.*`, normalUserNamePasswd)) + topts.CheckForStatus = "failure" + tgitea.WaitForPullRequestCommentMatch(t, topts) + + tgitea.WaitForStatus(t, topts, topts.TargetRefName, settings.PACApplicationNameDefaultValue) + 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, topts.TargetRefName, "") +} + +func TestGiteaACLOrgAllowed(t *testing.T) { + topts := &tgitea.TestOpts{ + TargetEvent: options.PullRequestEvent, + YAMLFiles: map[string]string{ + ".tekton/pr.yaml": "testdata/pipelinerun.yaml", + }, + NoCleanup: true, + ExpectEvents: false, + } + defer tgitea.TestPR(t, topts)() + secondcnx, _, err := tgitea.CreateGiteaUserSecondCnx(topts, topts.TargetRefName, topts.GiteaPassword) + assert.NilError(t, err) + + tgitea.CreateForkPullRequest(t, topts, secondcnx, "read", "echo Hello from user "+topts.TargetRefName) + topts.CheckForStatus = "success" + tgitea.WaitForStatus(t, topts, topts.TargetRefName, "") +} + +func TestGiteaACLOrgSkipped(t *testing.T) { + topts := &tgitea.TestOpts{ + TargetEvent: options.PullRequestEvent, + YAMLFiles: map[string]string{ + ".tekton/pr.yaml": "testdata/pipelinerun.yaml", + }, + NoCleanup: true, + ExpectEvents: false, + } + defer tgitea.TestPR(t, topts)() + secondcnx, _, err := tgitea.CreateGiteaUserSecondCnx(topts, topts.TargetRefName, topts.GiteaPassword) + assert.NilError(t, err) + + topts.PullRequest = tgitea.CreateForkPullRequest(t, topts, secondcnx, "", "echo Hello from user "+topts.TargetRefName) + topts.CheckForStatus = "success" + tgitea.WaitForStatus(t, topts, topts.PullRequest.Head.Sha, "") + topts.Regexp = regexp.MustCompile(`.*is skipping this commit.*`) + tgitea.WaitForPullRequestCommentMatch(t, topts) +} + +func TestGiteaACLCommentsAllowing(t *testing.T) { + tests := []struct { + name, comment string + }{ + { + name: "OK to Test", + comment: "/ok-to-test", + }, + { + name: "Retest", + comment: "/retest", + }, + { + name: "Test PR", + comment: "/test pr", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + topts := &tgitea.TestOpts{ + TargetEvent: options.PullRequestEvent, + YAMLFiles: map[string]string{ + ".tekton/pr.yaml": "testdata/pipelinerun.yaml", + }, + NoCleanup: true, + ExpectEvents: false, + } + defer tgitea.TestPR(t, topts)() + secondcnx, _, err := tgitea.CreateGiteaUserSecondCnx(topts, topts.TargetRefName, topts.GiteaPassword) + assert.NilError(t, err) + + topts.PullRequest = tgitea.CreateForkPullRequest(t, topts, secondcnx, "", "echo Hello from user "+topts.TargetRefName) + topts.CheckForStatus = "success" + tgitea.WaitForStatus(t, topts, topts.PullRequest.Head.Sha, "") + topts.Regexp = regexp.MustCompile(`.*is skipping this commit.*`) + tgitea.WaitForPullRequestCommentMatch(t, topts) + + tgitea.PostCommentOnPullRequest(t, topts, tt.comment) + topts.Regexp = successRegexp + tgitea.WaitForPullRequestCommentMatch(t, topts) + }) + } +} + +func TestGiteAClusterTasks(t *testing.T) { + // we need to verify sure to create clustertask before pushing the files + // so we have to create a new client and do more manual things we get for free in TestPR + topts := &tgitea.TestOpts{ + TargetEvent: "pull_request, push", + YAMLFiles: map[string]string{ + ".tekton/prcluster.yaml": "testdata/pipelinerunclustertasks.yaml", + }, + ExpectEvents: false, + } + topts.TargetRefName = names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + topts.TargetNS = topts.TargetRefName + + // create first the cluster tasks + ctname := fmt.Sprintf(".tekton/%s.yaml", topts.TargetNS) + newyamlFiles := map[string]string{ctname: "testdata/clustertask.yaml"} + entries, err := payload.GetEntries(newyamlFiles, topts.TargetNS, "main", "pull_request", map[string]string{}) + assert.NilError(t, err) + //nolint: staticcheck + ct := v1beta1.ClusterTask{} + assert.NilError(t, yaml.Unmarshal([]byte(entries[ctname]), &ct)) + ct.Name = "clustertask-" + topts.TargetNS + + run := ¶ms.Run{} + ctx := context.Background() + assert.NilError(t, run.Clients.NewClients(ctx, &run.Info)) + // TODO(chmou): this is for v1beta1, we need to figure out a way how to do that on v1 + _, err = run.Clients.Tekton.TektonV1beta1().ClusterTasks().Create(context.TODO(), &ct, metav1.CreateOptions{}) + assert.NilError(t, err) + assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, run)) + run.Clients.Log.Infof("%s has been created", ct.GetName()) + defer (func() { + assert.NilError(t, topts.ParamsRun.Clients.Tekton.TektonV1beta1().ClusterTasks().Delete(context.TODO(), ct.Name, metav1.DeleteOptions{})) + run.Clients.Log.Infof("%s is deleted", ct.GetName()) + })() + + // start PR + defer tgitea.TestPR(t, topts)() + + // wait for it + waitOpts := twait.Opts{ + RepoName: topts.TargetNS, + Namespace: topts.TargetNS, + // 0 means 1 🙃 (we test for >, while we actually should do >=, but i + // need to go all over the code to verify it's not going to break + // anything else) + MinNumberStatus: 0, + PollTimeout: twait.DefaultTimeout, + TargetSHA: topts.PullRequest.Head.Sha, + } + err = twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts) + assert.NilError(t, err) + + topts.CheckForStatus = "success" + tgitea.WaitForStatus(t, topts, topts.TargetRefName, "") +} + +// Local Variables: +// compile-command: "go test -tags=e2e -v -run TestGiteaPush ." +// End: diff --git a/test/gitea_test.go b/test/gitea_test.go index 165919416..2569dab37 100644 --- a/test/gitea_test.go +++ b/test/gitea_test.go @@ -26,7 +26,6 @@ import ( pacrepo "github.com/openshift-pipelines/pipelines-as-code/test/pkg/repository" "github.com/openshift-pipelines/pipelines-as-code/pkg/git" - "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/params/settings" tknpactest "github.com/openshift-pipelines/pipelines-as-code/test/pkg/cli" @@ -35,9 +34,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/secret" twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait" - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" - "gopkg.in/yaml.v2" "gotest.tools/v3/assert" "gotest.tools/v3/env" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -206,7 +203,7 @@ func TestGiteaConcurrencyExclusivenessMultipleRuns(t *testing.T) { } topts.CheckForStatus = "success" - tgitea.WaitForStatus(t, topts, topts.TargetRefName) + tgitea.WaitForStatus(t, topts, topts.TargetRefName, "") topts.Regexp = successRegexp tgitea.WaitForPullRequestCommentMatch(t, topts) @@ -230,89 +227,7 @@ func TestGiteaRetestAfterPush(t *testing.T) { assert.NilError(t, err) tgitea.PushFilesToRefGit(t, topts, entries, topts.TargetRefName) topts.CheckForStatus = "success" - tgitea.WaitForStatus(t, topts, topts.TargetRefName) -} - -func TestGiteaACLOrgAllowed(t *testing.T) { - topts := &tgitea.TestOpts{ - TargetEvent: options.PullRequestEvent, - YAMLFiles: map[string]string{ - ".tekton/pr.yaml": "testdata/pipelinerun.yaml", - }, - NoCleanup: true, - ExpectEvents: false, - } - defer tgitea.TestPR(t, topts)() - secondcnx, err := tgitea.CreateGiteaUser(topts.GiteaCNX.Client, topts.GiteaAPIURL, topts.TargetRefName, topts.GiteaPassword) - assert.NilError(t, err) - - tgitea.CreateForkPullRequest(t, topts, secondcnx, "read", "echo Hello from user "+topts.TargetRefName) - topts.CheckForStatus = "success" - tgitea.WaitForStatus(t, topts, topts.TargetRefName) -} - -func TestGiteaACLOrgSkipped(t *testing.T) { - topts := &tgitea.TestOpts{ - TargetEvent: options.PullRequestEvent, - YAMLFiles: map[string]string{ - ".tekton/pr.yaml": "testdata/pipelinerun.yaml", - }, - NoCleanup: true, - ExpectEvents: false, - } - defer tgitea.TestPR(t, topts)() - secondcnx, err := tgitea.CreateGiteaUser(topts.GiteaCNX.Client, topts.GiteaAPIURL, topts.TargetRefName, topts.GiteaPassword) - assert.NilError(t, err) - - topts.PullRequest = tgitea.CreateForkPullRequest(t, topts, secondcnx, "", "echo Hello from user "+topts.TargetRefName) - topts.CheckForStatus = "success" - tgitea.WaitForStatus(t, topts, topts.PullRequest.Head.Sha) - topts.Regexp = regexp.MustCompile(`.*is skipping this commit.*`) - tgitea.WaitForPullRequestCommentMatch(t, topts) -} - -func TestGiteaACLCommentsAllowing(t *testing.T) { - tests := []struct { - name, comment string - }{ - { - name: "OK to Test", - comment: "/ok-to-test", - }, - { - name: "Retest", - comment: "/retest", - }, - { - name: "Test PR", - comment: "/test pr", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - topts := &tgitea.TestOpts{ - TargetEvent: options.PullRequestEvent, - YAMLFiles: map[string]string{ - ".tekton/pr.yaml": "testdata/pipelinerun.yaml", - }, - NoCleanup: true, - ExpectEvents: false, - } - defer tgitea.TestPR(t, topts)() - secondcnx, err := tgitea.CreateGiteaUser(topts.GiteaCNX.Client, topts.GiteaAPIURL, topts.TargetRefName, topts.GiteaPassword) - assert.NilError(t, err) - - topts.PullRequest = tgitea.CreateForkPullRequest(t, topts, secondcnx, "", "echo Hello from user "+topts.TargetRefName) - topts.CheckForStatus = "success" - tgitea.WaitForStatus(t, topts, topts.PullRequest.Head.Sha) - topts.Regexp = regexp.MustCompile(`.*is skipping this commit.*`) - tgitea.WaitForPullRequestCommentMatch(t, topts) - - tgitea.PostCommentOnPullRequest(t, topts, tt.comment) - topts.Regexp = successRegexp - tgitea.WaitForPullRequestCommentMatch(t, topts) - }) - } + tgitea.WaitForStatus(t, topts, topts.TargetRefName, "") } func TestGiteaConfigMaxKeepRun(t *testing.T) { @@ -328,7 +243,7 @@ func TestGiteaConfigMaxKeepRun(t *testing.T) { } defer tgitea.TestPR(t, topts)() tgitea.PostCommentOnPullRequest(t, topts, "/retest") - tgitea.WaitForStatus(t, topts, topts.TargetRefName) + tgitea.WaitForStatus(t, topts, topts.TargetRefName, "") waitOpts := twait.Opts{ RepoName: topts.TargetNS, @@ -368,7 +283,7 @@ func TestGiteaPush(t *testing.T) { assert.NilError(t, err) assert.Assert(t, resp.StatusCode < 400, resp) assert.Assert(t, merged) - tgitea.WaitForStatus(t, topts, topts.PullRequest.Head.Sha) + tgitea.WaitForStatus(t, topts, topts.PullRequest.Head.Sha, "") time.Sleep(5 * time.Second) prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{ LabelSelector: pacapi.EventType + "=push", @@ -377,63 +292,6 @@ func TestGiteaPush(t *testing.T) { assert.Equal(t, len(prs.Items), 1, "should have only one push pipelinerun") } -func TestGiteaClusterTasks(t *testing.T) { - // we need to verify sure to create clustertask before pushing the files - // so we have to create a new client and do more manual things we get for free in TestPR - topts := &tgitea.TestOpts{ - TargetEvent: "pull_request, push", - YAMLFiles: map[string]string{ - ".tekton/prcluster.yaml": "testdata/pipelinerunclustertasks.yaml", - }, - ExpectEvents: false, - } - topts.TargetRefName = names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") - topts.TargetNS = topts.TargetRefName - - // create first the cluster tasks - ctname := fmt.Sprintf(".tekton/%s.yaml", topts.TargetNS) - newyamlFiles := map[string]string{ctname: "testdata/clustertask.yaml"} - entries, err := payload.GetEntries(newyamlFiles, topts.TargetNS, "main", "pull_request", map[string]string{}) - assert.NilError(t, err) - //nolint: staticcheck - ct := v1beta1.ClusterTask{} - assert.NilError(t, yaml.Unmarshal([]byte(entries[ctname]), &ct)) - ct.Name = "clustertask-" + topts.TargetNS - - run := ¶ms.Run{} - ctx := context.Background() - assert.NilError(t, run.Clients.NewClients(ctx, &run.Info)) - // TODO(chmou): this is for v1beta1, we need to figure out a way how to do that on v1 - _, err = run.Clients.Tekton.TektonV1beta1().ClusterTasks().Create(context.TODO(), &ct, metav1.CreateOptions{}) - assert.NilError(t, err) - assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, run)) - run.Clients.Log.Infof("%s has been created", ct.GetName()) - defer (func() { - assert.NilError(t, topts.ParamsRun.Clients.Tekton.TektonV1beta1().ClusterTasks().Delete(context.TODO(), ct.Name, metav1.DeleteOptions{})) - run.Clients.Log.Infof("%s is deleted", ct.GetName()) - })() - - // start PR - defer tgitea.TestPR(t, topts)() - - // wait for it - waitOpts := twait.Opts{ - RepoName: topts.TargetNS, - Namespace: topts.TargetNS, - // 0 means 1 🙃 (we test for >, while we actually should do >=, but i - // need to go all over the code to verify it's not going to break - // anything else) - MinNumberStatus: 0, - PollTimeout: twait.DefaultTimeout, - TargetSHA: topts.PullRequest.Head.Sha, - } - err = twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts) - assert.NilError(t, err) - - topts.CheckForStatus = "success" - tgitea.WaitForStatus(t, topts, topts.TargetRefName) -} - func TestGiteaWithCLI(t *testing.T) { t.Parallel() topts := &tgitea.TestOpts{ @@ -575,7 +433,7 @@ func TestGiteaWithCLIGeneratePipeline(t *testing.T) { _, err = git.RunGit(tmpdir, "push", "origin", topts.TargetRefName) assert.NilError(t, err) - tgitea.WaitForStatus(t, topts, topts.TargetRefName) + tgitea.WaitForStatus(t, topts, topts.TargetRefName, "") prs, err := topts.ParamsRun.Clients.Tekton.TektonV1().PipelineRuns(topts.TargetNS).List(context.Background(), metav1.ListOptions{ LabelSelector: pacapi.EventType + "=pull_request", @@ -846,7 +704,7 @@ func TestGiteaProvenance(t *testing.T) { topts.PullRequest = pr topts.ParamsRun.Clients.Log.Infof("PullRequest %s has been created", pr.HTMLURL) topts.CheckForStatus = "success" - tgitea.WaitForStatus(t, topts, topts.TargetRefName) + tgitea.WaitForStatus(t, topts, topts.TargetRefName, "") } // Local Variables: diff --git a/test/pkg/bitbucketcloud/setup.go b/test/pkg/bitbucketcloud/setup.go index 179541951..0238cd8c1 100644 --- a/test/pkg/bitbucketcloud/setup.go +++ b/test/pkg/bitbucketcloud/setup.go @@ -47,7 +47,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, bitbucketcloud.Provid URL: bitbucketCloudAPIURL, User: bitbucketCloudUser, } - if err := bbc.SetClient(ctx, nil, event); err != nil { + if err := bbc.SetClient(ctx, nil, event, nil); err != nil { return nil, options.E2E{}, bitbucketcloud.Provider{}, err } return run, e2eoptions, bbc, nil diff --git a/test/pkg/gitea/crd.go b/test/pkg/gitea/crd.go index 3680485be..57002e6ac 100644 --- a/test/pkg/gitea/crd.go +++ b/test/pkg/gitea/crd.go @@ -10,7 +10,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func createToken(topts *TestOpts) (string, error) { +func CreateToken(topts *TestOpts) (string, error) { token, _, err := topts.GiteaCNX.Client.CreateAccessToken(gitea.CreateAccessTokenOption{ Name: topts.TargetNS, Scopes: []gitea.AccessTokenScope{gitea.AccessTokenScopeAll}, @@ -22,12 +22,7 @@ func createToken(topts *TestOpts) (string, error) { } func CreateCRD(ctx context.Context, topts *TestOpts) error { - token, err := createToken(topts) - if err != nil { - return err - } - - if err := secret.Create(ctx, topts.ParamsRun, map[string]string{"token": token}, topts.TargetNS, "gitea-secret"); err != nil { + if err := secret.Create(ctx, topts.ParamsRun, map[string]string{"token": topts.Token}, topts.TargetNS, "gitea-secret"); err != nil { return err } repository := &v1alpha1.Repository{ diff --git a/test/pkg/gitea/scm.go b/test/pkg/gitea/scm.go index 388a393b3..19732a22e 100644 --- a/test/pkg/gitea/scm.go +++ b/test/pkg/gitea/scm.go @@ -9,6 +9,7 @@ import ( "net/url" "os" "path/filepath" + "regexp" "testing" "code.gitea.io/sdk/gitea" @@ -16,6 +17,7 @@ import ( "github.com/openshift-pipelines/pipelines-as-code/pkg/git" pgitea "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" + "go.uber.org/zap" "gotest.tools/v3/assert" "gotest.tools/v3/env" "gotest.tools/v3/fs" @@ -40,7 +42,11 @@ func InitGitRepo(t *testing.T) (string, func()) { func PushFilesToRefGit(t *testing.T, topts *TestOpts, entries map[string]string, baseRefFrom string) { tmpdir := fs.NewDir(t, t.Name()) - defer tmpdir.Remove() + defer (func() { + if os.Getenv("TEST_NOCLEANUP") == "" { + tmpdir.Remove() + } + })() defer env.ChangeWorkingDir(t, tmpdir.Path())() path := tmpdir.Path() _, err := git.RunGit(path, "init") @@ -82,7 +88,7 @@ func MakeGitCloneURL(targetURL, giteaUsername, giteaPassword string) (string, er // parse hostname of giteaURL parsedURL, err := url.Parse(targetURL) if err != nil { - return "", err + return "", fmt.Errorf("failed to parse url %s: %w", targetURL, err) } return fmt.Sprintf("%s://%s:%s@%s%s", parsedURL.Scheme, giteaUsername, giteaPassword, parsedURL.Host, parsedURL.Path), nil @@ -156,16 +162,40 @@ func GetIssueTimeline(ctx context.Context, topts *TestOpts) (Timelines, error) { return tls, nil } -func CreateGiteaRepo(giteaClient *gitea.Client, user, name, hookURL string) (*gitea.Repository, error) { +func CreateGiteaRepo(giteaClient *gitea.Client, user, name, hookURL string, onOrg bool, logger *zap.SugaredLogger) (*gitea.Repository, error) { + var repo *gitea.Repository + var err error // Create a new repo - repo, _, err := giteaClient.AdminCreateRepo(user, gitea.CreateRepoOption{ - Name: name, - Description: "This is a repo it's a wonderful thing", - AutoInit: true, - }) + if onOrg { + logger.Infof("Creating org %s", name) + user = "org-" + name + _, _, err := giteaClient.CreateOrg(gitea.CreateOrgOption{ + Name: user, + }) + if err != nil { + return nil, fmt.Errorf("failed to create org: %w", err) + } + logger.Infof("Creating gitea repository on org %s", name) + repo, _, err = giteaClient.CreateOrgRepo(user, gitea.CreateRepoOption{ + Name: name, + Description: "This is a repo it's a wonderful thing", + AutoInit: true, + }) + if err != nil { + return nil, fmt.Errorf("failed to create repo: %w", err) + } + } else { + logger.Infof("Creating gitea repository %s for user %s", name, user) + repo, _, err = giteaClient.AdminCreateRepo(user, gitea.CreateRepoOption{ + Name: name, + Description: "This is a repo it's a wonderful thing", + AutoInit: true, + }) + } if err != nil { - return nil, err + return nil, fmt.Errorf("failed to create repo: %w", err) } + logger.Infof("Creating webhook to smee url on gitea repository %s", name) _, _, err = giteaClient.CreateRepoHook(user, repo.Name, gitea.CreateHookOption{ Type: "gitea", Active: true, @@ -179,7 +209,34 @@ func CreateGiteaRepo(giteaClient *gitea.Client, user, name, hookURL string) (*gi return repo, err } -func CreateGiteaUser(giteaClient *gitea.Client, apiURL, username, password string) (pgitea.Provider, error) { +func CreateTeam(topts *TestOpts, orgName, teamName string) (*gitea.Team, error) { + team, _, err := topts.GiteaCNX.Client.CreateTeam(orgName, gitea.CreateTeamOption{ + Permission: gitea.AccessModeWrite, + Units: []gitea.RepoUnitType{ + gitea.RepoUnitPulls, + }, + Name: teamName, + }) + topts.ParamsRun.Clients.Log.Infof("Team %s has been created on Org %s", team.Name, orgName) + return team, err +} + +func RemoveCommentMatching(topts *TestOpts, commentString *regexp.Regexp) error { + comments, _, err := topts.GiteaCNX.Client.ListIssueComments(topts.Opts.Organization, topts.Opts.Repo, topts.PullRequest.Index, gitea.ListIssueCommentOptions{}) + if err != nil { + return err + } + for _, c := range comments { + if commentString.MatchString(c.Body) { + topts.ParamsRun.Clients.Log.Infof("Removing comment %d matching %s", c.ID, commentString.String()) + _, err := topts.GiteaCNX.Client.DeleteIssueComment(topts.Opts.Organization, topts.Opts.Repo, c.ID) + return err + } + } + return fmt.Errorf("no comment matching %s found", commentString.String()) +} + +func CreateGiteaUser(giteaClient *gitea.Client, username, password string) (*gitea.User, error) { visibility := gitea.VisibleTypePublic opts := gitea.CreateUserOption{ LoginName: username, @@ -191,31 +248,42 @@ func CreateGiteaUser(giteaClient *gitea.Client, apiURL, username, password strin } newuser, _, err := giteaClient.AdminCreateUser(opts) if err != nil { - return pgitea.Provider{}, err + return &gitea.User{}, err + } + return newuser, nil +} + +// func CreateGiteaUserSecondCnx(giteaClient *gitea.Client, apiURL, username, password string) (pgitea.Provider, *gitea.User, error) { +func CreateGiteaUserSecondCnx(topts *TestOpts, username, password string) (pgitea.Provider, *gitea.User, error) { + newuser, err := CreateGiteaUser(topts.GiteaCNX.Client, username, password) + if err != nil { + return pgitea.Provider{}, newuser, fmt.Errorf("failed to create user: %w", err) } - secondprovider, err := CreateProvider(context.Background(), - apiURL, newuser.UserName, password) + secondprovider, err := CreateProvider(context.Background(), topts.GiteaAPIURL, newuser.UserName, password) if err != nil { - return pgitea.Provider{}, err + return pgitea.Provider{}, newuser, fmt.Errorf("failed to create provider: %w", err) } - return secondprovider, err + return secondprovider, newuser, err } -func CreateForkPullRequest(t *testing.T, topts *TestOpts, secondcnx pgitea.Provider, - accessMode, command string, -) *gitea.PullRequest { +func CreateForkPullRequest(t *testing.T, topts *TestOpts, secondcnx pgitea.Provider, accessMode, command string) *gitea.PullRequest { + forkuserinfo, _, err := secondcnx.Client.GetMyUserInfo() + assert.NilError(t, err) forkrepo, _, err := secondcnx.Client.CreateFork(topts.Opts.Organization, topts.TargetRefName, gitea.CreateForkOption{}) assert.NilError(t, err) - cloneURL, err := MakeGitCloneURL(forkrepo.CloneURL, topts.TargetRefName, topts.GiteaPassword) + topts.ParamsRun.Clients.Log.Infof("Forked repository %s has been created", forkrepo.CloneURL) + cloneURL, err := MakeGitCloneURL(forkrepo.CloneURL, forkuserinfo.UserName, secondcnx.Password) assert.NilError(t, err) newopts := &TestOpts{ - GitCloneURL: cloneURL, TargetRefName: topts.TargetRefName, ParamsRun: topts.ParamsRun, + GitCloneURL: cloneURL, + TargetRefName: topts.TargetRefName, + ParamsRun: topts.ParamsRun, } processed, err := payload.ApplyTemplate("testdata/pipelinerun-alt.yaml", map[string]string{ "TargetNamespace": topts.TargetNS, "TargetEvent": topts.TargetEvent, - "TargetBranch": topts.TargetRefName, + "TargetBranch": topts.DefaultBranch, "PipelineName": "pr", "Command": command, }) @@ -229,12 +297,12 @@ func CreateForkPullRequest(t *testing.T, topts *TestOpts, secondcnx pgitea.Provi pr, _, err := secondcnx.Client.CreatePullRequest(topts.Opts.Organization, topts.TargetRefName, gitea.CreatePullRequestOption{ - Head: fmt.Sprintf("%s:%s", topts.TargetRefName, topts.TargetRefName), - Base: topts.TargetRefName, + Base: topts.DefaultBranch, + Head: fmt.Sprintf("%s:%s", forkrepo.Owner.UserName, topts.TargetRefName), Title: fmt.Sprintf("New PR from %s", topts.TargetRefName), }) assert.NilError(t, err) - topts.ParamsRun.Clients.Log.Infof("Created pr %s", pr.HTMLURL) + topts.ParamsRun.Clients.Log.Infof("Created pr %s branch:%s from fork %s, branch:%s", pr.HTMLURL, topts.DefaultBranch, forkrepo.FullName, topts.TargetRefName) return pr } diff --git a/test/pkg/gitea/setup.go b/test/pkg/gitea/setup.go index eae8b3f3f..a1dfed2be 100644 --- a/test/pkg/gitea/setup.go +++ b/test/pkg/gitea/setup.go @@ -31,7 +31,7 @@ func CreateProvider(ctx context.Context, giteaURL, user, password string) (gitea User: user, Token: password, } - if err := gprovider.SetClient(ctx, nil, event); err != nil { + if err := gprovider.SetClient(ctx, nil, event, 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 89bd78485..c5d5bc535 100644 --- a/test/pkg/gitea/test.go +++ b/test/pkg/gitea/test.go @@ -25,6 +25,7 @@ import ( ) type TestOpts struct { + OnOrg bool NoPullRequestCreation bool SkipEventsCheck bool NoCleanup bool @@ -50,6 +51,7 @@ type TestOpts struct { GiteaPassword string ExpectEvents bool InternalGiteaURL string + Token string } func PostCommentOnPullRequest(t *testing.T, topt *TestOpts, body string) { @@ -65,11 +67,15 @@ func TestPR(t *testing.T, topts *TestOpts) func() { ctx := context.Background() if topts.ParamsRun == nil { runcnx, opts, giteacnx, err := Setup(ctx) + assert.NilError(t, err, fmt.Errorf("cannot do gitea setup: %w", err)) topts.GiteaCNX = giteacnx topts.ParamsRun = runcnx topts.Opts = opts - assert.NilError(t, err, fmt.Errorf("cannot do gitea setup: %w", err)) } + giteaURL := os.Getenv("TEST_GITEA_API_URL") + giteaPassword := os.Getenv("TEST_GITEA_PASSWORD") + topts.GiteaAPIURL = giteaURL + topts.GiteaPassword = giteaPassword hookURL := os.Getenv("TEST_GITEA_SMEEURL") topts.InternalGiteaURL = os.Getenv("TEST_GITEA_INTERNAL_URL") if topts.InternalGiteaURL == "" { @@ -88,7 +94,7 @@ func TestPR(t *testing.T, topts *TestOpts) func() { assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, topts.ParamsRun)) } - repoInfo, err := CreateGiteaRepo(topts.GiteaCNX.Client, topts.Opts.Organization, topts.TargetRefName, hookURL) + repoInfo, err := CreateGiteaRepo(topts.GiteaCNX.Client, topts.Opts.Organization, topts.TargetRefName, hookURL, topts.OnOrg, topts.ParamsRun.Clients.Log) assert.NilError(t, err) topts.Opts.Repo = repoInfo.Name topts.Opts.Organization = repoInfo.Owner.UserName @@ -100,9 +106,11 @@ func TestPR(t *testing.T, topts *TestOpts) func() { defer TearDown(ctx, t, topts) } } - err = CreateCRD(ctx, topts) + topts.Token, err = CreateToken(topts) assert.NilError(t, err) + assert.NilError(t, CreateCRD(ctx, topts)) + url, err := MakeGitCloneURL(repoInfo.CloneURL, os.Getenv("TEST_GITEA_USERNAME"), os.Getenv("TEST_GITEA_PASSWORD")) assert.NilError(t, err) topts.GitCloneURL = url @@ -127,13 +135,9 @@ func TestPR(t *testing.T, topts *TestOpts) func() { assert.NilError(t, err) topts.PullRequest = pr topts.ParamsRun.Clients.Log.Infof("PullRequest %s has been created", pr.HTMLURL) - giteaURL := os.Getenv("TEST_GITEA_API_URL") - giteaPassword := os.Getenv("TEST_GITEA_PASSWORD") - topts.GiteaAPIURL = giteaURL - topts.GiteaPassword = giteaPassword if topts.CheckForStatus != "" { - WaitForStatus(t, topts, topts.TargetRefName) + WaitForStatus(t, topts, topts.TargetRefName, "") } if topts.Regexp != nil { @@ -168,19 +172,25 @@ func TestPR(t *testing.T, topts *TestOpts) func() { return cleanup } -func WaitForStatus(t *testing.T, topts *TestOpts, ref string) { +func WaitForStatus(t *testing.T, topts *TestOpts, ref, forcontext string) { i := 0 for { - cs, _, err := topts.GiteaCNX.Client.GetCombinedStatus(topts.Opts.Organization, topts.Opts.Repo, ref) + statuses, _, err := topts.GiteaCNX.Client.ListStatuses(topts.Opts.Organization, topts.Opts.Repo, ref, gitea.ListStatusesOption{}) assert.NilError(t, err) - if cs.State != "" && cs.State != "pending" { - assert.Equal(t, string(cs.State), topts.CheckForStatus) - topts.ParamsRun.Clients.Log.Infof("Status is %s", cs.State) - + for _, cstatus := range statuses { + if cstatus.State == "pending" { + continue + } + if forcontext != "" && cstatus.Context != forcontext { + continue + } + assert.Equal(t, string(cstatus.State), topts.CheckForStatus) + topts.ParamsRun.Clients.Log.Infof("Status on SHA: %s is %s", ref, cstatus.State) if topts.CheckForNumberStatus != 0 { - assert.Equal(t, len(cs.Statuses), topts.CheckForNumberStatus) + assert.Equal(t, len(statuses), topts.CheckForNumberStatus) } - break + + return } if i > 50 { t.Fatalf("gitea status has not been updated") diff --git a/test/pkg/github/setup.go b/test/pkg/github/setup.go index 1a1abb143..164e9d2cc 100644 --- a/test/pkg/github/setup.go +++ b/test/pkg/github/setup.go @@ -92,7 +92,7 @@ func Setup(ctx context.Context, viaDirectWebhook bool) (*params.Run, options.E2E Token: githubToken, URL: githubURL, } - if err := gprovider.SetClient(ctx, nil, event); err != nil { + if err := gprovider.SetClient(ctx, nil, event, 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 48bf34125..f22a7d561 100644 --- a/test/pkg/gitlab/setup.go +++ b/test/pkg/gitlab/setup.go @@ -57,7 +57,7 @@ func Setup(ctx context.Context) (*params.Run, options.E2E, gitlab.Provider, erro Token: gitlabToken, URL: gitlabURL, }, - }) + }, nil) if err != nil { return nil, options.E2E{}, gitlab.Provider{}, err } diff --git a/test/pkg/repository/create.go b/test/pkg/repository/create.go index 0a606f2b3..fb5d242b1 100644 --- a/test/pkg/repository/create.go +++ b/test/pkg/repository/create.go @@ -21,6 +21,6 @@ func CreateRepo(ctx context.Context, targetNS string, cs *params.Run, repository if err != nil { return err } - cs.Clients.Log.Infof("Repository created in %s", repo.GetNamespace()) + cs.Clients.Log.Infof("PipelinesAsCode Repository %s has been created in namespace %s", repo.GetName(), repo.GetNamespace()) return nil }