From 47eb4ade0470509fe8420923daa98f98acd31225 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Fri, 27 Oct 2023 12:41:25 -0400 Subject: [PATCH] Introduce Params and Results into StepActions CRD This PR introduces `params` and `results` into the `StepAction` CRD. It allows the `StepAction` to declare the params it needs and the results it will produce. The follow up PRs will contain the interaction of how a `Task` referencing the `StepAction` resolves them. All the tests were borrowed from `pkg/apis/pipeline/v1/...` that overlapped with `paramSpec` and `Results`. --- docs/pipeline-api.md | 134 +++- docs/stepactions.md | 58 ++ hack/ignored-openapi-violations.list | 6 + pkg/apis/pipeline/v1/param_types.go | 14 +- pkg/apis/pipeline/v1/param_types_test.go | 120 ++++ pkg/apis/pipeline/v1/pipeline_validation.go | 8 +- pkg/apis/pipeline/v1/task_validation.go | 24 +- .../pipeline/v1alpha1/openapi_generated.go | 94 ++- .../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_defaults.go | 11 + .../pipeline/v1alpha1/stepaction_types.go | 10 + .../v1alpha1/stepaction_validation.go | 118 ++++ .../v1alpha1/stepaction_validation_test.go | 607 +++++++++++++++++- pkg/apis/pipeline/v1alpha1/swagger.json | 48 ++ .../v1alpha1/zz_generated.deepcopy.go | 38 ++ 18 files changed, 1620 insertions(+), 30 deletions(-) create mode 100644 pkg/apis/pipeline/v1alpha1/result_defaults_test.go create mode 100644 pkg/apis/pipeline/v1alpha1/result_types.go create mode 100644 pkg/apis/pipeline/v1alpha1/result_validation.go create mode 100644 pkg/apis/pipeline/v1alpha1/result_validation_test.go diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index d3d80b89e81..d2f6a4d93b7 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -1709,7 +1709,7 @@ If Enum is not set, no input validation is performed for the param.

ParamSpecs ([]github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamSpec alias)

-(Appears on:PipelineSpec, TaskSpec) +(Appears on:PipelineSpec, TaskSpec, StepActionSpec)

ParamSpecs is a list of ParamSpec

@@ -3214,7 +3214,7 @@ this field is false and so declared workspaces are required.

PropertySpec

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

PropertySpec defines the struct for object keys

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

ResultsType (string alias)

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

ResultsType indicates the type of a result; @@ -6569,6 +6569,35 @@ string

If Script is not empty, the Step cannot have an Command and the Args will be passed to the Script.

+ + +params
+ + +ParamSpecs + + + + +(Optional) +

Params is a list of input parameters required to run the stepAction. +Params must be supplied as inputs in Steps unless they declare a defaultvalue.

+ + + + +results
+ + +[]StepActionResult + + + + +(Optional) +

Results are values that this StepAction can output

+ + @@ -7234,6 +7263,76 @@ 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

@@ -7329,6 +7428,35 @@ string

If Script is not empty, the Step cannot have an Command and the Args will be passed to the Script.

+ + +params
+ + +ParamSpecs + + + + +(Optional) +

Params is a list of input parameters required to run the stepAction. +Params must be supplied as inputs in Steps unless they declare a defaultvalue.

+ + + + +results
+ + +[]StepActionResult + + + + +(Optional) +

Results are values that this StepAction can output

+ +

