Enable relative TaskRefs within remote Pipelines

[martindbrv]

Description

The current implementation doesn't allow to reference tasks relative to a remote pipeline (e.g., the task definition is in the same remote location as the pipeline). The issue is described here:

https://issues.redhat.com/browse/SRVKP-4332

Changes

This PR includes a simple function that makes use of the task overriding functionality (see the relative path example).
Previously, only relative overrides for tasks within the triggering event's .tekton folder were supported. Now, it simply checks if there is a pipelinesascode.tekton.dev/task with a relative path value in the remote pipeline's definition and then assembles the tasks's FQDN based on the remote pipeline's location.

Consider the following scenario:

Repository A, contains:

• .tekton/pipelinerun.yaml

apiVersion: tekton.dev/v1
kind: PipelineRun
metadata:
  name: hello-world
  annotations:
    pipelinesascode.tekton.dev/on-target-branch: "[main]"
    pipelinesascode.tekton.dev/on-event: "[push]"
    pipelinesascode.tekton.dev/pipeline: "https://github.com/user/repositoryb/blob/main/pipeline.yaml"
spec:
  pipelineRef:
    name: hello-world

Repository B, contains:

• pipeline.yaml

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: hello-world
  annotations:
    pipelinesascode.tekton.dev/task: "./task.yaml"
spec:
  tasks:
    - name: say-hello
      taskRef:
        name: hello

• task.yaml

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: hello
spec:
  steps:
    - name: echo
      image: alpine
      script: |
        #!/bin/sh
        echo "Hello, World!"

Previously, you had to specify the FQDN of the task either within the PipelineRun or the Pipeline. Now, you can still do that but you can also use relative references as shown above in the hello-world pipeline.

Note

I will modify the docs to include info about the new functionality. I will also write some additional tests.
Just wanted to get some initial feedback before doing these things.

Submitter Checklist

• 📝 Ensure your commit message is clear and informative. Refer to the How to write a git commit message guide. Include the commit message in the PR body rather than linking to an external site (e.g., Jira ticket).

• ♽ Run make test lint before submitting a PR to avoid unnecessary CI processing. Consider installing pre-commit and running pre-commit install in the repository root for an efficient workflow.

• ✨ We use linters to maintain clean and consistent code. Run make lint before submitting a PR. Some linters offer a --fix mode, executable with make fix-linters (ensure markdownlint and golangci-lint are installed).

• 📖 Document any user-facing features or changes in behavior.

• 🧪 While 100% coverage isn't required, we encourage unit tests for code changes where possible.

• 🎁 If feasible, add an end-to-end test. See README for details.

• 🔎 Address any CI test flakiness before merging, or provide a valid reason to bypass it (e.g., token rate limitations).

• If adding a provider feature, fill in the following details:

  • GitHub App
  • GitHub Webhook
  • Gitea/Forgejo
  • GitLab
  • Bitbucket Cloud
  • Bitbucket Data Center

[chmouel]

/ok-to-test

[chmouel]

Hey you PR make sense, but we need to review carefully for side effects, will have a review on this week

[chmouel]

/ok-to-test

[chmouel]

🚀 Mirrored PR Created for E2E Testing

A mirrored PR has been opened for end-to-end testing: View PR

⏳ Follow progress there for E2E results.
If you need to update the PR with new changes, please ask a maintainer to rerun hack/mirror-pr.sh.

[chmouel]

🤖 AI Review provided by gemini:gemini-2.5-pro:

Of course! This is an excellent way to review code. I will carefully analyze the three commits, their evolution, and the final state of the code.

High-Level Summary

This is a well-structured pull request that adds a valuable feature: resolving relative task paths within a remote pipeline definition. The series of three commits shows a good iterative development process:

1. PATCH 1/3: Introduces the core functionality.
2. PATCH 2/3: Fixes a bug in the initial implementation where URLs were being incorrectly concatenated.
3. PATCH 3/3: Fixes a more subtle but critical bug related to non-deterministic behavior caused by Go's map iteration order, while also making a small code improvement.

The commit messages are clear and explain the "why" behind each change. The PR description provides excellent context and a clear example.

Detailed Code Analysis

I'll focus my analysis on the final state of the code after all three patches are applied, as that's what will be merged. I will also comment on the evolution of the code.

Final Code (after PATCH 3/3)

// file: pkg/resolve/remote.go

