From ad975ccab1b791827cf9ed65ee04d6d4d16e7eb1 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Wed, 1 Nov 2023 17:22:10 +0000 Subject: [PATCH] [TEP-0142] Remote Resolution for StepAction This commit is part of #7259. It adds the remote resolution for StepAction. Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com --- pkg/apis/pipeline/v1/container_types.go | 4 + pkg/reconciler/taskrun/resources/taskref.go | 31 ++++- .../taskrun/resources/taskref_test.go | 27 ++-- pkg/reconciler/taskrun/resources/taskspec.go | 14 +- .../taskrun/resources/taskspec_test.go | 122 +++++++++--------- pkg/reconciler/taskrun/taskrun.go | 3 +- 6 files changed, 119 insertions(+), 82 deletions(-) diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go index e0b979ffa86..922ac1c8d11 100644 --- a/pkg/apis/pipeline/v1/container_types.go +++ b/pkg/apis/pipeline/v1/container_types.go @@ -144,6 +144,10 @@ type Step struct { type Ref struct { // Name of the referenced step Name string `json:"name,omitempty"` + // ResolverRef allows referencing a StepAction in a remote location + // like a git repo. + // +optional + ResolverRef `json:",omitempty"` } // OnErrorType defines a list of supported exiting behavior of a container on error diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index 03896f7976a..d1683f6250b 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -124,7 +124,18 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset } // GetStepActionFunc is a factory function that will use the given Ref as context to return a valid GetStepAction function. -func GetStepActionFunc(tekton clientset.Interface, namespace string) GetStepAction { +func GetStepActionFunc(tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester, tr *v1.TaskRun, step *v1.Step) GetStepAction { + kind := "StepAction" + trName := tr.Name + namespace := tr.Namespace + if step.Ref != nil && step.Ref.Resolver != "" && requester != nil { + // Return an inline function that implements GetTask by calling Resolver.Get with the specified task type and + // casting it to a TaskObject. + return func(ctx context.Context, ref *v1.Ref) (*v1alpha1.StepAction, *v1.RefSource, error) { + resolver := resolution.NewResolver(requester, tr, string(ref.Resolver), trName, namespace, v1.Params{}) + return resolveStepAction(ctx, resolver, ref.Name, namespace, kind, k8s, tekton) + } + } local := &LocalStepActionRefResolver{ Namespace: namespace, Tektonclient: tekton, @@ -151,6 +162,18 @@ func resolveTask(ctx context.Context, resolver remote.Resolver, name, namespace return taskObj, refSource, vr, nil } +func resolveStepAction(ctx context.Context, resolver remote.Resolver, name, namespace, kind string, k8s kubernetes.Interface, tekton clientset.Interface) (*v1alpha1.StepAction, *v1.RefSource, error) { + obj, refSource, err := resolver.Get(ctx, strings.TrimSuffix(strings.ToLower(string(kind)), "s"), name) + if err != nil { + return nil, nil, err + } + switch obj := obj.(type) { + case *v1alpha1.StepAction: + return obj, refSource, nil + } + return nil, nil, errors.New("resource is not a step action") +} + // readRuntimeObjectAsTask tries to convert a generic runtime.Object // into a *v1.Task type so that its meta and spec fields // can be read. v1beta1 object will be converted to v1 and returned. @@ -239,12 +262,12 @@ type LocalStepActionRefResolver struct { // GetStepAction will resolve a StepAction from the local cluster using a versioned Tekton client. // It will return an error if it can't find an appropriate StepAction for any reason. -func (l *LocalStepActionRefResolver) GetStepAction(ctx context.Context, name string) (*v1alpha1.StepAction, *v1.RefSource, error) { +func (l *LocalStepActionRefResolver) GetStepAction(ctx context.Context, ref *v1.Ref) (*v1alpha1.StepAction, *v1.RefSource, error) { // If we are going to resolve this reference locally, we need a namespace scope. if l.Namespace == "" { - return nil, nil, fmt.Errorf("must specify namespace to resolve reference to step action %s", name) + return nil, nil, fmt.Errorf("must specify namespace to resolve reference to step action %s", ref.Name) } - stepAction, err := l.Tektonclient.TektonV1alpha1().StepActions(l.Namespace).Get(ctx, name, metav1.GetOptions{}) + stepAction, err := l.Tektonclient.TektonV1alpha1().StepActions(l.Namespace).Get(ctx, ref.Name, metav1.GetOptions{}) if err != nil { return nil, nil, err } diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index c97ead68377..60412c62808 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -372,7 +372,7 @@ func TestStepActionRef(t *testing.T) { Tektonclient: tektonclient, } - task, refSource, err := lc.GetStepAction(ctx, tc.ref.Name) + task, refSource, err := lc.GetStepAction(ctx, tc.ref) if err != nil { t.Fatalf("Received unexpected error ( %#v )", err) } @@ -435,7 +435,7 @@ func TestStepActionRef_Error(t *testing.T) { Tektonclient: tektonclient, } - _, _, err := lc.GetStepAction(ctx, tc.ref.Name) + _, _, err := lc.GetStepAction(ctx, tc.ref) if err == nil { t.Fatal("Expected error but found nil instead") } @@ -561,14 +561,26 @@ func TestGetStepActionFunc_Local(t *testing.T) { testcases := []struct { name string localStepActions []runtime.Object - ref *v1.Ref + taskRun *v1.TaskRun expected runtime.Object }{ { name: "local-step-action", localStepActions: []runtime.Object{simpleNamespacedStepAction}, - ref: &v1.Ref{ - Name: "simple", + taskRun: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-tr", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "simple", + }, + }}, + }, + }, }, expected: simpleNamespacedStepAction, }, @@ -577,10 +589,9 @@ func TestGetStepActionFunc_Local(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { tektonclient := fake.NewSimpleClientset(tc.localStepActions...) + fn := resources.GetStepActionFunc(tektonclient, nil, nil, tc.taskRun, &tc.taskRun.Spec.TaskSpec.Steps[0]) - fn := resources.GetStepActionFunc(tektonclient, "default") - - stepAction, refSource, err := fn(ctx, tc.ref.Name) + stepAction, refSource, err := fn(ctx, tc.taskRun.Spec.TaskSpec.Steps[0].Ref) if err != nil { t.Fatalf("failed to call stepActionfn: %s", err.Error()) } diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index 19954a78950..4457709ebd6 100644 --- a/pkg/reconciler/taskrun/resources/taskspec.go +++ b/pkg/reconciler/taskrun/resources/taskspec.go @@ -23,9 +23,12 @@ import ( v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned" resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution" + remoteresource "github.com/tektoncd/pipeline/pkg/resolution/resource" "github.com/tektoncd/pipeline/pkg/trustedresources" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" ) // ResolvedTask contains the data that is needed to execute @@ -39,7 +42,7 @@ type ResolvedTask struct { } // GetStepAction is a function used to retrieve StepActions. -type GetStepAction func(context.Context, string) (*v1alpha1.StepAction, *v1.RefSource, error) +type GetStepAction func(context.Context, *v1.Ref) (*v1alpha1.StepAction, *v1.RefSource, error) // GetTask is a function used to retrieve Tasks. // VerificationResult is the result from trusted resources if the feature is enabled. @@ -51,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, getStepAction GetStepAction) (*resolutionutil.ResolvedObjectMeta, *v1.TaskSpec, error) { +func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask, tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester) (*resolutionutil.ResolvedObjectMeta, *v1.TaskSpec, error) { taskMeta := metav1.ObjectMeta{} taskSpec := v1.TaskSpec{} var refSource *v1.RefSource @@ -89,7 +92,7 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask, getS return nil, nil, fmt.Errorf("taskRun %s not providing TaskRef or TaskSpec", taskRun.Name) } - steps, err := extractStepActions(ctx, taskSpec, getStepAction) + steps, err := extractStepActions(ctx, taskSpec, taskRun, tekton, k8s, requester) if err != nil { return nil, nil, err } else { @@ -105,13 +108,14 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask, getS } // extractStepActions extracts the StepActions and merges them with the inlined Step specification. -func extractStepActions(ctx context.Context, taskSpec v1.TaskSpec, getStepAction GetStepAction) ([]v1.Step, error) { +func extractStepActions(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() if step.Ref != nil { s.Ref = nil - stepAction, _, err := getStepAction(ctx, step.Ref.Name) + getStepAction := GetStepActionFunc(tekton, k8s, requester, taskRun, &step) + stepAction, _, err := getStepAction(ctx, step.Ref) if err != nil { return nil, err } diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index c5eaa6893b4..e9ee64ec4e9 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -27,6 +27,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/trustedresources" "github.com/tektoncd/pipeline/test/diff" @@ -34,20 +35,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func getStepAction(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) { - stepAction := &v1alpha1.StepAction{ - ObjectMeta: metav1.ObjectMeta{ - Name: "stepAction", - }, - Spec: v1alpha1.StepActionSpec{ - Image: "myimage", - Command: []string{"ls"}, - Args: []string{"-lh"}, - }, - } - return stepAction, nil, nil -} - func TestGetTaskSpec_Ref(t *testing.T) { task := &v1.Task{ ObjectMeta: metav1.ObjectMeta{ @@ -73,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, getStepAction) + resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) @@ -107,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, getStepAction) + resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) @@ -136,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, getStepAction) + _, _, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) if err == nil { t.Fatalf("Expected error resolving spec with no embedded or referenced task spec but didn't get error") } @@ -156,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, getStepAction) + _, _, err := resources.GetTaskData(context.Background(), tr, gt, nil, nil, nil) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -200,7 +187,7 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) { }, sampleRefSource.DeepCopy(), nil, nil } ctx := context.Background() - resolvedMeta, resolvedSpec, err := resources.GetTaskData(ctx, tr, getTask, getStepAction) + resolvedMeta, resolvedSpec, err := resources.GetTaskData(ctx, tr, getTask, nil, nil, nil) if err != nil { t.Fatalf("Unexpected error getting mocked data: %v", err) } @@ -234,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, getStepAction) + _, _, err := resources.GetTaskData(ctx, tr, getTask, nil, nil, nil) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -257,7 +244,7 @@ func TestGetTaskData_ResolvedNilTask(t *testing.T) { return nil, nil, nil, nil } ctx := context.Background() - _, _, err := resources.GetTaskData(ctx, tr, getTask, getStepAction) + _, _, err := resources.GetTaskData(ctx, tr, getTask, nil, nil, nil) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -304,7 +291,7 @@ func TestGetTaskData_VerificationResult(t *testing.T) { Spec: *sourceSpec.DeepCopy(), }, nil, verificationResult, nil } - r, _, err := resources.GetTaskData(context.Background(), tr, getTask, getStepAction) + r, _, err := resources.GetTaskData(context.Background(), tr, getTask, nil, nil, nil) if err != nil { t.Fatalf("Did not expect error but got: %s", err) } @@ -315,15 +302,16 @@ func TestGetTaskData_VerificationResult(t *testing.T) { func TestGetStepAction(t *testing.T) { tests := []struct { - name string - tr *v1.TaskRun - want *v1.TaskSpec - stepActionFunc func(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) + name string + tr *v1.TaskRun + want *v1.TaskSpec + stepAction *v1alpha1.StepAction }{{ name: "step-action-with-command-args", tr: &v1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ - Name: "mytaskrun", + Name: "mytaskrun", + Namespace: "default", }, Spec: v1.TaskRunSpec{ TaskSpec: &v1.TaskSpec{ @@ -346,12 +334,23 @@ func TestGetStepAction(t *testing.T) { Timeout: &metav1.Duration{Duration: time.Hour}, }}, }, - stepActionFunc: getStepAction, + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepAction", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Command: []string{"ls"}, + Args: []string{"-lh"}, + }, + }, }, { name: "step-action-with-script", tr: &v1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ - Name: "mytaskrun", + Name: "mytaskrun", + Namespace: "default", }, Spec: v1.TaskRunSpec{ TaskSpec: &v1.TaskSpec{ @@ -369,23 +368,22 @@ func TestGetStepAction(t *testing.T) { Script: "ls", }}, }, - stepActionFunc: func(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) { - stepAction := &v1alpha1.StepAction{ - ObjectMeta: metav1.ObjectMeta{ - Name: "stepActionWithScript", - }, - Spec: v1alpha1.StepActionSpec{ - Image: "myimage", - Script: "ls", - }, - } - return stepAction, nil, nil + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepActionWithScript", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Script: "ls", + }, }, }, { name: "step-action-with-env", tr: &v1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ - Name: "mytaskrun", + Name: "mytaskrun", + Namespace: "default", }, Spec: v1.TaskRunSpec{ TaskSpec: &v1.TaskSpec{ @@ -406,28 +404,28 @@ func TestGetStepAction(t *testing.T) { }}, }}, }, - stepActionFunc: func(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) { - stepAction := &v1alpha1.StepAction{ - ObjectMeta: metav1.ObjectMeta{ - Name: "stepActionWithEnv", - }, - Spec: v1alpha1.StepActionSpec{ - Image: "myimage", - Env: []corev1.EnvVar{{ - Name: "env1", - Value: "value1", - }}, - }, - } - return stepAction, nil, nil + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepActionWithEnv", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Env: []corev1.EnvVar{{ + Name: "env1", + Value: "value1", + }}, + }, }, }} 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(context.Background(), tt.tr, gt, tt.stepActionFunc) + _, got, err := resources.GetTaskData(ctx, tt.tr, gt, tektonclient, nil, nil) if err != nil { t.Errorf("Did not expect an error but got : %s", err) } @@ -441,10 +439,10 @@ func TestGetStepAction_Error(t *testing.T) { tests := []struct { name string tr *v1.TaskRun - stepActionFunc func(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) + stepActionFunc func(ctx context.Context, ref *v1.Ref) (*v1alpha1.StepAction, *v1.RefSource, error) expectedError error }{{ - name: "step-action-error", + name: "namespace missing error", tr: &v1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: "mytaskrun", @@ -459,17 +457,15 @@ func TestGetStepAction_Error(t *testing.T) { }, }, }, - stepActionFunc: func(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) { - return nil, nil, fmt.Errorf("Error fetching Step Action.") - }, - expectedError: fmt.Errorf("Error fetching Step Action."), + + 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, tt.stepActionFunc) + _, _, err := resources.GetTaskData(context.Background(), tt.tr, gt, 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 fd0b2553468..fcbc19b8dd8 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -332,9 +332,8 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, return nil, nil, fmt.Errorf("failed to list VerificationPolicies from namespace %s with error %w", tr.Namespace, err) } getTaskfunc := resources.GetTaskFuncFromTaskRun(ctx, c.KubeClientSet, c.PipelineClientSet, c.resolutionRequester, tr, vp) - getStepActionfunc := resources.GetStepActionFunc(c.PipelineClientSet, tr.Namespace) - taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc, getStepActionfunc) + taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc, c.PipelineClientSet, c.KubeClientSet, c.resolutionRequester) switch { case errors.Is(err, remote.ErrRequestInProgress): message := fmt.Sprintf("TaskRun %s/%s awaiting remote resource", tr.Namespace, tr.Name)