Skip to content

Commit

Permalink
Introduce Value in TaskResults
Browse files Browse the repository at this point in the history
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 tektoncd#7259.
Following this PR, we will add support for extracting StepAction results
via termination message and sidecar logs.
  • Loading branch information
chitrangpatel committed Nov 10, 2023
1 parent 63e1a26 commit caeda0c
Show file tree
Hide file tree
Showing 15 changed files with 442 additions and 10 deletions.
32 changes: 30 additions & 2 deletions docs/pipeline-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1741,7 +1741,7 @@ Used to distinguish between a single string and an array of strings.</p>
<h3 id="tekton.dev/v1.ParamValue">ParamValue
</h3>
<p>
(<em>Appears on:</em><a href="#tekton.dev/v1.Param">Param</a>, <a href="#tekton.dev/v1.ParamSpec">ParamSpec</a>, <a href="#tekton.dev/v1.PipelineResult">PipelineResult</a>, <a href="#tekton.dev/v1.PipelineRunResult">PipelineRunResult</a>, <a href="#tekton.dev/v1.TaskRunResult">TaskRunResult</a>)
(<em>Appears on:</em><a href="#tekton.dev/v1.Param">Param</a>, <a href="#tekton.dev/v1.ParamSpec">ParamSpec</a>, <a href="#tekton.dev/v1.PipelineResult">PipelineResult</a>, <a href="#tekton.dev/v1.PipelineRunResult">PipelineRunResult</a>, <a href="#tekton.dev/v1.TaskResult">TaskResult</a>, <a href="#tekton.dev/v1.TaskRunResult">TaskRunResult</a>)
</p>
<div>
<p>ResultValue is a type alias of ParamValue</p>
Expand Down Expand Up @@ -4931,6 +4931,20 @@ string
<p>Description is a human-readable description of the result</p>
</td>
</tr>
<tr>
<td>
<code>value</code><br/>
<em>
<a href="#tekton.dev/v1.ParamValue">
ParamValue
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>Value the expression used to retrieve the value of the result from an underlying Step.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="tekton.dev/v1.TaskRunDebug">TaskRunDebug
Expand Down Expand Up @@ -10232,7 +10246,7 @@ Used to distinguish between a single string and an array of strings.</p>
<h3 id="tekton.dev/v1beta1.ParamValue">ParamValue
</h3>
<p>
(<em>Appears on:</em><a href="#tekton.dev/v1beta1.Param">Param</a>, <a href="#tekton.dev/v1beta1.ParamSpec">ParamSpec</a>, <a href="#tekton.dev/v1beta1.PipelineResult">PipelineResult</a>, <a href="#tekton.dev/v1beta1.PipelineRunResult">PipelineRunResult</a>, <a href="#tekton.dev/v1beta1.TaskRunResult">TaskRunResult</a>)
(<em>Appears on:</em><a href="#tekton.dev/v1beta1.Param">Param</a>, <a href="#tekton.dev/v1beta1.ParamSpec">ParamSpec</a>, <a href="#tekton.dev/v1beta1.PipelineResult">PipelineResult</a>, <a href="#tekton.dev/v1beta1.PipelineRunResult">PipelineRunResult</a>, <a href="#tekton.dev/v1beta1.TaskResult">TaskResult</a>, <a href="#tekton.dev/v1beta1.TaskRunResult">TaskRunResult</a>)
</p>
<div>
<p>ResultValue is a type alias of ParamValue</p>
Expand Down Expand Up @@ -14135,6 +14149,20 @@ string
<p>Description is a human-readable description of the result</p>
</td>
</tr>
<tr>
<td>
<code>value</code><br/>
<em>
<a href="#tekton.dev/v1beta1.ParamValue">
ParamValue
</a>
</em>
</td>
<td>
<em>(Optional)</em>
<p>Value the expression used to retrieve the value of the result from an underlying Step.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="tekton.dev/v1beta1.TaskRunConditionType">TaskRunConditionType
Expand Down
8 changes: 7 additions & 1 deletion pkg/apis/pipeline/v1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1/result_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 47 additions & 4 deletions pkg/apis/pipeline/v1/result_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ package v1
import (
"context"
"fmt"
"regexp"

"github.com/tektoncd/pipeline/pkg/apis/config"
"knative.dev/pkg/apis"
)

Expand All @@ -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
Expand All @@ -66,3 +65,47 @@ 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),
},
}
}
if tr.Value.StringVal != "" {
_, _, 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
}
158 changes: 158 additions & 0 deletions pkg/apis/pipeline/v1/result_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1763,6 +1763,10 @@
"type": {
"description": "Type is the user-specified type of the result. The possible type is currently \"string\" and will support \"array\" in following work.",
"type": "string"
},
"value": {
"description": "Value the expression used to retrieve the value of the result from an underlying Step.",
"$ref": "#/definitions/v1.ParamValue"
}
}
},
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion pkg/apis/pipeline/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit caeda0c

Please sign in to comment.