Validation for Finally Task Results not being referenced in Pipeline …

…Results

Prior to this commit, there was no validation for Finally
Task Results being referenced to Pipeline Results.

This commit creates validation for that case by making
sure all the Finally Task Results reference a `task_name`
within the Pipeline Results.

Fixes bug [#4923](#4923)

/kind bug
/cc @jerop

pkg/apis/pipeline/v1beta1/pipeline_validation.go

@@ -69,7 +69,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	errs = errs.Also(validatePipelineWorkspacesUsage(ps.Workspaces, ps.Tasks).ViaField("tasks"))
 	errs = errs.Also(validatePipelineWorkspacesUsage(ps.Workspaces, ps.Finally).ViaField("finally"))
 	// Validate the pipeline's results
-	errs = errs.Also(validatePipelineResults(ps.Results))
+	errs = errs.Also(validatePipelineResults(ps.Results, ps.Tasks))
 	errs = errs.Also(validateTasksAndFinallySection(ps))
 	errs = errs.Also(validateFinalTasks(ps.Tasks, ps.Finally))
 	errs = errs.Also(validateWhenExpressions(ps.Tasks, ps.Finally))
@@ -249,7 +249,8 @@ func filter(arr []string, cond func(string) bool) []string {
 }
 
 // validatePipelineResults ensure that pipeline result variables are properly configured
-func validatePipelineResults(results []PipelineResult) (errs *apis.FieldError) {
+func validatePipelineResults(results []PipelineResult, tasks []PipelineTask) (errs *apis.FieldError) {
+	taskMap := convertTaskToTaskMap(tasks)
 	for idx, result := range results {
 		expressions, ok := GetVarSubstitutionExpressionsForPipelineResult(result)
 		if !ok {
@@ -265,7 +266,12 @@ func validatePipelineResults(results []PipelineResult) (errs *apis.FieldError) {
 		expressions = filter(expressions, looksLikeResultRef)
 		resultRefs := NewResultRefs(expressions)
 		if len(expressions) != len(resultRefs) {
-			errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("expected all of the expressions %v to be result expressions but only %v were", expressions, resultRefs),
+			return errs.Also(apis.ErrInvalidValue(fmt.Sprintf("expected all of the expressions %v to be result expressions but only %v were", expressions, resultRefs),
+				"value").ViaFieldIndex("results", idx))
+		}
+
+		if !TaskContainsResult(result.Value, taskMap) {
+			return errs.Also(apis.ErrInvalidValue("invalid propogation of finally task results to pipeline results",
 				"value").ViaFieldIndex("results", idx))
 		}
 
@@ -274,6 +280,40 @@ func validatePipelineResults(results []PipelineResult) (errs *apis.FieldError) {
 	return errs
 }
 
+// put task names in a dictionary
+func convertTaskToTaskMap(tasks []PipelineTask) map[string]string {
+	taskMap := make(map[string]string)
+	for _, val := range tasks {
+		taskMap[val.Name] = ""
+	}
+
+	return taskMap
+}
+
+// validate result is referenced in a task name
+func TaskContainsResult(value string, taskMap map[string]string) bool {
+	sections := splitValue(value)
+	if len(sections[0]) < 3 || len(sections[3]) < 2 {
+		return false
+	}
+
+	if len(sections) < 2 {
+		return false
+	}
+	_, ok := taskMap[sections[1]]
+	return ok
+}
+
+// split result.Value into its respective sections
+func splitValue(value string) []string {
+	sections := strings.Split(value, ".")
+	sections[0] = sections[0][2:]                  // remove `$(`
+	sections[3] = sections[3][:len(sections[3])-1] // remove `)`
+
+	return sections
+
+}
+
 func validateTasksAndFinallySection(ps *PipelineSpec) *apis.FieldError {
 	if len(ps.Finally) != 0 && len(ps.Tasks) == 0 {
 		return apis.ErrInvalidValue(fmt.Sprintf("spec.tasks is empty but spec.finally has %d tasks", len(ps.Finally)), "finally")

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

@@ -1107,7 +1107,7 @@ func TestValidatePipelineResults_Success(t *testing.T) {
 		Description: "this is my pipeline result",
 		Value:       "$(tasks.a-task.results.gitrepo.commit)",
 	}}
-	if err := validatePipelineResults(results); err != nil {
+	if err := validatePipelineResults(results, []PipelineTask{PipelineTask{Name: "a-task"}}); err != nil {
 		t.Errorf("Pipeline.validatePipelineResults() returned error for valid pipeline: %s: %v", desc, err)
 	}
 }
@@ -1152,7 +1152,7 @@ func TestValidatePipelineResults_Failure(t *testing.T) {
 		},
 	}}
 	for _, tt := range tests {
-		err := validatePipelineResults(tt.results)
+		err := validatePipelineResults(tt.results, []PipelineTask{})
 		if err == nil {
 			t.Errorf("Pipeline.validatePipelineResults() did not return for invalid pipeline: %s", tt.desc)
 		}
@@ -1162,6 +1162,82 @@ func TestValidatePipelineResults_Failure(t *testing.T) {
 	}
 }
 
+func TestFinallyTaskResultsToPipelineResults_Failure(t *testing.T) {
+	tests := []struct {
+		desc          string
+		p             *Pipeline
+		expectedError apis.FieldError
+		wc            func(context.Context) context.Context
+	}{{
+		desc: "invalid propogation of finally task results from pipeline results",
+		p: &Pipeline{
+			ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
+			Spec: PipelineSpec{
+				Results: []PipelineResult{{
+					Name:  "commit",
+					Value: "$(tasks.clone-app-repo.results.current-date-unix-timestamp)",
+				}, {
+					Name:  "initialized",
+					Value: "$(tasks.check-git-commit.results.init)",
+				}},
+				Workspaces: []PipelineWorkspaceDeclaration{{
+					Name: "git-source",
+				}},
+				Tasks: []PipelineTask{{
+					Name: "clone-app-repo",
+					TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{
+						Results: []TaskResult{{
+							Name: "current-date-unix-timestamp",
+							Type: "string",
+						}},
+						Steps: []Step{{
+							Name: "foo", Image: "bar",
+						}},
+					}},
+				}},
+				Finally: []PipelineTask{{
+					Name: "check-git-commit",
+					Params: []Param{{
+						Name: "commit1", Value: ArrayOrString{Type: ParamTypeString, StringVal: "$(tasks.clone-app-repo.results.current-date-unix-timestamp)"},
+					}},
+					TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{
+						Params: []ParamSpec{{
+							Name: "commit1", Type: ParamTypeString,
+						}},
+						Results: []TaskResult{{
+							Name: "init",
+							Type: "string",
+						}},
+						Steps: []Step{{
+							Name: "foo2", Image: "bar",
+						}},
+					}},
+				}},
+			},
+		},
+		expectedError: apis.FieldError{
+			Message: `invalid value: invalid propogation of finally task results to pipeline results`,
+			Paths:   []string{"spec.results[1].value"},
+		},
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			ctx := context.Background()
+			if tt.wc != nil {
+				ctx = tt.wc(ctx)
+			}
+			err := tt.p.Validate(ctx)
+			if err == nil {
+				t.Errorf("Pipeline.validatePipelineResults() did not return for invalid pipeline: %s", tt.desc)
+			}
+			if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
+				t.Errorf("Pipeline.validateParamResults() errors diff %s", diff.PrintWantGot(d))
+			}
+		})
+	}
+}
+
 func TestValidatePipelineParameterVariables_Success(t *testing.T) {
 	tests := []struct {
 		name   string