Skip to content

Commit

Permalink
Introduce StepAction referencing syntax in Steps
Browse files Browse the repository at this point in the history 
This PR adds the referencing syntax for `StepActions` in `Stpes`.
It also adds the necessary conversion between `v1beta1` and `v1`
and the necessary validation.
  • Loading branch information
@chitrangpatel
chitrangpatel committed Oct 25, 2023
1 parent 08ee164 commit 10922cc
Show file tree
Hide file tree
Showing 10 changed files with 595 additions and 12 deletions.
48 changes: 48 additions & 0 deletions docs/stepactions.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,51 @@ 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
```

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
```
9 changes: 9 additions & 0 deletions pkg/apis/pipeline/v1/container_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 42 additions & 6 deletions pkg/apis/pipeline/v1/task_validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
200 changes: 200 additions & 0 deletions pkg/apis/pipeline/v1/task_validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -727,6 +760,63 @@ 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
Expand Down Expand Up @@ -1370,6 +1460,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
Expand Down
17 changes: 17 additions & 0 deletions pkg/apis/pipeline/v1beta1/container_conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 10922cc

Please sign in to comment.