diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index 4457709ebd6..8b312b8dbb6 100644 --- a/pkg/reconciler/taskrun/resources/taskspec.go +++ b/pkg/reconciler/taskrun/resources/taskspec.go @@ -54,7 +54,7 @@ type GetTaskRun func(string) (*v1.TaskRun, error) // GetTaskData will retrieve the Task metadata and Spec associated with the // provided TaskRun. This can come from a reference Task or from the TaskRun's // metadata and embedded TaskSpec. -func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask, tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester) (*resolutionutil.ResolvedObjectMeta, *v1.TaskSpec, error) { +func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask) (*resolutionutil.ResolvedObjectMeta, *v1.TaskSpec, error) { taskMeta := metav1.ObjectMeta{} taskSpec := v1.TaskSpec{} var refSource *v1.RefSource @@ -92,13 +92,6 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask, tekt return nil, nil, fmt.Errorf("taskRun %s not providing TaskRef or TaskSpec", taskRun.Name) } - steps, err := extractStepActions(ctx, taskSpec, taskRun, tekton, k8s, requester) - if err != nil { - return nil, nil, err - } else { - taskSpec.Steps = steps - } - taskSpec.SetDefaults(ctx) return &resolutionutil.ResolvedObjectMeta{ ObjectMeta: &taskMeta, @@ -107,8 +100,8 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask, tekt }, &taskSpec, nil } -// extractStepActions extracts the StepActions and merges them with the inlined Step specification. -func extractStepActions(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.TaskRun, tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester) ([]v1.Step, error) { +// GetStepActionsData extracts the StepActions and merges them with the inlined Step specification. +func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.TaskRun, tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester) ([]v1.Step, error) { steps := []v1.Step{} for _, step := range taskSpec.Steps { s := step.DeepCopy() diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index e9ee64ec4e9..d1e8d428fac 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -60,7 +60,7 @@ func TestGetTaskSpec_Ref(t *testing.T) { gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { return task, sampleRefSource.DeepCopy(), nil, nil } - resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) + resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) @@ -94,7 +94,7 @@ func TestGetTaskSpec_Embedded(t *testing.T) { gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { return nil, nil, nil, errors.New("shouldn't be called") } - resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) + resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) @@ -123,7 +123,7 @@ func TestGetTaskSpec_Invalid(t *testing.T) { gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { return nil, nil, nil, errors.New("shouldn't be called") } - _, _, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) + _, _, err := resources.GetTaskData(context.Background(), tr, gt) if err == nil { t.Fatalf("Expected error resolving spec with no embedded or referenced task spec but didn't get error") } @@ -143,7 +143,7 @@ func TestGetTaskSpec_Error(t *testing.T) { gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { return nil, nil, nil, errors.New("something went wrong") } - _, _, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) + _, _, err := resources.GetTaskData(context.Background(), tr, gt) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -187,7 +187,7 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) { }, sampleRefSource.DeepCopy(), nil, nil } ctx := context.Background() - resolvedMeta, resolvedSpec, err := resources.GetTaskData(ctx, tr, getTask, nil, nil, nil) + resolvedMeta, resolvedSpec, err := resources.GetTaskData(ctx, tr, getTask) if err != nil { t.Fatalf("Unexpected error getting mocked data: %v", err) } @@ -221,7 +221,7 @@ func TestGetPipelineData_ResolutionError(t *testing.T) { return nil, nil, nil, errors.New("something went wrong") } ctx := context.Background() - _, _, err := resources.GetTaskData(ctx, tr, getTask, nil, nil, nil) + _, _, err := resources.GetTaskData(ctx, tr, getTask) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -244,7 +244,7 @@ func TestGetTaskData_ResolvedNilTask(t *testing.T) { return nil, nil, nil, nil } ctx := context.Background() - _, _, err := resources.GetTaskData(ctx, tr, getTask, nil, nil, nil) + _, _, err := resources.GetTaskData(ctx, tr, getTask) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -291,7 +291,7 @@ func TestGetTaskData_VerificationResult(t *testing.T) { Spec: *sourceSpec.DeepCopy(), }, nil, verificationResult, nil } - r, _, err := resources.GetTaskData(context.Background(), tr, getTask, nil, nil, nil) + r, _, err := resources.GetTaskData(context.Background(), tr, getTask) if err != nil { t.Fatalf("Did not expect error but got: %s", err) } @@ -300,11 +300,11 @@ func TestGetTaskData_VerificationResult(t *testing.T) { } } -func TestGetStepAction(t *testing.T) { +func TestGetStepActionsData(t *testing.T) { tests := []struct { name string tr *v1.TaskRun - want *v1.TaskSpec + want []v1.Step stepAction *v1alpha1.StepAction }{{ name: "step-action-with-command-args", @@ -325,15 +325,13 @@ func TestGetStepAction(t *testing.T) { }, }, }, - want: &v1.TaskSpec{ - Steps: []v1.Step{{ - Image: "myimage", - Command: []string{"ls"}, - Args: []string{"-lh"}, - WorkingDir: "/bar", - Timeout: &metav1.Duration{Duration: time.Hour}, - }}, - }, + want: []v1.Step{{ + Image: "myimage", + Command: []string{"ls"}, + Args: []string{"-lh"}, + WorkingDir: "/bar", + Timeout: &metav1.Duration{Duration: time.Hour}, + }}, stepAction: &v1alpha1.StepAction{ ObjectMeta: metav1.ObjectMeta{ Name: "stepAction", @@ -362,12 +360,10 @@ func TestGetStepAction(t *testing.T) { }, }, }, - want: &v1.TaskSpec{ - Steps: []v1.Step{{ - Image: "myimage", - Script: "ls", - }}, - }, + want: []v1.Step{{ + Image: "myimage", + Script: "ls", + }}, stepAction: &v1alpha1.StepAction{ ObjectMeta: metav1.ObjectMeta{ Name: "stepActionWithScript", @@ -395,15 +391,13 @@ func TestGetStepAction(t *testing.T) { }, }, }, - want: &v1.TaskSpec{ - Steps: []v1.Step{{ - Image: "myimage", - Env: []corev1.EnvVar{{ - Name: "env1", - Value: "value1", - }}, + want: []v1.Step{{ + Image: "myimage", + Env: []corev1.EnvVar{{ + Name: "env1", + Value: "value1", }}, - }, + }}, stepAction: &v1alpha1.StepAction{ ObjectMeta: metav1.ObjectMeta{ Name: "stepActionWithEnv", @@ -419,13 +413,10 @@ func TestGetStepAction(t *testing.T) { }, }} for _, tt := range tests { - gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { - return nil, nil, nil, nil - } ctx := context.Background() tektonclient := fake.NewSimpleClientset(tt.stepAction) - _, got, err := resources.GetTaskData(ctx, tt.tr, gt, tektonclient, nil, nil) + got, err := resources.GetStepActionsData(ctx, *tt.tr.Spec.TaskSpec, tt.tr, tektonclient, nil, nil) if err != nil { t.Errorf("Did not expect an error but got : %s", err) } @@ -435,7 +426,7 @@ func TestGetStepAction(t *testing.T) { } } -func TestGetStepAction_Error(t *testing.T) { +func TestGetStepActionsData_Error(t *testing.T) { tests := []struct { name string tr *v1.TaskRun @@ -461,11 +452,7 @@ func TestGetStepAction_Error(t *testing.T) { expectedError: fmt.Errorf("must specify namespace to resolve reference to step action stepActionError"), }} for _, tt := range tests { - gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { - return nil, nil, nil, nil - } - - _, _, err := resources.GetTaskData(context.Background(), tt.tr, gt, nil, nil, nil) + _, err := resources.GetStepActionsData(context.Background(), *tt.tr.Spec.TaskSpec, tt.tr, nil, nil, nil) if err == nil { t.Fatalf("Expected to get an error but did not find any.") } diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index fcbc19b8dd8..f6d2a550a2d 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -333,7 +333,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, } getTaskfunc := resources.GetTaskFuncFromTaskRun(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, tr, vp) - taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc, c.PipelineClientSet, c.KubeClientSet, c.resolutionRequester) + taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc) switch { case errors.Is(err, remote.ErrRequestInProgress): message := fmt.Sprintf("TaskRun %s/%s awaiting remote resource", tr.Namespace, tr.Name) @@ -358,6 +358,29 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, } } + steps, err := resources.GetStepActionsData(ctx, *taskSpec, tr, c.PipelineClientSet, c.KubeClientSet, c.resolutionRequester) + switch { + case errors.Is(err, remote.ErrRequestInProgress): + message := fmt.Sprintf("TaskRun %s/%s awaiting remote StepAction", tr.Namespace, tr.Name) + tr.Status.MarkResourceOngoing(v1.TaskRunReasonResolvingTaskRef, message) + return nil, nil, err + case errors.Is(err, apiserver.ErrReferencedObjectValidationFailed), errors.Is(err, apiserver.ErrCouldntValidateObjectPermanent): + tr.Status.MarkResourceFailed(podconvert.ReasonTaskFailedValidation, err) + return nil, nil, controller.NewPermanentError(err) + case errors.Is(err, apiserver.ErrCouldntValidateObjectRetryable): + return nil, nil, err + case err != nil: + logger.Errorf("Failed to determine StepAction to use for taskrun %s: %v", tr.Name, err) + if resources.IsGetTaskErrTransient(err) { + return nil, nil, err + } + tr.Status.MarkResourceFailed(podconvert.ReasonFailedResolution, err) + return nil, nil, controller.NewPermanentError(err) + default: + // Store the fetched StepActions to taskspec + taskSpec.Steps = steps + } + if taskMeta.VerificationResult != nil { switch taskMeta.VerificationResult.VerificationResultType { case trustedresources.VerificationError: