[TEP-0142] Remote Resolution for StepAction

This commit is part of #7259. It adds the remote resolution for StepAction. Signed-off-by: Yongxuan Zhang [email protected]
tektoncd · Nov 5, 2023 · c2d2f48 · c2d2f48
1 parent a791d49
commit c2d2f48
Show file tree

Hide file tree

Showing 11 changed files with 428 additions and 18 deletions.
diff --git a/docs/stepactions.md b/docs/stepactions.md
@@ -17,7 +17,7 @@ A `StepAction` is the reusable and scriptable unit of work that is performed by
 
 A `Step` is not reusable, the work it performs is reusable and referenceable. `Steps` are in-lined in the `Task` definition and either perform work directly or perform a `StepAction`. A `StepAction` cannot be run stand-alone (unlike a `TaskRun` or a `PipelineRun`). It has to be referenced by a `Step`. Another way to ehink about this is that a `Step` is not composed of `StepActions` (unlike a `Task` being composed of `Steps` and `Sidecars`). Instead, a `Step` is an actionable component, meaning that it has the ability to refer to a `StepAction`. The author of the `StepAction` must be able to compose a `Step` using a `StepAction` and provide all the necessary context (or orchestration) to it.
 
- 
+
 ## Configuring a `StepAction`
 
 A `StepAction` definition supports the following fields:
@@ -29,7 +29,7 @@ A `StepAction` definition supports the following fields:
   - [`metadata`][kubernetes-overview] - Specifies metadata that uniquely identifies the
     `StepAction` resource object. For example, a `name`.
   - [`spec`][kubernetes-overview] - Specifies the configuration information for this `StepAction` resource object.
-  - `image` - Specifies the image to use for the `Step`. 
+  - `image` - Specifies the image to use for the `Step`.
     - The container image must abide by the [container contract](./container-contract.md).
 - Optional
   - `command`
@@ -50,7 +50,7 @@ spec:
   env:
     - name: HOME
       value: /home
-  image: ubuntu 
+  image: ubuntu
   command: ["ls"]
   args: ["-lh"]
 ```
@@ -169,3 +169,29 @@ spec:
         timeout: 1h
         onError: continue
 ```
+
+### Specifying Remote StepActions
+
+A `ref` field may specify a StepAction in a remote location such as git.
+Support for specific types of remote will depend on the Resolvers your
+cluster's operator has installed. For more information including a tutorial, please check [resolution docs](resolution.md). The below example demonstrates referencing a StepAction in git:
+
+```yaml
+apiVersion: tekton.dev/v1
+kind: TaskRun
+metadata:
+  generateName: step-action-run-
+spec:
+  TaskSpec:
+    steps:
+      - name: action-runner
+        ref:
+          resolver: git
+          params:
+            - name: url
+              value: https://github.com/repo/repo.git
+            - name: revision
+              value: main
+            - name: pathInRepo
+              value: remote_step.yaml
+```
diff --git a/examples/v1/taskruns/alpha/stepaction-git-resolver.yaml b/examples/v1/taskruns/alpha/stepaction-git-resolver.yaml
@@ -0,0 +1,18 @@
+# TODO(#7325): use StepAction from Catalog
+apiVersion: tekton.dev/v1
+kind: TaskRun
+metadata:
+  generateName: step-action-run-
+spec:
+  TaskSpec:
+    steps:
+      - name: action-runner
+        ref:
+          resolver: git
+          params:
+            - name: url
+              value: https://github.com/chitrangpatel/repo1M.git
+            - name: revision
+              value: main
+            - name: pathInRepo
+              value: basic_step.yaml
diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go
@@ -179,6 +179,9 @@ const (
 	// TaskRunReasonResolvingTaskRef indicates that the TaskRun is waiting for
 	// its taskRef to be asynchronously resolved.
 	TaskRunReasonResolvingTaskRef = "ResolvingTaskRef"
+	// TaskRunReasonResolvingStepActionRef indicates that the TaskRun is waiting for
+	// its StepAction's Ref to be asynchronously resolved.
+	TaskRunReasonResolvingStepActionRef = "ResolvingRef"
 	// TaskRunReasonImagePullFailed is the reason set when the step of a task fails due to image not being pulled
 	TaskRunReasonImagePullFailed TaskRunReason = "TaskRunImagePullFailed"
 	// TaskRunReasonResultLargerThanAllowedLimit is the reason set when one of the results exceeds its maximum allowed limit of 1 KB

diff --git a/pkg/reconciler/apiserver/apiserver.go b/pkg/reconciler/apiserver/apiserver.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/google/uuid"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -56,6 +57,13 @@ func DryRunValidate(ctx context.Context, namespace string, obj runtime.Object, t
 		if _, err := tekton.TektonV1beta1().Tasks(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
 			return handleDryRunCreateErr(err, obj.Name)
 		}
+	case *v1alpha1.StepAction:
+		dryRunObj := obj.DeepCopy()
+		dryRunObj.Name = dryRunObjName
+		dryRunObj.Namespace = namespace // Make sure the namespace is the same as the StepAction
+		if _, err := tekton.TektonV1alpha1().StepActions(namespace).Create(ctx, dryRunObj, metav1.CreateOptions{DryRun: []string{metav1.DryRunAll}}); err != nil {
+			return handleDryRunCreateErr(err, obj.Name)
+		}
 	default:
 		return fmt.Errorf("unsupported object GVK %s", obj.GetObjectKind().GroupVersionKind())
 	}

