Skip to content

Commit 7ee69df

Browse files
committed
feat(matcher): skip ACL when .tekton dir absent
Reorder matchRepoPR to fetch .tekton templates before ACL checks. When the .tekton directory is missing, the flow returns early without triggering permission checks or posting unwarranted status updates to the Git provider. Split verifyRepoAndUser into setupRepo and checkUserAccess, and extract fetchTektonTemplates from getPipelineRunsFromRepo. The existing verifyRepoAndUser is kept as a thin wrapper for flows that need setup + ACL without .tekton checks. Signed-off-by: Akshay Pant <akpant@redhat.com> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9c4e9cb commit 7ee69df

2 files changed

Lines changed: 275 additions & 42 deletions

File tree

pkg/pipelineascode/match.go

Lines changed: 70 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import (
2525
)
2626

2727
func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, *v1alpha1.Repository, error) {
28-
p.debugf("matchRepoPR: starting repo verification for url=%s", p.event.URL)
29-
repo, err := p.verifyRepoAndUser(ctx)
28+
p.debugf("matchRepoPR: starting repo setup for url=%s", p.event.URL)
29+
repo, err := p.setupRepo(ctx)
3030
if err != nil {
3131
return nil, nil, err
3232
}
@@ -40,8 +40,24 @@ func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, *v1alpha1.Re
4040
return nil, repo, p.cancelPipelineRunsOpsComment(ctx, repo)
4141
}
4242

43+
rawTemplates, err := p.fetchTektonTemplates(ctx, repo)
44+
if err != nil {
45+
return nil, repo, err
46+
}
47+
if rawTemplates == "" {
48+
return nil, repo, nil
49+
}
50+
allowed, err := p.checkUserAccess(ctx, repo)
51+
if err != nil {
52+
return nil, repo, err
53+
}
54+
if !allowed {
55+
p.debugf("matchRepoPR: ACL check denied access")
56+
return nil, repo, nil
57+
}
58+
4359
p.debugf("matchRepoPR: fetching pipelineruns from repo=%s/%s", repo.GetNamespace(), repo.GetName())
44-
matchedPRs, err := p.getPipelineRunsFromRepo(ctx, repo)
60+
matchedPRs, err := p.getPipelineRunsFromRepo(ctx, repo, rawTemplates)
4561
if err != nil {
4662
return nil, repo, err
4763
}
@@ -50,11 +66,11 @@ func (p *PacRun) matchRepoPR(ctx context.Context) ([]matcher.Match, *v1alpha1.Re
5066
return matchedPRs, repo, nil
5167
}
5268

53-
// verifyRepoAndUser verifies if the Repo CR exists for the Git Repository,
54-
// if the user has permission to run CI and also initialise provider client.
55-
func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, error) {
69+
// setupRepo matches the Repository CR, sets up the authenticated provider client,
70+
// and fetches commit info. It does NOT perform ACL checks.
71+
func (p *PacRun) setupRepo(ctx context.Context) (*v1alpha1.Repository, error) {
5672
// Match the Event URL to a Repository URL,
57-
p.debugf("verifyRepoAndUser: matching repository for url=%s", p.event.URL)
73+
p.debugf("setupRepo: matching repository for url=%s", p.event.URL)
5874
repo, err := matcher.MatchEventURLRepo(ctx, p.run, p.event, "")
5975
if err != nil {
6076
return nil, fmt.Errorf("error matching Repository for event: %w", err)
@@ -65,13 +81,13 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
6581
p.eventEmitter.EmitMessage(nil, zap.WarnLevel, "RepositoryNamespaceMatch", msg)
6682
return nil, nil
6783
}
68-
p.debugf("verifyRepoAndUser: matched repo=%s/%s", repo.GetNamespace(), repo.GetName())
84+
p.debugf("setupRepo: matched repo=%s/%s", repo.GetNamespace(), repo.GetName())
6985

7086
p.logger = p.logger.With("namespace", repo.Namespace)
7187
p.vcx.SetLogger(p.logger)
7288
p.eventEmitter.SetLogger(p.logger)
7389

74-
// Set up authenticated client with proper token scoping
90+
// Set up authenticated client with proper token scoping.
7591
// NOTE: This is typically already done in sinker.processEvent() for all event types,
7692
// but we call it here as a safety net for edge cases (e.g., tests calling Run() directly,
7793
// or if the early setup in sinker failed/was skipped). The call is idempotent.
@@ -80,21 +96,25 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
8096
if err != nil {
8197
return repo, err
8298
}
83-
p.debugf("verifyRepoAndUser: authenticated client setup complete")
99+
p.debugf("setupRepo: authenticated client setup complete")
84100

85101
// Get the SHA commit info, we want to get the URL and commit title
86102
if p.event.SHA == "" || p.event.SHATitle == "" || p.event.SHAURL == "" {
87-
p.debugf("verifyRepoAndUser: fetching commit info")
103+
p.debugf("setupRepo: fetching commit info")
88104
if err = p.vcx.GetCommitInfo(ctx, p.event); err != nil {
89105
return repo, fmt.Errorf("could not find commit info: %w", err)
90106
}
91-
p.debugf("verifyRepoAndUser: commit info loaded sha=%s title=%s", p.event.SHA, p.event.SHATitle)
107+
p.debugf("setupRepo: commit info loaded sha=%s title=%s", p.event.SHA, p.event.SHATitle)
92108
}
93109

110+
return repo, nil
111+
}
112+
113+
func (p *PacRun) checkUserAccess(ctx context.Context, repo *v1alpha1.Repository) (bool, error) {
94114
// Verify whether the sender of the GitOps command (e.g., /test) has the appropriate permissions to
95115
// trigger CI on the repository, as any user is able to comment on a pushed commit in open-source repositories.
96116
if p.event.TriggerTarget == triggertype.Push && opscomments.IsAnyOpsEventType(p.event.EventType) {
97-
p.debugf("verifyRepoAndUser: checking access for gitops comment on push")
117+
p.debugf("checkUserAccess: checking access for gitops comment on push")
98118
status := providerstatus.StatusOpts{
99119
Status: CompletedStatus,
100120
Title: "Permission denied",
@@ -103,15 +123,15 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
103123
AccessDenied: true,
104124
}
105125
if allowed, err := p.checkAccessOrError(ctx, repo, status, "by GitOps comment on push commit"); !allowed {
106-
return nil, err
126+
return false, err
107127
}
108128
}
109129

110130
// Check if the submitter is allowed to run this.
111131
// on push we don't need to check the policy since the user has pushed to the repo so it has access to it.
112132
// on comment we skip it for now, we are going to check later on
113133
if p.event.TriggerTarget != triggertype.Push && p.event.EventType != opscomments.NoOpsCommentEventType.String() {
114-
p.debugf("verifyRepoAndUser: checking access for trigger target=%s event_type=%s", p.event.TriggerTarget, p.event.EventType)
134+
p.debugf("checkUserAccess: checking access for trigger target=%s event_type=%s", p.event.TriggerTarget, p.event.EventType)
115135
status := providerstatus.StatusOpts{
116136
Status: queuedStatus,
117137
Title: "Pending approval, waiting for an /ok-to-test",
@@ -120,7 +140,7 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
120140
AccessDenied: true,
121141
}
122142
if allowed, err := p.checkAccessOrError(ctx, repo, status, "via "+p.event.TriggerTarget.String()); !allowed {
123-
return nil, err
143+
return false, err
124144
}
125145
// When /ok-to-test is approved, update the parent "Pipelines as Code CI" status to success
126146
// to indicate the approval was successful before pipelines start running.
@@ -136,16 +156,15 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
136156
}
137157
}
138158
}
139-
return repo, nil
159+
return true, nil
140160
}
141161

