From dd66156b3f0250945f2fc5c5bfac1f9773bd5e92 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 --- config/config-feature-flags.yaml | 3 + docs/pipeline-api.md | 86 ++++++ docs/stepactions.md | 84 ++++++ examples/v1/taskruns/alpha/stepaction.yaml | 19 ++ pkg/apis/config/feature_flags.go | 8 + pkg/apis/config/feature_flags_test.go | 4 + .../testdata/feature-flags-all-flags-set.yaml | 1 + ...ure-flags-invalid-enable-step-actions.yaml | 21 ++ pkg/apis/pipeline/v1/container_types.go | 9 + pkg/apis/pipeline/v1/openapi_generated.go | 29 +- pkg/apis/pipeline/v1/swagger.json | 14 + pkg/apis/pipeline/v1/task_validation.go | 48 +++- pkg/apis/pipeline/v1/task_validation_test.go | 199 ++++++++++++++ pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 21 ++ .../pipeline/v1beta1/container_conversion.go | 17 ++ pkg/apis/pipeline/v1beta1/container_types.go | 10 + .../pipeline/v1beta1/openapi_generated.go | 29 +- pkg/apis/pipeline/v1beta1/swagger.json | 14 + .../pipeline/v1beta1/task_conversion_test.go | 16 ++ pkg/apis/pipeline/v1beta1/task_validation.go | 49 +++- .../pipeline/v1beta1/task_validation_test.go | 200 ++++++++++++++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 21 ++ 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/custom_task_test.go | 1 + test/e2e-tests-kind-prow-alpha.env | 1 + test/e2e-tests.sh | 14 + test/parse/yaml.go | 11 + 33 files changed, 1515 insertions(+), 25 deletions(-) create mode 100644 examples/v1/taskruns/alpha/stepaction.yaml create mode 100644 pkg/apis/config/testdata/feature-flags-invalid-enable-step-actions.yaml diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 50eaa68519b..a49d26ccb78 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -124,3 +124,6 @@ data: # Setting this flag to "true" will enable the CEL evaluation in WhenExpression # This feature is in preview mode and not implemented yet. Please check #7244 for the updates. enable-cel-in-whenexpression: "false" + # Setting this flag to "true" will enable the use of StepActions in Steps + # This feature is in preview mode and not implemented yet. Please check #7259 for updates. + enable-step-actions: "false" diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 39d002be95c..852625ad198 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -3270,6 +3270,35 @@ github.com/tektoncd/pipeline/pkg/apis/config.FeatureFlags +

Ref +

+

+(Appears on:Step) +

+
+

Ref can be used to refer to a specific instance of a StepAction.

+
+ + + + + + + + + + + + + +
FieldDescription
+name
+ +string + +		 +

Name of the referenced step

+

RefSource

@@ -4353,6 +4382,20 @@ StepOutputConfig

Stores configuration for the stderr stream of the step.

+ + +ref
+ + +Ref + + + + +(Optional) +

Contains the reference to an existing stepaction

+ +

StepOutputConfig @@ -11758,6 +11801,35 @@ github.com/tektoncd/pipeline/pkg/apis/config.FeatureFlags +

Ref +

+

+(Appears on:Step) +

+
+

Ref can be used to refer to a specific instance of a StepAction.

+
+ + + + + + + + + + + + + +
FieldDescription
+name
+ +string + +		 +

Name of the referenced step

+

RefSource

@@ -12954,6 +13026,20 @@ StepOutputConfig

Stores configuration for the stderr stream of the step.

+ + +ref
+ + +Ref + + + + +(Optional) +

Contains the reference to an existing stepaction

+ +

