From ae6e32f8db634fb4897eebfcbacd6059bfbe3be7 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Tue, 24 Oct 2023 11:51:43 -0400 Subject: [PATCH] Referencing StepActions in Steps This PR allows the Step to reference a StepAction CRD deployed on the cluster. This capability is gated behind a feature flag `enable-step-actions`. Remote resolution of StepActions will be implemented in a follow-up PR. This is the second item on the implementation Issue https://github.com/tektoncd/pipeline/issues/7259 --- docs/stepactions.md | 34 +++ examples/v1/taskruns/alpha/stepaction.yaml | 19 ++ pkg/reconciler/taskrun/resources/taskref.go | 29 ++ .../taskrun/resources/taskref_test.go | 152 +++++++++++ pkg/reconciler/taskrun/resources/taskspec.go | 51 +++- .../taskrun/resources/taskspec_test.go | 249 +++++++++++++++++- pkg/reconciler/taskrun/taskrun.go | 3 +- pkg/reconciler/taskrun/taskrun_test.go | 116 ++++++++ test/controller.go | 11 + test/parse/yaml.go | 11 + 10 files changed, 665 insertions(+), 10 deletions(-) create mode 100644 examples/v1/taskruns/alpha/stepaction.yaml diff --git a/docs/stepactions.md b/docs/stepactions.md index b015bf17fd8..e46010b71d4 100644 --- a/docs/stepactions.md +++ b/docs/stepactions.md @@ -72,6 +72,40 @@ spec: name: step-action ``` +Upon resolution and execution of the `TaskRun`, the `Status` will look something like: + +```yaml +status: + completionTime: "2023-10-24T20:28:42Z" + conditions: + - lastTransitionTime: "2023-10-24T20:28:42Z" + message: All Steps have completed executing + reason: Succeeded + status: "True" + type: Succeeded + podName: step-action-run-pod + provenance: + featureFlags: + EnableStepActions: true + ... + startTime: "2023-10-24T20:28:32Z" + steps: + - container: step-action-runner + imageID: docker.io/library/alpine@sha256:eece025e432126ce23f223450a0326fbebde39cdf496a85d8c016293fc851978 + name: action-runner + terminated: + containerID: containerd://46a836588967202c05b594696077b147a0eb0621976534765478925bb7ce57f6 + exitCode: 0 + finishedAt: "2023-10-24T20:28:42Z" + reason: Completed + startedAt: "2023-10-24T20:28:42Z" + taskSpec: + steps: + - computeResources: {} + image: alpine + name: action-runner +``` + If a `Step` is referencing a `StepAction`, it cannot contain the fields supported by `StepActions`. This includes: - `image` - `command` diff --git a/examples/v1/taskruns/alpha/stepaction.yaml b/examples/v1/taskruns/alpha/stepaction.yaml new file mode 100644 index 00000000000..3acc1e263f4 --- /dev/null +++ b/examples/v1/taskruns/alpha/stepaction.yaml @@ -0,0 +1,19 @@ +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: step-action +spec: + image: alpine + script: | + echo "I am a Step Action!!!" +--- +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: step-action-run +spec: + TaskSpec: + steps: + - name: action-runner + ref: + name: step-action diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index a2b547cdbcc..bef295bfa82 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -123,6 +123,15 @@ 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 { + local := &LocalRefResolver{ + Namespace: namespace, + Tektonclient: tekton, + } + return local.GetStepAction +} + // resolveTask accepts an impl of remote.Resolver and attempts to // fetch a task with given name and verify the v1beta1 task if trusted resources is enabled. // An error is returned if the remoteresource doesn't work @@ -222,6 +231,26 @@ func (l *LocalTaskRefResolver) GetTask(ctx context.Context, name string) (*v1.Ta return task, nil, nil, nil } +// LocalRefResolver uses the current cluster to resolve a stepaction reference. +type LocalRefResolver struct { + Namespace string + Tektonclient clientset.Interface +} + +// 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 *LocalRefResolver) GetStepAction(ctx context.Context, name string) (*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) + } + stepAction, err := l.Tektonclient.TektonV1alpha1().StepActions(l.Namespace).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return nil, nil, err + } + return stepAction, nil, nil +} + // IsGetTaskErrTransient returns true if an error returned by GetTask is retryable. func IsGetTaskErrTransient(err error) bool { return strings.Contains(err.Error(), errEtcdLeaderChange) diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index ceb472255c7..dc007a1151f 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -51,6 +51,19 @@ import ( ) var ( + simpleNamespacedStepAction = &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "tekton.dev/v1alpha1", + Kind: "StepAction", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "something", + }, + } simpleNamespacedTask = &v1.Task{ ObjectMeta: metav1.ObjectMeta{ Name: "simple", @@ -296,6 +309,104 @@ func TestLocalTaskRef(t *testing.T) { } } +func TestLocalRef(t *testing.T) { + testcases := []struct { + name string + namespace string + stepactions []runtime.Object + ref *v1.Ref + expected runtime.Object + wantErr error + }{ + { + name: "local-step-action", + namespace: "default", + stepactions: []runtime.Object{ + &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple", + Namespace: "default", + }, + }, + &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + Namespace: "default", + }, + }, + }, + ref: &v1.Ref{ + Name: "simple", + }, + expected: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple", + Namespace: "default", + }, + }, + wantErr: nil, + }, { + name: "step-action-not-found", + namespace: "default", + stepactions: []runtime.Object{}, + ref: &v1.Ref{ + Name: "simple", + }, + expected: nil, + wantErr: errors.New(`stepactions.tekton.dev "simple" not found`), + }, { + name: "local-step-action-missing-namespace", + namespace: "", + stepactions: []runtime.Object{ + &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "simple", + Namespace: "default", + }, + }, + }, + ref: &v1.Ref{ + Name: "simple", + }, + wantErr: fmt.Errorf("must specify namespace to resolve reference to step action simple"), + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + tektonclient := fake.NewSimpleClientset(tc.stepactions...) + + lc := &resources.LocalRefResolver{ + Namespace: tc.namespace, + Tektonclient: tektonclient, + } + + task, refSource, err := lc.GetStepAction(ctx, tc.ref.Name) + if tc.wantErr != nil { + if err == nil { + t.Fatal("Expected error but found nil instead") + } + if tc.wantErr.Error() != err.Error() { + t.Fatalf("Received different error ( %#v )", err) + } + } else if tc.wantErr == nil && err != nil { + t.Fatalf("Received unexpected error ( %#v )", err) + } + + if d := cmp.Diff(tc.expected, task); tc.expected != nil && d != "" { + t.Error(diff.PrintWantGot(d)) + } + + // local cluster step actions have empty source for now. This may be changed in future. + if refSource != nil { + t.Errorf("expected refsource is nil, but got %v", refSource) + } + }) + } +} func TestGetTaskFunc_Local(t *testing.T) { ctx := context.Background() @@ -405,6 +516,47 @@ func TestGetTaskFunc_Local(t *testing.T) { } } +func TestGetStepActionFunc_Local(t *testing.T) { + ctx := context.Background() + + testcases := []struct { + name string + localStepActions []runtime.Object + ref *v1.Ref + expected runtime.Object + }{ + { + name: "local-step-action", + localStepActions: []runtime.Object{simpleNamespacedStepAction}, + ref: &v1.Ref{ + Name: "simple", + }, + expected: simpleNamespacedStepAction, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + tektonclient := fake.NewSimpleClientset(tc.localStepActions...) + + fn := resources.GetStepActionFunc(tektonclient, "default") + + stepAction, refSource, err := fn(ctx, tc.ref.Name) + if err != nil { + t.Fatalf("failed to call stepActionfn: %s", err.Error()) + } + + if diff := cmp.Diff(stepAction, tc.expected); tc.expected != nil && diff != "" { + t.Error(diff) + } + + // local cluster task has empty RefSource for now. This may be changed in future. + if refSource != nil { + t.Errorf("expected refSource is nil, but got %v", refSource) + } + }) + } +} func TestGetTaskFuncFromTaskRunSpecAlreadyFetched(t *testing.T) { ctx := context.Background() ctx, cancel := context.WithCancel(ctx) diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index 3bf08f923dc..3698ad9473c 100644 --- a/pkg/reconciler/taskrun/resources/taskspec.go +++ b/pkg/reconciler/taskrun/resources/taskspec.go @@ -21,10 +21,13 @@ import ( "errors" "fmt" + "github.com/tektoncd/pipeline/pkg/apis/config" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" resolutionutil "github.com/tektoncd/pipeline/pkg/internal/resolution" "github.com/tektoncd/pipeline/pkg/trustedresources" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" ) // ResolvedTask contains the data that is needed to execute @@ -37,6 +40,9 @@ type ResolvedTask struct { VerificationResult *trustedresources.VerificationResult } +// GetStepAction is a function used to retrieve StepActions. +type GetStepAction func(context.Context, string) (*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. type GetTask func(context.Context, string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) @@ -47,7 +53,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) (*resolutionutil.ResolvedObjectMeta, *v1.TaskSpec, error) { +func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask, getStepAction GetStepAction) (*resolutionutil.ResolvedObjectMeta, *v1.TaskSpec, error) { taskMeta := metav1.ObjectMeta{} taskSpec := v1.TaskSpec{} var refSource *v1.RefSource @@ -85,6 +91,13 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask) (*re return nil, nil, fmt.Errorf("taskRun %s not providing TaskRef or TaskSpec", taskRun.Name) } + steps, err := extractStepActions(ctx, taskSpec, taskRun, getStepAction) + if err != nil { + return nil, nil, err + } else { + taskSpec.Steps = steps + } + taskSpec.SetDefaults(ctx) return &resolutionutil.ResolvedObjectMeta{ ObjectMeta: &taskMeta, @@ -92,3 +105,39 @@ func GetTaskData(ctx context.Context, taskRun *v1.TaskRun, getTask GetTask) (*re VerificationResult: verificationResult, }, &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, getStepAction GetStepAction) ([]v1.Step, error) { + steps := []v1.Step{} + for _, step := range taskSpec.Steps { + s := step.DeepCopy() + if step.Ref != nil { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions { + return nil, apis.ErrGeneric("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions) + } + s.Ref = nil + stepAction, _, err := getStepAction(ctx, step.Ref.Name) + if err != nil { + return nil, err + } + stepActionSpec := stepAction.StepActionSpec() + s.Image = stepActionSpec.Image + if stepActionSpec.Command != nil { + s.Command = stepActionSpec.Command + } + if stepActionSpec.Args != nil { + s.Args = stepActionSpec.Args + } + if stepActionSpec.Script != "" { + s.Script = stepActionSpec.Script + } + if stepActionSpec.Env != nil { + s.Env = stepActionSpec.Env + } + steps = append(steps, *s) + } else { + steps = append(steps, step) + } + } + return steps, nil +} diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index f3456002e53..81b3028038b 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -19,17 +19,37 @@ package resources_test import ( "context" "errors" + "fmt" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/config" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/trustedresources" "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" ) +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{ @@ -55,7 +75,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) + resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt, getStepAction) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) @@ -89,7 +109,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) + resolvedObjectMeta, taskSpec, err := resources.GetTaskData(context.Background(), tr, gt, getStepAction) if err != nil { t.Fatalf("Did not expect error getting task spec but got: %s", err) @@ -118,7 +138,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) + _, _, err := resources.GetTaskData(context.Background(), tr, gt, getStepAction) if err == nil { t.Fatalf("Expected error resolving spec with no embedded or referenced task spec but didn't get error") } @@ -138,7 +158,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) + _, _, err := resources.GetTaskData(context.Background(), tr, gt, getStepAction) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -182,7 +202,7 @@ func TestGetTaskData_ResolutionSuccess(t *testing.T) { }, sampleRefSource.DeepCopy(), nil, nil } ctx := context.Background() - resolvedMeta, resolvedSpec, err := resources.GetTaskData(ctx, tr, getTask) + resolvedMeta, resolvedSpec, err := resources.GetTaskData(ctx, tr, getTask, getStepAction) if err != nil { t.Fatalf("Unexpected error getting mocked data: %v", err) } @@ -216,7 +236,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) + _, _, err := resources.GetTaskData(ctx, tr, getTask, getStepAction) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -239,7 +259,7 @@ func TestGetTaskData_ResolvedNilTask(t *testing.T) { return nil, nil, nil, nil } ctx := context.Background() - _, _, err := resources.GetTaskData(ctx, tr, getTask) + _, _, err := resources.GetTaskData(ctx, tr, getTask, getStepAction) if err == nil { t.Fatalf("Expected error when unable to find referenced Task but got none") } @@ -286,7 +306,7 @@ func TestGetTaskData_VerificationResult(t *testing.T) { Spec: *sourceSpec.DeepCopy(), }, nil, verificationResult, nil } - r, _, err := resources.GetTaskData(context.Background(), tr, getTask) + r, _, err := resources.GetTaskData(context.Background(), tr, getTask, getStepAction) if err != nil { t.Fatalf("Did not expect error but got: %s", err) } @@ -294,3 +314,216 @@ func TestGetTaskData_VerificationResult(t *testing.T) { t.Errorf(diff.PrintWantGot(d)) } } + +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: "step-action-with-command", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + WorkingDir: "/foo", + }, { + Ref: &v1.Ref{ + Name: "stepAction", + }, + WorkingDir: "/bar", + Timeout: &metav1.Duration{Duration: time.Hour}, + }}, + }, + }, + }, + want: &v1.TaskSpec{ + Steps: []v1.Step{{ + Image: "myimage", + Command: []string{"ls"}, + Args: []string{"-lh"}, + WorkingDir: "/foo", + }, { + Image: "myimage", + Command: []string{"ls"}, + Args: []string{"-lh"}, + WorkingDir: "/bar", + Timeout: &metav1.Duration{Duration: time.Hour}, + }}, + }, + stepActionFunc: getStepAction, + }, { + name: "step-action-with-script", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepActionWithScript", + }, + }}, + }, + }, + }, + want: &v1.TaskSpec{ + Steps: []v1.Step{{ + Image: "myimage", + Script: "ls", + }}, + }, + stepActionFunc: func(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) { + stepAction := &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepActionWithSctipy", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Script: "ls", + }, + } + return stepAction, nil, nil + }, + }, { + name: "step-action-with-env", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepActionWithEnv", + }, + }}, + }, + }, + }, + want: &v1.TaskSpec{ + Steps: []v1.Step{{ + Image: "myimage", + Env: []corev1.EnvVar{{ + Name: "env1", + Value: "value1", + }}, + }}, + }, + 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 + }, + }, { + name: "step-action-error", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepActionError", + }, + }}, + }, + }, + }, + stepActionFunc: func(ctx context.Context, n string) (*v1alpha1.StepAction, *v1.RefSource, error) { + return nil, nil, fmt.Errorf("Error fetching Step Action.") + }, + }} + for _, tt := range tests { + gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { + return nil, nil, nil, nil + } + + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + _, got, err := resources.GetTaskData(ctx, tt.tr, gt, tt.stepActionFunc) + if got == nil { + want := fmt.Errorf("Error fetching Step Action.") + if d := cmp.Diff(want.Error(), err.Error()); d != "" { + t.Errorf("the expected error did not match what was received: %s", diff.PrintWantGot(d)) + } + } else { + if err != nil { + t.Errorf("Did not expect an error but got : %s", err) + } + if d := cmp.Diff(tt.want, got); d != "" { + t.Errorf("the taskSpec did not match what was expected diff: %s", diff.PrintWantGot(d)) + } + } + } +} + +func TestGetStepAction_Error(t *testing.T) { + tests := []struct { + name string + tr *v1.TaskRun + enableStepActions bool + expectedError apis.FieldError + }{{ + name: "no feature flag defined", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + }, + }, + }, + enableStepActions: false, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true to reference StepActions in Steps.", + Paths: []string{"enable-step-actions"}, + }, + }} + for _, tt := range tests { + gt := func(ctx context.Context, n string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { + return nil, nil, nil, nil + } + + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tt.enableStepActions, + }, + }) + _, _, err := resources.GetTaskData(ctx, tt.tr, gt, getStepAction) + if err == nil { + t.Fatalf("Expected to get an error but did not find any.") + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + } +} diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index a72f278eab4..fd0b2553468 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -332,8 +332,9 @@ 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) + taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc, getStepActionfunc) switch { case errors.Is(err, remote.ErrRequestInProgress): message := fmt.Sprintf("TaskRun %s/%s awaiting remote resource", tr.Namespace, tr.Name) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 577034e7e3e..3e5e8fa4bac 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -77,6 +77,7 @@ import ( "k8s.io/client-go/tools/record" clock "k8s.io/utils/clock/testing" "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" cminformer "knative.dev/pkg/configmap/informer" "knative.dev/pkg/controller" "knative.dev/pkg/kmeta" @@ -2734,6 +2735,121 @@ spec: } } +func TestStepActionRef(t *testing.T) { + taskRun := parse.MustParseV1TaskRun(t, ` +metadata: + name: test-taskrun-referencing-step-action + namespace: foo +spec: + taskSpec: + steps: + - ref: + name: stepAction + name: step1 + workingDir: /foo + - ref: + name: stepAction2 + name: step2 + - name: inlined-step + image: "inlined-image" +`) + stepAction := parse.MustParseV1alpha1StepAction(t, ` +metadata: + name: stepAction + namespace: foo +spec: + image: myImage + command: ["ls"] +`) + stepAction2 := parse.MustParseV1alpha1StepAction(t, ` +metadata: + name: stepAction2 + namespace: foo +spec: + image: myImage + script: "echo hi" +`) + d := test.Data{ + TaskRuns: []*v1.TaskRun{taskRun}, + StepActions: []*v1alpha1.StepAction{stepAction, stepAction2}, + ConfigMaps: []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-step-actions": "true", + }, + }, + }, + } + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + createServiceAccount(t, testAssets, "default", taskRun.Namespace) + c := testAssets.Controller + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err == nil { + t.Fatalf("Could not reconcile the taskrun: %v", err) + } + getTaskRun, _ := testAssets.Clients.Pipeline.TektonV1().TaskRuns(taskRun.Namespace).Get(testAssets.Ctx, taskRun.Name, metav1.GetOptions{}) + got := getTaskRun.Status.TaskSpec.Steps + want := []v1.Step{{ + Image: "myImage", + Command: []string{"ls"}, + Name: "step1", + WorkingDir: "/foo", + }, { + Image: "myImage", + Script: "echo hi", + Name: "step2", + }, { + Image: "inlined-image", + Name: "inlined-step", + }} + if c := cmp.Diff(want, got); c != "" { + t.Errorf("TestStepActionRef errored with: %s", diff.PrintWantGot(c)) + } +} + +func TestStepActionRef_Error(t *testing.T) { + taskRun := parse.MustParseV1TaskRun(t, ` +metadata: + name: test-taskrun-referencing-step-action + namespace: foo +spec: + taskSpec: + steps: + - name: missing-step-action + ref: + name: noStepAction +`) + d := test.Data{ + TaskRuns: []*v1.TaskRun{taskRun}, + ConfigMaps: []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-step-actions": "true", + }, + }, + }, + } + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + createServiceAccount(t, testAssets, "default", taskRun.Namespace) + c := testAssets.Controller + c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)) + getTaskRun, _ := testAssets.Clients.Pipeline.TektonV1().TaskRuns(taskRun.Namespace).Get(testAssets.Ctx, taskRun.Name, metav1.GetOptions{}) + got := getTaskRun.Status.Conditions + want := duckv1.Conditions{{ + Type: "Succeeded", + Status: "False", + Reason: "TaskRunResolutionFailed", + Message: `stepactions.tekton.dev "noStepAction" not found`, + }} + ignore := cmpopts.IgnoreFields(apis.Condition{}, "LastTransitionTime") + if c := cmp.Diff(want, got, ignore); c != "" { + t.Errorf("TestStepActionRef_Error Conditions did not match: %s", diff.PrintWantGot(c)) + } +} + func TestExpandMountPath(t *testing.T) { expectedMountPath := "/temppath/replaced" expectedReplacedArgs := fmt.Sprintf("replacedArgs - %s", expectedMountPath) diff --git a/test/controller.go b/test/controller.go index 76fef3fa933..72af3b8f312 100644 --- a/test/controller.go +++ b/test/controller.go @@ -37,6 +37,7 @@ import ( fakepipelineruninformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1/pipelinerun/fake" faketaskinformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1/task/fake" faketaskruninformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1/taskrun/fake" + fakestepactioninformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1alpha1/stepaction/fake" fakeverificationpolicyinformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1alpha1/verificationpolicy/fake" fakeclustertaskinformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1beta1/clustertask/fake" fakecustomruninformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1beta1/customrun/fake" @@ -72,6 +73,7 @@ type Data struct { Pipelines []*v1.Pipeline TaskRuns []*v1.TaskRun Tasks []*v1.Task + StepActions []*v1alpha1.StepAction ClusterTasks []*v1beta1.ClusterTask CustomRuns []*v1beta1.CustomRun Pods []*corev1.Pod @@ -100,6 +102,7 @@ type Informers struct { Run informersv1alpha1.RunInformer CustomRun informersv1beta1.CustomRunInformer Task informersv1.TaskInformer + StepAction informersv1alpha1.StepActionInformer ClusterTask informersv1beta1.ClusterTaskInformer Pod coreinformers.PodInformer ConfigMap coreinformers.ConfigMapInformer @@ -185,6 +188,7 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers TaskRun: faketaskruninformer.Get(ctx), CustomRun: fakecustomruninformer.Get(ctx), Task: faketaskinformer.Get(ctx), + StepAction: fakestepactioninformer.Get(ctx), ClusterTask: fakeclustertaskinformer.Get(ctx), Pod: fakefilteredpodinformer.Get(ctx, v1.ManagedByLabelKey), ConfigMap: fakeconfigmapinformer.Get(ctx), @@ -225,6 +229,13 @@ func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers t.Fatal(err) } } + c.Pipeline.PrependReactor("*", "stepactions", AddToInformer(t, i.StepAction.Informer().GetIndexer())) + for _, sa := range d.StepActions { + sa := sa.DeepCopy() // Avoid assumptions that the informer's copy is modified. + if _, err := c.Pipeline.TektonV1alpha1().StepActions(sa.Namespace).Create(ctx, sa, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + } c.Pipeline.PrependReactor("*", "clustertasks", AddToInformer(t, i.ClusterTask.Informer().GetIndexer())) for _, ct := range d.ClusterTasks { ct := ct.DeepCopy() // Avoid assumptions that the informer's copy is modified. diff --git a/test/parse/yaml.go b/test/parse/yaml.go index 3ee9f8349a6..8b0a30e11c7 100644 --- a/test/parse/yaml.go +++ b/test/parse/yaml.go @@ -23,6 +23,17 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) +// MustParseV1alpha1StepAction takes YAML and parses it into a *v1alpha1.StepAction +func MustParseV1alpha1StepAction(t *testing.T, yaml string) *v1alpha1.StepAction { + t.Helper() + var sa v1alpha1.StepAction + yaml = `apiVersion: tekton.dev/v1alpha1 +kind: StepAction +` + yaml + mustParseYAML(t, yaml, &sa) + return &sa +} + // MustParseV1beta1TaskRun takes YAML and parses it into a *v1beta1.TaskRun func MustParseV1beta1TaskRun(t *testing.T, yaml string) *v1beta1.TaskRun { t.Helper()