Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions pkg/resolve/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,14 @@ func assembleTaskFQDNs(pipelineURL string, tasks []string) ([]string, error) {
return tasks, nil // no pipeline URL, return tasks as is
}

// Only HTTP(S) URLs can serve as base for relative task resolution.
// Only HTTP(S) URLs and repository file paths can serve as base for relative task resolution.
// Hub catalog references (e.g., "catalog://resource:version") use a
// different scheme where relative paths are meaningless.
lowered := strings.ToLower(pipelineURL)
if !strings.HasPrefix(lowered, "http://") && !strings.HasPrefix(lowered, "https://") {
isHTTP := strings.HasPrefix(lowered, "http://") || strings.HasPrefix(lowered, "https://")
isRepoPath := strings.Contains(lowered, "/") && !strings.Contains(lowered, "://")

if !isHTTP && !isRepoPath {
return tasks, nil
}

Choose a reason for hiding this comment

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

high

The condition strings.Contains(lowered, "/") for identifying a repository path is too restrictive. It will fail to identify pipeline files located at the root of the repository (e.g., pipeline.yaml), preventing relative task path resolution within them. This could lead to unresolved paths with .. being sent to the Git provider API, which is the original problem this PR is trying to solve.

To make this more robust, consider also checking for a file extension. Hub tasks are unlikely to have .yaml or .yml extensions, so this could be a reasonable way to distinguish them from file paths at the repository root.

Copy link
Member Author

Choose a reason for hiding this comment

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

This scenario does fail. Is this a valid specification?
Refer task in the manifest screenshot
Image

Error message
Image


Expand Down
4 changes: 2 additions & 2 deletions pkg/resolve/remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,10 +562,10 @@ func TestAssembleTaskFQDNs(t *testing.T) {
expected: []string{"https://example.com/repo/task.yaml"},
},
{
name: "repository file path URL returns tasks unchanged",
name: "repository file path URL resolves relative tasks",
pipelineURL: "share/pipelines/build.yaml",
tasks: []string{"../tasks/t.yaml", "tasks/other-task.yaml"},
expected: []string{"../tasks/t.yaml", "tasks/other-task.yaml"},
expected: []string{"share/tasks/t.yaml", "share/pipelines/tasks/other-task.yaml"},
Copy link
Member Author

Choose a reason for hiding this comment

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

I have tried to restore the original behaviour of the function but a bit doubtful if the second resolution is what we expect expect.

},
}
for _, tt := range tests {
Expand Down
7 changes: 4 additions & 3 deletions test/gitea_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,10 @@ func TestGiteaPullRequestTaskAnnotations(t *testing.T) {
Regexp: successRegexp,
TargetEvent: triggertype.PullRequest.String(),
YAMLFiles: map[string]string{
".tekton/pipeline.yaml": "testdata/pipeline_in_tektondir.yaml",
".other-tasks/task-referenced-internally.yaml": "testdata/task_referenced_internally.yaml",
".tekton/pr.yaml": "testdata/pipelinerun_remote_task_annotations.yaml",
".tekton/pipeline.yaml": "testdata/pipeline_in_tektondir.yaml",
".other-tasks/task-referenced-internally.yaml": "testdata/task_referenced_internally.yaml",
".other-tasks/task2-referenced-internally.yaml": "testdata/task2_referenced_internally.yaml",
Copy link
Member Author

Choose a reason for hiding this comment

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

Both tasks are same, one is referenced using its absolute path in the repo and the other with relative path

pipelinesascode.tekton.dev/task: "[.other-tasks/task-referenced-internally.yaml]"
pipelinesascode.tekton.dev/task-3: "[../.other-tasks/task2-referenced-internally.yaml]"

".tekton/pr.yaml": "testdata/pipelinerun_remote_task_annotations.yaml",
},
CheckForStatus: "success",
ExtraArgs: map[string]string{
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

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

These tests are skipped but still modified them (not tested).

Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ func verifyGHTokenScope(t *testing.T, remoteTaskURL, remoteTaskName string, data
defer configmap.ChangeGlobalConfig(ctx, t, runcnx, "pipelines-as-code", data)()

entries, err := payload.GetEntries(map[string]string{
".tekton/pr.yaml": "testdata/pipelinerun_remote_task_annotations.yaml",
".tekton/pipeline.yaml": "testdata/pipeline_in_tektondir.yaml",
".other-tasks/task-referenced-internally.yaml": "testdata/task_referenced_internally.yaml",
".tekton/pr.yaml": "testdata/pipelinerun_remote_task_annotations.yaml",
".tekton/pipeline.yaml": "testdata/pipeline_in_tektondir.yaml",
".other-tasks/task-referenced-internally.yaml": "testdata/task_referenced_internally.yaml",
".other-tasks/task2-referenced-internally.yaml": "testdata/task2_referenced_internally.yaml",
}, targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{
"RemoteTaskURL": remoteTaskURL,
"RemoteTaskName": remoteTaskName,
Expand Down
4 changes: 4 additions & 0 deletions test/testdata/pipeline_in_tektondir.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@ spec:
- name: task-referenced-internally
taskRef:
name: task-referenced-internally

- name: task2-referenced-internally
taskRef:
name: task2-referenced-internally
1 change: 1 addition & 0 deletions test/testdata/pipelinerun_remote_task_annotations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ metadata:
pipelinesascode.tekton.dev/task: "[.other-tasks/task-referenced-internally.yaml]"
pipelinesascode.tekton.dev/task-1: "[\\ .RemoteTaskURL //]"
pipelinesascode.tekton.dev/task-2: "default://pylint"
pipelinesascode.tekton.dev/task-3: "[../.other-tasks/task2-referenced-internally.yaml]"
spec:
pipelineRef:
name: pipeline-in-tekton-dir
12 changes: 12 additions & 0 deletions test/testdata/task2_referenced_internally.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
name: task2-referenced-internally
spec:
steps:
- name: task2-remote
image: gcr.io/distroless/python3:nonroot
script: |
#!/usr/bin/python3
print("Hello Task Referenced Internally")
Loading