TEP-0090: Split up ResolvePipelineRunTask

Prior to this commit, the logic for resolving `TaskRun` and its
and `Resources` needed for a given `PipelineTask` was included
within the `ResolvePipelineRunTask` function.

In this change, we split the logic into their own functions for
reuse and readability. We also fix the names of `PipelineTask`
variables to distinguish them from `Tasks`.

No functional changes made in this commit.

Author: jerop

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -463,7 +463,7 @@ func isCustomTask(ctx context.Context, rprt ResolvedPipelineRunTask) bool {
 
 // ResolvePipelineRunTask retrieves a single Task's instance using the getTask to fetch
 // the spec. If it is unable to retrieve an instance of a referenced Task, it  will return
-// an error, otherwise it returns a list of all of the Tasks retrieved.  It will retrieve
+// an error, otherwise it returns a list of all the Tasks retrieved.  It will retrieve
 // the Resources needed for the TaskRun using the mapping of providedResources.
 func ResolvePipelineRunTask(
 	ctx context.Context,
@@ -472,91 +472,124 @@ func ResolvePipelineRunTask(
 	getTaskRun resources.GetTaskRun,
 	getRun GetRun,
 	getCondition GetCondition,
-	task v1beta1.PipelineTask,
+	pipelineTask v1beta1.PipelineTask,
 	providedResources map[string]*resourcev1alpha1.PipelineResource,
 ) (*ResolvedPipelineRunTask, error) {
-
 	rprt := ResolvedPipelineRunTask{
-		PipelineTask: &task,
+		PipelineTask: &pipelineTask,
 	}
 	rprt.CustomTask = isCustomTask(ctx, rprt)
 	if rprt.IsCustomTask() {
-		rprt.RunName = getRunName(pipelineRun.Status.Runs, pipelineRun.Status.ChildReferences, task.Name, pipelineRun.Name)
+		rprt.RunName = getRunName(pipelineRun.Status.Runs, pipelineRun.Status.ChildReferences, pipelineTask.Name, pipelineRun.Name)
 		run, err := getRun(rprt.RunName)
 		if err != nil && !kerrors.IsNotFound(err) {
 			return nil, fmt.Errorf("error retrieving Run %s: %w", rprt.RunName, err)
 		}
 		rprt.Run = run
 	} else {
-		rprt.TaskRunName = GetTaskRunName(pipelineRun.Status.TaskRuns, pipelineRun.Status.ChildReferences, task.Name, pipelineRun.Name)
-
-		taskRun, err := getTaskRun(rprt.TaskRunName)
-		if err != nil {
-			if !kerrors.IsNotFound(err) {
-				return nil, fmt.Errorf("error retrieving TaskRun %s: %w", rprt.TaskRunName, err)
-			}
-		}
-		if taskRun != nil {
-			rprt.TaskRun = taskRun
+		rprt.TaskRunName = GetTaskRunName(pipelineRun.Status.TaskRuns, pipelineRun.Status.ChildReferences, pipelineTask.Name, pipelineRun.Name)
+		if err := resolvePipelineRunTaskWithTaskRun(ctx, &rprt, rprt.TaskRunName, pipelineRun, getTask, getTaskRun, getCondition, pipelineTask, providedResources); err != nil {
+			return nil, err
 		}
+	}
+	return &rprt, nil
+}
 
-		// Find the Task that this PipelineTask is using
-		spec, taskName, kind, err := resolveTask(ctx, taskRun, getTask, task)
-		if err != nil {
-			return nil, err
+func resolvePipelineRunTaskWithTaskRun(
+	ctx context.Context,
+	rprt *ResolvedPipelineRunTask,
+	taskRunName string,
+	pipelineRun v1beta1.PipelineRun,
+	getTask resources.GetTask,
+	getTaskRun resources.GetTaskRun,
+	getCondition GetCondition,
+	pipelineTask v1beta1.PipelineTask,
+	providedResources map[string]*resourcev1alpha1.PipelineResource,
+) error {
+	taskRun, err := getTaskRun(taskRunName)
+	if err != nil {
+		if !kerrors.IsNotFound(err) {
+			return fmt.Errorf("error retrieving TaskRun %s: %w", taskRunName, err)
 		}
+	}
+	rprt.TaskRun = taskRun
 
-		spec.SetDefaults(ctx)
-		rtr, err := resolvePipelineTaskResources(task, &spec, taskName, kind, providedResources)
+	rtr, err := resolveTaskResources(ctx, getTask, pipelineTask, providedResources, taskRun)
+	if err != nil {
+		return err
+	}
+	rprt.ResolvedTaskResources = rtr
+
+	// Get all conditions that this pipelineTask will be using, if any
+	if len(pipelineTask.Conditions) > 0 {
+		rcc, err := resolveConditionChecks(&pipelineTask, pipelineRun.Status.TaskRuns, pipelineRun.Status.ChildReferences, rprt.TaskRunName, getTaskRun, getCondition, providedResources)
 		if err != nil {
-			return nil, fmt.Errorf("couldn't match referenced resources with declared resources: %w", err)
+			return err
 		}
+		rprt.ResolvedConditionChecks = rcc
+	}
+	return nil
+}
 
-		rprt.ResolvedTaskResources = rtr
+func resolveTaskResources(
+	ctx context.Context,
+	getTask resources.GetTask,
+	pipelineTask v1beta1.PipelineTask,
+	providedResources map[string]*resourcev1alpha1.PipelineResource,
+	taskRun *v1beta1.TaskRun,
+) (*resources.ResolvedTaskResources, error) {
 
-		// Get all conditions that this pipelineTask will be using, if any
-		if len(task.Conditions) > 0 {
-			rcc, err := resolveConditionChecks(&task, pipelineRun.Status.TaskRuns, pipelineRun.Status.ChildReferences, rprt.TaskRunName, getTaskRun, getCondition, providedResources)
-			if err != nil {
-				return nil, err
-			}
-			rprt.ResolvedConditionChecks = rcc
-		}
+	spec, taskName, kind, err := resolveTask(ctx, taskRun, getTask, pipelineTask)
+	if err != nil {
+		return nil, err
 	}
-	return &rprt, nil
+
+	spec.SetDefaults(ctx)
+	rtr, err := resolvePipelineTaskResources(pipelineTask, &spec, taskName, kind, providedResources)
+	if err != nil {
+		return nil, fmt.Errorf("couldn't match referenced resources with declared resources: %w", err)
+	}
+
+	return rtr, nil
 }
 
-func resolveTask(ctx context.Context, taskRun *v1beta1.TaskRun, getTask resources.GetTask, task v1beta1.PipelineTask) (v1beta1.TaskSpec, string, v1beta1.TaskKind, error) {
+func resolveTask(
+	ctx context.Context,
+	taskRun *v1beta1.TaskRun,
+	getTask resources.GetTask,
+	pipelineTask v1beta1.PipelineTask,
+) (v1beta1.TaskSpec, string, v1beta1.TaskKind, error) {
 	var (
 		t        v1beta1.TaskObject
 		err      error
 		spec     v1beta1.TaskSpec
 		taskName string
 		kind     v1beta1.TaskKind
 	)
-	if task.TaskRef != nil {
+
+	if 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 {
 			spec = *taskRun.Status.TaskSpec
-			taskName = task.TaskRef.Name
+			taskName = pipelineTask.TaskRef.Name
 		} else {
-			t, err = getTask(ctx, task.TaskRef.Name)
+			t, err = getTask(ctx, pipelineTask.TaskRef.Name)
 			switch {
 			case errors.Is(err, remote.ErrorRequestInProgress):
 				return v1beta1.TaskSpec{}, "", "", err
 			case err != nil:
 				return v1beta1.TaskSpec{}, "", "", &TaskNotFoundError{
-					Name: task.TaskRef.Name,
+					Name: pipelineTask.TaskRef.Name,
 					Msg:  err.Error(),
 				}
 			default:
 				spec = t.TaskSpec()
 				taskName = t.TaskMetadata().Name
 			}
 		}
-		kind = task.TaskRef.Kind
-	} else {
-		spec = task.TaskSpec.TaskSpec
+		kind = pipelineTask.TaskRef.Kind
+	} else if pipelineTask.TaskSpec != nil {
+		spec = pipelineTask.TaskSpec.TaskSpec
 	}
 	return spec, taskName, kind, err
 }