func createTaskURL(eventPipelines map[string]*tektonv1.Pipeline, pipeline *tektonv1.Pipeline, tasks []string) ([]string, error) {
	var pURL *url.URL
	var err error
	for ep := range eventPipelines {
		// ensure the URL to be parsed matches the
		// remote pipeline of the current PipelineRun
		if eventPipelines[ep] != pipeline {
			continue
		}
		pURL, err = url.Parse(ep)
		if err != nil {
			return tasks, err
		}
		pPath := strings.SplitAfter(pURL.Path, "/")
		// pop the pipeline target path from the URL
		pPath = pPath[:len(pPath)-1]
		pURL.Path = strings.Join(pPath, "")
	}
	taskURLS := make([]string, len(tasks))
	for i, t := range tasks {
		tURL, err := url.Parse(t)
		if err == nil && tURL.Scheme != "" && tURL.Host != "" {
			taskURLS[i] = t
			continue // it's already an absolute URL
		}
		tURL = pURL
		tURL = tURL.JoinPath(t)
		taskURLS[i] = tURL.String()
	}
	return taskURLS, nil
}

---

🐛 Bug Analysis

I have found one potential bug in the final version of the code.

Potential Nil Pointer Dereference Panic

In the createTaskURL function, the first for loop is responsible for finding the URL of the current pipeline and parsing it into pURL.

	var pURL *url.URL
	var err error
	for ep := range eventPipelines {
		if eventPipelines[ep] != pipeline {
			continue
		}
		pURL, err = url.Parse(ep)
        //...
	}

If the pipeline object is not found in the eventPipelines map for any reason, the loop will complete and pURL will remain nil. The code will then proceed to the second loop:

	for i, t := range tasks {
        // ...
		tURL = pURL // tURL is now nil
		tURL = tURL.JoinPath(t) // This will cause a panic: nil pointer dereference
		taskURLS[i] = tURL.String()
	}

Recommendation: Add a check after the first loop to ensure pURL was successfully assigned.

// ... after the first for loop
	if pURL == nil {
		// This case should ideally not happen if the logic is correct elsewhere,
		// but it's a necessary safeguard.
		return nil, fmt.Errorf("could not find the remote URL for pipeline %s", pipeline.Name)
	}
// ... continue to the second for loop

---

💡 Code Improvements & Idiomatic Go

The code is functional, but it could be made more robust and idiomatic.

1. Simplifying Path Manipulation

The current code manually manipulates the URL path to get the parent directory:

pPath := strings.SplitAfter(pURL.Path, "/")
pPath = pPath[:len(pPath)-1]
pURL.Path = strings.Join(pPath, "")

This can be done more cleanly and expressively using the standard path package. The path.Dir function is designed for exactly this purpose.

Recommendation: Replace the three lines above with a single, more readable line.

import (
    "path" // Make sure to add this import
    // ...
)

// ... inside the loop
pURL.Path = path.Dir(pURL.Path)

This is more idiomatic and less prone to off-by-one errors than manual slice manipulation.

2. Function Naming

The function is named createTaskURL. However, its primary job is to resolve relative task paths into fully-qualified URLs. It doesn't always "create" them, as it passes through absolute URLs unchanged.

Recommendation: Consider renaming the function to something more descriptive of its action, such as resolveRelativeTaskURLs or makeTaskURLsAbsolute. This makes the code's intent clearer to future readers.

3. Inefficient Loop to Find the Pipeline URL

The code iterates over the eventPipelines map to find the key (URL) for a given value (pipeline object).

for ep := range eventPipelines {
    if eventPipelines[ep] != pipeline {
        continue
    }
    // ... found it
}

This is an O(n) operation. While likely not a performance bottleneck given the small number of pipelines expected per event, it's worth noting. If the calling function resolveRemoteResources could construct a reverse map (map[*tektonv1.Pipeline]string) or pass the pipeline's URL directly, this loop could be avoided. However, given the existing data structures, this is a necessary lookup. The fix for the non-determinism in PATCH 3/3 is excellent, and this implementation is a correct way to achieve it.

---

✅ Spelling and Grammar

The commit messages and PR description are very well-written. I found only one extremely minor typo in the PR description text.

• In the PR description: "...assembles the tasks's FQDN..."
• Correction: Should be "...assembles the task's FQDN..." (for a single task) or "...assembles the tasks' FQDNs..." (for multiple tasks).

Conclusion

This is a solid contribution. The author correctly identified the need for this feature and navigated a couple of tricky implementation details (URL accumulation and map iteration order) to arrive at a working solution.

Summary of Recommendations:

1. Critical: Fix the potential nil pointer panic by adding a check after the first loop in createTaskURL.
2. Highly Recommended: Improve code clarity and make it more idiomatic by using path.Dir for path manipulation.
3. Suggested: Consider renaming createTaskURL to better reflect its purpose (e.g., resolveRelativeTaskURLs).
4. Minor: Correct the minor grammatical error in the PR description.

