diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index fdb95ffb338..5705f9ced47 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -1741,7 +1741,7 @@ Used to distinguish between a single string and an array of strings.

ParamValue

-(Appears on:Param, ParamSpec, PipelineResult, PipelineRunResult, TaskRunResult) +(Appears on:Param, ParamSpec, PipelineResult, PipelineRunResult, TaskResult, TaskRunResult)

ResultValue is a type alias of ParamValue

@@ -4931,6 +4931,20 @@ string

Description is a human-readable description of the result

+ + +value
+ + +ParamValue + + + + +(Optional) +

Value the expression used to retrieve the value of the result from an underlying Step.

+ +

TaskRunDebug @@ -10232,7 +10246,7 @@ Used to distinguish between a single string and an array of strings.

ParamValue

-(Appears on:Param, ParamSpec, PipelineResult, PipelineRunResult, TaskRunResult) +(Appears on:Param, ParamSpec, PipelineResult, PipelineRunResult, TaskResult, TaskRunResult)

ResultValue is a type alias of ParamValue

@@ -14135,6 +14149,20 @@ string

Description is a human-readable description of the result

+ + +value
+ + +ParamValue + + + + +(Optional) +

Value the expression used to retrieve the value of the result from an underlying Step.

+ +

TaskRunConditionType diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 8ed6ba86be0..e16eee08c82 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -3425,12 +3425,18 @@ func schema_pkg_apis_pipeline_v1_TaskResult(ref common.ReferenceCallback) common Format: "", }, }, + "value": { + SchemaProps: spec.SchemaProps{ + Description: "Value the expression used to retrieve the value of the result from an underlying Step.", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamValue"), + }, + }, }, Required: []string{"name"}, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamValue", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec"}, } } diff --git a/pkg/apis/pipeline/v1/result_defaults.go b/pkg/apis/pipeline/v1/result_defaults.go index 7fc6733b459..6bb0c5fdab1 100644 --- 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_defaults_test.go b/pkg/apis/pipeline/v1/result_defaults_test.go index 2a160da3671..b1cb43c6ba6 100644 --- a/pkg/apis/pipeline/v1/result_defaults_test.go +++ b/pkg/apis/pipeline/v1/result_defaults_test.go @@ -18,15 +18,17 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/test/diff" ) func TestTaskResult_SetDefaults(t *testing.T) { tests := []struct { - name string - before *v1.TaskResult - after *v1.TaskResult + name string + before *v1.TaskResult + after *v1.TaskResult + enableStepActions bool }{{ name: "empty taskresult", before: nil, @@ -82,10 +84,26 @@ func TestTaskResult_SetDefaults(t *testing.T) { Type: v1.ResultsTypeObject, Properties: map[string]v1.PropertySpec{"key1": {v1.ParamTypeString}}, }, + }, { + name: "value is not provided by step actions enabled", + before: &v1.TaskResult{ + Name: "resultname", + Type: v1.ResultsTypeString, + }, + after: &v1.TaskResult{ + Name: "resultname", + Type: v1.ResultsTypeString, + Value: &v1.ParamValue{Type: v1.ParamTypeString}, + }, + enableStepActions: true, }} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tc.enableStepActions, + }, + }) tc.before.SetDefaults(ctx) if d := cmp.Diff(tc.after, tc.before); d != "" { t.Error(diff.PrintWantGot(d)) diff --git a/pkg/apis/pipeline/v1/result_types.go b/pkg/apis/pipeline/v1/result_types.go index 3a5b97d9190..59083ede906 100644 --- 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 index 0d19c2ab0af..c48a1eb95f1 100644 --- 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,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..results.) +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..results.) 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..results.). +// 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 index 1f2a2280855..f012ef4ec46 100644 --- 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..results.) 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/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index c48314355aa..23b19292f3b 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -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" } } }, diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 838f93869f2..628630cf446 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1504,6 +1504,11 @@ func (in *TaskResult) DeepCopyInto(out *TaskResult) { (*out)[key] = val } } + if in.Value != nil { + in, out := &in.Value, &out.Value + *out = new(ParamValue) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index f64529189c5..6408a47d891 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -4637,12 +4637,18 @@ func schema_pkg_apis_pipeline_v1beta1_TaskResult(ref common.ReferenceCallback) c Format: "", }, }, + "value": { + SchemaProps: spec.SchemaProps{ + Description: "Value the expression used to retrieve the value of the result from an underlying Step.", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ParamValue"), + }, + }, }, Required: []string{"name"}, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ParamValue", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec"}, } } diff --git a/pkg/apis/pipeline/v1beta1/result_conversion.go b/pkg/apis/pipeline/v1beta1/result_conversion.go index c695de103c4..8854a283cb7 100644 --- 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 index 12af7aa36ef..e7bdfbe2a3b 100644 --- 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_defaults_test.go b/pkg/apis/pipeline/v1beta1/result_defaults_test.go index 13dd6799233..0862c96fa86 100644 --- a/pkg/apis/pipeline/v1beta1/result_defaults_test.go +++ b/pkg/apis/pipeline/v1beta1/result_defaults_test.go @@ -18,15 +18,17 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" ) func TestTaskResult_SetDefaults(t *testing.T) { tests := []struct { - name string - before *v1beta1.TaskResult - after *v1beta1.TaskResult + name string + before *v1beta1.TaskResult + after *v1beta1.TaskResult + enableStepActions bool }{{ name: "empty taskresult", before: nil, @@ -82,10 +84,26 @@ func TestTaskResult_SetDefaults(t *testing.T) { Type: v1beta1.ResultsTypeObject, Properties: map[string]v1beta1.PropertySpec{"key1": {v1beta1.ParamTypeString}}, }, + }, { + name: "value is not provided by step actions enabled", + before: &v1beta1.TaskResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeString, + }, + after: &v1beta1.TaskResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeString, + Value: &v1beta1.ParamValue{Type: v1beta1.ParamTypeString}, + }, + enableStepActions: true, }} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tc.enableStepActions, + }, + }) tc.before.SetDefaults(ctx) if d := cmp.Diff(tc.after, tc.before); d != "" { t.Error(diff.PrintWantGot(d)) diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index b4e3764c891..2f484f47ddd 100644 --- 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 diff --git a/pkg/apis/pipeline/v1beta1/result_validation.go b/pkg/apis/pipeline/v1beta1/result_validation.go index ab9ee83c356..dff135c414e 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -17,6 +17,8 @@ import ( "context" "fmt" + "github.com/tektoncd/pipeline/pkg/apis/config" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "knative.dev/pkg/apis" ) @@ -35,13 +37,12 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { // 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 +67,35 @@ 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..results.) +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 := v1.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..results.) but got %v. Got error: %v", tr.Value.StringVal, err), + Paths: []string{ + fmt.Sprintf("%s.value", tr.Name), + }, + } + } + } + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/result_validation_test.go b/pkg/apis/pipeline/v1beta1/result_validation_test.go index 2e3b28711bf..c2951c1ac5b 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/result_validation_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" "knative.dev/pkg/apis" @@ -70,6 +71,36 @@ func TestResultsValidate(t *testing.T) { } } +func TestResultsValidateValue(t *testing.T) { + tests := []struct { + name string + Result v1beta1.TaskResult + }{{ + name: "valid result value", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeString, + Value: &v1beta1.ParamValue{ + Type: v1beta1.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 @@ -124,3 +155,75 @@ func TestResultsValidateError(t *testing.T) { }) } } + +func TestResultsValidateValueError(t *testing.T) { + tests := []struct { + name string + Result v1beta1.TaskResult + enableStepActions bool + expectedError apis.FieldError + }{{ + name: "enable-step-actions-not-enabled", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeString, + Value: &v1beta1.ParamValue{ + Type: v1beta1.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: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeString, + Value: &v1beta1.ParamValue{ + Type: v1beta1.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: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeString, + Value: &v1beta1.ParamValue{ + Type: v1beta1.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..results.) 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)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 0d5641dd2ed..6df62472fbe 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2564,6 +2564,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/v1beta1.ParamValue" } } }, diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go index 5ec20da9acb..1f59a33c18f 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go @@ -88,6 +88,20 @@ spec: value: hello ` + stepActionTaskResultYAML := ` +metadata: + name: foo + namespace: bar +spec: + results: + - name: stepActionResult + type: string + value: "$(steps.stepName.results.resultName)" + steps: + - name: stepName + ref: + name: "step-action" +` remoteStepActionTaskYAML := ` metadata: name: foo @@ -295,6 +309,9 @@ spec: stepActionTaskV1beta1 := parse.MustParseV1beta1Task(t, stepActionTaskYAML) stepActionTaskV1 := parse.MustParseV1Task(t, stepActionTaskYAML) + stepActionTaskResultV1beta1 := parse.MustParseV1beta1Task(t, stepActionTaskResultYAML) + stepActionTaskResultV1 := parse.MustParseV1Task(t, stepActionTaskResultYAML) + remoteStepActionTaskV1beta1 := parse.MustParseV1beta1Task(t, remoteStepActionTaskYAML) remoteStepActionTaskV1 := parse.MustParseV1Task(t, remoteStepActionTaskYAML) @@ -334,6 +351,10 @@ spec: name: "step action in task", v1beta1Task: stepActionTaskV1beta1, v1Task: stepActionTaskV1, + }, { + name: "value in task result", + v1beta1Task: stepActionTaskResultV1beta1, + v1Task: stepActionTaskResultV1, }, { name: "remote step action in task", v1beta1Task: remoteStepActionTaskV1beta1, diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 923c113107c..ca255ddee6c 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -2066,6 +2066,11 @@ func (in *TaskResult) DeepCopyInto(out *TaskResult) { (*out)[key] = val } } + if in.Value != nil { + in, out := &in.Value, &out.Value + *out = new(ParamValue) + (*in).DeepCopyInto(*out) + } return }