StepOutputConfig diff --git a/docs/stepactions.md b/docs/stepactions.md index 292304efe24..255dc2a8bdb 100644 --- a/docs/stepactions.md +++ b/docs/stepactions.md @@ -54,3 +54,87 @@ spec: command: ["ls"] args:: ["-lh"] ``` + +## Referencing a StepAction + +`StepActions` can be referenced from the `Step` using the `ref` field, as follows: + +```yaml +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: step-action-run +spec: + TaskSpec: + steps: + - name: action-runner + ref: + 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 + script: | + echo "I am a Step Action!!!" +``` + +If a `Step` is referencing a `StepAction`, it cannot contain the fields supported by `StepActions`. This includes: +- `image` +- `command` +- `args` +- `script` +- `env` + +Using any of the above fields and referencing a `StepAction` in the same `Step` is not allowed and will cause an validation error. + +```yaml +# This is not allowed and will result in a validation error. +# Because the image is expected to be provided by the StepAction +# and not inlined. +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: step-action-run +spec: + TaskSpec: + steps: + - name: action-runner + ref: + name: step-action + image: ubuntu +``` +Executing the above `TaskRun` will result in an error that looks like: + +``` +Error from server (BadRequest): error when creating "STDIN": admission webhook "validation.webhook.pipeline.tekton.dev" denied the request: validation failed: image cannot be used with Ref: spec.taskSpec.steps[0].image +``` 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/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 6b4940c0880..656de7c8a93 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -96,6 +96,10 @@ const ( EnableCELInWhenExpression = "enable-cel-in-whenexpression" // DefaultEnableCELInWhenExpression is the default value for EnableCELInWhenExpression DefaultEnableCELInWhenExpression = false + // EnableStepActions is the flag to enable the use of StepActions in Steps + EnableStepActions = "enable-step-actions" + // DefaultEnableStepActions is the default value for EnableStepActions + DefaultEnableStepActions = false disableAffinityAssistantKey = "disable-affinity-assistant" disableCredsInitKey = "disable-creds-init" @@ -145,6 +149,7 @@ type FeatureFlags struct { SetSecurityContext bool Coschedule string EnableCELInWhenExpression bool + EnableStepActions bool } // GetFeatureFlagsConfigName returns the name of the configmap containing all @@ -220,6 +225,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setFeature(EnableCELInWhenExpression, DefaultEnableCELInWhenExpression, &tc.EnableCELInWhenExpression); err != nil { return nil, err } + if err := setFeature(EnableStepActions, DefaultEnableStepActions, &tc.EnableStepActions); err != nil { + return nil, err + } // Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if // enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of // each feature's individual flag. diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 65756040698..afa954c8d4d 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -74,6 +74,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { SetSecurityContext: true, Coschedule: config.CoscheduleDisabled, EnableCELInWhenExpression: true, + EnableStepActions: true, }, fileName: "feature-flags-all-flags-set", }, @@ -273,6 +274,9 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) { }, { fileName: "feature-flags-invalid-enable-cel-in-whenexpression", want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax`, + }, { + fileName: "feature-flags-invalid-enable-step-actions", + want: `failed parsing feature flags config "invalid": strconv.ParseBool: parsing "invalid": invalid syntax`, }} { t.Run(tc.fileName, func(t *testing.T) { cm := test.ConfigMapFromTestFile(t, tc.fileName) diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml index 63015ec07c1..bf087e8a427 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -33,3 +33,4 @@ data: set-security-context: "true" keep-pod-on-cancel: "true" enable-cel-in-whenexpression: "true" + enable-step-actions: "true" diff --git a/pkg/apis/config/testdata/feature-flags-invalid-enable-step-actions.yaml b/pkg/apis/config/testdata/feature-flags-invalid-enable-step-actions.yaml new file mode 100644 index 00000000000..462a31704aa --- /dev/null +++ b/pkg/apis/config/testdata/feature-flags-invalid-enable-step-actions.yaml @@ -0,0 +1,21 @@ +# Copyright 2023 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: feature-flags + namespace: tekton-pipelines +data: + enable-step-actions: "invalid" diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go index ccef95cf767..ff3148cdbdd 100644 --- a/pkg/apis/pipeline/v1/container_types.go +++ b/pkg/apis/pipeline/v1/container_types.go @@ -135,6 +135,15 @@ type Step struct { // Stores configuration for the stderr stream of the step. // +optional StderrConfig *StepOutputConfig `json:"stderrConfig,omitempty"` + // Contains the reference to an existing stepaction + //+optional + Ref *Ref `json:"ref,omitempty"` +} + +// Ref can be used to refer to a specific instance of a StepAction. +type Ref struct { + // Name of the referenced step + Name string `json:"name,omitempty"` } // OnErrorType defines a list of supported exiting behavior of a container on error diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 6425ec1de3b..bc7334a9e01 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -61,6 +61,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PipelineWorkspaceDeclaration": schema_pkg_apis_pipeline_v1_PipelineWorkspaceDeclaration(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec": schema_pkg_apis_pipeline_v1_PropertySpec(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Provenance": schema_pkg_apis_pipeline_v1_Provenance(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Ref": schema_pkg_apis_pipeline_v1_Ref(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.RefSource": schema_pkg_apis_pipeline_v1_RefSource(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ResolverRef": schema_pkg_apis_pipeline_v1_ResolverRef(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ResultRef": schema_pkg_apis_pipeline_v1_ResultRef(ref), @@ -2184,6 +2185,26 @@ func schema_pkg_apis_pipeline_v1_Provenance(ref common.ReferenceCallback) common } } +func schema_pkg_apis_pipeline_v1_Ref(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "Ref can be used to refer to a specific instance of a StepAction.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Description: "Name of the referenced step", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1_RefSource(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -2924,12 +2945,18 @@ func schema_pkg_apis_pipeline_v1_Step(ref common.ReferenceCallback) common.OpenA Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepOutputConfig"), }, }, + "ref": { + SchemaProps: spec.SchemaProps{ + Description: "Contains the reference to an existing stepaction", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Ref"), + }, + }, }, Required: []string{"name"}, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.WorkspaceUsage", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Ref", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.WorkspaceUsage", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, } } diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 76d325bfff0..e41ad106bf1 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1093,6 +1093,16 @@ } } }, + "v1.Ref": { + "description": "Ref can be used to refer to a specific instance of a StepAction.", + "type": "object", + "properties": { + "name": { + "description": "Name of the referenced step", + "type": "string" + } + } + }, "v1.RefSource": { "description": "RefSource contains the information that can uniquely identify where a remote built definition came from i.e. Git repositories, Tekton Bundles in OCI registry and hub.", "type": "object", @@ -1445,6 +1455,10 @@ "description": "OnError defines the exiting behavior of a container on error can be set to [ continue | stopAndFail ]", "type": "string" }, + "ref": { + "description": "Contains the reference to an existing stepaction", + "$ref": "#/definitions/v1.Ref" + }, "script": { "description": "Script is the contents of an executable file to execute.\n\nIf Script is not empty, the Step cannot have an Command and the Args will be passed to the Script.", "type": "string" diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index a7fa0d442a7..8f7dbc9213e 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -268,17 +268,53 @@ func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) { } func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) { - if s.Image == "" { - errs = errs.Also(apis.ErrMissingField("Image")) - } - - if s.Script != "" { + if s.Ref != nil { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions { + return apis.ErrGeneric("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions) + } + if s.Image != "" { + errs = errs.Also(&apis.FieldError{ + Message: "image cannot be used with Ref", + Paths: []string{"image"}, + }) + } if len(s.Command) > 0 { errs = errs.Also(&apis.FieldError{ - Message: "script cannot be used with command", + Message: "command cannot be used with Ref", + Paths: []string{"command"}, + }) + } + if len(s.Args) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "args cannot be used with Ref", + Paths: []string{"args"}, + }) + } + if s.Script != "" { + errs = errs.Also(&apis.FieldError{ + Message: "script cannot be used with Ref", Paths: []string{"script"}, }) } + if s.Env != nil { + errs = errs.Also(&apis.FieldError{ + Message: "env cannot be used with Ref", + Paths: []string{"env"}, + }) + } + } else { + if s.Image == "" { + errs = errs.Also(apis.ErrMissingField("Image")) + } + + if s.Script != "" { + if len(s.Command) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "script cannot be used with command", + Paths: []string{"script"}, + }) + } + } } if s.Name != "" { diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index b04d43f190e..99b97f0ffc9 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -21,6 +21,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/config" cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -520,6 +521,38 @@ func TestTaskSpecValidate(t *testing.T) { } } +func TestTaskSpecStepActionReferenceValidate(t *testing.T) { + tests := []struct { + name string + Steps []v1.Step + }{{ + name: "valid stepaction ref", + Steps: []v1.Step{{ + Name: "mystep", + WorkingDir: "/foo", + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + ts.SetDefaults(ctx) + if err := ts.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + func TestTaskValidateError(t *testing.T) { type fields struct { Params []v1.ParamSpec @@ -727,6 +760,62 @@ func TestTaskValidateError(t *testing.T) { } } +func TestTaskValidateErrorWithStepActionRef(t *testing.T) { + tests := []struct { + name string + Steps []v1.Step + enableStepActions bool + expectedError apis.FieldError + }{{ + name: "invalid step - invalid step action", + Steps: []v1.Step{{ + Image: "image", + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "image cannot be used with Ref", + Paths: []string{"spec.steps[0].image"}, + }, + }, { + name: "invalid step - enableStepActions not on", + Steps: []v1.Step{{ + Image: "image", + 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{"spec.steps[0].enable-step-actions"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + task := &v1.Task{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: v1.TaskSpec{ + Steps: tt.Steps, + }} + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tt.enableStepActions, + }, + }) + task.SetDefaults(ctx) + err := task.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", task) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} func TestTaskSpecValidateError(t *testing.T) { type fields struct { Params []v1.ParamSpec @@ -1370,6 +1459,116 @@ func TestTaskSpecValidateError(t *testing.T) { } } +func TestTaskiSpecValidateErrorWithStepActionRef(t *testing.T) { + tests := []struct { + name string + Steps []v1.Step + enableStepActions bool + expectedError apis.FieldError + }{{ + name: "invalid Task Spec - enableStepActions not on", + Steps: []v1.Step{{ + Image: "image", + 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{"steps[0].enable-step-actions"}, + }, + }, { + name: "Cannot use image with Ref", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Image: "foo", + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "image cannot be used with Ref", + Paths: []string{"steps[0].image"}, + }, + }, { + name: "Cannot use command with Ref", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Command: []string{"foo"}, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "command cannot be used with Ref", + Paths: []string{"steps[0].command"}, + }, + }, { + name: "Cannot use args with Ref", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Args: []string{"foo"}, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "args cannot be used with Ref", + Paths: []string{"steps[0].args"}, + }, + }, { + name: "Cannot use script with Ref", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Script: "echo hi", + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "script cannot be used with Ref", + Paths: []string{"steps[0].script"}, + }, + }, { + name: "Cannot use env with Ref", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Env: []corev1.EnvVar{{ + Name: "env1", + Value: "value1", + }}, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "env cannot be used with Ref", + Paths: []string{"steps[0].env"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := v1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tt.enableStepActions, + }, + }) + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", ts) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestTaskSpecValidateErrorSidecarName(t *testing.T) { tests := []struct { name string diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index d47e242753c..c59e2c4f615 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1008,6 +1008,22 @@ func (in *Provenance) DeepCopy() *Provenance { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Ref) DeepCopyInto(out *Ref) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Ref. +func (in *Ref) DeepCopy() *Ref { + if in == nil { + return nil + } + out := new(Ref) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RefSource) DeepCopyInto(out *RefSource) { *out = *in @@ -1263,6 +1279,11 @@ func (in *Step) DeepCopyInto(out *Step) { *out = new(StepOutputConfig) **out = **in } + if in.Ref != nil { + in, out := &in.Ref, &out.Ref + *out = new(Ref) + **out = **in + } return } diff --git a/pkg/apis/pipeline/v1beta1/container_conversion.go b/pkg/apis/pipeline/v1beta1/container_conversion.go index 5bf1365fc7c..8e7171efa5e 100644 --- a/pkg/apis/pipeline/v1beta1/container_conversion.go +++ b/pkg/apis/pipeline/v1beta1/container_conversion.go @@ -22,6 +22,14 @@ import ( v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" ) +func (r Ref) convertTo(ctx context.Context, sink *v1.Ref) { + sink.Name = r.Name +} + +func (r *Ref) convertFrom(ctx context.Context, source v1.Ref) { + r.Name = source.Name +} + func (s Step) convertTo(ctx context.Context, sink *v1.Step) { sink.Name = s.Name sink.Image = s.Image @@ -47,6 +55,10 @@ func (s Step) convertTo(ctx context.Context, sink *v1.Step) { sink.OnError = (v1.OnErrorType)(s.OnError) sink.StdoutConfig = (*v1.StepOutputConfig)(s.StdoutConfig) sink.StderrConfig = (*v1.StepOutputConfig)(s.StderrConfig) + if s.Ref != nil { + sink.Ref = &v1.Ref{} + s.Ref.convertTo(ctx, sink.Ref) + } } func (s *Step) convertFrom(ctx context.Context, source v1.Step) { @@ -74,6 +86,11 @@ func (s *Step) convertFrom(ctx context.Context, source v1.Step) { s.OnError = (OnErrorType)(source.OnError) s.StdoutConfig = (*StepOutputConfig)(source.StdoutConfig) s.StderrConfig = (*StepOutputConfig)(source.StderrConfig) + if source.Ref != nil { + newRef := Ref{} + newRef.convertFrom(ctx, *source.Ref) + s.Ref = &newRef + } } func (s StepTemplate) convertTo(ctx context.Context, sink *v1.StepTemplate) { diff --git a/pkg/apis/pipeline/v1beta1/container_types.go b/pkg/apis/pipeline/v1beta1/container_types.go index 980ad392c8d..7b1a08b00c4 100644 --- a/pkg/apis/pipeline/v1beta1/container_types.go +++ b/pkg/apis/pipeline/v1beta1/container_types.go @@ -228,6 +228,16 @@ type Step struct { // Stores configuration for the stderr stream of the step. // +optional StderrConfig *StepOutputConfig `json:"stderrConfig,omitempty"` + + // Contains the reference to an existing stepaction + //+optional + Ref *Ref `json:"ref,omitempty"` +} + +// Ref can be used to refer to a specific instance of a StepAction. +type Ref struct { + // Name of the referenced step + Name string `json:"name,omitempty"` } // OnErrorType defines a list of supported exiting behavior of a container on error diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index eba5a872419..86c216b6aca 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -76,6 +76,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineWorkspaceDeclaration": schema_pkg_apis_pipeline_v1beta1_PipelineWorkspaceDeclaration(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec": schema_pkg_apis_pipeline_v1beta1_PropertySpec(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Provenance": schema_pkg_apis_pipeline_v1beta1_Provenance(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Ref": schema_pkg_apis_pipeline_v1beta1_Ref(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.RefSource": schema_pkg_apis_pipeline_v1beta1_RefSource(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ResolverRef": schema_pkg_apis_pipeline_v1beta1_ResolverRef(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ResultRef": schema_pkg_apis_pipeline_v1beta1_ResultRef(ref), @@ -3054,6 +3055,26 @@ func schema_pkg_apis_pipeline_v1beta1_Provenance(ref common.ReferenceCallback) c } } +func schema_pkg_apis_pipeline_v1beta1_Ref(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "Ref can be used to refer to a specific instance of a StepAction.", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Description: "Name of the referenced step", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1beta1_RefSource(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -3878,12 +3899,18 @@ func schema_pkg_apis_pipeline_v1beta1_Step(ref common.ReferenceCallback) common. Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepOutputConfig"), }, }, + "ref": { + SchemaProps: spec.SchemaProps{ + Description: "Contains the reference to an existing stepaction", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Ref"), + }, + }, }, Required: []string{"name"}, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceUsage", "k8s.io/api/core/v1.ContainerPort", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.Lifecycle", "k8s.io/api/core/v1.Probe", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Ref", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceUsage", "k8s.io/api/core/v1.ContainerPort", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.Lifecycle", "k8s.io/api/core/v1.Probe", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, } } diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 4ff7d3d43ab..2effd724b67 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -1555,6 +1555,16 @@ } } }, + "v1beta1.Ref": { + "description": "Ref can be used to refer to a specific instance of a StepAction.", + "type": "object", + "properties": { + "name": { + "description": "Name of the referenced step", + "type": "string" + } + } + }, "v1beta1.RefSource": { "description": "RefSource contains the information that can uniquely identify where a remote built definition came from i.e. Git repositories, Tekton Bundles in OCI registry and hub.", "type": "object", @@ -2071,6 +2081,10 @@ "description": "Periodic probe of container service readiness. Step will be removed from service endpoints if the probe fails. Cannot be updated. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#container-probes\n\nDeprecated: This field will be removed in a future release.", "$ref": "#/definitions/v1.Probe" }, + "ref": { + "description": "Contains the reference to an existing stepaction", + "$ref": "#/definitions/v1beta1.Ref" + }, "resources": { "description": "Compute Resources required by this Step. Cannot be updated. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/", "default": {}, diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go index 629ed816c7a..4428918523a 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go @@ -73,6 +73,15 @@ spec: steps: - image: foo - image: bar +` + stepActionTaskYAML := ` +metadata: + name: foo + namespace: bar +spec: + steps: + - ref: + name: "step-action" ` taskWithAllNoDeprecatedFieldsYAML := ` metadata: @@ -261,6 +270,9 @@ spec: multiStepTaskV1beta1 := parse.MustParseV1beta1Task(t, multiStepTaskYAML) multiStepTaskV1 := parse.MustParseV1Task(t, multiStepTaskYAML) + stepActionTaskV1beta1 := parse.MustParseV1beta1Task(t, stepActionTaskYAML) + stepActionTaskV1 := parse.MustParseV1Task(t, stepActionTaskYAML) + taskWithAllNoDeprecatedFieldsV1beta1 := parse.MustParseV1beta1Task(t, taskWithAllNoDeprecatedFieldsYAML) taskWithAllNoDeprecatedFieldsV1 := parse.MustParseV1Task(t, taskWithAllNoDeprecatedFieldsYAML) @@ -289,6 +301,10 @@ spec: name: "multi-steps task", v1beta1Task: multiStepTaskV1beta1, v1Task: multiStepTaskV1, + }, { + name: "step action in task", + v1beta1Task: stepActionTaskV1beta1, + v1Task: stepActionTaskV1, }, { name: "task conversion all non deprecated fields", v1beta1Task: taskWithAllNoDeprecatedFieldsV1beta1, diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index cfdd84433c6..e08c75809f1 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -240,19 +240,54 @@ func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) { } func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) { - if s.Image == "" { - errs = errs.Also(apis.ErrMissingField("Image")) - } - - if s.Script != "" { + if s.Ref != nil { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions { + return apis.ErrGeneric("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions) + } + if s.Image != "" { + errs = errs.Also(&apis.FieldError{ + Message: "image cannot be used with Ref", + Paths: []string{"image"}, + }) + } if len(s.Command) > 0 { errs = errs.Also(&apis.FieldError{ - Message: "script cannot be used with command", + Message: "command cannot be used with Ref", + Paths: []string{"command"}, + }) + } + if len(s.Args) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "args cannot be used with Ref", + Paths: []string{"args"}, + }) + } + if s.Script != "" { + errs = errs.Also(&apis.FieldError{ + Message: "script cannot be used with Ref", Paths: []string{"script"}, }) } - } + if s.Env != nil { + errs = errs.Also(&apis.FieldError{ + Message: "env cannot be used with Ref", + Paths: []string{"env"}, + }) + } + } else { + if s.Image == "" { + errs = errs.Also(apis.ErrMissingField("Image")) + } + if s.Script != "" { + if len(s.Command) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "script cannot be used with command", + Paths: []string{"script"}, + }) + } + } + } if s.Name != "" { if names.Has(s.Name) { errs = errs.Also(apis.ErrInvalidValue(s.Name, "name")) diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index c89bc74a8ae..e90b59173cc 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/config" cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -523,6 +524,38 @@ func TestTaskSpecValidate(t *testing.T) { } } +func TestTaskSpecStepActionReferenceValidate(t *testing.T) { + tests := []struct { + name string + Steps []v1beta1.Step + }{{ + name: "valid stepaction ref", + Steps: []v1beta1.Step{{ + Name: "mystep", + WorkingDir: "/foo", + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + }}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1beta1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + ts.SetDefaults(ctx) + if err := ts.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + func TestTaskValidateError(t *testing.T) { type fields struct { Params []v1beta1.ParamSpec @@ -730,6 +763,63 @@ func TestTaskValidateError(t *testing.T) { } } +func TestTaskValidateErrorWithStepActionRef(t *testing.T) { + tests := []struct { + name string + Steps []v1beta1.Step + enableStepActions bool + expectedError apis.FieldError + }{{ + name: "invalid step - invalid step action", + Steps: []v1beta1.Step{{ + Image: "image", + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "image cannot be used with Ref", + Paths: []string{"spec.steps[0].image"}, + }, + }, { + name: "invalid step - enableStepActions not on", + Steps: []v1beta1.Step{{ + Image: "image", + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + }}, + enableStepActions: false, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true to reference StepActions in Steps.", + Paths: []string{"spec.steps[0].enable-step-actions"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + task := &v1beta1.Task{ + ObjectMeta: metav1.ObjectMeta{Name: "foo"}, + Spec: v1beta1.TaskSpec{ + Steps: tt.Steps, + }} + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tt.enableStepActions, + }, + }) + task.SetDefaults(ctx) + err := task.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", task) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestTaskSpecValidateError(t *testing.T) { type fields struct { Params []v1beta1.ParamSpec @@ -1382,6 +1472,116 @@ func TestTaskSpecValidateError(t *testing.T) { } } +func TestTaskiSpecValidateErrorWithStepActionRef(t *testing.T) { + tests := []struct { + name string + Steps []v1beta1.Step + enableStepActions bool + expectedError apis.FieldError + }{{ + name: "invalid Task Spec - enableStepActions not on", + Steps: []v1beta1.Step{{ + Image: "image", + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + }}, + enableStepActions: false, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true to reference StepActions in Steps.", + Paths: []string{"steps[0].enable-step-actions"}, + }, + }, { + name: "Cannot use image with Ref", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + Image: "foo", + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "image cannot be used with Ref", + Paths: []string{"steps[0].image"}, + }, + }, { + name: "Cannot use command with Ref", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + Command: []string{"foo"}, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "command cannot be used with Ref", + Paths: []string{"steps[0].command"}, + }, + }, { + name: "Cannot use args with Ref", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + Args: []string{"foo"}, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "args cannot be used with Ref", + Paths: []string{"steps[0].args"}, + }, + }, { + name: "Cannot use script with Ref", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + Script: "echo hi", + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "script cannot be used with Ref", + Paths: []string{"steps[0].script"}, + }, + }, { + name: "Cannot use env with Ref", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + Env: []corev1.EnvVar{{ + Name: "env1", + Value: "value1", + }}, + }}, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "env cannot be used with Ref", + Paths: []string{"steps[0].env"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := v1beta1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tt.enableStepActions, + }, + }) + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", ts) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestTaskSpecValidateErrorSidecarName(t *testing.T) { tests := []struct { name string diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index aa44da6184e..db21dad9f79 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1455,6 +1455,22 @@ func (in *Provenance) DeepCopy() *Provenance { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Ref) DeepCopyInto(out *Ref) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Ref. +func (in *Ref) DeepCopy() *Ref { + if in == nil { + return nil + } + out := new(Ref) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RefSource) DeepCopyInto(out *RefSource) { *out = *in @@ -1735,6 +1751,11 @@ func (in *Step) DeepCopyInto(out *Step) { *out = new(StepOutputConfig) **out = **in } + if in.Ref != nil { + in, out := &in.Ref, &out.Ref + *out = new(Ref) + **out = **in + } return } diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index c33ec14c903..2eb3bb56f27 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 @@ -224,6 +233,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/custom_task_test.go b/test/custom_task_test.go index 604f66db2b5..f3b7e93b9b4 100644 --- a/test/custom_task_test.go +++ b/test/custom_task_test.go @@ -732,6 +732,7 @@ func getFeatureFlagsBaseOnAPIFlag(t *testing.T) *config.FeatureFlags { "enable-api-fields": "alpha", "results-from": "sidecar-logs", "enable-tekton-oci-bundles": "true", + "enable-step-actions": "true", }) if err != nil { t.Fatalf("error creating alpha feature flags configmap: %v", err) diff --git a/test/e2e-tests-kind-prow-alpha.env b/test/e2e-tests-kind-prow-alpha.env index 42f84533ab9..ee5f4c964bc 100644 --- a/test/e2e-tests-kind-prow-alpha.env +++ b/test/e2e-tests-kind-prow-alpha.env @@ -4,3 +4,4 @@ RUN_YAML_TESTS=true KO_DOCKER_REPO=registry.local:5000 E2E_GO_TEST_TIMEOUT=40m RESULTS_FROM=sidecar-logs +ENABLE_STEP_ACTIONS=true diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 97f62d4d172..298741b9eca 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -27,6 +27,7 @@ RUN_YAML_TESTS=${RUN_YAML_TESTS:="true"} SKIP_GO_E2E_TESTS=${SKIP_GO_E2E_TESTS:="false"} E2E_GO_TEST_TIMEOUT=${E2E_GO_TEST_TIMEOUT:="20m"} RESULTS_FROM=${RESULTS_FROM:-termination-message} +ENABLE_STEP_ACTIONS=${ENABLE_STEP_ACTIONS:="false"} failed=0 # Script entry point. @@ -77,6 +78,18 @@ function set_result_extraction_method() { kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" } +function set_enable_step_actions() { + local method="$1" + if [ "$method" != "false" ] && [ "$method" != "true" ]; then + printf "Invalid value for enable-step-actions %s\n" ${method} + exit 255 + fi + printf "Setting enable-step-actions to %s\n", ${method} + jsonpatch=$(printf "{\"data\": {\"enable-step-actions\": \"%s\"}}" $1) + echo "feature-flags ConfigMap patch: ${jsonpatch}" + kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" +} + function run_e2e() { # Run the integration tests header "Running Go e2e tests" @@ -96,6 +109,7 @@ function run_e2e() { add_spire "$PIPELINE_FEATURE_GATE" set_feature_gate "$PIPELINE_FEATURE_GATE" set_result_extraction_method "$RESULTS_FROM" +set_enable_step_actions "$ENABLE_STEP_ACTIONS" run_e2e (( failed )) && fail_test 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()