Skip to content

Commit bfb63c6

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 641eadc commit bfb63c6

2 files changed

Lines changed: 83 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: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,85 @@ func TestRemote(t *testing.T) {
512512
}
513513
}
514514

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

0 commit comments

Comments
 (0)