Add ValidateStepActionResultsVariables to validate stepResults only a…

…nd fix ValidateStepResultsVariables to include task results validation
tektoncd · Oct 29, 2024 · 49fb44c · 49fb44c
1 parent 96db451
commit 49fb44c
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 36 deletions.
diff --git a/examples/v1/taskruns/alpha/task-stepaction-results.yaml b/examples/v1/taskruns/alpha/task-stepaction-results.yaml
@@ -0,0 +1,48 @@
+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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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.