Skip to content

Commit

Permalink
fix: avoid panic when used pipelineRef or pipelineSpec in pipeline task
Browse files Browse the repository at this point in the history
fix #7720

Currently, the `pipelineRef` and `pipelineSpec` are only in preview mode
and not yet supported. If a user has configured this field and enabled alpha
features, it might bypass validation and enter into controller logic. It is now
necessary to implement relevant checks within the controller logic to clearly
prompt the user, instead of causing the program to panic.
  • Loading branch information
l-qing authored and tekton-robot committed Mar 4, 2024
1 parent a40423b commit f3dd478
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
10 changes: 8 additions & 2 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,8 @@ func resolveTask(
pipelineTask v1.PipelineTask,
) (*resources.ResolvedTask, error) {
rt := &resources.ResolvedTask{}
if pipelineTask.TaskRef != nil {
switch {
case pipelineTask.TaskRef != nil:
// If the TaskRun has already a stored TaskSpec in its status, use it as source of truth
if taskRun != nil && taskRun.Status.TaskSpec != nil {
rt.TaskSpec = taskRun.Status.TaskSpec
Expand All @@ -655,8 +656,13 @@ func resolveTask(
}
}
rt.Kind = pipelineTask.TaskRef.Kind
} else {
case pipelineTask.TaskSpec != nil:
rt.TaskSpec = &pipelineTask.TaskSpec.TaskSpec
default:
// If the alpha feature is enabled, and the user has configured pipelineSpec or pipelineRef, it will enter here.
// Currently, the controller is not yet adapted, and to avoid a panic, an error message is provided here.
// TODO: Adjust the logic here once the feature is supported in the future.
return nil, fmt.Errorf("Currently, Task %q does not support PipelineRef or PipelineSpec, please use TaskRef or TaskSpec instead", pipelineTask.Name)
}
rt.TaskSpec.SetDefaults(ctx)
return rt, nil
Expand Down
26 changes: 26 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"sort"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -2434,6 +2435,31 @@ func TestResolvePipelineRun_PipelineTaskHasNoResources(t *testing.T) {
}
}

func TestResolvePipelineRun_PipelineTaskHasPipelineRef(t *testing.T) {
pt := v1.PipelineTask{
Name: "pipeline-in-pipeline",
PipelineRef: &v1.PipelineRef{Name: "pipeline"},
}

getTask := func(ctx context.Context, name string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) {
return task, nil, nil, nil
}
getTaskRun := func(name string) (*v1.TaskRun, error) { return &trs[0], nil }
pr := v1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "pipelinerun",
},
}

_, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetCustomRun, pt, nil)
if err == nil {
t.Errorf("Error getting tasks for fake pipeline %s, expected an error but got nil.", p.ObjectMeta.Name)
}
if !strings.Contains(err.Error(), "does not support PipelineRef or PipelineSpec") {
t.Errorf("Error getting tasks for fake pipeline %s: expected contains keyword but got %s", p.ObjectMeta.Name, err)
}
}

func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) {
pts := []v1.PipelineTask{{
Name: "mytask1",
Expand Down

0 comments on commit f3dd478

Please sign in to comment.