Introduce Value in TaskResults

This PR introduces the field `Value` in `TaskResults`. This field is necessary to capture the results produced by `StepActions` if the Task needs to resolve name conflicts. This is part of issue #7259. Following this PR, we will add support for extracting StepAction results via termination message and sidecar logs.
tektoncd · Nov 10, 2023 · 37ff8ec · 37ff8ec
1 parent 63e1a26
commit 37ff8ec
Show file tree

Hide file tree

Showing 10 changed files with 398 additions and 10 deletions.
diff --git a/pkg/apis/pipeline/v1/result_defaults.go b/pkg/apis/pipeline/v1/result_defaults.go
@@ -13,13 +13,22 @@ limitations under the License.
 
 package v1
 
-import "context"
+import (
+	"context"
+
+	"github.com/tektoncd/pipeline/pkg/apis/config"
+)
 
 // SetDefaults set the default type for TaskResult
-func (tr *TaskResult) SetDefaults(context.Context) {
+func (tr *TaskResult) SetDefaults(ctx context.Context) {
 	if tr == nil {
 		return
 	}
+	if tr.Value == nil && config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions {
+		tr.Value = &ParamValue{
+			Type: ParamTypeString,
+		}
+	}
 	if tr.Type == "" {
 		if tr.Properties != nil {
 			// Set type to object if `properties` is given

diff --git a/pkg/apis/pipeline/v1/result_types.go b/pkg/apis/pipeline/v1/result_types.go
@@ -32,6 +32,10 @@ type TaskResult struct {
 	// Description is a human-readable description of the result
 	// +optional
 	Description string `json:"description,omitempty"`
+
+	// Value the expression used to retrieve the value of the result from an underlying Step.
+	// +optional
+	Value *ResultValue `json:"value,omitempty"`
 }
 
 // TaskRunResult used to describe the results of a task

diff --git a/pkg/apis/pipeline/v1/result_validation.go b/pkg/apis/pipeline/v1/result_validation.go
@@ -16,7 +16,9 @@ package v1
 import (
 	"context"
 	"fmt"
+	"regexp"
 
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"knative.dev/pkg/apis"
 )
 
@@ -31,17 +33,14 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) {
 		errs := validateObjectResult(tr)
 		return errs
 	case tr.Type == ResultsTypeArray:
-		return nil
 	// Resources created before the result. Type was introduced may not have Type set
 	// and should be considered valid
 	case tr.Type == "":
-		return nil
 	// By default, the result type is string
 	case tr.Type != ResultsTypeString:
 		return apis.ErrInvalidValue(tr.Type, "type", "type must be string")
 	}
-
-	return nil
+	return tr.validateValue(ctx)
 }
 
 // validateObjectResult validates the object result and check if the Properties is missing
@@ -66,3 +65,45 @@ func validateObjectResult(tr TaskResult) (errs *apis.FieldError) {
 	}
 	return nil
 }
+
+// validateValue validates the value of the TaskResult.
+// It requires that enable-step-actions is true, the value is of type string
+// and format $(steps.<stepName>.results.<resultName>)
+func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError) {
+	if tr.Value != nil {
+		if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions {
+			return apis.ErrGeneric("feature flag %s should be set to true to fetch Results from Steps using StepActions.", config.EnableStepActions)
+		}
+		if tr.Value.Type != ParamTypeString {
+			return &apis.FieldError{
+				Message: fmt.Sprintf(
+					"Invalid Type. Wanted string but got: \"%v\"", tr.Value.Type),
+				Paths: []string{
+					fmt.Sprintf("%s.type", tr.Name),
+				},
+			}
+		}
+		_, _, err := ExtractStepResultName(tr.Value.StringVal)
+		if err != nil {
+			return &apis.FieldError{
+				Message: fmt.Sprintf("Could not extract step and result name from TaskResult. Expect value to look like $(steps.<stepName>.results.<resultName>) but got %v. Got error: %v", tr.Value.StringVal, err),
+				Paths: []string{
+					fmt.Sprintf("%s.value", tr.Name),
+				},
+			}
+		}
+	}
+	return nil
+}
+
+// ExtractStepResultName extracts the step name and result name from a string matching
+// formtat $(steps.<stepName>.results.<resultName>).
+// If a match is not found, an error is retured.
+func ExtractStepResultName(value string) (string, string, error) {
+	re := regexp.MustCompile(`\$\(steps\.(.*?)\.results\.(.*?)\)`)
+	rs := re.FindStringSubmatch(value)
+	if len(rs) != 3 {
+		return "", "", fmt.Errorf("Did not find two matches.")
+	}
+	return rs[1], rs[2], nil
+}
diff --git a/pkg/apis/pipeline/v1/result_validation_test.go b/pkg/apis/pipeline/v1/result_validation_test.go
@@ -18,10 +18,12 @@ package v1_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"github.com/tektoncd/pipeline/test/diff"
 	"knative.dev/pkg/apis"
