Skip to content

Commit

Permalink
Add support for params between Step and StepActions
Browse files Browse the repository at this point in the history
Following the previous [PR](tektoncd#7317), which introduced Params to the `StepAction` CRD,
this PR integrates `param` usage between `Steps` and `StepActions`. This
completes support for params in `StepActions`. This work is part of
issue tektoncd#7259.
  • Loading branch information
chitrangpatel committed Nov 3, 2023
1 parent c6c6ff4 commit dd8eecc
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 6 deletions.
3 changes: 3 additions & 0 deletions pkg/reconciler/taskrun/resources/taskspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, tr *v1.TaskRu
if stepActionSpec.Env != nil {
s.Env = stepActionSpec.Env
}
if err := validateStepHasStepActionParameters(ctx, s.Params, stepActionSpec.Params); err != nil {
return nil, err
}
s = applyStepActionParameters(ctx, s, &taskSpec, tr, s.Params, stepActionSpec.Params)
s.Params = nil
steps = append(steps, *s)
Expand Down
78 changes: 72 additions & 6 deletions pkg/reconciler/taskrun/resources/taskspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ func TestGetStepActionsData(t *testing.T) {

got, err := resources.GetStepActionsData(ctx, *tt.tr.Spec.TaskSpec, tt.tr, tektonclient)
if err != nil {
t.Errorf("Did not expect an error but got : %s", err)
t.Fatalf("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))
Expand All @@ -757,10 +757,10 @@ func TestGetStepActionsData(t *testing.T) {

func TestGetStepActionsData_Error(t *testing.T) {
tests := []struct {
name string
tr *v1.TaskRun
stepActionFunc func(ctx context.Context, ref *v1.Ref) (*v1alpha1.StepAction, *v1.RefSource, error)
expectedError error
name string
tr *v1.TaskRun
stepAction *v1alpha1.StepAction
expectedError error
}{{
name: "namespace missing error",
tr: &v1.TaskRun{
Expand All @@ -777,10 +777,76 @@ func TestGetStepActionsData_Error(t *testing.T) {
},
},
},
stepAction: &v1alpha1.StepAction{},
expectedError: fmt.Errorf("must specify namespace to resolve reference to step action stepActionError"),
}, {
name: "params missing",
tr: &v1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "mytaskrun",
Namespace: "default",
},
Spec: v1.TaskRunSpec{
TaskSpec: &v1.TaskSpec{
Steps: []v1.Step{{
Ref: &v1.Ref{
Name: "stepaction",
},
}},
},
},
},
stepAction: &v1alpha1.StepAction{
ObjectMeta: metav1.ObjectMeta{
Name: "stepaction",
Namespace: "default",
},
Spec: v1alpha1.StepActionSpec{
Image: "myimage",
Params: v1.ParamSpecs{{
Name: "string-param",
Type: v1.ParamTypeString,
}},
},
},
expectedError: fmt.Errorf("non-existent params in Step: [string-param]"),
}, {
name: "params extra",
tr: &v1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "mytaskrun",
Namespace: "default",
},
Spec: v1.TaskRunSpec{
TaskSpec: &v1.TaskSpec{
Steps: []v1.Step{{
Ref: &v1.Ref{
Name: "stepaction",
},
Params: v1.Params{{
Name: "string-param",
Value: *v1.NewStructuredValues("$(params.stringparam)"),
}},
}},
},
},
},
stepAction: &v1alpha1.StepAction{
ObjectMeta: metav1.ObjectMeta{
Name: "stepaction",
Namespace: "default",
},
Spec: v1alpha1.StepActionSpec{
Image: "myimage",
},
},
expectedError: fmt.Errorf("extra params passed by Step to StepAction: [string-param]"),
}}
for _, tt := range tests {
_, err := resources.GetStepActionsData(context.Background(), *tt.tr.Spec.TaskSpec, tt.tr, nil)
ctx := context.Background()
tektonclient := fake.NewSimpleClientset(tt.stepAction)

_, err := resources.GetStepActionsData(ctx, *tt.tr.Spec.TaskSpec, tt.tr, tektonclient)
if err == nil {
t.Fatalf("Expected to get an error but did not find any.")
}
Expand Down
38 changes: 38 additions & 0 deletions pkg/reconciler/taskrun/resources/validate_params.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package resources

import (
"context"
"fmt"

v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
Expand Down Expand Up @@ -45,3 +46,40 @@ func ValidateOutOfBoundArrayParams(declarations v1.ParamSpecs, params v1.Params,
}
return nil
}

func validateStepHasStepActionParameters(ctx context.Context, stepParams v1.Params, stepActionDefaults []v1.ParamSpec) error {
stepActionParams := sets.String{}
requiredStepActionParams := []string{}
for _, sa := range stepActionDefaults {
stepActionParams.Insert(sa.Name)
if sa.Default == nil {
requiredStepActionParams = append(requiredStepActionParams, sa.Name)
}
}

stepProvidedParams := sets.String{}
extra := []string{}
for _, sp := range stepParams {
stepProvidedParams.Insert(sp.Name)
if !stepActionParams.Has(sp.Name) {
// Extra parameter that is not needed
extra = append(extra, sp.Name)
}
}
if len(extra) > 0 {
return fmt.Errorf("extra params passed by Step to StepAction: %v", extra)
}

missing := []string{}

for _, requiredParam := range requiredStepActionParams {
if !stepProvidedParams.Has(requiredParam) {
// Missing required param
missing = append(missing, requiredParam)
}
}
if len(missing) > 0 {
return fmt.Errorf("non-existent params in Step: %v", missing)
}
return nil
}

0 comments on commit dd8eecc

Please sign in to comment.