Skip to content

Commit 0fc3592

Browse files
chmouelsavitaashture
authored andcommitted
Remove PacOpts from IsAllowed
We already have it instantiated on the run object via SetClient, no need to pass it again. Signed-off-by: Chmouel Boudjnah <[email protected]>
1 parent 90ce7a4 commit 0fc3592

File tree

14 files changed

+34
-32
lines changed

14 files changed

+34
-32
lines changed

pkg/pipelineascode/match.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ is that what you want? make sure you use -n when generating the secret, eg: echo
116116

117117
// Check if the submitter is allowed to run this.
118118
if p.event.TriggerTarget != "push" {
119-
allowed, err := p.vcx.IsAllowed(ctx, p.event, p.run.Info.Pac)
119+
allowed, err := p.vcx.IsAllowed(ctx, p.event)
120120
if err != nil {
121121
return repo, err
122122
}

pkg/provider/bitbucketcloud/acl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketcloud/types"
1212
)
1313

14-
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, _ *info.PacOpts) (bool, error) {
14+
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, error) {
1515
// Check first if the user is in the owner file or part of the workspace
1616
allowed, err := v.checkMember(ctx, event)
1717
if err != nil {

pkg/provider/bitbucketcloud/acl_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,10 +180,7 @@ func TestIsAllowed(t *testing.T) {
180180
bbcloudtest.MuxFiles(t, mux, tt.event, tt.fields.filescontents, "")
181181

182182
v := &Provider{Client: bbclient}
183-
184-
pacopts := info.PacOpts{}
185-
186-
got, err := v.IsAllowed(ctx, tt.event, &pacopts)
183+
got, err := v.IsAllowed(ctx, tt.event)
187184
if (err != nil) != tt.wantErr {
188185
t.Errorf("Provider.IsAllowed() error = %v, wantErr %v", err, tt.wantErr)
189186
return

pkg/provider/bitbucketserver/acl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414

1515
type activitiesTypes struct{ Values []*bbv1.Activity }
1616

17-
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, _ *info.PacOpts) (bool, error) {
17+
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, error) {
1818
allowed, err := v.checkMemberShip(ctx, event)
1919
if err != nil {
2020
return false, err

pkg/provider/bitbucketserver/acl_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,7 @@ func TestIsAllowed(t *testing.T) {
217217
projectKey: tt.event.Organization,
218218
}
219219

220-
pacopts := info.PacOpts{}
221-
222-
got, err := v.IsAllowed(ctx, tt.event, &pacopts)
220+
got, err := v.IsAllowed(ctx, tt.event)
223221
if tt.wantErrSubstr != "" {
224222
assert.ErrorContains(t, err, tt.wantErrSubstr)
225223
return

pkg/provider/gitea/acl.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func (v *Provider) CheckPolicyAllowing(_ context.Context, event *info.Event, all
4444
return false, fmt.Sprintf("user: %s is not a member of any of the allowed teams: %v", event.Sender, allowedTeams)
4545
}
4646

47-
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, pac *info.PacOpts) (bool, error) {
47+
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, error) {
4848
aclPolicy := policy.Policy{
4949
Repository: v.repo,
5050
EventEmitter: v.eventEmitter,
@@ -74,7 +74,7 @@ func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, pac *info.P
7474
}
7575

7676
// Try to parse the comment from an owner who has issues a /ok-to-test
77-
ownerAllowed, err := v.aclAllowedOkToTestFromAnOwner(ctx, event, pac)
77+
ownerAllowed, err := v.aclAllowedOkToTestFromAnOwner(ctx, event)
7878
if err != nil {
7979
return false, err
8080
}
@@ -95,7 +95,7 @@ func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, pac *info.P
9595
// if there is a /ok-to-test in there running an aclCheck again on the comment
9696
// Sender if she is an OWNER and then allow it to run CI.
9797
// TODO: pull out the github logic from there in an agnostic way.
98-
func (v *Provider) aclAllowedOkToTestFromAnOwner(ctx context.Context, event *info.Event, pac *info.PacOpts) (bool, error) {
98+
func (v *Provider) aclAllowedOkToTestFromAnOwner(ctx context.Context, event *info.Event) (bool, error) {
9999
revent := info.NewEvent()
100100
event.DeepCopyInto(revent)
101101
revent.EventType = ""
@@ -108,14 +108,14 @@ func (v *Provider) aclAllowedOkToTestFromAnOwner(ctx context.Context, event *inf
108108
case *giteaStructs.IssueCommentPayload:
109109
// if we don't need to check old comments, then on issue comment we
110110
// need to check if comment have /ok-to-test and is from allowed user
111-
if !pac.RememberOKToTest {
111+
if !v.run.Info.Pac.RememberOKToTest {
112112
return v.aclAllowedOkToTestCurrentComment(ctx, revent, event.Comment.ID)
113113
}
114114
revent.URL = event.Issue.URL
115115
case *giteaStructs.PullRequestPayload:
116116
// if we don't need to check old comments, then on push event we don't need
117117
// to check anything for the non-allowed user
118-
if !pac.RememberOKToTest {
118+
if !v.run.Info.Pac.RememberOKToTest {
119119
return false, nil
120120
}
121121
revent.URL = event.PullRequest.HTMLURL

pkg/provider/gitea/acl_test.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"code.gitea.io/sdk/gitea"
1111
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
12+
"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
1213
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
1314
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
1415
giteaStructs "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/gitea/structs"
@@ -79,9 +80,11 @@ func TestCheckPolicyAllowing(t *testing.T) {
7980
ctx, _ := rtesting.SetupFakeContext(t)
8081
observer, _ := zapobserver.New(zap.InfoLevel)
8182
logger := zap.New(observer).Sugar()
82-
repo := &v1alpha1.Repository{Spec: v1alpha1.RepositorySpec{
83-
Settings: &v1alpha1.Settings{},
84-
}}
83+
repo := &v1alpha1.Repository{
84+
Spec: v1alpha1.RepositorySpec{
85+
Settings: &v1alpha1.Settings{},
86+
},
87+
}
8588
gprovider := Provider{
8689
Client: fakeclient,
8790
repo: repo,
@@ -286,14 +289,17 @@ func TestOkToTestComment(t *testing.T) {
286289
gprovider := Provider{
287290
Client: fakeclient,
288291
Logger: logger,
289-
}
290-
pacopts := info.PacOpts{
291-
Settings: &settings.Settings{
292-
RememberOKToTest: tt.rememberOkToTest,
292+
run: &params.Run{
293+
Info: info.Info{
294+
Pac: &info.PacOpts{
295+
Settings: &settings.Settings{
296+
RememberOKToTest: tt.rememberOkToTest,
297+
},
298+
},
299+
},
293300
},
294301
}
295-
296-
isAllowed, err := gprovider.IsAllowed(ctx, &tt.runevent, &pacopts)
302+
isAllowed, err := gprovider.IsAllowed(ctx, &tt.runevent)
297303
if tt.wantErr {
298304
assert.Assert(t, err != nil)
299305
} else {

pkg/provider/gitea/gitea.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ type Provider struct {
4848
Password string
4949
repo *v1alpha1.Repository
5050
eventEmitter *events.EventEmitter
51+
run *params.Run
5152
}
5253

5354
// GetTaskURI TODO: Implement ME
@@ -82,7 +83,7 @@ func (v *Provider) GetConfig() *info.ProviderConfig {
8283
}
8384
}
8485

85-
func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Event, repo *v1alpha1.Repository, emitter *events.EventEmitter) error {
86+
func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.Event, repo *v1alpha1.Repository, emitter *events.EventEmitter) error {
8687
var err error
8788
apiURL := runevent.Provider.URL
8889
// password is not exposed to CRD, it's only used from the e2e tests
@@ -100,6 +101,7 @@ func (v *Provider) SetClient(_ context.Context, _ *params.Run, runevent *info.Ev
100101
v.giteaInstanceURL = runevent.Provider.URL
101102
v.eventEmitter = emitter
102103
v.repo = repo
104+
v.run = run
103105
return nil
104106
}
105107

pkg/provider/github/acl.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (v *Provider) IsAllowedOwnersFile(ctx context.Context, event *info.Event) (
6161
return acl.UserInOwnerFile(ownerContent, event.Sender)
6262
}
6363

64-
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event, _ *info.PacOpts) (bool, error) {
64+
func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, error) {
6565
aclPolicy := policy.Policy{
6666
Repository: v.repo,
6767
EventEmitter: v.eventEmitter,

pkg/provider/github/acl_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ func TestOkToTestComment(t *testing.T) {
333333
Run: &params.Run{Info: info.Info{Pac: pacopts}},
334334
}
335335

336-
got, err := gprovider.IsAllowed(ctx, &tt.runevent, pacopts)
336+
got, err := gprovider.IsAllowed(ctx, &tt.runevent)
337337
if (err != nil) != tt.wantErr {
338338
t.Errorf("aclCheck() error = %v, wantErr %v", err, tt.wantErr)
339339
return

0 commit comments

Comments
 (0)