@@ -70,6 +72,36 @@ func TestResultsValidate(t *testing.T) {
 	}
 }
 
+func TestResultsValidateValue(t *testing.T) {
+	tests := []struct {
+		name   string
+		Result v1.TaskResult
+	}{{
+		name: "valid result value",
+		Result: v1.TaskResult{
+			Name:        "MY-RESULT",
+			Description: "my great result",
+			Type:        v1.ResultsTypeString,
+			Value: &v1.ParamValue{
+				Type:      v1.ParamTypeString,
+				StringVal: "$(steps.stepName.results.resultName)",
+			},
+		},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := config.ToContext(context.Background(), &config.Config{
+				FeatureFlags: &config.FeatureFlags{
+					EnableStepActions: true,
+				},
+			})
+			if err := tt.Result.Validate(ctx); err != nil {
+				t.Errorf("TaskSpec.Validate() = %v", err)
+			}
+		})
+	}
+}
+
 func TestResultsValidateError(t *testing.T) {
 	tests := []struct {
 		name          string
@@ -123,3 +155,129 @@ func TestResultsValidateError(t *testing.T) {
 		})
 	}
 }
+
+func TestResultsValidateValueError(t *testing.T) {
+	tests := []struct {
+		name              string
+		Result            v1.TaskResult
+		enableStepActions bool
+		expectedError     apis.FieldError
+	}{{
+		name: "enable-step-actions-not-enabled",
+		Result: v1.TaskResult{
+			Name:        "MY-RESULT",
+			Description: "my great result",
+			Type:        v1.ResultsTypeString,
+			Value: &v1.ParamValue{
+				Type:      v1.ParamTypeString,
+				StringVal: "$(steps.stepName.results.resultName)",
+			},
+		},
+		enableStepActions: false,
+		expectedError: apis.FieldError{
+			Message: "feature flag %s should be set to true to fetch Results from Steps using StepActions.",
+			Paths:   []string{"enable-step-actions"},
+		},
+	}, {
+		name: "invalid result value type",
+		Result: v1.TaskResult{
+			Name:        "MY-RESULT",
+			Description: "my great result",
+			Type:        v1.ResultsTypeString,
+			Value: &v1.ParamValue{
+				Type: v1.ParamTypeArray,
+			},
+		},
+		enableStepActions: true,
+		expectedError: apis.FieldError{
+			Message: `Invalid Type. Wanted string but got: "array"`,
+			Paths:   []string{"MY-RESULT.type"},
+		},
+	}, {
+		name: "invalid result value format",
+		Result: v1.TaskResult{
+			Name:        "MY-RESULT",
+			Description: "my great result",
+			Type:        v1.ResultsTypeString,
+			Value: &v1.ParamValue{
+				Type:      v1.ParamTypeString,
+				StringVal: "not a valid format",
+			},
+		},
+		enableStepActions: true,
+		expectedError: apis.FieldError{
+			Message: `Could not extract step and result name from TaskResult. Expect value to look like $(steps.<stepName>.results.<resultName>) but got not a valid format. Got error: Did not find two matches.`,
+			Paths:   []string{"MY-RESULT.value"},
+		},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := config.ToContext(context.Background(), &config.Config{
+				FeatureFlags: &config.FeatureFlags{
+					EnableStepActions: tt.enableStepActions,
+				},
+			})
+			err := tt.Result.Validate(ctx)
+			if err == nil {
+				t.Fatalf("Expected an error, got nothing for %v", tt.Result)
+			}
+			if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
+				t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d))
+			}
+		})
+	}
+}
+
+func TestExtractStepResultName(t *testing.T) {
+	tests := []struct {
+		name       string
+		value      string
+		wantStep   string
+		wantResult string
+	}{{
+		name:       "valid string format",
+		value:      "$(steps.Foo.results.Bar)",
+		wantStep:   "Foo",
+		wantResult: "Bar",
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotStep, gotResult, err := v1.ExtractStepResultName(tt.value)
+			if err != nil {
+				t.Errorf("Did not expect an error but got: %v", err)
+			}
+			if d := cmp.Diff(tt.wantStep, gotStep); d != "" {
+				t.Errorf(diff.PrintWantGot(d))
+			}
+			if d := cmp.Diff(tt.wantResult, gotResult); d != "" {
+				t.Errorf(diff.PrintWantGot(d))
+			}
+		})
+	}
+}
+
+func TestExtractStepResultNameError(t *testing.T) {
+	tests := []struct {
+		name    string
+		value   string
+		wantErr error
+	}{{
+		name:    "invalid string format",
+		value:   "not valid",
+		wantErr: fmt.Errorf("Did not find two matches."),
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotStep, gotResult, err := v1.ExtractStepResultName(tt.value)
+			if d := cmp.Diff(tt.wantErr.Error(), err.Error()); d != "" {
+				t.Errorf(diff.PrintWantGot(d))
+			}
+			if gotStep != "" {
+				t.Errorf("Expected an empty string but fot: %v", gotStep)
+			}
+			if gotResult != "" {
+				t.Errorf("Expected an empty string but fot: %v", gotResult)
+			}
+		})
+	}
+}
diff --git a/pkg/apis/pipeline/v1beta1/result_conversion.go b/pkg/apis/pipeline/v1beta1/result_conversion.go
@@ -33,6 +33,10 @@ func (r TaskResult) convertTo(ctx context.Context, sink *v1.TaskResult) {
 		}
 		sink.Properties = properties
 	}
