From 54bda5014c86b49d7f92d3b0eedcc808b8330ef4 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Thu, 9 Nov 2023 23:17:53 -0500 Subject: [PATCH] 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 https://github.com/tektoncd/pipeline/issues/7259. Following this PR, we will add support for extracting StepAction results via termination message and sidecar logs. --- docs/pipeline-api.md | 32 ++- pkg/apis/pipeline/v1/openapi_generated.go | 8 +- pkg/apis/pipeline/v1/result_types.go | 4 + pkg/apis/pipeline/v1/result_validation.go | 70 +++++- .../pipeline/v1/result_validation_test.go | 208 ++++++++++++++++++ pkg/apis/pipeline/v1/swagger.json | 4 + pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 5 + .../pipeline/v1beta1/openapi_generated.go | 8 +- .../pipeline/v1beta1/result_conversion.go | 8 + pkg/apis/pipeline/v1beta1/result_types.go | 4 + .../pipeline/v1beta1/result_validation.go | 58 ++++- .../v1beta1/result_validation_test.go | 154 +++++++++++++ pkg/apis/pipeline/v1beta1/swagger.json | 4 + .../pipeline/v1beta1/task_conversion_test.go | 21 ++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 5 + 15 files changed, 575 insertions(+), 18 deletions(-) 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_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..7122e18ef60 100644 --- a/pkg/apis/pipeline/v1/result_validation.go +++ b/pkg/apis/pipeline/v1/result_validation.go @@ -16,7 +16,10 @@ package v1 import ( "context" "fmt" + "regexp" + "github.com/tektoncd/pipeline/pkg/apis/config" + "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" ) @@ -28,20 +31,16 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { switch { case tr.Type == ResultsTypeObject: - errs := validateObjectResult(tr) - return errs + errs = errs.Also(validateObjectResult(tr)) 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") + errs = errs.Also(apis.ErrInvalidValue(tr.Type, "type", "type must be string")) } - - return nil + return errs.Also(tr.validateValue(ctx)) } // validateObjectResult validates the object result and check if the Properties is missing @@ -66,3 +65,60 @@ 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 { + return 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 != "" { + stepName, resultName, err := ExtractStepResultName(tr.Value.StringVal) + if err != nil { + return &apis.FieldError{ + Message: fmt.Sprintf("%v", err), + Paths: []string{fmt.Sprintf("%s.value", tr.Name)}, + } + } + if e := validation.IsDNS1123Label(stepName); len(e) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("invalid extracted step name %q", stepName), + Paths: []string{fmt.Sprintf("%s.value", tr.Name)}, + Details: "stepName in $(steps..results.) must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", + }) + } + if !resultNameFormatRegex.MatchString(resultName) { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("invalid extracted result name %q", resultName), + Paths: []string{fmt.Sprintf("%s.value", tr.Name)}, + Details: fmt.Sprintf("resultName in $(steps..results.) must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '%s')", ResultNameFormat), + }) + } + } + return errs +} + +// 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("Could not extract step name and result name. Expected value to look like $(steps..results.) but got \"%v\"", value) + } + 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..664c09f54fa 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" @@ -123,3 +125,209 @@ func TestResultsValidateError(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.step-name.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 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 array", + Result: v1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1.ResultsTypeArray, + 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 type object", + Result: v1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1.ResultsTypeObject, + Properties: map[string]v1.PropertySpec{"hello": {Type: v1.ParamTypeString}}, + Value: &v1.ParamValue{ + Type: v1.ParamTypeObject, + }, + }, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: `Invalid Type. Wanted string but got: "object"`, + 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 name and result name. Expected value to look like $(steps..results.) but got "not a valid format"`, + Paths: []string{"MY-RESULT.value"}, + }, + }, { + name: "invalid string format invalid step name", + Result: v1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1.ResultsTypeString, + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.foo.foo.results.Bar)", + }, + }, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "invalid extracted step name \"foo.foo\"", + Paths: []string{"MY-RESULT.value"}, + Details: "stepName in $(steps..results.) must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", + }, + }, { + name: "invalid string format invalid result name", + Result: v1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1.ResultsTypeString, + Value: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "$(steps.foo.results.-bar)", + }, + }, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "invalid extracted result name \"-bar\"", + Paths: []string{"MY-RESULT.value"}, + Details: fmt.Sprintf("resultName in $(steps..results.) must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '%s')", v1.ResultNameFormat), + }, + }} + 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(`Could not extract step name and result name. Expected value to look like $(steps..results.) but got "not valid"`), + }} + 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 got: %v", gotStep) + } + if gotResult != "" { + t.Errorf("Expected an empty string but got: %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_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..a9f776b5277 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -17,6 +17,9 @@ import ( "context" "fmt" + "github.com/tektoncd/pipeline/pkg/apis/config" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" ) @@ -28,20 +31,16 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { switch { case tr.Type == ResultsTypeObject: - errs := validateObjectResult(tr) - return errs + errs = errs.Also(validateObjectResult(tr)) case tr.Type == ResultsTypeArray: - return errs // 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") + errs = errs.Also(apis.ErrInvalidValue(tr.Type, "type", "type must be string")) } - - return nil + return errs.Also(tr.validateValue(ctx)) } // validateObjectResult validates the object result and check if the Properties is missing @@ -66,3 +65,48 @@ 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 { + return 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 != "" { + stepName, resultName, err := v1.ExtractStepResultName(tr.Value.StringVal) + if err != nil { + return &apis.FieldError{ + Message: fmt.Sprintf("%v", err), + Paths: []string{fmt.Sprintf("%s.value", tr.Name)}, + } + } + if e := validation.IsDNS1123Label(stepName); len(e) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("invalid extracted step name %q", stepName), + Paths: []string{fmt.Sprintf("%s.value", tr.Name)}, + Details: "stepName in $(steps..results.) must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", + }) + } + if !resultNameFormatRegex.MatchString(resultName) { + errs = errs.Also(&apis.FieldError{ + Message: fmt.Sprintf("invalid extracted result name %q", resultName), + Paths: []string{fmt.Sprintf("%s.value", tr.Name)}, + Details: fmt.Sprintf("resultName in $(steps..results.) must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '%s')", ResultNameFormat), + }) + } + } + return errs +} diff --git a/pkg/apis/pipeline/v1beta1/result_validation_test.go b/pkg/apis/pipeline/v1beta1/result_validation_test.go index 2e3b28711bf..18d01f640fb 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/result_validation_test.go @@ -18,10 +18,12 @@ package v1beta1_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" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" "knative.dev/pkg/apis" @@ -124,3 +126,155 @@ func TestResultsValidateError(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.step-name.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 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 array", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeArray, + 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 type object", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeObject, + Properties: map[string]v1beta1.PropertySpec{"hello": {Type: v1beta1.ParamTypeString}}, + Value: &v1beta1.ParamValue{ + Type: v1beta1.ParamTypeObject, + }, + }, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: `Invalid Type. Wanted string but got: "object"`, + 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 name and result name. Expected value to look like $(steps..results.) but got "not a valid format"`, + Paths: []string{"MY-RESULT.value"}, + }, + }, { + name: "invalid string format invalid step name", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeString, + Value: &v1beta1.ParamValue{ + Type: v1beta1.ParamTypeString, + StringVal: "$(steps.foo.foo.results.Bar)", + }, + }, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "invalid extracted step name \"foo.foo\"", + Paths: []string{"MY-RESULT.value"}, + Details: "stepName in $(steps..results.) must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", + }, + }, { + name: "invalid string format invalid result name", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + Type: v1beta1.ResultsTypeString, + Value: &v1beta1.ParamValue{ + Type: v1beta1.ParamTypeString, + StringVal: "$(steps.foo.results.-bar)", + }, + }, + enableStepActions: true, + expectedError: apis.FieldError{ + Message: "invalid extracted result name \"-bar\"", + Paths: []string{"MY-RESULT.value"}, + Details: fmt.Sprintf("resultName in $(steps..results.) must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '%s')", v1beta1.ResultNameFormat), + }, + }} + 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 }