From 43fd366502b9deb1cf4d2c0b2609c8f0fb7d016d Mon Sep 17 00:00:00 2001 From: Jawed khelil Date: Mon, 16 Sep 2024 16:20:44 +0200 Subject: [PATCH] Add ValidateStepActionResultsVariables to validate stepResults only and fix ValidateStepResultsVariables to include task results validation --- .../alpha/task-stepaction-results.yaml | 46 +++++++++++++++++++ pkg/apis/pipeline/v1/task_validation.go | 24 ++++++++-- .../v1alpha1/stepaction_validation.go | 2 +- .../v1alpha1/stepaction_validation_test.go | 13 ------ .../pipeline/v1beta1/stepaction_validation.go | 2 +- .../v1beta1/stepaction_validation_test.go | 13 ------ pkg/apis/pipeline/v1beta1/task_validation.go | 21 +++++++-- 7 files changed, 85 insertions(+), 36 deletions(-) create mode 100644 examples/v1/taskruns/alpha/task-stepaction-results.yaml diff --git a/examples/v1/taskruns/alpha/task-stepaction-results.yaml b/examples/v1/taskruns/alpha/task-stepaction-results.yaml new file mode 100644 index 00000000000..cc1f3b679c2 --- /dev/null +++ b/examples/v1/taskruns/alpha/task-stepaction-results.yaml @@ -0,0 +1,46 @@ +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: step-action +spec: + params: + - name: message + type: string + env: + - name: message + value: $(params.message) + image: mirror.gcr.io/bash + script: | + #!/usr/bin/env bash + echo ${message} +--- +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + generateName: reference-step-result-in-step- +spec: + taskSpec: + description: | + A simple task to demonstrate how to reference a step result in another step when used alongside with task result + results: + - name: resultsDir + type: string + steps: + - name: collect + image: mirror.gcr.io/bash + results: + - name: message + type: string + script: | + #!/usr/bin/env sh + set -x + message="scott" + + echo -n "${message}" > $(step.results.message.path) + echo -n "tom" > $(results.resultsDir.path) + - name: reduce + params: + - name: message + value: $(steps.collect.results.message) + ref: + name: step-action diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 4232d295d2b..a59cbf04ec7 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -95,7 +95,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { }) } - errs = errs.Also(validateSteps(ctx, mergedSteps).ViaField("steps")) + errs = errs.Also(validateSteps(ctx, mergedSteps, ts.Results).ViaField("steps")) errs = errs.Also(validateSidecars(ts.Sidecars).ViaField("sidecars")) errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params")) errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params)) @@ -251,13 +251,13 @@ func ValidateVolumes(volumes []corev1.Volume) (errs *apis.FieldError) { return errs } -func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) { +func validateSteps(ctx context.Context, steps []Step, taskResults []TaskResult) (errs *apis.FieldError) { // Task must not have duplicate step names. names := sets.NewString() for idx, s := range steps { errs = errs.Also(validateStep(ctx, s, names).ViaIndex(idx)) if s.Results != nil { - errs = errs.Also(ValidateStepResultsVariables(ctx, s.Results, s.Script).ViaIndex(idx)) + errs = errs.Also(ValidateStepResultsVariables(ctx, s.Results, taskResults, s.Script).ViaIndex(idx)) errs = errs.Also(ValidateStepResults(ctx, s.Results).ViaIndex(idx).ViaField("results")) } if len(s.When) > 0 { @@ -853,12 +853,26 @@ func ValidateStepResults(ctx context.Context, results []StepResult) (errs *apis. } // ValidateStepResultsVariables validates if the StepResults referenced in step script are defined in step's results. -func ValidateStepResultsVariables(ctx context.Context, results []StepResult, script string) (errs *apis.FieldError) { +func ValidateStepResultsVariables(ctx context.Context, results []StepResult, taskResults []TaskResult, script string) (errs *apis.FieldError) { + resultsNames := sets.NewString() + taskResultsNames := sets.NewString() + for _, r := range results { + resultsNames.Insert(r.Name) + } + for _, r := range taskResults { + taskResultsNames.Insert(r.Name) + } + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "step.results", resultsNames).ViaField("script")) + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "results", taskResultsNames).ViaField("script")) + return errs +} + +// ValidateStepActionResultsVariables validates if the StepResults referenced in step script are defined in step's results. +func ValidateStepActionResultsVariables(ctx context.Context, results []StepResult, script string) (errs *apis.FieldError) { resultsNames := sets.NewString() for _, r := range results { resultsNames.Insert(r.Name) } errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "step.results", resultsNames).ViaField("script")) - errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "results", resultsNames).ViaField("script")) return errs } diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go index f61d137978a..4af720dad34 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go @@ -68,7 +68,7 @@ func (ss *StepActionSpec) Validate(ctx context.Context) (errs *apis.FieldError) errs = errs.Also(validateUsageOfDeclaredParameters(ctx, *ss)) errs = errs.Also(v1.ValidateParameterTypes(ctx, ss.Params).ViaField("params")) errs = errs.Also(validateParameterVariables(ctx, *ss, ss.Params)) - errs = errs.Also(v1.ValidateStepResultsVariables(ctx, ss.Results, ss.Script)) + errs = errs.Also(v1.ValidateStepActionResultsVariables(ctx, ss.Results, ss.Script)) errs = errs.Also(v1.ValidateStepResults(ctx, ss.Results).ViaField("results")) errs = errs.Also(validateVolumeMounts(ss.VolumeMounts, ss.Params).ViaField("volumeMounts")) return errs diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go index e1b3c0f3b8d..ee79af415a6 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go @@ -613,19 +613,6 @@ func TestStepActionSpecValidateError(t *testing.T) { Message: `windows script support requires "enable-api-fields" feature gate to be "alpha" but it is "beta"`, Paths: []string{}, }, - }, { - name: "step script refers to nonexistent result", - fields: fields{ - Image: "my-image", - Script: ` - #!/usr/bin/env bash - date | tee $(results.non-exist.path)`, - Results: []v1.StepResult{{Name: "a-result"}}, - }, - expectedError: apis.FieldError{ - Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(results.non-exist.path)"`, - Paths: []string{"script"}, - }, }, { name: "step script refers to nonexistent stepresult", fields: fields{ diff --git a/pkg/apis/pipeline/v1beta1/stepaction_validation.go b/pkg/apis/pipeline/v1beta1/stepaction_validation.go index 0955c7e4f70..10a352a4318 100644 --- a/pkg/apis/pipeline/v1beta1/stepaction_validation.go +++ b/pkg/apis/pipeline/v1beta1/stepaction_validation.go @@ -68,7 +68,7 @@ func (ss *StepActionSpec) Validate(ctx context.Context) (errs *apis.FieldError) errs = errs.Also(validateUsageOfDeclaredParameters(ctx, *ss)) errs = errs.Also(v1.ValidateParameterTypes(ctx, ss.Params).ViaField("params")) errs = errs.Also(validateParameterVariables(ctx, *ss, ss.Params)) - errs = errs.Also(v1.ValidateStepResultsVariables(ctx, ss.Results, ss.Script)) + errs = errs.Also(v1.ValidateStepActionResultsVariables(ctx, ss.Results, ss.Script)) errs = errs.Also(v1.ValidateStepResults(ctx, ss.Results).ViaField("results")) errs = errs.Also(validateVolumeMounts(ss.VolumeMounts, ss.Params).ViaField("volumeMounts")) return errs diff --git a/pkg/apis/pipeline/v1beta1/stepaction_validation_test.go b/pkg/apis/pipeline/v1beta1/stepaction_validation_test.go index 49efacdf91a..028fd7c2030 100644 --- a/pkg/apis/pipeline/v1beta1/stepaction_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/stepaction_validation_test.go @@ -613,19 +613,6 @@ func TestStepActionSpecValidateError(t *testing.T) { Message: `windows script support requires "enable-api-fields" feature gate to be "alpha" but it is "beta"`, Paths: []string{}, }, - }, { - name: "step script refers to nonexistent result", - fields: fields{ - Image: "my-image", - Script: ` - #!/usr/bin/env bash - date | tee $(results.non-exist.path)`, - Results: []v1.StepResult{{Name: "a-result"}}, - }, - expectedError: apis.FieldError{ - Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(results.non-exist.path)"`, - Paths: []string{"script"}, - }, }, { name: "step script refers to nonexistent stepresult", fields: fields{ diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 4d03a950125..48bbcbc8f33 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -95,7 +95,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { }) } - errs = errs.Also(validateSteps(ctx, mergedSteps).ViaField("steps")) + errs = errs.Also(validateSteps(ctx, mergedSteps, ts.Results).ViaField("steps")) errs = errs.Also(validateSidecarNames(ts.Sidecars)) errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params")) errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params)) @@ -240,13 +240,13 @@ func ValidateVolumes(volumes []corev1.Volume) (errs *apis.FieldError) { return errs } -func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) { +func validateSteps(ctx context.Context, steps []Step, taskResults []TaskResult) (errs *apis.FieldError) { // Task must not have duplicate step names. names := sets.NewString() for idx, s := range steps { errs = errs.Also(validateStep(ctx, s, names).ViaIndex(idx)) if s.Results != nil { - errs = errs.Also(v1.ValidateStepResultsVariables(ctx, s.Results, s.Script).ViaIndex(idx)) + errs = errs.Also(ValidateStepResultsVariables(ctx, s.Results, taskResults, s.Script).ViaIndex(idx)) errs = errs.Also(v1.ValidateStepResults(ctx, s.Results).ViaIndex(idx).ViaField("results")) } if len(s.When) > 0 { @@ -256,6 +256,21 @@ func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) { return errs } +// ValidateStepResultsVariables validates if the StepResults referenced in step script are defined in step's results or in task's results +func ValidateStepResultsVariables(ctx context.Context, results []v1.StepResult, taskResults []TaskResult, script string) (errs *apis.FieldError) { + resultsNames := sets.NewString() + taskResultsNames := sets.NewString() + for _, r := range results { + resultsNames.Insert(r.Name) + } + for _, r := range taskResults { + taskResultsNames.Insert(r.Name) + } + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "step.results", resultsNames).ViaField("script")) + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "results", taskResultsNames).ViaField("script")) + return errs +} + // isCreateOrUpdateAndDiverged checks if the webhook event was create or update // if create, it returns true. // if update, it checks if the step results have diverged and returns if diverged.