After addressing the nil pointer bug, this change will be a great addition to the codebase.

[zakisk]

@martindbrv can you please squash all commits into one? we generally follow having changes in one-commit as they seems relative.

[Copilot]

Pull Request Overview

This PR adds support for resolving task references that are specified with relative paths in remote Pipelines by computing their fully qualified URLs based on the pipeline’s location.

• Introduce assembleTaskFQDNs to build absolute task URLs from relative paths
• Update resolveRemoteResources to leverage the new function when fetching remote tasks
• Extend tests, helper functions, testdata, and documentation to cover relative-task scenarios

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/test/tekton/genz.go	Add MakeTaskB helper to marshal a Task to YAML
pkg/resolve/resolve.go	Add PipelineURL field to FetchedResourcesForRun
pkg/resolve/remote.go	Introduce assembleTaskFQDNs and integrate it into remote resolution
pkg/resolve/remote_test.go	Update TestRemote to handle multiple expected PipelineRun outputs and relative tasks
pkg/resolve/testdata/remote-pipeline-with-relative-tasks.yaml	New testdata for relative tasks case
pkg/resolve/testdata/remote-pipeline-with-relative-tasks-1.yaml	Additional variant of testdata for relative tasks
docs/content/docs/guide/resolver.md	Document the new “Relative Tasks” feature

Comments suppressed due to low confidence (5)

pkg/test/tekton/genz.go:153

• [nitpick] The function name MakeTaskB is ambiguous. Consider renaming it to MarshalTask or MakeTaskYAML to clearly communicate that it returns a YAML byte slice.

func MakeTaskB(name string, taskSpec tektonv1.TaskSpec) ([]byte, error) {

pkg/test/tekton/genz.go:153

• Exported function MakeTaskB is missing a GoDoc comment. Please add a descriptive comment explaining its purpose and usage.

func MakeTaskB(name string, taskSpec tektonv1.TaskSpec) ([]byte, error) {

pkg/resolve/remote.go:53

• [nitpick] There are no unit tests covering the error path when pipelineURL is invalid or when url.Parse fails. Consider adding tests to ensure assembleTaskFQDNs handles parse errors correctly.

	return taskURLS, nil

pkg/resolve/remote.go:30

• [nitpick] Input task paths may include accidental whitespace, causing url.Parse to fail or generate URLs with spaces. Consider trimming each task string (strings.TrimSpace) before parsing.

func assembleTaskFQDNs(pipelineURL string, tasks []string) ([]string, error) {

pkg/resolve/remote.go:44

• If t contains leading/trailing whitespace, url.Parse will error and fallback to joining a malformed path. You should t = strings.TrimSpace(t) first to avoid unexpected URLs.

		tURL, err := url.Parse(t)

[chmouel]

This helps, i think fore it's good to go. It would be nice if we can get a E2E tests but i understand it's kinda hard for you to write one since there is a lot of things you need to get setup.. So creating a issue for it is fine as well and some maint can do this...

[martindbrv]

No worries, I can write e2e tests. Just to clarify, except that I have to setup Gitea, I suppose I need to create accounts for each provider if I want to run the corresponding tests, right?

[martindbrv]

Also, I am not completely sure if the Gitea e2e tests fail (TestGiteaStepActions) because of my changes since they succeeded before the last merge

[zakisk]

if the Gitea e2e tests fail (TestGiteaStepActions) because of my changes

@martindbrv that wasn't because of your changes

[zakisk]

@martindbrv are you gonna add E2E?

[martindbrv]

@zakisk Hi. Yeah, I am going to add Gitea tests. Is that gonna be enough for now?

[zakisk]

Yes

[chmouel]

I have fixed your PR and pushed to your branch for the markdownlint issues (make fix-markdownlint)

[chmouel]

the linter error is unrelated to your pr and will be fixed in #2175

[zakisk]

/lgtm

[pipelines-as-code]

Congrats @martindbrv your PR Has been approved 🎉

✅ Pull Request Approved

Approval Status:

• Required Approvals: 1
• Current Approvals: 2

👥 Reviewers Who Approved:

Reviewer	Permission Level	Approval Status
@chmouel	admin	✅
@zakisk	admin	✅

📝 Next Steps

• Ensure all required checks pass
• Comply with branch protection rules
• Request a maintainer to merge using the /merge command (or merge it
  directly if you have repository permission).

Automated by the PAC Boussole 🧭

[chmouel]

Thank you so much for your contribution and sorry for taking so much time to review this...

[chmouel]

/merge

[martindbrv]

Thank you so much for your contribution and sorry for taking so much time to review this...

Happy to help, and no worries at all! I appreciate the review. Also, I'll get the Gitea tests into a new PR as soon as I can.