Clean up getting substitution expressions

Prior to this change, the functions for fetching substitution expressions were inconsistent in the codebase. This change updates the functions for fetching substitition expressions from Parameters and Pipeline Results such that they are member functions, in line with related functions. /kind cleanup
tektoncd · Sep 19, 2023 · 4446f4e · 4446f4e
1 parent e35a4db
commit 4446f4e
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 48 deletions.
diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go
@@ -154,6 +154,29 @@ type Param struct {
 	Value ParamValue `json:"value"`
 }
 
+// GetVarSubstitutionExpressions extracts all the value between "$(" and ")"" for a Parameter
+func (p Param) GetVarSubstitutionExpressions() ([]string, bool) {
+	var allExpressions []string
+	switch p.Value.Type {
+	case ParamTypeArray:
+		// array type
+		for _, value := range p.Value.ArrayVal {
+			allExpressions = append(allExpressions, validateString(value)...)
+		}
+	case ParamTypeString:
+		// string type
+		allExpressions = append(allExpressions, validateString(p.Value.StringVal)...)
+	case ParamTypeObject:
+		// object type
+		for _, value := range p.Value.ObjectVal {
+			allExpressions = append(allExpressions, validateString(value)...)
+		}
+	default:
+		return nil, false
+	}
+	return allExpressions, len(allExpressions) != 0
+}
+
 // ExtractNames returns a set of unique names
 func (ps Params) ExtractNames() sets.String {
 	names := sets.String{}

diff --git a/pkg/apis/pipeline/v1/pipeline_types.go b/pkg/apis/pipeline/v1/pipeline_types.go
@@ -330,3 +330,15 @@ type PipelineList struct {
 	metav1.ListMeta `json:"metadata,omitempty"`
 	Items           []Pipeline `json:"items"`
 }
+
+// GetVarSubstitutionExpressions extracts all the value between "$(" and ")"" for a PipelineResult
+func (result PipelineResult) GetVarSubstitutionExpressions() ([]string, bool) {
+	allExpressions := validateString(result.Value.StringVal)
+	for _, v := range result.Value.ArrayVal {
+		allExpressions = append(allExpressions, validateString(v)...)
+	}
+	for _, v := range result.Value.ObjectVal {
+		allExpressions = append(allExpressions, validateString(v)...)
+	}
+	return allExpressions, len(allExpressions) != 0
+}
diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go
@@ -530,7 +530,7 @@ func validateExecutionStatusVariablesInFinally(tasksNames sets.String, finally [
 
 func (pt *PipelineTask) validateExecutionStatusVariablesDisallowed() (errs *apis.FieldError) {
 	for _, param := range pt.Params {
-		if expressions, ok := GetVarSubstitutionExpressionsForParam(param); ok {
+		if expressions, ok := param.GetVarSubstitutionExpressions(); ok {
 			errs = errs.Also(validateContainsExecutionStatusVariablesDisallowed(expressions, "value").
 				ViaFieldKey("params", param.Name))
 		}
@@ -546,7 +546,7 @@ func (pt *PipelineTask) validateExecutionStatusVariablesDisallowed() (errs *apis
 
 func (pt *PipelineTask) validateExecutionStatusVariablesAllowed(ptNames sets.String) (errs *apis.FieldError) {
 	for _, param := range pt.Params {
-		if expressions, ok := GetVarSubstitutionExpressionsForParam(param); ok {
+		if expressions, ok := param.GetVarSubstitutionExpressions(); ok {
 			errs = errs.Also(validateExecutionStatusVariablesExpressions(expressions, ptNames, "value").
 				ViaFieldKey("params", param.Name))
 		}
@@ -625,7 +625,7 @@ func validatePipelineResults(results []PipelineResult, tasks []PipelineTask, fin
 	pipelineTaskNames := getPipelineTasksNames(tasks)
 	pipelineFinallyTaskNames := getPipelineTasksNames(finally)
 	for idx, result := range results {
-		expressions, ok := GetVarSubstitutionExpressionsForPipelineResult(result)
+		expressions, ok := result.GetVarSubstitutionExpressions()
 		if !ok {
 			errs = errs.Also(apis.ErrInvalidValue("expected pipeline results to be task result expressions but no expressions were found",
 				"value").ViaFieldIndex("results", idx))
@@ -713,7 +713,7 @@ func validateFinalTasks(tasks []PipelineTask, finalTasks []PipelineTask) (errs *
 func validateTaskResultReferenceInFinallyTasks(finalTasks []PipelineTask, ts sets.String, fts sets.String) (errs *apis.FieldError) {
 	for idx, t := range finalTasks {
 		for _, p := range t.Params {
-			if expressions, ok := GetVarSubstitutionExpressionsForParam(p); ok {
+			if expressions, ok := p.GetVarSubstitutionExpressions(); ok {
 				errs = errs.Also(validateResultsVariablesExpressionsInFinally(expressions, ts, fts, "value").ViaFieldKey(
 					"params", p.Name).ViaFieldIndex("finally", idx))
 			}

diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go
@@ -131,7 +131,7 @@ func (ps *PipelineRunSpec) validatePipelineRunParameters(ctx context.Context) (e
 
 	// Validate that task results aren't used in param values
 	for _, param := range ps.Params {
-		expressions, ok := GetVarSubstitutionExpressionsForParam(param)
+		expressions, ok := param.GetVarSubstitutionExpressions()
 		if ok {
 			if LooksLikeContainsResultRefs(expressions) {
 				expressions = filter(expressions, looksLikeResultRef)

diff --git a/pkg/apis/pipeline/v1/resultref.go b/pkg/apis/pipeline/v1/resultref.go
@@ -105,41 +105,6 @@ func looksLikeResultRef(expression string) bool {
 	return len(subExpressions) >= 4 && (subExpressions[0] == ResultTaskPart || subExpressions[0] == ResultFinallyPart) && subExpressions[2] == ResultResultPart
 }
 
-// GetVarSubstitutionExpressionsForParam extracts all the value between "$(" and ")"" for a parameter
-func GetVarSubstitutionExpressionsForParam(param Param) ([]string, bool) {
-	var allExpressions []string
-	switch param.Value.Type {
-	case ParamTypeArray:
-		// array type
-		for _, value := range param.Value.ArrayVal {
-			allExpressions = append(allExpressions, validateString(value)...)
-		}
-	case ParamTypeString:
-		// string type
-		allExpressions = append(allExpressions, validateString(param.Value.StringVal)...)
-	case ParamTypeObject:
-		// object type
-		for _, value := range param.Value.ObjectVal {
-			allExpressions = append(allExpressions, validateString(value)...)
-		}
-	default:
-		return nil, false
-	}
-	return allExpressions, len(allExpressions) != 0
-}
-
-// GetVarSubstitutionExpressionsForPipelineResult extracts all the value between "$(" and ")"" for a pipeline result
-func GetVarSubstitutionExpressionsForPipelineResult(result PipelineResult) ([]string, bool) {
-	allExpressions := validateString(result.Value.StringVal)
-	for _, v := range result.Value.ArrayVal {
-		allExpressions = append(allExpressions, validateString(v)...)
-	}
-	for _, v := range result.Value.ObjectVal {
-		allExpressions = append(allExpressions, validateString(v)...)
-	}
-	return allExpressions, len(allExpressions) != 0
-}
-
 func validateString(value string) []string {
 	expressions := VariableSubstitutionRegex.FindAllString(value, -1)
 	if expressions == nil {
@@ -208,7 +173,7 @@ func ParseResultName(resultName string) (string, string) {
 func PipelineTaskResultRefs(pt *PipelineTask) []*ResultRef {
 	refs := []*ResultRef{}
 	for _, p := range pt.extractAllParams() {
-		expressions, _ := GetVarSubstitutionExpressionsForParam(p)
+		expressions, _ := p.GetVarSubstitutionExpressions()
 		refs = append(refs, NewResultRefs(expressions)...)
 	}
 	for _, whenExpression := range pt.When {

diff --git a/pkg/apis/pipeline/v1/resultref_test.go b/pkg/apis/pipeline/v1/resultref_test.go
@@ -172,7 +172,7 @@ func TestNewResultReference(t *testing.T) {
 		}},
 	}} {
 		t.Run(tt.name, func(t *testing.T) {
-			expressions, ok := v1.GetVarSubstitutionExpressionsForParam(tt.param)
+			expressions, ok := tt.param.GetVarSubstitutionExpressions()
 			if !ok && tt.want != nil {
 				t.Fatalf("expected to find expressions but didn't find any")
 			} else {
@@ -272,7 +272,7 @@ func TestHasResultReference(t *testing.T) {
 		}},
 	}} {
 		t.Run(tt.name, func(t *testing.T) {
-			expressions, ok := v1.GetVarSubstitutionExpressionsForParam(tt.param)
+			expressions, ok := tt.param.GetVarSubstitutionExpressions()
 			if !ok {
 				t.Fatalf("expected to find expressions but didn't find any")
 			}
@@ -355,7 +355,7 @@ func TestLooksLikeResultRef(t *testing.T) {
 		want: true,
 	}} {
 		t.Run(tt.name, func(t *testing.T) {
-			expressions, ok := v1.GetVarSubstitutionExpressionsForParam(tt.param)
+			expressions, ok := tt.param.GetVarSubstitutionExpressions()
 			if ok {
 				if got := v1.LooksLikeContainsResultRefs(expressions); got != tt.want {
 					t.Errorf("LooksLikeContainsResultRefs() = %v, want %v", got, tt.want)
@@ -755,7 +755,7 @@ func TestGetVarSubstitutionExpressionsForPipelineResult(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			get, _ := v1.GetVarSubstitutionExpressionsForPipelineResult(tt.result)
+			get, _ := tt.result.GetVarSubstitutionExpressions()
 			if d := cmp.Diff(tt.want, get, cmpopts.SortSlices(sortStrings)); d != "" {
 				t.Error(diff.PrintWantGot(d))
 			}

diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go
@@ -333,7 +333,7 @@ func ApplyTaskResultsToPipelineResults(
 	arrayReplacements := map[string][]string{}
 	objectReplacements := map[string]map[string]string{}
 	for _, pipelineResult := range results {
-		variablesInPipelineResult, _ := v1.GetVarSubstitutionExpressionsForPipelineResult(pipelineResult)
+		variablesInPipelineResult, _ := pipelineResult.GetVarSubstitutionExpressions()
 		if len(variablesInPipelineResult) == 0 {
 			continue
 		}

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
@@ -697,7 +697,7 @@ func (t *ResolvedPipelineTask) hasResultReferences() bool {
 		matrixParams = t.PipelineTask.Params
 	}
 	for _, param := range append(t.PipelineTask.Params, matrixParams...) {
-		if ps, ok := v1.GetVarSubstitutionExpressionsForParam(param); ok {
+		if ps, ok := param.GetVarSubstitutionExpressions(); ok {
 			if v1.LooksLikeContainsResultRefs(ps) {
 				return true
 			}

diff --git a/pkg/reconciler/pipelinerun/resources/validate_dependencies.go b/pkg/reconciler/pipelinerun/resources/validate_dependencies.go
@@ -46,7 +46,7 @@ func ValidatePipelineTaskResults(state PipelineRunState) error {
 func ValidatePipelineResults(ps *v1.PipelineSpec, state PipelineRunState) error {
 	ptMap := state.ToMap()
 	for _, result := range ps.Results {
-		expressions, _ := v1.GetVarSubstitutionExpressionsForPipelineResult(result)
+		expressions, _ := result.GetVarSubstitutionExpressions()
 		refs := v1.NewResultRefs(expressions)
 		for _, ref := range refs {
 			if err := validateResultRef(ref, ptMap); err != nil {