VerificationPolicySpec diff --git a/docs/stepactions.md b/docs/stepactions.md index e46010b71d4..c1acec55d5f 100644 --- a/docs/stepactions.md +++ b/docs/stepactions.md @@ -38,6 +38,8 @@ A `StepAction` definition supports the following fields: - `script` - cannot be used at the same time as using `command`. - `env` + - [`params`](#declaring-params) + - [`results`](#declaring-results) The non-functional example below demonstrates the use of most of the above-mentioned fields: @@ -55,6 +57,62 @@ spec: args: ["-lh"] ``` +### Declaring Parameters + +Like with `Tasks`, a `StepAction` must declare all the parameters that it used. The same rules for `Parameter` [name](./tasks.md/#parameter-name), [type](./tasks.md/#parameter-type) (including [object](./tasks.md/#object-type), [array](./tasks.md/#array-type) and [string](./tasks.md/#string-type)) apply as when declaring them in `Tasks`. A `StepAction` can also provide [default value](./tasks.md/#default-value) to a `Parameter`. + + `Parameters` are passed to the `StepAction` from its corresponding `Step` referencing it. + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: stepaction-using-params +spec: + params: + - name: gitrepo + type: object + properties: + url: + type: string + commit: + type: string + - name: flags + type: array + - name: outputPath + type: string + default: "/workspace" + image: some-git-image + args: [ + "-url=$(params.gitrepo.url)", + "-revision=$(params.gitrepo.commit)", + "-output=$(params.outputPath)", + "$(params.flags[*])", + ] +``` + +### Declaring Results + +A `StepAction` also declares the results that it will emit. + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: stepaction-declaring-results +spec: + results: + - name: current-date-unix-timestamp + description: The current date in unix timestamp format + - name: current-date-human-readable + description: The current date in human readable format + image: bash:latest + script: | + #!/usr/bin/env bash + date +%s | tee $(results.current-date-unix-timestamp.path) + date | tee $(results.current-date-human-readable.path) +``` + ## Referencing a StepAction `StepActions` can be referenced from the `Step` using the `ref` field, as follows: diff --git a/hack/ignored-openapi-violations.list b/hack/ignored-openapi-violations.list index fc91c45ba27..43afc5a82d9 100644 --- a/hack/ignored-openapi-violations.list +++ b/hack/ignored-openapi-violations.list @@ -38,6 +38,12 @@ API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1,StepTemplate,DeprecatedTerminationMessagePath API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1,StepTemplate,DeprecatedTerminationMessagePolicy API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/resolution/v1alpha1,ResolutionRequestSpec,Parameters +API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,ParamValue,ArrayVal +API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,ParamValue,ObjectVal +API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,ParamValue,StringVal +API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,ParamValue,Type +API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,StepActionSpec,Params +API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,StepActionSpec,Results API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,RunSpec,Workspaces API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,VerificationPolicySpec,Authorities API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,VerificationPolicySpec,Resources diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go index 2792aae99f4..4f09b5bc649 100644 --- a/pkg/apis/pipeline/v1/param_types.go +++ b/pkg/apis/pipeline/v1/param_types.go @@ -108,8 +108,8 @@ func (pp *ParamSpec) setDefaultsForProperties() { } } -// getNames returns all the names of the declared parameters -func (ps ParamSpecs) getNames() []string { +// GetNames returns all the names of the declared parameters +func (ps ParamSpecs) GetNames() []string { var names []string for _, p := range ps { names = append(names, p.Name) @@ -117,8 +117,8 @@ func (ps ParamSpecs) getNames() []string { return names } -// sortByType splits the input params into string params, array params, and object params, in that order -func (ps ParamSpecs) sortByType() (ParamSpecs, ParamSpecs, ParamSpecs) { +// SortByType splits the input params into string params, array params, and object params, in that order +func (ps ParamSpecs) SortByType() (ParamSpecs, ParamSpecs, ParamSpecs) { var stringParams, arrayParams, objectParams ParamSpecs for _, p := range ps { switch p.Type { @@ -135,10 +135,10 @@ func (ps ParamSpecs) sortByType() (ParamSpecs, ParamSpecs, ParamSpecs) { return stringParams, arrayParams, objectParams } -// validateNoDuplicateNames returns an error if any of the params have the same name -func (ps ParamSpecs) validateNoDuplicateNames() *apis.FieldError { +// ValidateNoDuplicateNames returns an error if any of the params have the same name +func (ps ParamSpecs) ValidateNoDuplicateNames() *apis.FieldError { var errs *apis.FieldError - names := ps.getNames() + names := ps.GetNames() for dup := range findDups(names) { errs = errs.Also(apis.ErrGeneric("parameter appears more than once", "").ViaFieldKey("params", dup)) } diff --git a/pkg/apis/pipeline/v1/param_types_test.go b/pkg/apis/pipeline/v1/param_types_test.go index 606e4777c42..9a67bd00ca3 100644 --- a/pkg/apis/pipeline/v1/param_types_test.go +++ b/pkg/apis/pipeline/v1/param_types_test.go @@ -28,6 +28,7 @@ import ( v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/test/diff" "k8s.io/apimachinery/pkg/util/sets" + "knative.dev/pkg/apis" ) func TestParamSpec_SetDefaults(t *testing.T) { @@ -688,3 +689,122 @@ func TestParseTaskandResultName(t *testing.T) { }) } } + +func TestGetNames(t *testing.T) { + tcs := []struct { + name string + params v1.ParamSpecs + want []string + }{{ + name: "names from param spec", + params: v1.ParamSpecs{{ + Name: "foo", + }, { + Name: "bar", + }}, + want: []string{"foo", "bar"}, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + got := tc.params.GetNames() + if d := cmp.Diff(tc.want, got); d != "" { + t.Errorf(diff.PrintWantGot(d)) + } + }) + } +} + +func TestSortByType(t *testing.T) { + tcs := []struct { + name string + params v1.ParamSpecs + want []v1.ParamSpecs + }{{ + name: "sort by type", + params: v1.ParamSpecs{{ + Name: "array1", + Type: "array", + }, { + Name: "string1", + Type: "string", + }, { + Name: "object1", + Type: "object", + }, { + Name: "array2", + Type: "array", + }, { + Name: "string2", + Type: "string", + }, { + Name: "object2", + Type: "object", + }}, + want: []v1.ParamSpecs{ + {{ + Name: "string1", + Type: "string", + }, { + Name: "string2", + Type: "string", + }}, + {{ + Name: "array1", + Type: "array", + }, { + Name: "array2", + Type: "array", + }}, + {{ + Name: "object1", + Type: "object", + }, { + Name: "object2", + Type: "object", + }}, + }, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + s, a, o := tc.params.SortByType() + got := []v1.ParamSpecs{s, a, o} + if d := cmp.Diff(tc.want, got); d != "" { + t.Errorf(diff.PrintWantGot(d)) + } + }) + } +} + +func TestValidateNoDuplicateNames(t *testing.T) { + tcs := []struct { + name string + params v1.ParamSpecs + expectedError *apis.FieldError + }{{ + name: "no duplicates", + params: v1.ParamSpecs{{ + Name: "foo", + }, { + Name: "bar", + }}, + }, { + name: "duplicates", + params: v1.ParamSpecs{{ + Name: "foo", + }, { + Name: "foo", + }}, + expectedError: &apis.FieldError{ + Message: `parameter appears more than once`, + Paths: []string{"params[foo]"}, + }, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + got := tc.params.ValidateNoDuplicateNames() + if d := cmp.Diff(tc.expectedError.Error(), got.Error()); d != "" { + t.Errorf(diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 59bade7b9b1..2dc3b884f7d 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -394,9 +394,9 @@ func (ps *PipelineSpec) validatePipelineParameterUsage(ctx context.Context) (err // validatePipelineTaskParameterUsage validates that parameters referenced in the Pipeline Tasks are declared by the Pipeline func validatePipelineTaskParameterUsage(tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) { - allParamNames := sets.NewString(params.getNames()...) - _, arrayParams, objectParams := params.sortByType() - arrayParamNames := sets.NewString(arrayParams.getNames()...) + allParamNames := sets.NewString(params.GetNames()...) + _, arrayParams, objectParams := params.SortByType() + arrayParamNames := sets.NewString(arrayParams.GetNames()...) objectParameterNameKeys := map[string][]string{} for _, p := range objectParams { for k := range p.Properties { @@ -437,7 +437,7 @@ func validatePipelineTasksWorkspacesUsage(wss []PipelineWorkspaceDeclaration, pt func ValidatePipelineParameterVariables(ctx context.Context, tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) { // validates all the types within a slice of ParamSpecs errs = errs.Also(ValidateParameterTypes(ctx, params).ViaField("params")) - errs = errs.Also(params.validateNoDuplicateNames()) + errs = errs.Also(params.ValidateNoDuplicateNames()) errs = errs.Also(params.validateParamEnums(ctx).ViaField("params")) for i, task := range tasks { errs = errs.Also(task.Params.validateDuplicateParameters().ViaField("params").ViaIndex(i)) diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index f520146bb1e..0eabc27c9a8 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -129,16 +129,16 @@ func (ts *TaskSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError { // ValidateUsageOfDeclaredParameters validates that all parameters referenced in the Task are declared by the Task. func ValidateUsageOfDeclaredParameters(ctx context.Context, steps []Step, params ParamSpecs) *apis.FieldError { var errs *apis.FieldError - _, _, objectParams := params.sortByType() - allParameterNames := sets.NewString(params.getNames()...) + _, _, objectParams := params.SortByType() + allParameterNames := sets.NewString(params.GetNames()...) errs = errs.Also(validateVariables(ctx, steps, "params", allParameterNames)) errs = errs.Also(validateObjectUsage(ctx, steps, objectParams)) - errs = errs.Also(validateObjectParamsHaveProperties(ctx, params)) + errs = errs.Also(ValidateObjectParamsHaveProperties(ctx, params)) return errs } -// validateObjectParamsHaveProperties returns an error if any declared object params are missing properties -func validateObjectParamsHaveProperties(ctx context.Context, params ParamSpecs) *apis.FieldError { +// ValidateObjectParamsHaveProperties returns an error if any declared object params are missing properties +func ValidateObjectParamsHaveProperties(ctx context.Context, params ParamSpecs) *apis.FieldError { var errs *apis.FieldError for _, p := range params { if p.Type == ParamTypeObject && p.Properties == nil { @@ -435,12 +435,12 @@ func (p ParamSpec) ValidateObjectType(ctx context.Context) *apis.FieldError { // ValidateParameterVariables validates all variables within a slice of ParamSpecs against a slice of Steps func ValidateParameterVariables(ctx context.Context, steps []Step, params ParamSpecs) *apis.FieldError { var errs *apis.FieldError - errs = errs.Also(params.validateNoDuplicateNames()) + errs = errs.Also(params.ValidateNoDuplicateNames()) errs = errs.Also(params.validateParamEnums(ctx).ViaField("params")) - stringParams, arrayParams, objectParams := params.sortByType() - stringParameterNames := sets.NewString(stringParams.getNames()...) - arrayParameterNames := sets.NewString(arrayParams.getNames()...) - errs = errs.Also(validateNameFormat(stringParameterNames.Insert(arrayParameterNames.List()...), objectParams)) + stringParams, arrayParams, objectParams := params.SortByType() + stringParameterNames := sets.NewString(stringParams.GetNames()...) + arrayParameterNames := sets.NewString(arrayParams.GetNames()...) + errs = errs.Also(ValidateNameFormat(stringParameterNames.Insert(arrayParameterNames.List()...), objectParams)) return errs.Also(validateArrayUsage(steps, "params", arrayParameterNames)) } @@ -561,8 +561,8 @@ func validateVariables(ctx context.Context, steps []Step, prefix string, vars se return errs } -// validateNameFormat validates that the name format of all param types follows the rules -func validateNameFormat(stringAndArrayParams sets.String, objectParams []ParamSpec) (errs *apis.FieldError) { +// ValidateNameFormat validates that the name format of all param types follows the rules +func ValidateNameFormat(stringAndArrayParams sets.String, objectParams []ParamSpec) (errs *apis.FieldError) { // checking string or array name format // ---- invalidStringAndArrayNames := []string{} diff --git a/pkg/apis/pipeline/v1alpha1/openapi_generated.go b/pkg/apis/pipeline/v1alpha1/openapi_generated.go index 876cb0bdc84..7638264c094 100644 --- a/pkg/apis/pipeline/v1alpha1/openapi_generated.go +++ b/pkg/apis/pipeline/v1alpha1/openapi_generated.go @@ -41,6 +41,7 @@ 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), @@ -747,6 +748,59 @@ 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{ @@ -829,11 +883,49 @@ func schema_pkg_apis_pipeline_v1alpha1_StepActionSpec(ref common.ReferenceCallba Format: "", }, }, + "params": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "Params is a list of input parameters required to run the stepAction. Params must be supplied as inputs in Steps unless they declare a defaultvalue.", + 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.ParamSpec"), + }, + }, + }, + }, + }, + "results": { + VendorExtensible: spec.VendorExtensible{ + Extensions: spec.Extensions{ + "x-kubernetes-list-type": "atomic", + }, + }, + SchemaProps: spec.SchemaProps{ + Description: "Results are values that this StepAction can output", + 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/v1alpha1.StepActionResult"), + }, + }, + }, + }, + }, }, }, }, Dependencies: []string{ - "k8s.io/api/core/v1.EnvVar"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1.StepActionResult", "k8s.io/api/core/v1.EnvVar"}, } } diff --git a/pkg/apis/pipeline/v1alpha1/result_defaults_test.go b/pkg/apis/pipeline/v1alpha1/result_defaults_test.go new file mode 100644 index 00000000000..302be5b969a --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/result_defaults_test.go @@ -0,0 +1,96 @@ +/* +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 new file mode 100644 index 00000000000..df2214875bd --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/result_types.go @@ -0,0 +1,62 @@ +/* +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 new file mode 100644 index 00000000000..3a63ddb976a --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/result_validation.go @@ -0,0 +1,77 @@ +/* +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 new file mode 100644 index 00000000000..72f55096fd6 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/result_validation_test.go @@ -0,0 +1,125 @@ +/* +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_defaults.go b/pkg/apis/pipeline/v1alpha1/stepaction_defaults.go index 8b30d937e9c..b0471f66488 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_defaults.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_defaults.go @@ -23,4 +23,15 @@ var _ apis.Defaultable = (*StepAction)(nil) // SetDefaults implements apis.Defaultable func (s *StepAction) SetDefaults(ctx context.Context) { + s.Spec.SetDefaults(ctx) +} + +// SetDefaults set any defaults for the StepAction spec +func (ss *StepActionSpec) SetDefaults(ctx context.Context) { + for i := range ss.Params { + ss.Params[i].SetDefaults(ctx) + } + for i := range ss.Results { + ss.Results[i].SetDefaults(ctx) + } } diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_types.go b/pkg/apis/pipeline/v1alpha1/stepaction_types.go index 03bc5608642..ce8d0934b6d 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_types.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_types.go @@ -14,6 +14,7 @@ limitations under the License. package v1alpha1 import ( + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -111,6 +112,15 @@ type StepActionSpec struct { // If Script is not empty, the Step cannot have an Command and the Args will be passed to the Script. // +optional Script string `json:"script,omitempty"` + // Params is a list of input parameters required to run the stepAction. + // Params must be supplied as inputs in Steps unless they declare a defaultvalue. + // +optional + // +listType=atomic + Params v1.ParamSpecs `json:"params,omitempty"` + // Results are values that this StepAction can output + // +optional + // +listType=atomic + Results []StepActionResult `json:"results,omitempty"` } // StepActionObject is implemented by StepAction diff --git a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go index 6209026c1b1..47674cfc62f 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_validation.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_validation.go @@ -15,11 +15,15 @@ package v1alpha1 import ( "context" + "fmt" "strings" "github.com/tektoncd/pipeline/pkg/apis/config" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/substitution" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" "knative.dev/pkg/webhook/resourcesemantics" ) @@ -58,5 +62,119 @@ func (ss *StepActionSpec) Validate(ctx context.Context) (errs *apis.FieldError) errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script")) } } + 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")) + return errs +} + +// validateUsageOfDeclaredParameters validates that all parameters referenced in the Task are declared by the Task. +func validateUsageOfDeclaredParameters(ctx context.Context, sas StepActionSpec) *apis.FieldError { + params := sas.Params + var errs *apis.FieldError + _, _, objectParams := params.SortByType() + allParameterNames := sets.NewString(params.GetNames()...) + errs = errs.Also(validateStepActionVariables(ctx, sas, "params", allParameterNames)) + errs = errs.Also(validateObjectUsage(ctx, sas, objectParams)) + errs = errs.Also(v1.ValidateObjectParamsHaveProperties(ctx, params)) + 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 +} + +// validateParameterVariables validates all variables within a slice of ParamSpecs against a StepAction +func validateParameterVariables(ctx context.Context, sas StepActionSpec, params v1.ParamSpecs) *apis.FieldError { + var errs *apis.FieldError + errs = errs.Also(params.ValidateNoDuplicateNames()) + stringParams, arrayParams, objectParams := params.SortByType() + stringParameterNames := sets.NewString(stringParams.GetNames()...) + arrayParameterNames := sets.NewString(arrayParams.GetNames()...) + errs = errs.Also(v1.ValidateNameFormat(stringParameterNames.Insert(arrayParameterNames.List()...), objectParams)) + return errs.Also(validateStepActionArrayUsage(sas, "params", arrayParameterNames)) +} + +// validateObjectUsage validates the usage of individual attributes of an object param and the usage of the entire object +func validateObjectUsage(ctx context.Context, sas StepActionSpec, params v1.ParamSpecs) (errs *apis.FieldError) { + objectParameterNames := sets.NewString() + for _, p := range params { + // collect all names of object type params + objectParameterNames.Insert(p.Name) + + // collect all keys for this object param + objectKeys := sets.NewString() + for key := range p.Properties { + objectKeys.Insert(key) + } + + // check if the object's key names are referenced correctly i.e. param.objectParam.key1 + errs = errs.Also(validateStepActionVariables(ctx, sas, fmt.Sprintf("params\\.%s", p.Name), objectKeys)) + } + + return errs.Also(validateStepActionObjectUsageAsWhole(sas, "params", objectParameterNames)) +} + +// validateStepActionObjectUsageAsWhole returns an error if the StepAction contains references to the entire input object params in fields where these references are prohibited +func validateStepActionObjectUsageAsWhole(sas StepActionSpec, prefix string, vars sets.String) *apis.FieldError { + errs := substitution.ValidateNoReferencesToEntireProhibitedVariables(sas.Image, prefix, vars).ViaField("image") + errs = errs.Also(substitution.ValidateNoReferencesToEntireProhibitedVariables(sas.Script, prefix, vars).ViaField("script")) + for i, cmd := range sas.Command { + errs = errs.Also(substitution.ValidateNoReferencesToEntireProhibitedVariables(cmd, prefix, vars).ViaFieldIndex("command", i)) + } + for i, arg := range sas.Args { + errs = errs.Also(substitution.ValidateNoReferencesToEntireProhibitedVariables(arg, prefix, vars).ViaFieldIndex("args", i)) + } + for _, env := range sas.Env { + errs = errs.Also(substitution.ValidateNoReferencesToEntireProhibitedVariables(env.Value, prefix, vars).ViaFieldKey("env", env.Name)) + } + return errs +} + +// validateStepActionArrayUsage returns an error if the Step contains references to the input array params in fields where these references are prohibited +func validateStepActionArrayUsage(sas StepActionSpec, prefix string, arrayParamNames sets.String) *apis.FieldError { + errs := substitution.ValidateNoReferencesToProhibitedVariables(sas.Image, prefix, arrayParamNames).ViaField("image") + errs = errs.Also(substitution.ValidateNoReferencesToProhibitedVariables(sas.Script, prefix, arrayParamNames).ViaField("script")) + for i, cmd := range sas.Command { + errs = errs.Also(substitution.ValidateVariableReferenceIsIsolated(cmd, prefix, arrayParamNames).ViaFieldIndex("command", i)) + } + for i, arg := range sas.Args { + errs = errs.Also(substitution.ValidateVariableReferenceIsIsolated(arg, prefix, arrayParamNames).ViaFieldIndex("args", i)) + } + for _, env := range sas.Env { + errs = errs.Also(substitution.ValidateNoReferencesToProhibitedVariables(env.Value, prefix, arrayParamNames).ViaFieldKey("env", env.Name)) + } + return errs +} + +// validateStepActionVariables returns an error if the StepAction contains references to any unknown variables +func validateStepActionVariables(ctx context.Context, sas StepActionSpec, prefix string, vars sets.String) *apis.FieldError { + errs := substitution.ValidateNoReferencesToUnknownVariables(sas.Image, prefix, vars).ViaField("image") + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(sas.Script, prefix, vars).ViaField("script")) + for i, cmd := range sas.Command { + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(cmd, prefix, vars).ViaFieldIndex("command", i)) + } + for i, arg := range sas.Args { + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(arg, prefix, vars).ViaFieldIndex("args", i)) + } + for _, env := range sas.Env { + errs = errs.Also(substitution.ValidateNoReferencesToUnknownVariables(env.Value, prefix, vars).ViaFieldKey("env", env.Name)) + } + 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 72871f81555..9e371c1edba 100644 --- a/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/stepaction_validation_test.go @@ -15,10 +15,12 @@ package v1alpha1_test import ( "context" + "fmt" "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/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" @@ -64,6 +66,8 @@ func TestStepActionSpecValidate(t *testing.T) { Args []string Script string Env []corev1.EnvVar + Params []v1.ParamSpec + Results []v1alpha1.StepActionResult } tests := []struct { name string @@ -91,6 +95,161 @@ func TestStepActionSpecValidate(t *testing.T) { Value: "/tekton/home", }}, }, + }, { + name: "valid params type explicit", + fields: fields{ + Image: "myimage", + Params: []v1.ParamSpec{{ + Name: "stringParam", + Type: v1.ParamTypeString, + Description: "param", + Default: v1.NewStructuredValues("default"), + }, { + Name: "objectParam", + Type: v1.ParamTypeObject, + Description: "param", + Properties: map[string]v1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + Default: v1.NewObject(map[string]string{ + "key1": "var1", + "key2": "var2", + }), + }, { + Name: "objectParamWithoutDefault", + Type: v1.ParamTypeObject, + Description: "param", + Properties: map[string]v1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + }, { + Name: "objectParamWithDefaultPartialKeys", + Type: v1.ParamTypeObject, + Description: "param", + Properties: map[string]v1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + Default: v1.NewObject(map[string]string{ + "key1": "default", + }), + }}, + }, + }, { + name: "valid string param usage", + fields: fields{ + Image: "url", + Params: []v1.ParamSpec{{ + Name: "baz", + }, { + Name: "foo-is-baz", + }}, + Args: []string{"--flag=$(params.baz) && $(params.foo-is-baz)"}, + }, + }, { + name: "valid array param usage", + fields: fields{ + Image: "url", + Params: []v1.ParamSpec{{ + Name: "baz", + Type: v1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1.ParamTypeArray, + }}, + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"$(params.baz)", "middle string", "$(params.foo-is-baz)"}, + }, + }, { + name: "valid object param usage", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "gitrepo", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{ + "url": {}, + "commit": {}, + }, + }}, + Image: "some-git-image", + Args: []string{"-url=$(params.gitrepo.url)", "-commit=$(params.gitrepo.commit)"}, + }, + }, { + name: "valid star array usage", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Type: v1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1.ParamTypeArray, + }}, + Image: "myimage", + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"$(params.baz[*])", "middle string", "$(params.foo-is-baz[*])"}, + }, + }, { + name: "valid step with parameterized script", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + }, { + Name: "foo-is-baz", + }}, + Image: "my-image", + Script: ` + #!/usr/bin/env bash + hello $(params.baz)`, + }, + }, { + name: "valid result", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1alpha1.StepActionResult{{ + Name: "MY-RESULT", + Description: "my great result", + }}, + }, + }, { + name: "valid result type string", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1alpha1.StepActionResult{{ + Name: "MY-RESULT", + Type: "string", + Description: "my great result", + }}, + }, + }, { + name: "valid result type array", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1alpha1.StepActionResult{{ + Name: "MY-RESULT", + Type: v1.ResultsTypeArray, + Description: "my great result", + }}, + }, + }, { + name: "valid result type object", + fields: fields{ + Image: "my-image", + Args: []string{"arg"}, + Results: []v1alpha1.StepActionResult{{ + 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) { @@ -100,8 +259,12 @@ func TestStepActionSpecValidate(t *testing.T) { Args: tt.fields.Args, Script: tt.fields.Script, Env: tt.fields.Env, + Params: tt.fields.Params, + Results: tt.fields.Results, } - if err := sa.Validate(context.Background()); err != nil { + ctx := context.Background() + sa.SetDefaults(ctx) + if err := sa.Validate(ctx); err != nil { t.Errorf("StepActionSpec.Validate() = %v", err) } }) @@ -115,6 +278,8 @@ func TestStepActionValidateError(t *testing.T) { Args []string Script string Env []corev1.EnvVar + Params []v1.ParamSpec + Results []v1alpha1.StepActionResult } tests := []struct { name string @@ -123,12 +288,117 @@ func TestStepActionValidateError(t *testing.T) { }{{ name: "inexistent image field", fields: fields{ - Args: []string{"--flag=$(params.inexistent)"}, + Args: []string{"flag"}, }, expectedError: apis.FieldError{ Message: `missing field(s)`, Paths: []string{"spec.Image"}, }, + }, { + name: "object used in a string field", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "gitrepo", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{ + "url": {}, + "commit": {}, + }, + }}, + Image: "$(params.gitrepo)", + Args: []string{"echo"}, + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "$(params.gitrepo)"`, + Paths: []string{"spec.image"}, + }, + }, { + name: "object star used in a string field", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "gitrepo", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{ + "url": {}, + "commit": {}, + }, + }}, + Image: "$(params.gitrepo[*])", + Args: []string{"echo"}, + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "$(params.gitrepo[*])"`, + Paths: []string{"spec.image"}, + }, + }, { + name: "object used in a field that can accept array type", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "gitrepo", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{ + "url": {}, + "commit": {}, + }, + }}, + Image: "myimage", + Args: []string{"$(params.gitrepo)"}, + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "$(params.gitrepo)"`, + Paths: []string{"spec.args[0]"}, + }, + }, { + name: "object star used in a field that can accept array type", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "gitrepo", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{ + "url": {}, + "commit": {}, + }, + }}, + Image: "some-git-image", + Args: []string{"$(params.gitrepo[*])"}, + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "$(params.gitrepo[*])"`, + Paths: []string{"spec.args[0]"}, + }, + }, { + name: "non-existent individual key of an object param is used in task step", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "gitrepo", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{ + "url": {}, + "commit": {}, + }, + }}, + Image: "some-git-image", + Args: []string{"$(params.gitrepo.non-exist-key)"}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.gitrepo.non-exist-key)"`, + Paths: []string{"spec.args[0]"}, + }, + }, { + name: "Inexistent param variable with existing", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "foo", + Description: "param", + Default: v1.NewStructuredValues("default"), + }}, + Image: "myimage", + Args: []string{"$(params.foo) && $(params.inexistent)"}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.foo) && $(params.inexistent)"`, + Paths: []string{"spec.args[0]"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -140,9 +410,12 @@ func TestStepActionValidateError(t *testing.T) { Args: tt.fields.Args, Script: tt.fields.Script, Env: tt.fields.Env, + Params: tt.fields.Params, + Results: tt.fields.Results, }, } ctx := context.Background() + sa.SetDefaults(ctx) err := sa.Validate(ctx) if err == nil { t.Fatalf("Expected an error, got nothing for %v", sa) @@ -161,6 +434,8 @@ func TestStepActionSpecValidateError(t *testing.T) { Args []string Script string Env []corev1.EnvVar + Params []v1.ParamSpec + Results []v1alpha1.StepActionResult } tests := []struct { name string @@ -169,7 +444,7 @@ func TestStepActionSpecValidateError(t *testing.T) { }{{ name: "inexistent image field", fields: fields{ - Args: []string{"--flag=$(params.inexistent)"}, + Args: []string{"flag"}, }, expectedError: apis.FieldError{ Message: `missing field(s)`, @@ -196,6 +471,329 @@ func TestStepActionSpecValidateError(t *testing.T) { Message: `windows script support requires "enable-api-fields" feature gate to be "alpha" but it is "beta"`, Paths: []string{}, }, + }, { + name: "step script refers to nonexistent result", + fields: fields{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + date | tee $(results.non-exist.path)`, + Results: []v1alpha1.StepActionResult{{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: "invalid param name format", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "_validparam1", + Description: "valid param name format", + }, { + Name: "valid_param2", + Description: "valid param name format", + }, { + Name: "", + Description: "invalid param name format", + }, { + Name: "a^b", + Description: "invalid param name format", + }, { + Name: "0ab", + Description: "invalid param name format", + }, { + Name: "f oo", + Description: "invalid param name format", + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: fmt.Sprintf("The format of following array and string variable names is invalid: %s", []string{"", "0ab", "a^b", "f oo"}), + Paths: []string{"params"}, + Details: "String/Array Names: \nMust only contain alphanumeric characters, hyphens (-), underscores (_), and dots (.)\nMust begin with a letter or an underscore (_)", + }, + }, { + name: "invalid object param format - object param name and key name shouldn't contain dots.", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "invalid.name1", + Description: "object param name contains dots", + Properties: map[string]v1.PropertySpec{ + "invalid.key1": {}, + "mykey2": {}, + }, + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: fmt.Sprintf("Object param name and key name format is invalid: %v", map[string][]string{ + "invalid.name1": {"invalid.key1"}, + }), + Paths: []string{"params"}, + Details: "Object Names: \nMust only contain alphanumeric characters, hyphens (-), underscores (_) \nMust begin with a letter or an underscore (_)", + }, + }, { + name: "duplicated param names", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "foo", + Type: v1.ParamTypeString, + Description: "parameter", + Default: v1.NewStructuredValues("value1"), + }, { + Name: "foo", + Type: v1.ParamTypeString, + Description: "parameter", + Default: v1.NewStructuredValues("value2"), + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: `parameter appears more than once`, + Paths: []string{"params[foo]"}, + }, + }, { + name: "invalid param type", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "validparam", + Type: v1.ParamTypeString, + Description: "parameter", + Default: v1.NewStructuredValues("default"), + }, { + Name: "param-with-invalid-type", + Type: "invalidtype", + Description: "invalidtypedesc", + Default: v1.NewStructuredValues("default"), + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: `invalid value: invalidtype`, + Paths: []string{"params.param-with-invalid-type.type"}, + }, + }, { + name: "param mismatching default/type 1", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "task", + Type: v1.ParamTypeArray, + Description: "param", + Default: v1.NewStructuredValues("default"), + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: `"array" type does not match default value's type: "string"`, + Paths: []string{"params.task.type", "params.task.default.type"}, + }, + }, { + name: "param mismatching default/type 2", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "task", + Type: v1.ParamTypeString, + Description: "param", + Default: v1.NewStructuredValues("default", "array"), + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: `"string" type does not match default value's type: "array"`, + Paths: []string{"params.task.type", "params.task.default.type"}, + }, + }, { + name: "param mismatching default/type 3", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "task", + Type: v1.ParamTypeArray, + Description: "param", + Default: v1.NewObject(map[string]string{ + "key1": "var1", + "key2": "var2", + }), + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: `"array" type does not match default value's type: "object"`, + Paths: []string{"params.task.type", "params.task.default.type"}, + }, + }, { + name: "param mismatching default/type 4", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "task", + Type: v1.ParamTypeObject, + Description: "param", + Properties: map[string]v1.PropertySpec{"key1": {}}, + Default: v1.NewStructuredValues("var"), + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: `"object" type does not match default value's type: "string"`, + Paths: []string{"params.task.type", "params.task.default.type"}, + }, + }, { + name: "PropertySpec type is set with unsupported type", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "task", + Type: v1.ParamTypeObject, + Description: "param", + Properties: map[string]v1.PropertySpec{ + "key1": {Type: "number"}, + "key2": {Type: "string"}, + }, + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: fmt.Sprintf("The value type specified for these keys %v is invalid", []string{"key1"}), + Paths: []string{"params.task.properties"}, + }, + }, { + name: "Properties is missing", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "task", + Type: v1.ParamTypeObject, + Description: "param", + }}, + Image: "myImage", + }, + expectedError: apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"task.properties"}, + }, + }, { + name: "array used in unaccepted field", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Type: v1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1.ParamTypeArray, + }}, + Image: "$(params.baz)", + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"$(params.baz)", "middle string", "url"}, + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "$(params.baz)"`, + Paths: []string{"image"}, + }, + }, { + name: "array star used in unaccepted field", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Type: v1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1.ParamTypeArray, + }}, + Image: "$(params.baz[*])", + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"$(params.baz)", "middle string", "url"}, + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "$(params.baz[*])"`, + Paths: []string{"image"}, + }, + }, { + name: "array star used illegaly in script field", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Type: v1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1.ParamTypeArray, + }}, + Script: "$(params.baz[*])", + Image: "my-image", + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "$(params.baz[*])"`, + Paths: []string{"script"}, + }, + }, { + name: "array not properly isolated", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Type: v1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1.ParamTypeArray, + }}, + Image: "someimage", + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"not isolated: $(params.baz)", "middle string", "url"}, + }, + expectedError: apis.FieldError{ + Message: `variable is not properly isolated in "not isolated: $(params.baz)"`, + Paths: []string{"args[0]"}, + }, + }, { + name: "array star not properly isolated", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Type: v1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1.ParamTypeArray, + }}, + Image: "someimage", + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"not isolated: $(params.baz[*])", "middle string", "url"}, + }, + expectedError: apis.FieldError{ + Message: `variable is not properly isolated in "not isolated: $(params.baz[*])"`, + Paths: []string{"args[0]"}, + }, + }, { + name: "inferred array not properly isolated", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Default: v1.NewStructuredValues("implied", "array", "type"), + }, { + Name: "foo-is-baz", + Default: v1.NewStructuredValues("implied", "array", "type"), + }}, + Image: "someimage", + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"not isolated: $(params.baz)", "middle string", "url"}, + }, + expectedError: apis.FieldError{ + Message: `variable is not properly isolated in "not isolated: $(params.baz)"`, + Paths: []string{"args[0]"}, + }, + }, { + name: "inferred array star not properly isolated", + fields: fields{ + Params: []v1.ParamSpec{{ + Name: "baz", + Default: v1.NewStructuredValues("implied", "array", "type"), + }, { + Name: "foo-is-baz", + Default: v1.NewStructuredValues("implied", "array", "type"), + }}, + Image: "someimage", + Command: []string{"$(params.foo-is-baz)"}, + Args: []string{"not isolated: $(params.baz[*])", "middle string", "url"}, + }, + expectedError: apis.FieldError{ + Message: `variable is not properly isolated in "not isolated: $(params.baz[*])"`, + Paths: []string{"args[0]"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -205,8 +803,11 @@ func TestStepActionSpecValidateError(t *testing.T) { Args: tt.fields.Args, Script: tt.fields.Script, Env: tt.fields.Env, + Params: tt.fields.Params, + Results: tt.fields.Results, } ctx := context.Background() + sa.SetDefaults(ctx) err := sa.Validate(ctx) if err == nil { t.Fatalf("Expected an error, got nothing for %v", sa) diff --git a/pkg/apis/pipeline/v1alpha1/swagger.json b/pkg/apis/pipeline/v1alpha1/swagger.json index 46e98e44f28..14d777f80ea 100644 --- a/pkg/apis/pipeline/v1alpha1/swagger.json +++ b/pkg/apis/pipeline/v1alpha1/swagger.json @@ -384,6 +384,36 @@ } } }, + "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", @@ -421,6 +451,24 @@ "description": "Image reference name to run for this StepAction. More info: https://kubernetes.io/docs/concepts/containers/images", "type": "string" }, + "params": { + "description": "Params is a list of input parameters required to run the stepAction. Params must be supplied as inputs in Steps unless they declare a defaultvalue.", + "type": "array", + "items": { + "default": {}, + "$ref": "#/definitions/v1.ParamSpec" + }, + "x-kubernetes-list-type": "atomic" + }, + "results": { + "description": "Results are values that this StepAction can output", + "type": "array", + "items": { + "default": {}, + "$ref": "#/definitions/v1alpha1.StepActionResult" + }, + "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" diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 494ace1360c..7ee5bef9874 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -23,6 +23,7 @@ package v1alpha1 import ( pod "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" + pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" v1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -277,6 +278,29 @@ 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 @@ -297,6 +321,20 @@ func (in *StepActionSpec) DeepCopyInto(out *StepActionSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Params != nil { + in, out := &in.Params, &out.Params + *out = make(pipelinev1.ParamSpecs, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.Results != nil { + in, out := &in.Results, &out.Results + *out = make([]StepActionResult, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return }