142-
// getPipelineRunsFromRepo fetches pipelineruns from git repository and prepare them for creation.
143-
func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Repository) ([]matcher.Match, error) {
162+
func (p *PacRun) fetchTektonTemplates(ctx context.Context, repo *v1alpha1.Repository) (string, error) {
144163
provenance := "source"
145164
if repo.Spec.Settings != nil && repo.Spec.Settings.PipelineRunProvenance != "" {
146165
provenance = repo.Spec.Settings.PipelineRunProvenance
147166
}
148-
p.debugf("getPipelineRunsFromRepo: repo=%s/%s provenance=%s", repo.GetNamespace(), repo.GetName(), provenance)
167+
p.debugf("fetchTektonTemplates: repo=%s/%s provenance=%s", repo.GetNamespace(), repo.GetName(), provenance)
149168
rawTemplates, err := p.vcx.GetTektonDir(ctx, p.event, tektonDir, provenance)
150169
if err != nil && p.event.TriggerTarget == triggertype.PullRequest && strings.Contains(err.Error(), "error unmarshalling yaml file") {
151170
// make the error a bit more friendly for users who don't know what marshalling or intricacies of the yaml parser works
@@ -163,16 +182,16 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
163182
},
164183
},
165184
)
166-
return nil, nil
185+
return "", nil
167186
}
168187

169-
return nil, err
188+
return "", err
170189
}
171190

172191
if err != nil {
173-
p.debugf("getPipelineRunsFromRepo: GetTektonDir returned error: %v", err)
192+
p.debugf("fetchTektonTemplates: GetTektonDir returned error: %v", err)
174193
} else {
175-
p.debugf("getPipelineRunsFromRepo: fetched templates length=%d", len(rawTemplates))
194+
p.debugf("fetchTektonTemplates: fetched templates length=%d", len(rawTemplates))
176195
}
177196

178197
if rawTemplates == "" && p.event.EventType == opscomments.OkToTestCommentEventType.String() {
@@ -198,8 +217,35 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
198217
msg = fmt.Sprintf("cannot locate templates in %s/ directory for this repository in %s", tektonDir, p.event.HeadBranch)
199218
}
200219
p.eventEmitter.EmitMessage(nil, logLevel, reason, msg)
220+
return "", nil
221+
}
222+
223+
return rawTemplates, nil
224+
}
225+
226+
// verifyRepoAndUser verifies if the Repo CR exists for the Git Repository,
227+
// if the user has permission to run CI and also initialise provider client.
228+
func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, error) {
229+
repo, err := p.setupRepo(ctx)
230+
if err != nil {
231+
return repo, err
232+
}
233+
if repo == nil {
201234
return nil, nil
202235
}
236+
allowed, err := p.checkUserAccess(ctx, repo)
237+
if err != nil {
238+
return nil, err
239+
}
240+
if !allowed {
241+
return nil, nil
242+
}
243+
return repo, nil
244+
}
245+
246+
// getPipelineRunsFromRepo processes pre-fetched .tekton templates and prepares PipelineRuns for creation.
247+
func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Repository, rawTemplates string) ([]matcher.Match, error) {
248+
p.debugf("getPipelineRunsFromRepo: repo=%s/%s templates_length=%d", repo.GetNamespace(), repo.GetName(), len(rawTemplates))
203249

204250
// check for condition if need update the pipelinerun with regexp from the
205251
// "raw" pipelinerun string

0 commit comments

Comments
 (0)