Skip to content

Commit 092b5ed

Browse files
committed
fix(resolve): deep-copy cached resources before inlining
Use DeepCopy when reusing cached Pipeline and Task objects across PipelineRuns. Without this, inlineTasks mutates the cached original, contaminating subsequent runs that reference the same remote pipeline. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Akshay Pant <akpant@redhat.com>
1 parent 4f6e21b commit 092b5ed

2 files changed

Lines changed: 82 additions & 4 deletions

File tree

pkg/resolve/remote.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ func resolveRemoteResources(ctx context.Context, rt *matcher.RemoteTasks, types
122122
// making sure that the pipeline with same annotation name is not fetched
123123
if alreadyFetchedResource(fetchedResourcesForEvent.Pipelines, remotePipeline) {
124124
rt.Logger.Debugf("skipping already fetched pipeline %s in annotations on pipelinerun %s", remotePipeline, pipelinerun.GetName())
125-
// already fetched, then just get the pipeline to add to run specific Resources
126-
pipeline = fetchedResourcesForEvent.Pipelines[remotePipeline]
125+
// already fetched, deep-copy so inlining for this run doesn't mutate the cached original
126+
pipeline = fetchedResourcesForEvent.Pipelines[remotePipeline].DeepCopy()
127127
} else {
128128
// seems like a new pipeline, fetch it based on name in annotation
129129
pipeline, err = rt.GetPipelineFromAnnotationName(ctx, remotePipeline)
@@ -181,7 +181,7 @@ func resolveRemoteResources(ctx context.Context, rt *matcher.RemoteTasks, types
181181
// if task is already fetched in the event, then just copy the task
182182
if alreadyFetchedResource(fetchedResourcesForEvent.Tasks, remoteTask) {
183183
rt.Logger.Debugf("skipping already fetched task %s in annotations on pipelinerun %s", remoteTask, pipelinerun.GetName())
184-
task = fetchedResourcesForEvent.Tasks[remoteTask]
184+
task = fetchedResourcesForEvent.Tasks[remoteTask].DeepCopy()
185185
} else {
186186
// get the task from annotation name
187187
task, err = rt.GetTaskFromAnnotationName(ctx, remoteTask)
@@ -213,7 +213,7 @@ func resolveRemoteResources(ctx context.Context, rt *matcher.RemoteTasks, types
213213

214214
// if PipelineRef is used then, first resolve pipeline and replace all taskRef{Finally/Task} of Pipeline, then put inlinePipeline in PipelineRun
215215
if pipelinerun.Spec.PipelineRef != nil && pipelinerun.Spec.PipelineRef.Resolver == "" {
216-
pipelineResolved := fetchedResourcesForPipelineRun.Pipeline
216+
pipelineResolved := fetchedResourcesForPipelineRun.Pipeline.DeepCopy()
217217
turns, err := inlineTasks(pipelineResolved.Spec.Tasks, ropt, fetchedResourcesForPipelineRun)
218218
if err != nil {
219219
return nil, err

pkg/resolve/remote_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,84 @@ func TestRemote(t *testing.T) {
512512
}
513513
}
514514

515+
// Verifies that cached remote pipelines are deep-copied before modification.
516+
// Without deep copy, when the first PipelineRun applies its task annotation,
517+
// it would mutate the cached pipeline and leak that task into the second run.
518+
func TestSharedRemotePipelineCacheNotMutated(t *testing.T) {
519+
taskName := "shared-task"
520+
521+
pipelineTask := tektonv1.TaskSpec{
522+
Steps: []tektonv1.Step{
523+
{Name: "from-pipeline", Image: "alpine", Command: []string{"true"}},
524+
},
525+
}
526+
pipelinerunTask := tektonv1.TaskSpec{
527+
Steps: []tektonv1.Step{
528+
{Name: "from-pipelinerun", Image: "busybox", Command: []string{"false"}},
529+
},
530+
}
531+
532+
// remote pipeline with a single TaskRef
533+
pipeline := ttkn.MakePipeline("shared-pipeline", []tektonv1.PipelineTask{
534+
{Name: taskName, TaskRef: &tektonv1.TaskRef{Name: taskName}},
535+
}, map[string]string{
536+
apipac.Task: "http://remote/shared-task",
537+
})
538+
pipelineB, err := yaml.Marshal(pipeline)
539+
assert.NilError(t, err)
540+
541+
pipelineTaskB, err := ttkn.MakeTaskB(taskName, pipelineTask)
542+
assert.NilError(t, err)
543+
pipelinerunTaskB, err := ttkn.MakeTaskB(taskName, pipelinerunTask)
544+
assert.NilError(t, err)
545+
546+
remoteURLS := map[string]map[string]string{
547+
"http://remote/shared-pipeline": {"body": string(pipelineB), "code": "200"},
548+
"http://remote/shared-task": {"body": string(pipelineTaskB), "code": "200"},
549+
"http://remote/pr-task": {"body": string(pipelinerunTaskB), "code": "200"},
550+
}
551+
552+
// first run: overrides the task via pipelinerun annotation
553+
// second run: same pipeline, no override — should get the original task
554+
pipelineruns := []*tektonv1.PipelineRun{
555+
ttkn.MakePR("first-run", map[string]string{
556+
apipac.Pipeline: "http://remote/shared-pipeline",
557+
apipac.Task: "http://remote/pr-task",
558+
}, tektonv1.PipelineRunSpec{
559+
PipelineRef: &tektonv1.PipelineRef{Name: "shared-pipeline"},
560+
}),
561+
ttkn.MakePR("second-run", map[string]string{
562+
apipac.Pipeline: "http://remote/shared-pipeline",
563+
}, tektonv1.PipelineRunSpec{
564+
PipelineRef: &tektonv1.PipelineRef{Name: "shared-pipeline"},
565+
}),
566+
}
567+
568+
observer, _ := zapobserver.New(zap.InfoLevel)
569+
logger := zap.New(observer).Sugar()
570+
ctx, _ := rtesting.SetupFakeContext(t)
571+
httpTestClient := httptesthelper.MakeHTTPTestClient(remoteURLS)
572+
rt := &matcher.RemoteTasks{
573+
ProviderInterface: &testprovider.TestProviderImp{},
574+
Logger: logger,
575+
Run: &params.Run{
576+
Clients: clients.Clients{HTTP: *httpTestClient},
577+
},
578+
}
579+
580+
ret, err := resolveRemoteResources(ctx, rt, TektonTypes{PipelineRuns: pipelineruns}, &Opts{RemoteTasks: true, GenerateName: true})
581+
assert.NilError(t, err)
582+
assert.Equal(t, len(ret), 2)
583+
584+
firstStep := ret[0].Spec.PipelineSpec.Tasks[0].TaskSpec.Steps[0]
585+
assert.Equal(t, firstStep.Name, "from-pipelinerun")
586+
assert.Equal(t, firstStep.Image, "busybox")
587+
588+
secondStep := ret[1].Spec.PipelineSpec.Tasks[0].TaskSpec.Steps[0]
589+
assert.Equal(t, secondStep.Name, "from-pipeline")
590+
assert.Equal(t, secondStep.Image, "alpine")
591+
}
592+
515593
func TestAssembleTaskFQDNs(t *testing.T) {
516594
tests := []struct {
517595
name string

0 commit comments

Comments
 (0)