+	if r.Value != nil {
+		sink.Value = &v1.ParamValue{}
+		r.Value.convertTo(ctx, sink.Value)
+	}
 }
 
 func (r *TaskResult) convertFrom(ctx context.Context, source v1.TaskResult) {
@@ -46,4 +50,8 @@ func (r *TaskResult) convertFrom(ctx context.Context, source v1.TaskResult) {
 		}
 		r.Properties = properties
 	}
+	if source.Value != nil {
+		r.Value = &ParamValue{}
+		r.Value.convertFrom(ctx, *source.Value)
+	}
 }
diff --git a/pkg/apis/pipeline/v1beta1/result_defaults.go b/pkg/apis/pipeline/v1beta1/result_defaults.go
@@ -13,13 +13,22 @@ limitations under the License.
 
 package v1beta1
 
-import "context"
+import (
+	"context"
+
+	"github.com/tektoncd/pipeline/pkg/apis/config"
+)
 
 // SetDefaults set the default type for TaskResult
-func (tr *TaskResult) SetDefaults(context.Context) {
+func (tr *TaskResult) SetDefaults(ctx context.Context) {
 	if tr == nil {
 		return
 	}
+	if tr.Value == nil && config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions {
+		tr.Value = &ParamValue{
+			Type: ParamTypeString,
+		}
+	}
 	if tr.Type == "" {
 		if tr.Properties != nil {
 			// Set type to object if `properties` is given

diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go
@@ -32,6 +32,10 @@ type TaskResult struct {
 	// Description is a human-readable description of the result
 	// +optional
 	Description string `json:"description,omitempty"`
+
+	// Value the expression used to retrieve the value of the result from an underlying Step.
+	// +optional
+	Value *ResultValue `json:"value,omitempty"`
 }
 
 // TaskRunResult used to describe the results of a task