diff --git a/pkg/reconciler/apiserver/apiserver_test.go b/pkg/reconciler/apiserver/apiserver_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"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/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake"
 	"github.com/tektoncd/pipeline/pkg/reconciler/apiserver"
@@ -34,6 +35,9 @@ func TestDryRunCreate_Valid_DifferentGVKs(t *testing.T) {
 	}, {
 		name: "v1beta1 pipeline",
 		obj:  &v1beta1.Pipeline{},
+	}, {
+		name: "v1alpha1 stepaction",
+		obj:  &v1alpha1.StepAction{},
 	}, {
 		name:    "unsupported gvk",
 		obj:     &v1beta1.ClusterTask{},
@@ -71,6 +75,10 @@ func TestDryRunCreate_Invalid_DifferentGVKs(t *testing.T) {
 		name:    "v1beta1 pipeline",
 		obj:     &v1beta1.Pipeline{},
 		wantErr: apiserver.ErrReferencedObjectValidationFailed,
+	}, {
+		name:    "v1alpha1 stepaction",
+		obj:     &v1alpha1.StepAction{},
+		wantErr: apiserver.ErrReferencedObjectValidationFailed,
 	}, {
 		name:    "unsupported gvk",
 		obj:     &v1beta1.ClusterTask{},
@@ -85,6 +93,9 @@ func TestDryRunCreate_Invalid_DifferentGVKs(t *testing.T) {
 			tektonclient.PrependReactor("create", "pipelines", func(action ktesting.Action) (bool, runtime.Object, error) {
 				return true, nil, apierrors.NewBadRequest("bad request")
 			})
+			tektonclient.PrependReactor("create", "stepactions", func(action ktesting.Action) (bool, runtime.Object, error) {
+				return true, nil, apierrors.NewBadRequest("bad request")
+			})
 			err := apiserver.DryRunValidate(context.Background(), "default", tc.obj, tektonclient)
 			if d := cmp.Diff(tc.wantErr, err, cmpopts.EquateErrors()); d != "" {
 				t.Errorf("wrong error: %s", d)

diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go
@@ -124,7 +124,21 @@ 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 {
+// also requires a kubeclient, tektonclient, requester in case it needs to find that task in
+// cluster or authorize against an external repositroy. It will figure out whether it needs to look in the cluster or in
+// a remote image to fetch the reference.
+func GetStepActionFunc(tekton clientset.Interface, k8s kubernetes.Interface, requester remoteresource.Requester, tr *v1.TaskRun, step *v1.Step) GetStepAction {
+	trName := tr.Name
+	namespace := tr.Namespace
+	if step.Ref != nil && step.Ref.Resolver != "" && requester != nil {
+		// Return an inline function that implements GetStepAction by calling Resolver.Get with the specified StepAction type and
+		// casting it to a StepAction.
+		return func(ctx context.Context, name string) (*v1alpha1.StepAction, *v1.RefSource, error) {
+			// TODO(#7259): support params replacements for resolver params
+			resolver := resolution.NewResolver(requester, tr, string(step.Ref.Resolver), trName, namespace, step.Ref.Params)
+			return resolveStepAction(ctx, resolver, name, namespace, k8s, tekton)
+		}
+	}
 	local := &LocalStepActionRefResolver{
 		Namespace:    namespace,
 		Tektonclient: tekton,
@@ -151,6 +165,21 @@ 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 string, k8s kubernetes.Interface, tekton clientset.Interface) (*v1alpha1.StepAction, *v1.RefSource, error) {
+	obj, refSource, err := resolver.Get(ctx, "StepAction", name)
+	if err != nil {
+		return nil, nil, err
+	}
+	switch obj := obj.(type) { //nolint:gocritic
+	case *v1alpha1.StepAction:
+		if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil {
+			return nil, nil, err
+		}
+		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.

diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go
@@ -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.Name)
 			if err != nil {
 				t.Fatalf("failed to call stepActionfn: %s", err.Error())
 			}
@@ -596,6 +607,109 @@ func TestGetStepActionFunc_Local(t *testing.T) {
 		})
 	}
 }
+
+func TestGetStepActionFunc_RemoteResolution_Success(t *testing.T) {
+	ctx := context.Background()
+	stepRef := &v1.Ref{ResolverRef: v1.ResolverRef{Resolver: "git"}}
+
+	testcases := []struct {
+		name           string
+		stepActionYAML string
+		wantStepAction *v1alpha1.StepAction
+		wantErr        bool
+	}{{
+		name: "remote StepAction",
+		stepActionYAML: strings.Join([]string{
+			"kind: StepAction",
+			"apiVersion: tekton.dev/v1alpha1",
+			stepActionYAMLString,
+		}, "\n"),
+		wantStepAction: parse.MustParseV1alpha1StepAction(t, stepActionYAMLString),
+	}}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			resolved := test.NewResolvedResource([]byte(tc.stepActionYAML), nil /* annotations */, sampleRefSource.DeepCopy(), nil /* data error */)
+			requester := test.NewRequester(resolved, nil)
+			tr := &v1.TaskRun{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
+				Spec: v1.TaskRunSpec{
+					TaskSpec: &v1.TaskSpec{
+						Steps: []v1.Step{{
+							Ref: stepRef,
+						}},
+					},
+					ServiceAccountName: "default",
+				},
+			}
+			tektonclient := fake.NewSimpleClientset()
+			fn := resources.GetStepActionFunc(tektonclient, nil, requester, tr, &tr.Spec.TaskSpec.Steps[0])
+
+			resolvedStepAction, resolvedRefSource, err := fn(ctx, tr.Spec.TaskSpec.Steps[0].Ref.Name)
+			if tc.wantErr {
+				if err == nil {
+					t.Fatalf("expected an error when calling GetStepActionFunc but got none")
+				}
+			} else {
+				if err != nil {
+					t.Fatalf("failed to call fn: %s", err.Error())
+				}
+
+				if d := cmp.Diff(sampleRefSource, resolvedRefSource); d != "" {
+					t.Errorf("refSources did not match: %s", diff.PrintWantGot(d))
+				}
+
+				if d := cmp.Diff(tc.wantStepAction, resolvedStepAction); d != "" {
+					t.Errorf("resolvedStepActions did not match: %s", diff.PrintWantGot(d))
+				}
+			}
+		})
+	}
+}
+
+func TestGetStepActionFunc_RemoteResolution_Error(t *testing.T) {
+	ctx := context.Background()
+	stepRef := &v1.Ref{ResolverRef: v1.ResolverRef{Resolver: "git"}}
+
+	testcases := []struct {
+		name       string
+		resolvesTo []byte
+	}{{
+		name:       "invalid data",
+		resolvesTo: []byte("INVALID YAML"),
+	}, {
+		name: "resolved not StepAction",
+		resolvesTo: []byte(strings.Join([]string{
+			"kind: Task",
+			"apiVersion: tekton.dev/v1beta1",
+			taskYAMLString,
+		}, "\n")),
+	},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			resource := test.NewResolvedResource(tc.resolvesTo, nil, nil, nil)
+			requester := test.NewRequester(resource, nil)
+			tr := &v1.TaskRun{
+				ObjectMeta: metav1.ObjectMeta{Namespace: "default"},
+				Spec: v1.TaskRunSpec{
+					TaskSpec: &v1.TaskSpec{
+						Steps: []v1.Step{{
+							Ref: stepRef,
+						}},
+					},
+					ServiceAccountName: "default",
+				},
+			}
+			tektonclient := fake.NewSimpleClientset()
+			fn := resources.GetStepActionFunc(tektonclient, nil, requester, tr, &tr.Spec.TaskSpec.Steps[0])
+			if _, _, err := fn(ctx, tr.Spec.TaskSpec.Steps[0].Ref.Name); err == nil {
+				t.Fatalf("expected error due to invalid pipeline data but saw none")
+			}
+		})
+	}
+}
+
 func TestGetTaskFuncFromTaskRunSpecAlreadyFetched(t *testing.T) {
 	ctx := context.Background()
 	ctx, cancel := context.WithCancel(ctx)
@@ -1515,6 +1629,15 @@ spec:
       echo "hello world!"
 `
 
+var stepActionYAMLString = `
+metadata:
+  name: foo
+  namespace: default
+spec:
+  image: myImage
+  command: ["ls"]
+`
+
 var remoteTaskYamlWithoutDefaults = `
 metadata:
   name: simple

diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go
@@ -25,8 +25,10 @@ import (
 	"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
@@ -99,14 +101,13 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask) (*re
 }
 
 // GetStepActionsData extracts the StepActions and merges them with the inlined Step specification.
-func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, tr *v1.TaskRun, tekton clientset.Interface) ([]v1.Step, error) {
+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()
 		if step.Ref != nil {
-			s.Ref = nil
-			getStepAction := GetStepActionFunc(tekton, tr.Namespace)
-			stepAction, _, err := getStepAction(ctx, step.Ref.Name)
+			getStepAction := GetStepActionFunc(tekton, k8s, requester, taskRun, s)
+			stepAction, _, err := getStepAction(ctx, s.Ref.Name)
 			if err != nil {
 				return nil, err
 			}
@@ -124,6 +125,7 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, tr *v1.TaskRu
 			if stepActionSpec.Env != nil {
 				s.Env = stepActionSpec.Env
 			}
+			s.Ref = nil
 			steps = append(steps, *s)
 		} else {
 			steps = append(steps, step)