From f96950d93a5a348cb33bb6b5fd772ae9585be6c5 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Wed, 25 Oct 2023 15:39:04 -0400 Subject: [PATCH] Introduce StepAction referencing syntax in Steps This PR adds the referencing syntax for `StepActions` in `Stpes`. It also adds the necessary conversion between `v1beta1` and `v1` and the necessary validation. --- docs/pipeline-api.md | 86 +++++++++++ docs/stepactions.md | 83 +++++++++- 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 | 143 ++++++++++++++++++ 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 | 48 +++++- .../pipeline/v1beta1/task_validation_test.go | 143 ++++++++++++++++++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 21 +++ 16 files changed, 716 insertions(+), 15 deletions(-) diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 39d002be95c..c3c820c4da3 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..b015bf17fd8 100644 --- a/docs/stepactions.md +++ b/docs/stepactions.md @@ -52,5 +52,86 @@ spec: value: /home image: ubuntu command: ["ls"] - args:: ["-lh"] + 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 +``` + +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 +``` + +When a `Step` is referencing a `StepAction`, it can contain the following fields: +- `computeResources` +- `workspaces` (Isolated workspaces) +- `volumeDevices` +- `imagePullPolicy` +- `onError` +- `stdoutConfig` +- `stderrConfig` +- `securityContext` +- `envFrom` +- `timeout` + +Using any of the above fields and referencing a `StepAction` is allowed and will not cause an error. For example, the `TaskRun` below will execute without any errors: + +```yaml +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: step-action-run +spec: + TaskSpec: + steps: + - name: action-runner + ref: + name: step-action + computeResources: + requests: + memory: 1Gi + cpu: 500m + timeout: 1h + onError: continue ``` diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go index ccef95cf767..e0b979ffa86 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..8dc34c03672 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..03246208237 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 e85179d60d1..cdad33d7a80 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -264,17 +264,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..f3f8326d795 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 @@ -1370,6 +1403,116 @@ func TestTaskSpecValidateError(t *testing.T) { } } +func TestTaskSpecValidateErrorWithStepActionRef(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..539e01233f4 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..430d41c2b54 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..1392a89640e 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..1136be3d78b 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) @@ -293,6 +305,10 @@ spec: name: "task conversion all non deprecated fields", v1beta1Task: taskWithAllNoDeprecatedFieldsV1beta1, v1Task: taskWithAllNoDeprecatedFieldsV1, + }, { + name: "step action in task", + v1beta1Task: stepActionTaskV1beta1, + v1Task: stepActionTaskV1, }, { name: "task conversion deprecated fields", v1beta1Task: taskWithDeprecatedFieldsV1beta1, diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 36646a4d1d2..a4a38800fe7 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -270,17 +270,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/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 484c90cb748..962abf02ef6 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 @@ -1382,6 +1415,116 @@ func TestTaskSpecValidateError(t *testing.T) { } } +func TestTaskSpecValidateErrorWithStepActionRef(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 }