Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clean up getting substitution expressions #7121

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Clean up getting substitution expressions #7121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions pkg/apis/pipeline/v1/param_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
12 changes: 12 additions & 0 deletions pkg/apis/pipeline/v1/pipeline_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1/pipeline_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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))
}
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/pipeline/v1/pipelinerun_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
37 changes: 1 addition & 36 deletions pkg/apis/pipeline/v1/resultref.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions pkg/apis/pipeline/v1/resultref_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/apply.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/resources/validate_dependencies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading