From 13f391d9464ce669fc64c3fd0311965a3b311192 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Thu, 16 Nov 2023 20:07:18 -0500 Subject: [PATCH] TEP-0142: Introduce StepResults in Steps This PR introduces `StepResults` in Steps. --- docs/pipeline-api.md | 264 +++++++++++----- pkg/apis/pipeline/v1/container_types.go | 9 + pkg/apis/pipeline/v1/openapi_generated.go | 75 ++++- pkg/apis/pipeline/v1/result_defaults.go | 23 ++ pkg/apis/pipeline/v1/result_defaults_test.go | 72 +++++ pkg/apis/pipeline/v1/result_types.go | 19 ++ pkg/apis/pipeline/v1/result_validation.go | 44 +++ .../pipeline/v1/result_validation_test.go | 109 +++++++ pkg/apis/pipeline/v1/swagger.json | 39 +++ pkg/apis/pipeline/v1/task_validation.go | 42 ++- pkg/apis/pipeline/v1/task_validation_test.go | 281 ++++++++++++++++- pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 30 ++ .../pipeline/v1alpha1/openapi_generated.go | 58 +--- .../pipeline/v1alpha1/result_defaults_test.go | 96 ------ pkg/apis/pipeline/v1alpha1/result_types.go | 62 ---- .../pipeline/v1alpha1/result_validation.go | 77 ----- .../v1alpha1/result_validation_test.go | 125 -------- .../pipeline/v1alpha1/stepaction_types.go | 2 +- .../v1alpha1/stepaction_validation.go | 22 +- .../v1alpha1/stepaction_validation_test.go | 29 +- pkg/apis/pipeline/v1alpha1/swagger.json | 32 +- .../v1alpha1/zz_generated.deepcopy.go | 25 +- .../pipeline/v1beta1/container_conversion.go | 12 + pkg/apis/pipeline/v1beta1/container_types.go | 9 + .../pipeline/v1beta1/openapi_generated.go | 75 ++++- .../pipeline/v1beta1/result_conversion.go | 26 ++ pkg/apis/pipeline/v1beta1/result_defaults.go | 22 ++ .../pipeline/v1beta1/result_defaults_test.go | 72 +++++ pkg/apis/pipeline/v1beta1/result_types.go | 16 + .../pipeline/v1beta1/result_validation.go | 41 +++ .../v1beta1/result_validation_test.go | 109 +++++++ pkg/apis/pipeline/v1beta1/swagger.json | 39 +++ .../pipeline/v1beta1/task_conversion_test.go | 28 ++ pkg/apis/pipeline/v1beta1/task_validation.go | 40 ++- .../pipeline/v1beta1/task_validation_test.go | 282 ++++++++++++++++-- .../pipeline/v1beta1/zz_generated.deepcopy.go | 30 ++ 36 files changed, 1720 insertions(+), 616 deletions(-) delete mode 100644 pkg/apis/pipeline/v1alpha1/result_defaults_test.go delete mode 100644 pkg/apis/pipeline/v1alpha1/result_types.go delete mode 100644 pkg/apis/pipeline/v1alpha1/result_validation.go delete mode 100644 pkg/apis/pipeline/v1alpha1/result_validation_test.go diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 5705f9ced47..735eef41145 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -3214,7 +3214,7 @@ this field is false and so declared workspaces are required.

PropertySpec

-(Appears on:ParamSpec, TaskResult, StepActionResult) +(Appears on:ParamSpec, StepResult, TaskResult)

PropertySpec defines the struct for object keys

@@ -3506,7 +3506,7 @@ string

ResultsType (string alias)

-(Appears on:PipelineResult, TaskResult, TaskRunResult, StepActionResult) +(Appears on:PipelineResult, StepResult, TaskResult, TaskRunResult)

ResultsType indicates the type of a result; @@ -4441,6 +4441,25 @@ Params

Params declares parameters passed to this step action.

+ + +results
+ + +[]StepResult + + + + +(Optional) +

Results declares StepResults produced by the step. +This is field is at an alpha stability level and gated by “enable-step-actions” feature flag. +It can be used in an inlined step when used to store results to +$(step.results.resultName.path). When referencing StepActions, +it cannot be used. The results declared by the StepActions will be stored here instead. +Using it along side referencing StepActions will result in a validation error.

+ +

StepOutputConfig @@ -4473,6 +4492,76 @@ string +

StepResult +

+

+(Appears on:Step, StepActionSpec) +

+
+

StepResult used to describe the results of a step action

+
+ + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+name
+ +string + +		 +

Name the given name

+
+type
+ + +ResultsType + + +		 +(Optional) +

Type is the user-specified type of the result. The possible type +is currently “string” and will support “array” in following work.

+
+properties
+ + +map[string]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec + + +		 +(Optional) +

Properties is the JSON Schema properties to support key-value pairs results.

+
+description
+ +string + +		 +(Optional) +

Description is a human-readable description of the result

+

StepState

@@ -6616,8 +6705,8 @@ Params must be supplied as inputs in Steps unless they declare a defaultvalue. results
- -[]StepActionResult + +[]StepResult @@ -7323,76 +7412,6 @@ Refer Go’s ParseDuration documentation for expected format: StepActionResult - -

-(Appears on:StepActionSpec) -

-
-

StepActionResult used to describe the results of a task

-
- - - - - - - - - - - - - - - - - - - - - - - - - -
FieldDescription
-name
- -string - -		 -

Name the given name

-
-type
- - -ResultsType - - -		 -(Optional) -

Type is the user-specified type of the result. The possible type -is currently “string” and will support “array” in following work.

-
-properties
- - -map[string]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec - - -		 -(Optional) -

Properties is the JSON Schema properties to support key-value pairs results.

-
-description
- -string - -		 -(Optional) -

Description is a human-readable description of the result

-

StepActionSpec

@@ -7507,8 +7526,8 @@ Params must be supplied as inputs in Steps unless they declare a defaultvalue. results
- -[]StepActionResult + +[]StepResult @@ -11980,7 +11999,7 @@ this field is false and so declared workspaces are required.

PropertySpec

-(Appears on:ParamSpec, TaskResult) +(Appears on:ParamSpec, StepResult, TaskResult)

PropertySpec defines the struct for object keys

@@ -12284,7 +12303,7 @@ string

ResultsType (string alias)

-(Appears on:PipelineResult, TaskResult, TaskRunResult) +(Appears on:PipelineResult, StepResult, TaskResult, TaskRunResult)

ResultsType indicates the type of a result; @@ -13333,6 +13352,25 @@ Params

Params declares parameters passed to this step action.

+ + +results
+ + +[]StepResult + + + + +(Optional) +

Results declares StepResults produced by the step. +This is field is at an alpha stability level and gated by “enable-step-actions” feature flag. +It can be used in an inlined step when used to store results to +$(step.results.resultName.path). When referencing StepActions, +it cannot be used. The results declared by the StepActions will be stored here instead. +Using it along side referencing StepActions will result in a validation error.

+ +

StepOutputConfig @@ -13365,6 +13403,76 @@ string +

StepResult +

+

+(Appears on:Step) +

+
+

StepResult used to describe the results of a step action

+
+ + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
+name
+ +string + +		 +

Name the given name

+
+type
+ + +ResultsType + + +		 +(Optional) +

Type is the user-specified type of the result. The possible type +is currently “string” and will support “array” in following work.

+
+properties
+ + +map[string]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec + + +		 +(Optional) +

Properties is the JSON Schema properties to support key-value pairs results.

+
+description
+ +string + +		 +(Optional) +

Description is a human-readable description of the result

+

StepState

diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go index a3cfa06384a..edca691571a 100644 --- a/pkg/apis/pipeline/v1/container_types.go +++ b/pkg/apis/pipeline/v1/container_types.go @@ -142,6 +142,15 @@ type Step struct { // +optional // +listType=atomic Params Params `json:"params,omitempty"` + // Results declares StepResults produced by the step. + // This is field is at an alpha stability level and gated by "enable-step-actions" feature flag. + // It can be used in an inlined step when used to store results to + // $(step.results.resultName.path). When referencing StepActions, + // it cannot be used. The results declared by the StepActions will be stored here instead. + // Using it along side referencing StepActions will result in a validation error. + // +optional + // +listType=atomic + Results []StepResult `json:"results,omitempty"` } // Ref can be used to refer to a specific instance of a StepAction. diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index e16eee08c82..2d33ca1cdb9 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -70,6 +70,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.SkippedTask": schema_pkg_apis_pipeline_v1_SkippedTask(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Step": schema_pkg_apis_pipeline_v1_Step(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepOutputConfig": schema_pkg_apis_pipeline_v1_StepOutputConfig(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepResult": schema_pkg_apis_pipeline_v1_StepResult(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepState": schema_pkg_apis_pipeline_v1_StepState(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepTemplate": schema_pkg_apis_pipeline_v1_StepTemplate(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Task": schema_pkg_apis_pipeline_v1_Task(ref), @@ -2985,12 +2986,31 @@ func schema_pkg_apis_pipeline_v1_Step(ref common.ReferenceCallback) common.OpenA }, }, }, + "results": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "Results declares StepResults produced by the step. This is field is at an alpha stability level and gated by \"enable-step-actions\" feature flag. It can be used in an inlined step when used to store results to $(step.results.resultName.path). When referencing StepActions, it cannot be used. The results declared by the StepActions will be stored here instead. Using it along side referencing StepActions will result in a validation error.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepResult"), + }, + }, + }, + }, + }, }, Required: []string{"name"}, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Param", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Ref", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.WorkspaceUsage", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Param", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.Ref", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.WorkspaceUsage", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, } } @@ -3014,6 +3034,59 @@ func schema_pkg_apis_pipeline_v1_StepOutputConfig(ref common.ReferenceCallback) } } +func schema_pkg_apis_pipeline_v1_StepResult(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "StepResult used to describe the results of a step action", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Description: "Name the given name", + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + "type": { + SchemaProps: spec.SchemaProps{ + 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{"string"}, + Format: "", + }, + }, + "properties": { + SchemaProps: spec.SchemaProps{ + Description: "Properties is the JSON Schema properties to support key-value pairs results.", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec"), + }, + }, + }, + }, + }, + "description": { + SchemaProps: spec.SchemaProps{ + Description: "Description is a human-readable description of the result", + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"name"}, + }, + }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec"}, + } +} + func schema_pkg_apis_pipeline_v1_StepState(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/pkg/apis/pipeline/v1/result_defaults.go b/pkg/apis/pipeline/v1/result_defaults.go index 7fc6733b459..51dc7bd7e80 100644 --- a/pkg/apis/pipeline/v1/result_defaults.go +++ b/pkg/apis/pipeline/v1/result_defaults.go @@ -37,3 +37,26 @@ func (tr *TaskResult) SetDefaults(context.Context) { } } } + +// SetDefaults set the default type for StepResult +func (sr *StepResult) SetDefaults(context.Context) { + if sr == nil { + return + } + if sr.Type == "" { + if sr.Properties != nil { + // Set type to object if `properties` is given + sr.Type = ResultsTypeObject + } else { + // ResultsTypeString is the default value + sr.Type = ResultsTypeString + } + } + + // Set default type of object values to string + for key, propertySpec := range sr.Properties { + if propertySpec.Type == "" { + sr.Properties[key] = PropertySpec{Type: ParamType(ResultsTypeString)} + } + } +} diff --git a/pkg/apis/pipeline/v1/result_defaults_test.go b/pkg/apis/pipeline/v1/result_defaults_test.go index 2a160da3671..a611983dec1 100644 --- a/pkg/apis/pipeline/v1/result_defaults_test.go +++ b/pkg/apis/pipeline/v1/result_defaults_test.go @@ -93,3 +93,75 @@ func TestTaskResult_SetDefaults(t *testing.T) { }) } } + +func TestStepResult_SetDefaults(t *testing.T) { + tests := []struct { + name string + before *v1.StepResult + after *v1.StepResult + }{{ + name: "empty taskresult", + before: nil, + after: nil, + }, { + name: "inferred string type", + before: &v1.StepResult{ + Name: "resultname", + }, + after: &v1.StepResult{ + Name: "resultname", + Type: v1.ResultsTypeString, + }, + }, { + name: "string type specified not changed", + before: &v1.StepResult{ + Name: "resultname", + Type: v1.ResultsTypeString, + }, + after: &v1.StepResult{ + Name: "resultname", + Type: v1.ResultsTypeString, + }, + }, { + name: "array type specified not changed", + before: &v1.StepResult{ + Name: "resultname", + Type: v1.ResultsTypeArray, + }, + after: &v1.StepResult{ + Name: "resultname", + Type: v1.ResultsTypeArray, + }, + }, { + name: "inferred object type from properties - PropertySpec type is provided", + before: &v1.StepResult{ + Name: "resultname", + Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, + }, + after: &v1.StepResult{ + Name: "resultname", + Type: v1.ResultsTypeObject, + Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, + }, + }, { + name: "inferred type from properties - PropertySpec type is not provided", + before: &v1.StepResult{ + Name: "resultname", + Properties: map[string]v1.PropertySpec{"key1": {}}, + }, + after: &v1.StepResult{ + Name: "resultname", + Type: v1.ResultsTypeObject, + Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, + }, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + 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 59083ede906..99a576f7abb 100644 --- a/pkg/apis/pipeline/v1/result_types.go +++ b/pkg/apis/pipeline/v1/result_types.go @@ -38,6 +38,25 @@ type TaskResult struct { Value *ResultValue `json:"value,omitempty"` } +// StepResult used to describe the results of a step action +type StepResult struct { + // Name the given name + Name string `json:"name"` + + // Type is the user-specified type of the result. The possible type + // is currently "string" and will support "array" in following work. + // +optional + Type ResultsType `json:"type,omitempty"` + + // Properties is the JSON Schema properties to support key-value pairs results. + // +optional + Properties map[string]PropertySpec `json:"properties,omitempty"` + + // Description is a human-readable description of the result + // +optional + Description string `json:"description,omitempty"` +} + // TaskRunResult used to describe the results of a task type TaskRunResult struct { // Name the given name diff --git a/pkg/apis/pipeline/v1/result_validation.go b/pkg/apis/pipeline/v1/result_validation.go index 7122e18ef60..ec391042b70 100644 --- a/pkg/apis/pipeline/v1/result_validation.go +++ b/pkg/apis/pipeline/v1/result_validation.go @@ -111,6 +111,50 @@ func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError) return errs } +// Validate implements apis.Validatable +func (sr StepResult) Validate(ctx context.Context) (errs *apis.FieldError) { + if !resultNameFormatRegex.MatchString(sr.Name) { + return apis.ErrInvalidKeyName(sr.Name, "name", fmt.Sprintf("Name 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)) + } + + switch { + case sr.Type == ResultsTypeObject: + return validateObjectStepResult(sr) + case sr.Type == ResultsTypeArray: + return nil + // The Type is string by default if it is empty. + case sr.Type == "": + return nil + case sr.Type == ResultsTypeString: + return nil + default: + return apis.ErrInvalidValue(sr.Type, "type", fmt.Sprintf("invalid type %s", sr.Type)) + } +} + +// validateObjectStepResult validates the object result and check if the Properties is missing +// for Properties values it will check if the type is string. +func validateObjectStepResult(sr StepResult) (errs *apis.FieldError) { + if ParamType(sr.Type) == ParamTypeObject && sr.Properties == nil { + return apis.ErrMissingField(fmt.Sprintf("%s.properties", sr.Name)) + } + + invalidKeys := []string{} + for key, propertySpec := range sr.Properties { + if propertySpec.Type != ParamTypeString { + invalidKeys = append(invalidKeys, key) + } + } + + if len(invalidKeys) != 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("the value type specified for these keys %v is invalid, the type must be string", invalidKeys), + Paths: []string{fmt.Sprintf("%s.properties", sr.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. diff --git a/pkg/apis/pipeline/v1/result_validation_test.go b/pkg/apis/pipeline/v1/result_validation_test.go index 664c09f54fa..4026e0b923c 100644 --- a/pkg/apis/pipeline/v1/result_validation_test.go +++ b/pkg/apis/pipeline/v1/result_validation_test.go @@ -331,3 +331,112 @@ func TestExtractStepResultNameError(t *testing.T) { }) } } + +func TestStepResultsValidate(t *testing.T) { + tests := []struct { + name string + Result v1.StepResult + }{{ + name: "valid result type empty", + Result: v1.StepResult{ + Name: "MY-RESULT", + Description: "my great result", + }, + }, { + name: "valid result type string", + Result: v1.StepResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeString, + Description: "my great result", + }, + }, { + name: "valid result type array", + Result: v1.StepResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeArray, + Description: "my great result", + }, + }, { + name: "valid result type object", + Result: v1.StepResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1.PropertySpec{"hello": {Type: v1.ParamTypeString}}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + if err := tt.Result.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + +func TestStepResultsValidateError(t *testing.T) { + tests := []struct { + name string + Result v1.StepResult + expectedError apis.FieldError + }{{ + name: "invalid result name", + Result: v1.StepResult{ + Name: "_MY-RESULT", + Type: v1.ResultsTypeString, + Description: "wrong name", + }, + expectedError: apis.FieldError{ + Message: `invalid key name "_MY-RESULT"`, + Paths: []string{"name"}, + Details: "Name 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 '^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$')", + }, + }, { + name: "invalid result type", + Result: v1.StepResult{ + Name: "MY-RESULT", + Type: "wrong", + Description: "my great result", + }, + expectedError: apis.FieldError{ + Message: `invalid value: wrong`, + Paths: []string{"type"}, + Details: "invalid type wrong", + }, + }, { + name: "invalid object properties type", + Result: v1.StepResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1.PropertySpec{"hello": {Type: "wrong type"}}, + }, + expectedError: apis.FieldError{ + Message: "the value type specified for these keys [hello] is invalid, the type must be string", + Paths: []string{"MY-RESULT.properties"}, + }, + }, { + name: "invalid object properties empty", + Result: v1.StepResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + }, + expectedError: apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"MY-RESULT.properties"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.Result.Validate(context.Background()) + 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/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 23b19292f3b..d6fa25ab9c8 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1476,6 +1476,15 @@ "description": "Contains the reference to an existing StepAction.", "$ref": "#/definitions/v1.Ref" }, + "results": { + "description": "Results declares StepResults produced by the step. This is field is at an alpha stability level and gated by \"enable-step-actions\" feature flag. It can be used in an inlined step when used to store results to $(step.results.resultName.path). When referencing StepActions, it cannot be used. The results declared by the StepActions will be stored here instead. Using it along side referencing StepActions will result in a validation error.", + "type": "array", + "items": { + "default": {}, + "$ref": "#/definitions/v1.StepResult" + }, + "x-kubernetes-list-type": "atomic" + }, "script": { "description": "Script is the contents of an executable file to execute.\n\nIf Script is not empty, the Step cannot have an Command and the Args will be passed to the Script.", "type": "string" @@ -1543,6 +1552,36 @@ } } }, + "v1.StepResult": { + "description": "StepResult used to describe the results of a step action", + "type": "object", + "required": [ + "name" + ], + "properties": { + "description": { + "description": "Description is a human-readable description of the result", + "type": "string" + }, + "name": { + "description": "Name the given name", + "type": "string", + "default": "" + }, + "properties": { + "description": "Properties is the JSON Schema properties to support key-value pairs results.", + "type": "object", + "additionalProperties": { + "default": {}, + "$ref": "#/definitions/v1.PropertySpec" + } + }, + "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" + } + } + }, "v1.StepState": { "description": "StepState reports the results of running a step in a Task.", "type": "object", diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 7fb4c70be9d..1711eae77f3 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -259,16 +259,24 @@ func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) { names := sets.NewString() for idx, s := range steps { errs = errs.Also(validateStep(ctx, s, names).ViaIndex(idx)) + if s.Results != nil { + errs = errs.Also(ValidateStepResultsVariables(ctx, s.Results, s.Script).ViaIndex(idx)) + errs = errs.Also(ValidateStepResults(ctx, s.Results).ViaIndex(idx).ViaField("results")) + } } return errs } +func isCreateOrUpdate(ctx context.Context) bool { + return apis.IsInCreate(ctx) || apis.IsInUpdate(ctx) +} + func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) { if s.Ref != nil { - if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdate(ctx) { return apis.ErrGeneric("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions) } - errs = s.Ref.Validate(ctx) + errs = errs.Also(s.Ref.Validate(ctx)) if s.Image != "" { errs = errs.Also(&apis.FieldError{ Message: "image cannot be used with Ref", @@ -305,6 +313,12 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi Paths: []string{"volumeMounts"}, }) } + if len(s.Results) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "results cannot be used with Ref", + Paths: []string{"results"}, + }) + } } else { if len(s.Params) > 0 { errs = errs.Also(&apis.FieldError{ @@ -312,6 +326,11 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi Paths: []string{"params"}, }) } + if len(s.Results) > 0 { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdate(ctx) { + return apis.ErrGeneric("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions) + } + } if s.Image == "" { errs = errs.Also(apis.ErrMissingField("Image")) } @@ -673,3 +692,22 @@ func (ts *TaskSpec) GetIndexingReferencesToArrayParams() sets.String { } return sets.NewString(arrayIndexParamRefs...) } + +// ValidateStepResults validates that all of the declared StepResults are valid. +func ValidateStepResults(ctx context.Context, results []StepResult) (errs *apis.FieldError) { + for index, result := range results { + errs = errs.Also(result.Validate(ctx).ViaIndex(index)) + } + return errs +} + +// ValidateStepResultsVariables validates if the StepResults referenced in step script are defined in step's results. +func ValidateStepResultsVariables(ctx context.Context, results []StepResult, script string) (errs *apis.FieldError) { + resultsNames := sets.NewString() + for _, r := range results { + resultsNames.Insert(r.Name) + } + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "step.results", resultsNames).ViaField("script")) + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "results", resultsNames).ViaField("script")) + return errs +} diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 5bc80e018fc..9da4d19774d 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -1413,26 +1413,82 @@ func TestTaskSpecValidateError(t *testing.T) { } } -func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { +func TestTaskSpecValidateErrorWithStepActionRef_CreateUpdateEvent(t *testing.T) { tests := []struct { - name string - Steps []v1.Step - enableStepActions bool - expectedError apis.FieldError + name string + Steps []v1.Step + isCreate bool + isUpdate bool + expectedError apis.FieldError }{{ - name: "invalid Task Spec - enableStepActions not on", + name: "is create ctx", Steps: []v1.Step{{ - Image: "image", Ref: &v1.Ref{ Name: "stepAction", }, }}, - enableStepActions: false, + isCreate: true, + isUpdate: false, expectedError: apis.FieldError{ Message: "feature flag %s should be set to true to reference StepActions in Steps.", Paths: []string{"steps[0].enable-step-actions"}, }, }, { + name: "is update ctx", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + isCreate: false, + isUpdate: true, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true to reference StepActions in Steps.", + Paths: []string{"steps[0].enable-step-actions"}, + }, + }, { + name: "ctx is not create or update", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + isCreate: false, + isUpdate: false, + expectedError: apis.FieldError{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := v1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: false, + }, + }) + if tt.isCreate { + ctx = apis.WithinCreate(ctx) + } + if tt.isUpdate { + ctx = apis.WithinUpdate(ctx, apis.GetBaseline(ctx)) + } + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + 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 TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { + tests := []struct { + name string + Steps []v1.Step + expectedError apis.FieldError + }{{ name: "Cannot use image with Ref", Steps: []v1.Step{{ Ref: &v1.Ref{ @@ -1440,7 +1496,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Image: "foo", }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "image cannot be used with Ref", Paths: []string{"steps[0].image"}, @@ -1453,7 +1508,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Command: []string{"foo"}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "command cannot be used with Ref", Paths: []string{"steps[0].command"}, @@ -1466,7 +1520,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Args: []string{"foo"}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "args cannot be used with Ref", Paths: []string{"steps[0].args"}, @@ -1479,7 +1532,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Script: "echo hi", }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "script cannot be used with Ref", Paths: []string{"steps[0].script"}, @@ -1495,7 +1547,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { Value: "value1", }}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "env cannot be used with Ref", Paths: []string{"steps[0].env"}, @@ -1508,7 +1559,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { Name: "param", }}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "params cannot be used without Ref", Paths: []string{"steps[0].params"}, @@ -1524,11 +1574,24 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { MountPath: "/registry-config", }}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "volumeMounts cannot be used with Ref", Paths: []string{"steps[0].volumeMounts"}, }, + }, { + name: "Cannot use results with Ref", + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Results: []v1.StepResult{{ + Name: "result", + }}, + }}, + expectedError: apis.FieldError{ + Message: "results cannot be used with Ref", + Paths: []string{"steps[0].results"}, + }, }, } for _, tt := range tests { @@ -1538,9 +1601,10 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { } ctx := config.ToContext(context.Background(), &config.Config{ FeatureFlags: &config.FeatureFlags{ - EnableStepActions: tt.enableStepActions, + EnableStepActions: true, }, }) + ctx = apis.WithinCreate(ctx) ts.SetDefaults(ctx) err := ts.Validate(ctx) if err == nil { @@ -2412,3 +2476,188 @@ func TestParamEnum_Failure(t *testing.T) { } } } + +func TestTaskSpecValidate_StepResults(t *testing.T) { + type fields struct { + Image string + Args []string + Script string + Results []v1.StepResult + } + tests := []struct { + name string + fields fields + }{{ + name: "valid result", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1.StepResult{{ + Name: "MY-RESULT", + Description: "my great result", + }}, + }, + }, { + name: "valid result type string", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Script: "echo $(step.results.MY-RESULT.path)", + Results: []v1.StepResult{{ + Name: "MY-RESULT", + Type: "string", + Description: "my great result", + }}, + }, + }, { + name: "valid result type array", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1.StepResult{{ + Name: "MY-RESULT", + Type: v1.ResultsTypeArray, + Description: "my great result", + }}, + }, + }, { + name: "valid result type object", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1.StepResult{{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1.PropertySpec{ + "url": {Type: "string"}, + "commit": {Type: "string"}, + }, + }}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1.TaskSpec{ + Steps: []v1.Step{{ + Image: tt.fields.Image, + Args: tt.fields.Args, + Script: tt.fields.Script, + Results: tt.fields.Results, + }}, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + ts.SetDefaults(ctx) + if err := ts.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + +func TestTaskSpecValidate_StepResults_Error(t *testing.T) { + type fields struct { + Image string + Script string + Results []v1.StepResult + } + tests := []struct { + name string + fields fields + expectedError apis.FieldError + isCreate bool + isUpdate bool + enableStepActions bool + }{{ + name: "step result not allowed withoue enable step actions - create event", + fields: fields{ + Image: "my-image", + Results: []v1.StepResult{{Name: "a-result"}}, + }, + enableStepActions: false, + isCreate: true, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true in order to use Results in Steps.", + Paths: []string{"steps[0].enable-step-actions"}, + }, + }, { + name: "step result not allowed withoue enable step actions - update event", + fields: fields{ + Image: "my-image", + Results: []v1.StepResult{{Name: "a-result"}}, + }, + enableStepActions: false, + isUpdate: true, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true in order to use Results in Steps.", + Paths: []string{"steps[0].enable-step-actions"}, + }, + }, { + name: "step result not allowed withoue enable step actions - neither create nor update", + fields: fields{ + Image: "my-image", + Results: []v1.StepResult{{Name: "a-result"}}, + }, + enableStepActions: false, + expectedError: apis.FieldError{}, + }, { + name: "step script refers to nonexistent result", + fields: fields{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + date | tee $(results.non-exist.path)`, + Results: []v1.StepResult{{Name: "a-result"}}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(results.non-exist.path)"`, + Paths: []string{"steps[0].script"}, + }, + enableStepActions: true, + }, { + name: "step script refers to nonexistent stepresult", + fields: fields{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + date | tee $(step.results.non-exist.path)`, + Results: []v1.StepResult{{Name: "a-result"}}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(step.results.non-exist.path)"`, + Paths: []string{"steps[0].script"}, + }, + enableStepActions: true, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1.TaskSpec{ + Steps: []v1.Step{{ + Image: tt.fields.Image, + Script: tt.fields.Script, + Results: tt.fields.Results, + }}, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tt.enableStepActions, + }, + }) + if tt.isCreate { + ctx = apis.WithinCreate(ctx) + } + if tt.isUpdate { + ctx = apis.WithinUpdate(ctx, apis.GetBaseline(ctx)) + } + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("StepActionSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 628630cf446..dd332d9188a 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1297,6 +1297,13 @@ func (in *Step) DeepCopyInto(out *Step) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Results != nil { + in, out := &in.Results, &out.Results + *out = make([]StepResult, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -1326,6 +1333,29 @@ func (in *StepOutputConfig) DeepCopy() *StepOutputConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *StepResult) DeepCopyInto(out *StepResult) { + *out = *in + if in.Properties != nil { + in, out := &in.Properties, &out.Properties + *out = make(map[string]PropertySpec, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StepResult. +func (in *StepResult) DeepCopy() *StepResult { + if in == nil { + return nil + } + out := new(StepResult) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StepState) DeepCopyInto(out *StepState) { *out = *in diff --git a/pkg/apis/pipeline/v1alpha1/openapi_generated.go b/pkg/apis/pipeline/v1alpha1/openapi_generated.go index cb18896855e..3426a815b7f 100644 --- a/pkg/apis/pipeline/v1alpha1/openapi_generated.go +++ b/pkg/apis/pipeline/v1alpha1/openapi_generated.go @@ -41,7 +41,6 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.RunSpec": schema_pkg_apis_pipeline_v1alpha1_RunSpec(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepAction": schema_pkg_apis_pipeline_v1alpha1_StepAction(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionList": schema_pkg_apis_pipeline_v1alpha1_StepActionList(ref), - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionResult": schema_pkg_apis_pipeline_v1alpha1_StepActionResult(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionSpec": schema_pkg_apis_pipeline_v1alpha1_StepActionSpec(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.VerificationPolicy": schema_pkg_apis_pipeline_v1alpha1_VerificationPolicy(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.VerificationPolicyList": schema_pkg_apis_pipeline_v1alpha1_VerificationPolicyList(ref), @@ -748,59 +747,6 @@ func schema_pkg_apis_pipeline_v1alpha1_StepActionList(ref common.ReferenceCallba } } -func schema_pkg_apis_pipeline_v1alpha1_StepActionResult(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Description: "StepActionResult used to describe the results of a task", - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "name": { - SchemaProps: spec.SchemaProps{ - Description: "Name the given name", - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "type": { - SchemaProps: spec.SchemaProps{ - 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{"string"}, - Format: "", - }, - }, - "properties": { - SchemaProps: spec.SchemaProps{ - Description: "Properties is the JSON Schema properties to support key-value pairs results.", - Type: []string{"object"}, - AdditionalProperties: &spec.SchemaOrBool{ - Allows: true, - Schema: &spec.Schema{ - SchemaProps: spec.SchemaProps{ - Default: map[string]interface{}{}, - Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec"), - }, - }, - }, - }, - }, - "description": { - SchemaProps: spec.SchemaProps{ - Description: "Description is a human-readable description of the result", - Type: []string{"string"}, - Format: "", - }, - }, - }, - Required: []string{"name"}, - }, - }, - Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PropertySpec"}, - } -} - func schema_pkg_apis_pipeline_v1alpha1_StepActionSpec(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ @@ -915,7 +861,7 @@ func schema_pkg_apis_pipeline_v1alpha1_StepActionSpec(ref common.ReferenceCallba Schema: &spec.Schema{ SchemaProps: spec.SchemaProps{ Default: map[string]interface{}{}, - Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionResult"), + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepResult"), }, }, }, @@ -952,7 +898,7 @@ func schema_pkg_apis_pipeline_v1alpha1_StepActionSpec(ref common.ReferenceCallba }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionResult", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeMount"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.StepResult", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeMount"}, } } diff --git a/pkg/apis/pipeline/v1alpha1/result_defaults_test.go b/pkg/apis/pipeline/v1alpha1/result_defaults_test.go deleted file mode 100644 index 302be5b969a..00000000000 --- a/pkg/apis/pipeline/v1alpha1/result_defaults_test.go +++ /dev/null @@ -1,96 +0,0 @@ -/* -Copyright 2022 The Tekton Authors -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1alpha1_test - -import ( - "context" - "testing" - - "github.com/google/go-cmp/cmp" - v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" - v1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/test/diff" -) - -func TestStepActionResult_SetDefaults(t *testing.T) { - tests := []struct { - name string - before *v1alpha1.StepActionResult - after *v1alpha1.StepActionResult - }{{ - name: "empty taskresult", - before: nil, - after: nil, - }, { - name: "inferred string type", - before: &v1alpha1.StepActionResult{ - Name: "resultname", - }, - after: &v1alpha1.StepActionResult{ - Name: "resultname", - Type: v1.ResultsTypeString, - }, - }, { - name: "string type specified not changed", - before: &v1alpha1.StepActionResult{ - Name: "resultname", - Type: v1.ResultsTypeString, - }, - after: &v1alpha1.StepActionResult{ - Name: "resultname", - Type: v1.ResultsTypeString, - }, - }, { - name: "array type specified not changed", - before: &v1alpha1.StepActionResult{ - Name: "resultname", - Type: v1.ResultsTypeArray, - }, - after: &v1alpha1.StepActionResult{ - Name: "resultname", - Type: v1.ResultsTypeArray, - }, - }, { - name: "inferred object type from properties - PropertySpec type is provided", - before: &v1alpha1.StepActionResult{ - Name: "resultname", - Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, - }, - after: &v1alpha1.StepActionResult{ - Name: "resultname", - Type: v1.ResultsTypeObject, - Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, - }, - }, { - name: "inferred type from properties - PropertySpec type is not provided", - before: &v1alpha1.StepActionResult{ - Name: "resultname", - Properties: map[string]v1.PropertySpec{"key1": {}}, - }, - after: &v1alpha1.StepActionResult{ - Name: "resultname", - Type: v1.ResultsTypeObject, - Properties: map[string]v1.PropertySpec{"key1": {Type: v1.ParamTypeString}}, - }, - }} - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() - tc.before.SetDefaults(ctx) - if d := cmp.Diff(tc.after, tc.before); d != "" { - t.Error(diff.PrintWantGot(d)) - } - }) - } -} diff --git a/pkg/apis/pipeline/v1alpha1/result_types.go b/pkg/apis/pipeline/v1alpha1/result_types.go deleted file mode 100644 index df2214875bd..00000000000 --- a/pkg/apis/pipeline/v1alpha1/result_types.go +++ /dev/null @@ -1,62 +0,0 @@ -/* -Copyright 2023 The Tekton Authors -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1alpha1 - -import ( - "context" - - v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" -) - -// StepActionResult used to describe the results of a task -type StepActionResult struct { - // Name the given name - Name string `json:"name"` - - // Type is the user-specified type of the result. The possible type - // is currently "string" and will support "array" in following work. - // +optional - Type v1.ResultsType `json:"type,omitempty"` - - // Properties is the JSON Schema properties to support key-value pairs results. - // +optional - Properties map[string]v1.PropertySpec `json:"properties,omitempty"` - - // Description is a human-readable description of the result - // +optional - Description string `json:"description,omitempty"` -} - -// SetDefaults set the default type for StepActionResult -func (sar *StepActionResult) SetDefaults(context.Context) { - if sar == nil { - return - } - if sar.Type == "" { - if sar.Properties != nil { - // Set type to object if `properties` is given - sar.Type = v1.ResultsTypeObject - } else { - // ResultsTypeString is the default value - sar.Type = v1.ResultsTypeString - } - } - - // Set default type of object values to string - for key, propertySpec := range sar.Properties { - if propertySpec.Type == "" { - sar.Properties[key] = v1.PropertySpec{Type: v1.ParamType(v1.ResultsTypeString)} - } - } -} diff --git a/pkg/apis/pipeline/v1alpha1/result_validation.go b/pkg/apis/pipeline/v1alpha1/result_validation.go deleted file mode 100644 index 3a63ddb976a..00000000000 --- a/pkg/apis/pipeline/v1alpha1/result_validation.go +++ /dev/null @@ -1,77 +0,0 @@ -/* -Copyright 2023 The Tekton Authors -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1alpha1 - -import ( - "context" - "fmt" - "regexp" - - v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" - "knative.dev/pkg/apis" -) - -const ( - // resultNameFormat Constant used to define the regex Result.Name should follow - resultNameFormat = `^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$` -) - -var resultNameFormatRegex = regexp.MustCompile(resultNameFormat) - -// validate implements apis.Validatable -func (sar StepActionResult) validate(ctx context.Context) (errs *apis.FieldError) { - if !resultNameFormatRegex.MatchString(sar.Name) { - return apis.ErrInvalidKeyName(sar.Name, "name", fmt.Sprintf("Name 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)) - } - - switch { - case sar.Type == v1.ResultsTypeObject: - errs := validateObjectResult(sar) - return errs - case sar.Type == v1.ResultsTypeArray: - return nil - // Resources created before the result. Type was introduced may not have Type set - // and should be considered valid - case sar.Type == "": - return nil - // By default, the result type is string - case sar.Type != v1.ResultsTypeString: - return apis.ErrInvalidValue(sar.Type, "type", "type must be string") - } - - return nil -} - -// validateObjectResult validates the object result and check if the Properties is missing -// for Properties values it will check if the type is string. -func validateObjectResult(sar StepActionResult) (errs *apis.FieldError) { - if v1.ParamType(sar.Type) == v1.ParamTypeObject && sar.Properties == nil { - return apis.ErrMissingField(fmt.Sprintf("%s.properties", sar.Name)) - } - - invalidKeys := []string{} - for key, propertySpec := range sar.Properties { - if propertySpec.Type != v1.ParamTypeString { - invalidKeys = append(invalidKeys, key) - } - } - - if len(invalidKeys) != 0 { - return &apis.FieldError{ - Message: fmt.Sprintf("The value type specified for these keys %v is invalid, the type must be string", invalidKeys), - Paths: []string{fmt.Sprintf("%s.properties", sar.Name)}, - } - } - return nil -} diff --git a/pkg/apis/pipeline/v1alpha1/result_validation_test.go b/pkg/apis/pipeline/v1alpha1/result_validation_test.go deleted file mode 100644 index 72f55096fd6..00000000000 --- a/pkg/apis/pipeline/v1alpha1/result_validation_test.go +++ /dev/null @@ -1,125 +0,0 @@ -/* -Copyright 2023 The Tekton Authors - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package v1alpha1 - -import ( - "context" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" - "github.com/tektoncd/pipeline/test/diff" - "knative.dev/pkg/apis" -) - -func TestResultsValidate(t *testing.T) { - tests := []struct { - name string - Result StepActionResult - }{{ - name: "valid result type empty", - Result: StepActionResult{ - Name: "MY-RESULT", - Description: "my great result", - }, - }, { - name: "valid result type string", - Result: StepActionResult{ - Name: "MY-RESULT", - Type: v1.ResultsTypeString, - Description: "my great result", - }, - }, { - name: "valid result type array", - Result: StepActionResult{ - Name: "MY-RESULT", - Type: v1.ResultsTypeArray, - Description: "my great result", - }, - }, { - name: "valid result type object", - Result: StepActionResult{ - Name: "MY-RESULT", - Type: v1.ResultsTypeObject, - Description: "my great result", - Properties: map[string]v1.PropertySpec{"hello": {Type: v1.ParamTypeString}}, - }, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - if err := tt.Result.validate(ctx); err != nil { - t.Errorf("TaskSpec.Validate() = %v", err) - } - }) - } -} - -func TestResultsValidateError(t *testing.T) { - tests := []struct { - name string - Result StepActionResult - expectedError apis.FieldError - }{{ - name: "invalid result type", - Result: StepActionResult{ - Name: "MY-RESULT", - Type: "wrong", - Description: "my great result", - }, - expectedError: apis.FieldError{ - Message: `invalid value: wrong`, - Paths: []string{"type"}, - Details: "type must be string", - }, - }, { - name: "invalid object properties type", - Result: StepActionResult{ - Name: "MY-RESULT", - Type: v1.ResultsTypeObject, - Description: "my great result", - Properties: map[string]v1.PropertySpec{"hello": {Type: "wrong type"}}, - }, - expectedError: apis.FieldError{ - Message: "The value type specified for these keys [hello] is invalid, the type must be string", - Paths: []string{"MY-RESULT.properties"}, - }, - }, { - name: "invalid object properties empty", - Result: StepActionResult{ - Name: "MY-RESULT", - Type: v1.ResultsTypeObject, - Description: "my great result", - }, - expectedError: apis.FieldError{ - Message: "missing field(s)", - Paths: []string{"MY-RESULT.properties"}, - }, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - err := tt.Result.validate(context.Background()) - 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/v1alpha1/stepaction_types.go b/pkg/apis/pipeline/v1alpha1/stepaction_types.go index 97752a2b421..4209f1de4ca 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_types.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_types.go @@ -120,7 +120,7 @@ type StepActionSpec struct { // Results are values that this StepAction can output // +optional // +listType=atomic - Results []StepActionResult `json:"results,omitempty"` + Results []v1.StepResult `json:"results,omitempty"` // SecurityContext defines the security options the Step should be run with. // If set, the fields of SecurityContext override the equivalent fields of PodSecurityContext. // More info: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go index f170410674a..f2d2ccfb7f2 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go @@ -66,8 +66,8 @@ func (ss *StepActionSpec) Validate(ctx context.Context) (errs *apis.FieldError) errs = errs.Also(validateUsageOfDeclaredParameters(ctx, *ss)) errs = errs.Also(v1.ValidateParameterTypes(ctx, ss.Params).ViaField("params")) errs = errs.Also(validateParameterVariables(ctx, *ss, ss.Params)) - errs = errs.Also(validateStepActionResultsVariables(ctx, *ss)) - errs = errs.Also(validateResults(ctx, ss.Results).ViaField("results")) + errs = errs.Also(v1.ValidateStepResultsVariables(ctx, ss.Results, ss.Script)) + errs = errs.Also(v1.ValidateStepResults(ctx, ss.Results).ViaField("results")) errs = errs.Also(validateVolumeMounts(ss.VolumeMounts, ss.Params).ViaField("volumeMounts")) return errs } @@ -84,13 +84,6 @@ func validateUsageOfDeclaredParameters(ctx context.Context, sas StepActionSpec) return errs } -func validateResults(ctx context.Context, results []StepActionResult) (errs *apis.FieldError) { - for index, result := range results { - errs = errs.Also(result.validate(ctx).ViaIndex(index)) - } - return errs -} - func validateVolumeMounts(volumeMounts []corev1.VolumeMount, params v1.ParamSpecs) (errs *apis.FieldError) { if len(volumeMounts) == 0 { return @@ -200,14 +193,3 @@ func validateStepActionVariables(ctx context.Context, sas StepActionSpec, prefix } return errs } - -// validateStepActionResultsVariables validates if the results referenced in step script are defined in task results -func validateStepActionResultsVariables(ctx context.Context, sas StepActionSpec) (errs *apis.FieldError) { - results := sas.Results - resultsNames := sets.NewString() - for _, r := range results { - resultsNames.Insert(r.Name) - } - errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(sas.Script, "results", resultsNames).ViaField("script")) - return errs -} diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go index 2288ea4fdae..cd85acdd132 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go @@ -67,7 +67,7 @@ func TestStepActionSpecValidate(t *testing.T) { Script string Env []corev1.EnvVar Params []v1.ParamSpec - Results []v1alpha1.StepActionResult + Results []v1.StepResult VolumeMounts []corev1.VolumeMount } tests := []struct { @@ -209,7 +209,7 @@ func TestStepActionSpecValidate(t *testing.T) { fields: fields{ Image: "my-image", Args: []string{"arg"}, - Results: []v1alpha1.StepActionResult{{ + Results: []v1.StepResult{{ Name: "MY-RESULT", Description: "my great result", }}, @@ -219,7 +219,7 @@ func TestStepActionSpecValidate(t *testing.T) { fields: fields{ Image: "my-image", Args: []string{"arg"}, - Results: []v1alpha1.StepActionResult{{ + Results: []v1.StepResult{{ Name: "MY-RESULT", Type: "string", Description: "my great result", @@ -230,7 +230,7 @@ func TestStepActionSpecValidate(t *testing.T) { fields: fields{ Image: "my-image", Args: []string{"arg"}, - Results: []v1alpha1.StepActionResult{{ + Results: []v1.StepResult{{ Name: "MY-RESULT", Type: v1.ResultsTypeArray, Description: "my great result", @@ -241,7 +241,7 @@ func TestStepActionSpecValidate(t *testing.T) { fields: fields{ Image: "my-image", Args: []string{"arg"}, - Results: []v1alpha1.StepActionResult{{ + Results: []v1.StepResult{{ Name: "MY-RESULT", Type: v1.ResultsTypeObject, Description: "my great result", @@ -310,7 +310,7 @@ func TestStepActionValidateError(t *testing.T) { Script string Env []corev1.EnvVar Params []v1.ParamSpec - Results []v1alpha1.StepActionResult + Results []v1.StepResult VolumeMounts []corev1.VolumeMount } tests := []struct { @@ -590,7 +590,7 @@ func TestStepActionSpecValidateError(t *testing.T) { Script string Env []corev1.EnvVar Params []v1.ParamSpec - Results []v1alpha1.StepActionResult + Results []v1.StepResult } tests := []struct { name string @@ -633,12 +633,25 @@ func TestStepActionSpecValidateError(t *testing.T) { Script: ` #!/usr/bin/env bash date | tee $(results.non-exist.path)`, - Results: []v1alpha1.StepActionResult{{Name: "a-result"}}, + Results: []v1.StepResult{{Name: "a-result"}}, }, expectedError: apis.FieldError{ Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(results.non-exist.path)"`, Paths: []string{"script"}, }, + }, { + name: "step script refers to nonexistent stepresult", + fields: fields{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + date | tee $(step.results.non-exist.path)`, + Results: []v1.StepResult{{Name: "a-result"}}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(step.results.non-exist.path)"`, + Paths: []string{"script"}, + }, }, { name: "invalid param name format", fields: fields{ diff --git a/pkg/apis/pipeline/v1alpha1/swagger.json b/pkg/apis/pipeline/v1alpha1/swagger.json index eea6d5d9a67..4423cb8c3cb 100644 --- a/pkg/apis/pipeline/v1alpha1/swagger.json +++ b/pkg/apis/pipeline/v1alpha1/swagger.json @@ -384,36 +384,6 @@ } } }, - "v1alpha1.StepActionResult": { - "description": "StepActionResult used to describe the results of a task", - "type": "object", - "required": [ - "name" - ], - "properties": { - "description": { - "description": "Description is a human-readable description of the result", - "type": "string" - }, - "name": { - "description": "Name the given name", - "type": "string", - "default": "" - }, - "properties": { - "description": "Properties is the JSON Schema properties to support key-value pairs results.", - "type": "object", - "additionalProperties": { - "default": {}, - "$ref": "#/definitions/v1.PropertySpec" - } - }, - "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" - } - } - }, "v1alpha1.StepActionSpec": { "description": "StepActionSpec contains the actionable components of a step.", "type": "object", @@ -465,7 +435,7 @@ "type": "array", "items": { "default": {}, - "$ref": "#/definitions/v1alpha1.StepActionResult" + "$ref": "#/definitions/v1.StepResult" }, "x-kubernetes-list-type": "atomic" }, diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 4e02d12fe6d..c25623d07dd 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -278,29 +278,6 @@ func (in *StepActionList) DeepCopyObject() runtime.Object { return nil } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *StepActionResult) DeepCopyInto(out *StepActionResult) { - *out = *in - if in.Properties != nil { - in, out := &in.Properties, &out.Properties - *out = make(map[string]pipelinev1.PropertySpec, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StepActionResult. -func (in *StepActionResult) DeepCopy() *StepActionResult { - if in == nil { - return nil - } - out := new(StepActionResult) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StepActionSpec) DeepCopyInto(out *StepActionSpec) { *out = *in @@ -330,7 +307,7 @@ func (in *StepActionSpec) DeepCopyInto(out *StepActionSpec) { } if in.Results != nil { in, out := &in.Results, &out.Results - *out = make([]StepActionResult, len(*in)) + *out = make([]pipelinev1.StepResult, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } diff --git a/pkg/apis/pipeline/v1beta1/container_conversion.go b/pkg/apis/pipeline/v1beta1/container_conversion.go index 620a655f124..4f025e7dd25 100644 --- a/pkg/apis/pipeline/v1beta1/container_conversion.go +++ b/pkg/apis/pipeline/v1beta1/container_conversion.go @@ -71,6 +71,12 @@ func (s Step) convertTo(ctx context.Context, sink *v1.Step) { p.convertTo(ctx, &new) sink.Params = append(sink.Params, new) } + sink.Results = nil + for _, r := range s.Results { + new := v1.StepResult{} + r.convertTo(ctx, &new) + sink.Results = append(sink.Results, new) + } } func (s *Step) convertFrom(ctx context.Context, source v1.Step) { @@ -109,6 +115,12 @@ func (s *Step) convertFrom(ctx context.Context, source v1.Step) { new.ConvertFrom(ctx, p) s.Params = append(s.Params, new) } + s.Results = nil + for _, r := range source.Results { + new := StepResult{} + new.convertFrom(ctx, r) + s.Results = append(s.Results, new) + } } func (s StepTemplate) convertTo(ctx context.Context, sink *v1.StepTemplate) { diff --git a/pkg/apis/pipeline/v1beta1/container_types.go b/pkg/apis/pipeline/v1beta1/container_types.go index 97bf664d0b3..4f9be74b62f 100644 --- a/pkg/apis/pipeline/v1beta1/container_types.go +++ b/pkg/apis/pipeline/v1beta1/container_types.go @@ -236,6 +236,15 @@ type Step struct { // +optional // +listType=atomic Params Params `json:"params,omitempty"` + // Results declares StepResults produced by the step. + // This is field is at an alpha stability level and gated by "enable-step-actions" feature flag. + // It can be used in an inlined step when used to store results to + // $(step.results.resultName.path). When referencing StepActions, + // it cannot be used. The results declared by the StepActions will be stored here instead. + // Using it along side referencing StepActions will result in a validation error. + // +optional + // +listType=atomic + Results []StepResult `json:"results,omitempty"` } // Ref can be used to refer to a specific instance of a StepAction. diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 6408a47d891..cf29c4ad102 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -85,6 +85,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.SkippedTask": schema_pkg_apis_pipeline_v1beta1_SkippedTask(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Step": schema_pkg_apis_pipeline_v1beta1_Step(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepOutputConfig": schema_pkg_apis_pipeline_v1beta1_StepOutputConfig(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepResult": schema_pkg_apis_pipeline_v1beta1_StepResult(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepState": schema_pkg_apis_pipeline_v1beta1_StepState(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepTemplate": schema_pkg_apis_pipeline_v1beta1_StepTemplate(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Task": schema_pkg_apis_pipeline_v1beta1_Task(ref), @@ -3939,12 +3940,31 @@ func schema_pkg_apis_pipeline_v1beta1_Step(ref common.ReferenceCallback) common. }, }, }, + "results": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "Results declares StepResults produced by the step. This is field is at an alpha stability level and gated by \"enable-step-actions\" feature flag. It can be used in an inlined step when used to store results to $(step.results.resultName.path). When referencing StepActions, it cannot be used. The results declared by the StepActions will be stored here instead. Using it along side referencing StepActions will result in a validation error.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepResult"), + }, + }, + }, + }, + }, }, Required: []string{"name"}, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Param", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Ref", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceUsage", "k8s.io/api/core/v1.ContainerPort", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.Lifecycle", "k8s.io/api/core/v1.Probe", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Param", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Ref", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepOutputConfig", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.StepResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceUsage", "k8s.io/api/core/v1.ContainerPort", "k8s.io/api/core/v1.EnvFromSource", "k8s.io/api/core/v1.EnvVar", "k8s.io/api/core/v1.Lifecycle", "k8s.io/api/core/v1.Probe", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.SecurityContext", "k8s.io/api/core/v1.VolumeDevice", "k8s.io/api/core/v1.VolumeMount", "k8s.io/apimachinery/pkg/apis/meta/v1.Duration"}, } } @@ -3968,6 +3988,59 @@ func schema_pkg_apis_pipeline_v1beta1_StepOutputConfig(ref common.ReferenceCallb } } +func schema_pkg_apis_pipeline_v1beta1_StepResult(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "StepResult used to describe the results of a step action", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "name": { + SchemaProps: spec.SchemaProps{ + Description: "Name the given name", + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + "type": { + SchemaProps: spec.SchemaProps{ + 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{"string"}, + Format: "", + }, + }, + "properties": { + SchemaProps: spec.SchemaProps{ + Description: "Properties is the JSON Schema properties to support key-value pairs results.", + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec"), + }, + }, + }, + }, + }, + "description": { + SchemaProps: spec.SchemaProps{ + Description: "Description is a human-readable description of the result", + Type: []string{"string"}, + Format: "", + }, + }, + }, + Required: []string{"name"}, + }, + }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec"}, + } +} + func schema_pkg_apis_pipeline_v1beta1_StepState(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/pkg/apis/pipeline/v1beta1/result_conversion.go b/pkg/apis/pipeline/v1beta1/result_conversion.go index 8854a283cb7..46d7bcab3ff 100644 --- a/pkg/apis/pipeline/v1beta1/result_conversion.go +++ b/pkg/apis/pipeline/v1beta1/result_conversion.go @@ -55,3 +55,29 @@ func (r *TaskResult) convertFrom(ctx context.Context, source v1.TaskResult) { r.Value.convertFrom(ctx, *source.Value) } } + +func (r StepResult) convertTo(ctx context.Context, sink *v1.StepResult) { + sink.Name = r.Name + sink.Type = v1.ResultsType(r.Type) + sink.Description = r.Description + if r.Properties != nil { + properties := make(map[string]v1.PropertySpec) + for k, v := range r.Properties { + properties[k] = v1.PropertySpec{Type: v1.ParamType(v.Type)} + } + sink.Properties = properties + } +} + +func (r *StepResult) convertFrom(ctx context.Context, source v1.StepResult) { + r.Name = source.Name + r.Type = ResultsType(source.Type) + r.Description = source.Description + if source.Properties != nil { + properties := make(map[string]PropertySpec) + for k, v := range source.Properties { + properties[k] = PropertySpec{Type: ParamType(v.Type)} + } + r.Properties = properties + } +} diff --git a/pkg/apis/pipeline/v1beta1/result_defaults.go b/pkg/apis/pipeline/v1beta1/result_defaults.go index 12af7aa36ef..c1f80a33470 100644 --- a/pkg/apis/pipeline/v1beta1/result_defaults.go +++ b/pkg/apis/pipeline/v1beta1/result_defaults.go @@ -37,3 +37,25 @@ func (tr *TaskResult) SetDefaults(context.Context) { } } } + +// SetDefaults set the default type for StepResult +func (sr *StepResult) SetDefaults(context.Context) { + if sr == nil { + return + } + if sr.Type == "" { + if sr.Properties != nil { + // Set type to object if `properties` is given + sr.Type = ResultsTypeObject + } else { + // ResultsTypeString is the default value + sr.Type = ResultsTypeString + } + } + // Set default type of object values to string + for key, propertySpec := range sr.Properties { + if propertySpec.Type == "" { + sr.Properties[key] = PropertySpec{Type: ParamType(ResultsTypeString)} + } + } +} diff --git a/pkg/apis/pipeline/v1beta1/result_defaults_test.go b/pkg/apis/pipeline/v1beta1/result_defaults_test.go index 13dd6799233..1853a05be22 100644 --- a/pkg/apis/pipeline/v1beta1/result_defaults_test.go +++ b/pkg/apis/pipeline/v1beta1/result_defaults_test.go @@ -93,3 +93,75 @@ func TestTaskResult_SetDefaults(t *testing.T) { }) } } + +func TestStepResult_SetDefaults(t *testing.T) { + tests := []struct { + name string + before *v1beta1.StepResult + after *v1beta1.StepResult + }{{ + name: "empty taskresult", + before: nil, + after: nil, + }, { + name: "inferred string type", + before: &v1beta1.StepResult{ + Name: "resultname", + }, + after: &v1beta1.StepResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeString, + }, + }, { + name: "string type specified not changed", + before: &v1beta1.StepResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeString, + }, + after: &v1beta1.StepResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeString, + }, + }, { + name: "array type specified not changed", + before: &v1beta1.StepResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeArray, + }, + after: &v1beta1.StepResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeArray, + }, + }, { + name: "inferred object type from properties - PropertySpec type is provided", + before: &v1beta1.StepResult{ + Name: "resultname", + Properties: map[string]v1beta1.PropertySpec{"key1": {Type: v1beta1.ParamTypeString}}, + }, + after: &v1beta1.StepResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeObject, + Properties: map[string]v1beta1.PropertySpec{"key1": {Type: v1beta1.ParamTypeString}}, + }, + }, { + name: "inferred type from properties - PropertySpec type is not provided", + before: &v1beta1.StepResult{ + Name: "resultname", + Properties: map[string]v1beta1.PropertySpec{"key1": {}}, + }, + after: &v1beta1.StepResult{ + Name: "resultname", + Type: v1beta1.ResultsTypeObject, + Properties: map[string]v1beta1.PropertySpec{"key1": {Type: v1beta1.ParamTypeString}}, + }, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + 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 2f484f47ddd..225c5791d23 100644 --- a/pkg/apis/pipeline/v1beta1/result_types.go +++ b/pkg/apis/pipeline/v1beta1/result_types.go @@ -38,6 +38,22 @@ type TaskResult struct { Value *ResultValue `json:"value,omitempty"` } +// StepResult used to describe the results of a step action +type StepResult struct { + // Name the given name + Name string `json:"name"` + // Type is the user-specified type of the result. The possible type + // is currently "string" and will support "array" in following work. + // +optional + Type ResultsType `json:"type,omitempty"` + // Properties is the JSON Schema properties to support key-value pairs results. + // +optional + Properties map[string]PropertySpec `json:"properties,omitempty"` + // Description is a human-readable description of the result + // +optional + Description string `json:"description,omitempty"` +} + // TaskRunResult used to describe the results of a task type TaskRunResult struct { // Name the given name diff --git a/pkg/apis/pipeline/v1beta1/result_validation.go b/pkg/apis/pipeline/v1beta1/result_validation.go index a9f776b5277..9e7b4987cf3 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -110,3 +110,44 @@ func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError) } return errs } + +// Validate implements apis.Validatable +func (sr StepResult) Validate(ctx context.Context) (errs *apis.FieldError) { + if !resultNameFormatRegex.MatchString(sr.Name) { + return apis.ErrInvalidKeyName(sr.Name, "name", fmt.Sprintf("Name 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)) + } + switch { + case sr.Type == ResultsTypeObject: + return validateObjectStepResult(sr) + case sr.Type == ResultsTypeArray: + return nil + // The Type is string by default if it is empty. + case sr.Type == "": + return nil + case sr.Type == ResultsTypeString: + return nil + default: + return apis.ErrInvalidValue(sr.Type, "type", fmt.Sprintf("invalid type %s", sr.Type)) + } +} + +// validateObjectStepResult validates the object result and check if the Properties is missing +// for Properties values it will check if the type is string. +func validateObjectStepResult(sr StepResult) (errs *apis.FieldError) { + if ParamType(sr.Type) == ParamTypeObject && sr.Properties == nil { + return apis.ErrMissingField(fmt.Sprintf("%s.properties", sr.Name)) + } + invalidKeys := []string{} + for key, propertySpec := range sr.Properties { + if propertySpec.Type != ParamTypeString { + invalidKeys = append(invalidKeys, key) + } + } + if len(invalidKeys) != 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("the value type specified for these keys %v is invalid, the type must be string", invalidKeys), + Paths: []string{fmt.Sprintf("%s.properties", sr.Name)}, + } + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/result_validation_test.go b/pkg/apis/pipeline/v1beta1/result_validation_test.go index 18d01f640fb..434db1c929e 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/result_validation_test.go @@ -278,3 +278,112 @@ func TestResultsValidateValueError(t *testing.T) { }) } } + +func TestStepResultsValidate(t *testing.T) { + tests := []struct { + name string + Result v1beta1.StepResult + }{{ + name: "valid result type empty", + Result: v1beta1.StepResult{ + Name: "MY-RESULT", + Description: "my great result", + }, + }, { + name: "valid result type string", + Result: v1beta1.StepResult{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeString, + Description: "my great result", + }, + }, { + name: "valid result type array", + Result: v1beta1.StepResult{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeArray, + Description: "my great result", + }, + }, { + name: "valid result type object", + Result: v1beta1.StepResult{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1beta1.PropertySpec{"hello": {Type: v1beta1.ParamTypeString}}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + if err := tt.Result.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + +func TestStepResultsValidateError(t *testing.T) { + tests := []struct { + name string + Result v1beta1.StepResult + expectedError apis.FieldError + }{{ + name: "invalid result name", + Result: v1beta1.StepResult{ + Name: "_MY-RESULT", + Type: v1beta1.ResultsTypeString, + Description: "wrong name", + }, + expectedError: apis.FieldError{ + Message: `invalid key name "_MY-RESULT"`, + Paths: []string{"name"}, + Details: "Name 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 '^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$')", + }, + }, { + name: "invalid result type", + Result: v1beta1.StepResult{ + Name: "MY-RESULT", + Type: "wrong", + Description: "my great result", + }, + expectedError: apis.FieldError{ + Message: `invalid value: wrong`, + Paths: []string{"type"}, + Details: "invalid type wrong", + }, + }, { + name: "invalid object properties type", + Result: v1beta1.StepResult{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1beta1.PropertySpec{"hello": {Type: "wrong type"}}, + }, + expectedError: apis.FieldError{ + Message: "the value type specified for these keys [hello] is invalid, the type must be string", + Paths: []string{"MY-RESULT.properties"}, + }, + }, { + name: "invalid object properties empty", + Result: v1beta1.StepResult{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeObject, + Description: "my great result", + }, + expectedError: apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"MY-RESULT.properties"}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.Result.Validate(context.Background()) + 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 6df62472fbe..8ad1cb6754d 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2107,6 +2107,15 @@ "default": {}, "$ref": "#/definitions/v1.ResourceRequirements" }, + "results": { + "description": "Results declares StepResults produced by the step. This is field is at an alpha stability level and gated by \"enable-step-actions\" feature flag. It can be used in an inlined step when used to store results to $(step.results.resultName.path). When referencing StepActions, it cannot be used. The results declared by the StepActions will be stored here instead. Using it along side referencing StepActions will result in a validation error.", + "type": "array", + "items": { + "default": {}, + "$ref": "#/definitions/v1beta1.StepResult" + }, + "x-kubernetes-list-type": "atomic" + }, "script": { "description": "Script is the contents of an executable file to execute.\n\nIf Script is not empty, the Step cannot have an Command and the Args will be passed to the Script.", "type": "string" @@ -2198,6 +2207,36 @@ } } }, + "v1beta1.StepResult": { + "description": "StepResult used to describe the results of a step action", + "type": "object", + "required": [ + "name" + ], + "properties": { + "description": { + "description": "Description is a human-readable description of the result", + "type": "string" + }, + "name": { + "description": "Name the given name", + "type": "string", + "default": "" + }, + "properties": { + "description": "Properties is the JSON Schema properties to support key-value pairs results.", + "type": "object", + "additionalProperties": { + "default": {}, + "$ref": "#/definitions/v1beta1.PropertySpec" + } + }, + "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" + } + } + }, "v1beta1.StepState": { "description": "StepState reports the results of running a step in a Task.", "type": "object", diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go index 1f59a33c18f..ddf97617f8c 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go @@ -74,6 +74,27 @@ spec: steps: - image: foo - image: bar +` + stepResultTaskYAML := ` +metadata: + name: foo + namespace: bar + generation: 1 +spec: + displayName: "task-display-name" + description: test + steps: + - image: foo + results: + - name: res + type: string + - name: arr + type: array + - name: obj + type: object + properties: + key: + type: string ` stepActionTaskYAML := ` metadata: @@ -306,6 +327,9 @@ spec: multiStepTaskV1beta1 := parse.MustParseV1beta1Task(t, multiStepTaskYAML) multiStepTaskV1 := parse.MustParseV1Task(t, multiStepTaskYAML) + stepResultTaskV1beta1 := parse.MustParseV1beta1Task(t, stepResultTaskYAML) + stepResultTaskV1 := parse.MustParseV1Task(t, stepResultTaskYAML) + stepActionTaskV1beta1 := parse.MustParseV1beta1Task(t, stepActionTaskYAML) stepActionTaskV1 := parse.MustParseV1Task(t, stepActionTaskYAML) @@ -347,6 +371,10 @@ spec: name: "task conversion all non deprecated fields", v1beta1Task: taskWithAllNoDeprecatedFieldsV1beta1, v1Task: taskWithAllNoDeprecatedFieldsV1, + }, { + name: "step results in task", + v1beta1Task: stepResultTaskV1beta1, + v1Task: stepResultTaskV1, }, { name: "step action in task", v1beta1Task: stepActionTaskV1beta1, diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 127d20cd788..1f354c9aa11 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -265,13 +265,21 @@ func validateSteps(ctx context.Context, steps []Step) (errs *apis.FieldError) { names := sets.NewString() for idx, s := range steps { errs = errs.Also(validateStep(ctx, s, names).ViaIndex(idx)) + if s.Results != nil { + errs = errs.Also(validateStepResultsVariables(ctx, s.Results, s.Script).ViaIndex(idx)) + errs = errs.Also(validateStepResults(ctx, s.Results).ViaIndex(idx).ViaField("results")) + } } return errs } +func isCreateOrUpdate(ctx context.Context) bool { + return apis.IsInCreate(ctx) || apis.IsInUpdate(ctx) +} + func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) { if s.Ref != nil { - if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdate(ctx) { return apis.ErrGeneric("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions) } errs = errs.Also(s.Ref.Validate(ctx)) @@ -311,6 +319,12 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi Paths: []string{"volumeMounts"}, }) } + if len(s.Results) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "results cannot be used with Ref", + Paths: []string{"results"}, + }) + } } else { if len(s.Params) > 0 { errs = errs.Also(&apis.FieldError{ @@ -318,6 +332,11 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi Paths: []string{"params"}, }) } + if len(s.Results) > 0 { + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdate(ctx) { + return apis.ErrGeneric("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions) + } + } if s.Image == "" { errs = errs.Also(apis.ErrMissingField("Image")) } @@ -680,3 +699,22 @@ func (ts *TaskSpec) GetIndexingReferencesToArrayParams() sets.String { } return sets.NewString(arrayIndexParamRefs...) } + +// ValidateStepResults validates that all of the declared StepResults are valid. +func validateStepResults(ctx context.Context, results []StepResult) (errs *apis.FieldError) { + for index, result := range results { + errs = errs.Also(result.Validate(ctx).ViaIndex(index)) + } + return errs +} + +// validateStepResultsVariables validates if the StepResults referenced in step script are defined in step's results. +func validateStepResultsVariables(ctx context.Context, results []StepResult, script string) (errs *apis.FieldError) { + resultsNames := sets.NewString() + for _, r := range results { + resultsNames.Insert(r.Name) + } + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "step.results", resultsNames).ViaField("script")) + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(script, "results", resultsNames).ViaField("script")) + return errs +} diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 936b32ca2bb..b0db4521ea7 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -1426,26 +1426,82 @@ func TestTaskSpecValidateError(t *testing.T) { } } -func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { +func TestTaskSpecValidateErrorWithStepActionRef_CreateUpdateEvent(t *testing.T) { tests := []struct { - name string - Steps []v1beta1.Step - enableStepActions bool - expectedError apis.FieldError + name string + Steps []v1beta1.Step + isCreate bool + isUpdate bool + expectedError apis.FieldError }{{ - name: "invalid Task Spec - enableStepActions not on", + name: "is create ctx", Steps: []v1beta1.Step{{ - Image: "image", Ref: &v1beta1.Ref{ Name: "stepAction", }, }}, - enableStepActions: false, + isCreate: true, + isUpdate: false, expectedError: apis.FieldError{ Message: "feature flag %s should be set to true to reference StepActions in Steps.", Paths: []string{"steps[0].enable-step-actions"}, }, }, { + name: "is update ctx", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + }}, + isCreate: false, + isUpdate: true, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true to reference StepActions in Steps.", + Paths: []string{"steps[0].enable-step-actions"}, + }, + }, { + name: "ctx is not create or update", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + }}, + isCreate: false, + isUpdate: false, + expectedError: apis.FieldError{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := v1beta1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: false, + }, + }) + if tt.isCreate { + ctx = apis.WithinCreate(ctx) + } + if tt.isUpdate { + ctx = apis.WithinUpdate(ctx, apis.GetBaseline(ctx)) + } + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + 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 TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { + tests := []struct { + name string + Steps []v1beta1.Step + expectedError apis.FieldError + }{{ name: "Cannot use image with Ref", Steps: []v1beta1.Step{{ Ref: &v1beta1.Ref{ @@ -1453,7 +1509,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Image: "foo", }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "image cannot be used with Ref", Paths: []string{"steps[0].image"}, @@ -1466,7 +1521,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Command: []string{"foo"}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "command cannot be used with Ref", Paths: []string{"steps[0].command"}, @@ -1479,7 +1533,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Args: []string{"foo"}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "args cannot be used with Ref", Paths: []string{"steps[0].args"}, @@ -1492,7 +1545,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { }, Script: "echo hi", }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "script cannot be used with Ref", Paths: []string{"steps[0].script"}, @@ -1508,7 +1560,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { Value: "value1", }}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "env cannot be used with Ref", Paths: []string{"steps[0].env"}, @@ -1521,7 +1572,6 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { Name: "param", }}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "params cannot be used without Ref", Paths: []string{"steps[0].params"}, @@ -1537,12 +1587,25 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { MountPath: "/registry-config", }}, }}, - enableStepActions: true, expectedError: apis.FieldError{ Message: "volumeMounts cannot be used with Ref", Paths: []string{"steps[0].volumeMounts"}, + }, + }, { + name: "Cannot use results with Ref", + Steps: []v1beta1.Step{{ + Ref: &v1beta1.Ref{ + Name: "stepAction", + }, + Results: []v1beta1.StepResult{{ + Name: "result", + }}, }}, - } + expectedError: apis.FieldError{ + Message: "results cannot be used with Ref", + Paths: []string{"steps[0].results"}, + }, + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ts := v1beta1.TaskSpec{ @@ -1550,7 +1613,7 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { } ctx := config.ToContext(context.Background(), &config.Config{ FeatureFlags: &config.FeatureFlags{ - EnableStepActions: tt.enableStepActions, + EnableStepActions: true, }, }) ts.SetDefaults(ctx) @@ -2248,3 +2311,188 @@ func TestParamEnum_Failure(t *testing.T) { } } } + +func TestTaskSpecValidate_StepResults(t *testing.T) { + type fields struct { + Image string + Args []string + Script string + Results []v1beta1.StepResult + } + tests := []struct { + name string + fields fields + }{{ + name: "valid result", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1beta1.StepResult{{ + Name: "MY-RESULT", + Description: "my great result", + }}, + }, + }, { + name: "valid result type string", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Script: "echo $(step.results.MY-RESULT.path)", + Results: []v1beta1.StepResult{{ + Name: "MY-RESULT", + Type: "string", + Description: "my great result", + }}, + }, + }, { + name: "valid result type array", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1beta1.StepResult{{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeArray, + Description: "my great result", + }}, + }, + }, { + name: "valid result type object", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1beta1.StepResult{{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1beta1.PropertySpec{ + "url": {Type: "string"}, + "commit": {Type: "string"}, + }, + }}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Image: tt.fields.Image, + Args: tt.fields.Args, + Script: tt.fields.Script, + Results: tt.fields.Results, + }}, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + ts.SetDefaults(ctx) + if err := ts.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + +func TestTaskSpecValidate_StepResults_Error(t *testing.T) { + type fields struct { + Image string + Script string + Results []v1beta1.StepResult + } + tests := []struct { + name string + fields fields + enableStepActions bool + isCreate bool + isUpdate bool + expectedError apis.FieldError + }{{ + name: "step result not allowed withoue enable step actions - create event", + fields: fields{ + Image: "my-image", + Results: []v1beta1.StepResult{{Name: "a-result"}}, + }, + enableStepActions: false, + isCreate: true, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true in order to use Results in Steps.", + Paths: []string{"steps[0].enable-step-actions"}, + }, + }, { + name: "step result not allowed withoue enable step actions - update event", + fields: fields{ + Image: "my-image", + Results: []v1beta1.StepResult{{Name: "a-result"}}, + }, + enableStepActions: false, + isUpdate: true, + expectedError: apis.FieldError{ + Message: "feature flag %s should be set to true in order to use Results in Steps.", + Paths: []string{"steps[0].enable-step-actions"}, + }, + }, { + name: "step result not allowed withoue enable step actions - neither create nor update", + fields: fields{ + Image: "my-image", + Results: []v1beta1.StepResult{{Name: "a-result"}}, + }, + enableStepActions: false, + expectedError: apis.FieldError{}, + }, { + name: "step script refers to nonexistent result", + fields: fields{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + date | tee $(results.non-exist.path)`, + Results: []v1beta1.StepResult{{Name: "a-result"}}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(results.non-exist.path)"`, + Paths: []string{"steps[0].script"}, + }, + enableStepActions: true, + }, { + name: "step script refers to nonexistent stepresult", + fields: fields{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + date | tee $(step.results.non-exist.path)`, + Results: []v1beta1.StepResult{{Name: "a-result"}}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "\n\t\t\t#!/usr/bin/env bash\n\t\t\tdate | tee $(step.results.non-exist.path)"`, + Paths: []string{"steps[0].script"}, + }, + enableStepActions: true, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Image: tt.fields.Image, + Script: tt.fields.Script, + Results: tt.fields.Results, + }}, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: tt.enableStepActions, + }, + }) + if tt.isCreate { + ctx = apis.WithinCreate(ctx) + } + if tt.isUpdate { + ctx = apis.WithinUpdate(ctx, apis.GetBaseline(ctx)) + } + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("StepActionSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index ca255ddee6c..02a0f9dc77f 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1769,6 +1769,13 @@ func (in *Step) DeepCopyInto(out *Step) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Results != nil { + in, out := &in.Results, &out.Results + *out = make([]StepResult, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -1798,6 +1805,29 @@ func (in *StepOutputConfig) DeepCopy() *StepOutputConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *StepResult) DeepCopyInto(out *StepResult) { + *out = *in + if in.Properties != nil { + in, out := &in.Properties, &out.Properties + *out = make(map[string]PropertySpec, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new StepResult. +func (in *StepResult) DeepCopy() *StepResult { + if in == nil { + return nil + } + out := new(StepResult) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *StepState) DeepCopyInto(out *StepState) { *out = *in