diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index 6366a75d7bd..8941ebff560 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation.go @@ -87,7 +87,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError { return err } // Validate that the parameters type are correct - if err := v1beta1.ValidateParameterTypes(ts.Params); err != nil { + if err := v1beta1.ValidateParameterTypes(ctx, ts.Params); err != nil { return err } diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 9d547860475..bc7495422b8 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -75,6 +75,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineTaskRun": schema_pkg_apis_pipeline_v1beta1_PipelineTaskRun(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineTaskRunSpec": schema_pkg_apis_pipeline_v1beta1_PipelineTaskRunSpec(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineWorkspaceDeclaration": schema_pkg_apis_pipeline_v1beta1_PipelineWorkspaceDeclaration(ref), + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec": schema_pkg_apis_pipeline_v1beta1_PropertySpec(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ResolverParam": schema_pkg_apis_pipeline_v1beta1_ResolverParam(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ResolverRef": schema_pkg_apis_pipeline_v1beta1_ResolverRef(ref), "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ResultRef": schema_pkg_apis_pipeline_v1beta1_ResultRef(ref), @@ -369,7 +370,7 @@ func schema_pkg_apis_pipeline_v1beta1_ArrayOrString(ref common.ReferenceCallback return common.OpenAPIDefinition{ Schema: spec.Schema{ SchemaProps: spec.SchemaProps{ - Description: "ArrayOrString is a type that can hold a single string or string array. Used in JSON unmarshalling so that a single JSON field can accept either an individual string or an array of strings.", + Description: "ArrayOrString is a type that can hold a single string or string array. Used in JSON unmarshalling so that a single JSON field can accept either an individual string or an array of strings. consideration the object case after the community reaches an agreement on it.", Type: []string{"object"}, Properties: map[string]spec.Schema{ "type": { @@ -406,8 +407,23 @@ func schema_pkg_apis_pipeline_v1beta1_ArrayOrString(ref common.ReferenceCallback }, }, }, + "objectVal": { + SchemaProps: spec.SchemaProps{ + Type: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, - Required: []string{"type", "stringVal", "arrayVal"}, + Required: []string{"type", "stringVal", "arrayVal", "objectVal"}, }, }, } @@ -1120,7 +1136,7 @@ func schema_pkg_apis_pipeline_v1beta1_ParamSpec(ref common.ReferenceCallback) co }, "type": { SchemaProps: spec.SchemaProps{ - Description: "Type is the user-specified type of the parameter. The possible types are currently \"string\" and \"array\", and \"string\" is the default.", + Description: "Type is the user-specified type of the parameter. The possible types are currently \"string\", \"array\" and \"object\", and \"string\" is the default.", Type: []string{"string"}, Format: "", }, @@ -1132,6 +1148,21 @@ func schema_pkg_apis_pipeline_v1beta1_ParamSpec(ref common.ReferenceCallback) co Format: "", }, }, + "properties": { + SchemaProps: spec.SchemaProps{ + Description: "Properties is the JSON Schema properties to support key-value pairs parameter.", + 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"), + }, + }, + }, + }, + }, "default": { SchemaProps: spec.SchemaProps{ Description: "Default is the value a parameter takes if no input value is supplied. If default is set, a Task may be executed without a supplied value for the parameter.", @@ -1143,7 +1174,7 @@ func schema_pkg_apis_pipeline_v1beta1_ParamSpec(ref common.ReferenceCallback) co }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PropertySpec"}, } } @@ -2894,6 +2925,25 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineWorkspaceDeclaration(ref common.Re } } +func schema_pkg_apis_pipeline_v1beta1_PropertySpec(ref common.ReferenceCallback) common.OpenAPIDefinition { + return common.OpenAPIDefinition{ + Schema: spec.Schema{ + SchemaProps: spec.SchemaProps{ + Description: "PropertySpec defines the struct for object keys", + Type: []string{"object"}, + Properties: map[string]spec.Schema{ + "type": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, + } +} + func schema_pkg_apis_pipeline_v1beta1_ResolverParam(ref common.ReferenceCallback) common.OpenAPIDefinition { return common.OpenAPIDefinition{ Schema: spec.Schema{ diff --git a/pkg/apis/pipeline/v1beta1/param_context.go b/pkg/apis/pipeline/v1beta1/param_context.go index a2d3882f310..abbcfb7292d 100644 --- a/pkg/apis/pipeline/v1beta1/param_context.go +++ b/pkg/apis/pipeline/v1beta1/param_context.go @@ -58,6 +58,9 @@ func addContextParams(ctx context.Context, in []Param) context.Context { if v.StringVal != "" { p.Value.Type = ParamTypeString } + if v.ObjectVal != nil { + p.Value.Type = ParamTypeObject + } } out[p.Name] = ParamSpec{ Name: p.Name, @@ -86,6 +89,7 @@ func addContextParamSpec(ctx context.Context, in []ParamSpec) context.Context { Name: p.Name, Type: p.Type, Description: p.Description, + Properties: p.Properties, Default: p.Default, } out[p.Name] = cps @@ -127,19 +131,26 @@ func getContextParams(ctx context.Context, overlays ...Param) []Param { // If there is no overlay, pass through the param to the next level. // e.g. for strings $(params.name), for arrays $(params.name[*]). + // for object: {key: param's name, value: $(params.name[*])} p := Param{ Name: ps.Name, } - if ps.Type == ParamTypeString { - p.Value = ArrayOrString{ - Type: ParamTypeString, - StringVal: fmt.Sprintf("$(params.%s)", ps.Name), - } - } else { + switch ps.Type { + case ParamTypeArray: p.Value = ArrayOrString{ Type: ParamTypeArray, ArrayVal: []string{fmt.Sprintf("$(params.%s[*])", ps.Name)}, } + case ParamTypeObject: + p.Value = ArrayOrString{ + Type: ParamTypeObject, + ObjectVal: map[string]string{ps.Name: fmt.Sprintf("$(params.%s[*])", ps.Name)}, + } + default: + p.Value = ArrayOrString{ + Type: ParamTypeString, + StringVal: fmt.Sprintf("$(params.%s)", ps.Name), + } } out = append(out, p) } @@ -161,6 +172,7 @@ func getContextParamSpecs(ctx context.Context) []ParamSpec { Name: ps.Name, Type: ps.Type, Description: ps.Description, + Properties: ps.Properties, Default: ps.Default, }) } diff --git a/pkg/apis/pipeline/v1beta1/param_context_test.go b/pkg/apis/pipeline/v1beta1/param_context_test.go index d51dc1b5637..5989e3be9c3 100644 --- a/pkg/apis/pipeline/v1beta1/param_context_test.go +++ b/pkg/apis/pipeline/v1beta1/param_context_test.go @@ -56,11 +56,30 @@ func TestAddContextParams(t *testing.T) { }, }, }, + { + name: "add-object-param", + params: []Param{{Name: "c", Value: *NewObject(map[string]string{"key1": "val1", "key2": "val2"})}}, + want: paramCtxVal{ + "a": ParamSpec{ + Name: "a", + Type: ParamTypeString, + }, + "b": ParamSpec{ + Name: "b", + Type: ParamTypeArray, + }, + "c": ParamSpec{ + Name: "c", + Type: ParamTypeObject, + }, + }, + }, { name: "existing-param", params: []Param{ {Name: "a", Value: *NewArrayOrString("foo1")}, {Name: "b", Value: *NewArrayOrString("bar1", "baz1")}, + {Name: "c", Value: *NewObject(map[string]string{"key1": "val1", "key2": "val2"})}, }, want: paramCtxVal{ "a": ParamSpec{ @@ -71,6 +90,10 @@ func TestAddContextParams(t *testing.T) { Name: "b", Type: ParamTypeArray, }, + "c": ParamSpec{ + Name: "c", + Type: ParamTypeObject, + }, }, }, { @@ -91,6 +114,10 @@ func TestAddContextParams(t *testing.T) { Name: "b", Type: ParamTypeArray, }, + "c": ParamSpec{ + Name: "c", + Type: ParamTypeObject, + }, }, }, } { @@ -118,11 +145,18 @@ func TestAddContextParamSpec(t *testing.T) { name: "add-paramspec", params: []ParamSpec{{ Name: "a", + }, { + Name: "b", + Properties: map[string]PropertySpec{"key1": {}}, }}, want: paramCtxVal{ "a": ParamSpec{ Name: "a", }, + "b": ParamSpec{ + Name: "b", + Properties: map[string]PropertySpec{"key1": {}}, + }, }, }, { @@ -132,6 +166,12 @@ func TestAddContextParamSpec(t *testing.T) { Type: ParamTypeArray, Default: NewArrayOrString("foo", "bar"), Description: "tacocat", + }, { + Name: "b", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{"key2": {}}, + Default: NewObject(map[string]string{"key2": "val"}), + Description: "my object", }}, want: paramCtxVal{ "a": ParamSpec{ @@ -140,6 +180,13 @@ func TestAddContextParamSpec(t *testing.T) { Default: NewArrayOrString("foo", "bar"), Description: "tacocat", }, + "b": ParamSpec{ + Name: "b", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{"key2": {}}, + Default: NewObject(map[string]string{"key2": "val"}), + Description: "my object", + }, }, }, { @@ -159,6 +206,13 @@ func TestAddContextParamSpec(t *testing.T) { Default: NewArrayOrString("foo", "bar"), Description: "tacocat", }, + "b": ParamSpec{ + Name: "b", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{"key2": {}}, + Default: NewObject(map[string]string{"key2": "val"}), + Description: "my object", + }, }, }, } { @@ -187,6 +241,13 @@ func TestGetContextParams(t *testing.T) { Default: NewArrayOrString("bar"), Description: "racecar", }, + { + Name: "c", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{"key1": {}}, + Default: NewObject(map[string]string{"key1": "val1"}), + Description: "my object", + }, } ctx = addContextParamSpec(ctx, want) @@ -210,6 +271,13 @@ func TestGetContextParams(t *testing.T) { ArrayVal: []string{"$(params.b[*])"}, }, }, + { + Name: "c", + Value: ArrayOrString{ + Type: ParamTypeObject, + ObjectVal: map[string]string{"c": "$(params.c[*])"}, + }, + }, }, }, { @@ -217,6 +285,9 @@ func TestGetContextParams(t *testing.T) { overlay: []Param{{ Name: "a", Value: *NewArrayOrString("tacocat"), + }, { + Name: "c", + Value: *NewObject(map[string]string{"key2": "val2"}), }}, want: []Param{ { @@ -230,6 +301,13 @@ func TestGetContextParams(t *testing.T) { ArrayVal: []string{"$(params.b[*])"}, }, }, + { + Name: "c", + Value: ArrayOrString{ + Type: ParamTypeObject, + ObjectVal: map[string]string{"key2": "val2"}, + }, + }, }, }, } { @@ -257,6 +335,13 @@ func TestGetContextParamSpecs(t *testing.T) { Default: NewArrayOrString("bar"), Description: "racecar", }, + { + Name: "c", + Type: ParamTypeObject, + Properties: map[string]PropertySpec{"key1": {}}, + Default: NewObject(map[string]string{"key1": "val1"}), + Description: "my object", + }, } ctx = addContextParamSpec(ctx, want) diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 4e863ffc052..4f2c594bc3f 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -38,13 +38,16 @@ type ParamSpec struct { // Name declares the name by which a parameter is referenced. Name string `json:"name"` // Type is the user-specified type of the parameter. The possible types - // are currently "string" and "array", and "string" is the default. + // are currently "string", "array" and "object", and "string" is the default. // +optional Type ParamType `json:"type,omitempty"` // Description is a user-facing description of the parameter that may be // used to populate a UI. // +optional Description string `json:"description,omitempty"` + // Properties is the JSON Schema properties to support key-value pairs parameter. + // +optional + Properties map[string]PropertySpec `json:"properties,omitempty"` // Default is the value a parameter takes if no input value is supplied. If // default is set, a Task may be executed without a supplied value for the // parameter. @@ -52,25 +55,47 @@ type ParamSpec struct { Default *ArrayOrString `json:"default,omitempty"` } +// PropertySpec defines the struct for object keys +type PropertySpec struct { + Type ParamType `json:"type,omitempty"` +} + // SetDefaults set the default type func (pp *ParamSpec) SetDefaults(ctx context.Context) { - if pp != nil && pp.Type == "" { - if pp.Default != nil { - // propagate the parsed ArrayOrString's type to the parent ParamSpec's type - if pp.Default.Type != "" { - // propagate the default type if specified - pp.Type = pp.Default.Type - } else { - // determine the type based on the array or string values when default value is specified but not the type - if pp.Default.ArrayVal != nil { - pp.Type = ParamTypeArray - } else { - pp.Type = ParamTypeString - } - } - } else { - // ParamTypeString is the default value (when no type can be inferred from the default value) - pp.Type = ParamTypeString + if pp == nil { + return + } + + // Propagate inferred type to the parent ParamSpec's type, and default type to the PropertySpec's type + // The sequence to look at is type in ParamSpec -> properties -> type in default -> array/string/object value in default + // If neither `properties` or `default` section is provided, ParamTypeString will be the default type. + switch { + case pp.Type != "": + // If param type is provided by the author, do nothing but just set default type for PropertySpec in case `properties` section is provided. + pp.setDefaultsForProperties(ctx) + case pp.Properties != nil: + pp.Type = ParamTypeObject + // Also set default type for PropertySpec + pp.setDefaultsForProperties(ctx) + case pp.Default == nil: + // ParamTypeString is the default value (when no type can be inferred from the default value) + pp.Type = ParamTypeString + case pp.Default.Type != "": + pp.Type = pp.Default.Type + case pp.Default.ArrayVal != nil: + pp.Type = ParamTypeArray + case pp.Default.ObjectVal != nil: + pp.Type = ParamTypeObject + default: + pp.Type = ParamTypeString + } +} + +// setDefaultsForProperties sets default type for PropertySpec (string) if it's not specified +func (pp *ParamSpec) setDefaultsForProperties(ctx context.Context) { + for key, propertySpec := range pp.Properties { + if propertySpec.Type == "" { + pp.Properties[key] = PropertySpec{Type: ParamTypeString} } } } @@ -93,31 +118,40 @@ type ParamType string const ( ParamTypeString ParamType = "string" ParamTypeArray ParamType = "array" + ParamTypeObject ParamType = "object" ) // AllParamTypes can be used for ParamType validation. -var AllParamTypes = []ParamType{ParamTypeString, ParamTypeArray} +var AllParamTypes = []ParamType{ParamTypeString, ParamTypeArray, ParamTypeObject} // ArrayOrString is modeled after IntOrString in kubernetes/apimachinery: // ArrayOrString is a type that can hold a single string or string array. // Used in JSON unmarshalling so that a single JSON field can accept // either an individual string or an array of strings. +// TODO (@chuangw6): This struct will be renamed or be embedded in a new struct to take into +// consideration the object case after the community reaches an agreement on it. type ArrayOrString struct { Type ParamType `json:"type"` // Represents the stored type of ArrayOrString. StringVal string `json:"stringVal"` // +listType=atomic - ArrayVal []string `json:"arrayVal"` + ArrayVal []string `json:"arrayVal"` + ObjectVal map[string]string `json:"objectVal"` } // UnmarshalJSON implements the json.Unmarshaller interface. func (arrayOrString *ArrayOrString) UnmarshalJSON(value []byte) error { - if value[0] == '"' { + switch value[0] { + case '[': + arrayOrString.Type = ParamTypeArray + return json.Unmarshal(value, &arrayOrString.ArrayVal) + case '{': + arrayOrString.Type = ParamTypeObject + return json.Unmarshal(value, &arrayOrString.ObjectVal) + default: arrayOrString.Type = ParamTypeString return json.Unmarshal(value, &arrayOrString.StringVal) } - arrayOrString.Type = ParamTypeArray - return json.Unmarshal(value, &arrayOrString.ArrayVal) } // MarshalJSON implements the json.Marshaller interface. @@ -127,6 +161,8 @@ func (arrayOrString ArrayOrString) MarshalJSON() ([]byte, error) { return json.Marshal(arrayOrString.StringVal) case ParamTypeArray: return json.Marshal(arrayOrString.ArrayVal) + case ParamTypeObject: + return json.Marshal(arrayOrString.ObjectVal) default: return []byte{}, fmt.Errorf("impossible ArrayOrString.Type: %q", arrayOrString.Type) } @@ -160,6 +196,14 @@ func NewArrayOrString(value string, values ...string) *ArrayOrString { } } +// NewObject creates an ArrayOrString of type ParamTypeObject using the provided key-value pairs +func NewObject(pairs map[string]string) *ArrayOrString { + return &ArrayOrString{ + Type: ParamTypeObject, + ObjectVal: pairs, + } +} + // ArrayReference returns the name of the parameter from array parameter reference // returns arrayParam from $(params.arrayParam[*]) func ArrayReference(a string) string { diff --git a/pkg/apis/pipeline/v1beta1/param_types_test.go b/pkg/apis/pipeline/v1beta1/param_types_test.go index 45c3e8b014a..1ec6610cb0f 100644 --- a/pkg/apis/pipeline/v1beta1/param_types_test.go +++ b/pkg/apis/pipeline/v1beta1/param_types_test.go @@ -73,7 +73,44 @@ func TestParamSpec_SetDefaults(t *testing.T) { }, }, }, { - name: "fully defined ParamSpec", + name: "inferred type from default value - object", + before: &v1beta1.ParamSpec{ + Name: "parametername", + Default: &v1beta1.ArrayOrString{ + ObjectVal: map[string]string{"url": "test", "path": "test"}, + }, + }, + defaultsApplied: &v1beta1.ParamSpec{ + Name: "parametername", + Type: v1beta1.ParamTypeObject, + Default: &v1beta1.ArrayOrString{ + ObjectVal: map[string]string{"url": "test", "path": "test"}, + }, + }, + }, { + name: "inferred type from properties - PropertySpec type is not provided", + before: &v1beta1.ParamSpec{ + Name: "parametername", + Properties: map[string]v1beta1.PropertySpec{"key1": {}}, + }, + defaultsApplied: &v1beta1.ParamSpec{ + Name: "parametername", + Type: v1beta1.ParamTypeObject, + Properties: map[string]v1beta1.PropertySpec{"key1": {Type: "string"}}, + }, + }, { + name: "inferred type from properties - PropertySpec type is provided", + before: &v1beta1.ParamSpec{ + Name: "parametername", + Properties: map[string]v1beta1.PropertySpec{"key2": {Type: "string"}}, + }, + defaultsApplied: &v1beta1.ParamSpec{ + Name: "parametername", + Type: v1beta1.ParamTypeObject, + Properties: map[string]v1beta1.PropertySpec{"key2": {Type: "string"}}, + }, + }, { + name: "fully defined ParamSpec - array", before: &v1beta1.ParamSpec{ Name: "parametername", Type: v1beta1.ParamTypeArray, @@ -90,6 +127,24 @@ func TestParamSpec_SetDefaults(t *testing.T) { ArrayVal: []string{"array"}, }, }, + }, { + name: "fully defined ParamSpec - object", + before: &v1beta1.ParamSpec{ + Name: "parametername", + Type: v1beta1.ParamTypeObject, + Description: "a description", + Default: &v1beta1.ArrayOrString{ + ObjectVal: map[string]string{"url": "test", "path": "test"}, + }, + }, + defaultsApplied: &v1beta1.ParamSpec{ + Name: "parametername", + Type: v1beta1.ParamTypeObject, + Description: "a description", + Default: &v1beta1.ArrayOrString{ + ObjectVal: map[string]string{"url": "test", "path": "test"}, + }, + }, }} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -182,7 +237,7 @@ func TestArrayOrString_UnmarshalJSON(t *testing.T) { }, { input: map[string]interface{}{"val": nil}, - result: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeArray, ArrayVal: nil}, + result: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, ArrayVal: nil}, }, { input: map[string]interface{}{"val": []string{}}, @@ -196,6 +251,10 @@ func TestArrayOrString_UnmarshalJSON(t *testing.T) { input: map[string]interface{}{"val": []string{"multiple", "elements"}}, result: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"multiple", "elements"}}, }, + { + input: map[string]interface{}{"val": map[string]string{"key1": "val1", "key2": "val2"}}, + result: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeObject, ObjectVal: map[string]string{"key1": "val1", "key2": "val2"}}, + }, } for _, c := range cases { @@ -231,6 +290,7 @@ func TestArrayOrString_MarshalJSON(t *testing.T) { {*v1beta1.NewArrayOrString("123"), "{\"val\":\"123\"}"}, {*v1beta1.NewArrayOrString("123", "1234"), "{\"val\":[\"123\",\"1234\"]}"}, {*v1beta1.NewArrayOrString("a", "a", "a"), "{\"val\":[\"a\",\"a\",\"a\"]}"}, + {*v1beta1.NewObject(map[string]string{"key1": "var1", "key2": "var2"}), "{\"val\":{\"key1\":\"var1\",\"key2\":\"var2\"}}"}, } for _, c := range cases { diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 99ef08835e6..9cd98f072b9 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -299,12 +299,13 @@ } }, "v1beta1.ArrayOrString": { - "description": "ArrayOrString is a type that can hold a single string or string array. Used in JSON unmarshalling so that a single JSON field can accept either an individual string or an array of strings.", + "description": "ArrayOrString is a type that can hold a single string or string array. Used in JSON unmarshalling so that a single JSON field can accept either an individual string or an array of strings. consideration the object case after the community reaches an agreement on it.", "type": "object", "required": [ "type", "stringVal", - "arrayVal" + "arrayVal", + "objectVal" ], "properties": { "arrayVal": { @@ -315,6 +316,13 @@ }, "x-kubernetes-list-type": "atomic" }, + "objectVal": { + "type": "object", + "additionalProperties": { + "type": "string", + "default": "" + } + }, "stringVal": { "description": "Represents the stored type of ArrayOrString.", "type": "string", @@ -720,8 +728,16 @@ "type": "string", "default": "" }, + "properties": { + "description": "Properties is the JSON Schema properties to support key-value pairs parameter.", + "type": "object", + "additionalProperties": { + "default": {}, + "$ref": "#/definitions/v1beta1.PropertySpec" + } + }, "type": { - "description": "Type is the user-specified type of the parameter. The possible types are currently \"string\" and \"array\", and \"string\" is the default.", + "description": "Type is the user-specified type of the parameter. The possible types are currently \"string\", \"array\" and \"object\", and \"string\" is the default.", "type": "string" } } @@ -1642,6 +1658,15 @@ } } }, + "v1beta1.PropertySpec": { + "description": "PropertySpec defines the struct for object keys", + "type": "object", + "properties": { + "type": { + "type": "string" + } + } + }, "v1beta1.ResolverParam": { "description": "ResolverParam is a single parameter passed to a resolver.", "type": "object", diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 35383626013..66accdd81e6 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -25,6 +25,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/substitution" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -62,7 +63,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validateSteps(ctx, mergedSteps).ViaField("steps")) errs = errs.Also(ts.Resources.Validate(ctx).ViaField("resources")) - errs = errs.Also(ValidateParameterTypes(ts.Params).ViaField("params")) + errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params")) errs = errs.Also(ValidateParameterVariables(ts.Steps, ts.Params)) errs = errs.Also(ValidateResourcesVariables(ts.Steps, ts.Resources)) errs = errs.Also(validateTaskContextVariables(ts.Steps)) @@ -237,8 +238,13 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi } // ValidateParameterTypes validates all the types within a slice of ParamSpecs -func ValidateParameterTypes(params []ParamSpec) (errs *apis.FieldError) { +func ValidateParameterTypes(ctx context.Context, params []ParamSpec) (errs *apis.FieldError) { for _, p := range params { + if p.Type == ParamTypeObject { + // Object type parameter is an alpha feature and will fail validation if it's used in a task spec + // when the enable-api-fields feature gate is not "alpha". + errs = errs.Also(ValidateEnabledAPIFields(ctx, "object type parameter", config.AlphaAPIFields)) + } errs = errs.Also(p.ValidateType()) } return errs @@ -268,6 +274,33 @@ func (p ParamSpec) ValidateType() *apis.FieldError { }, } } + + // Check object type and its PropertySpec type + return p.ValidateObjectType() +} + +// ValidateObjectType checks that object type parameter does not miss the +// definition of `properties` section and the type of a PropertySpec is allowed. +// (Currently, only string is allowed) +func (p ParamSpec) ValidateObjectType() *apis.FieldError { + if p.Type == ParamTypeObject && p.Properties == nil { + return apis.ErrMissingField(fmt.Sprintf("%s.properties", p.Name)) + } + + invalidKeys := []string{} + for key, propertySpec := range p.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", invalidKeys), + Paths: []string{fmt.Sprintf("%s.properties", p.Name)}, + } + } + return nil } @@ -275,16 +308,21 @@ func (p ParamSpec) ValidateType() *apis.FieldError { func ValidateParameterVariables(steps []Step, params []ParamSpec) *apis.FieldError { parameterNames := sets.NewString() arrayParameterNames := sets.NewString() + objectParamSpecs := []ParamSpec{} for _, p := range params { parameterNames.Insert(p.Name) if p.Type == ParamTypeArray { arrayParameterNames.Insert(p.Name) } + if p.Type == ParamTypeObject { + objectParamSpecs = append(objectParamSpecs, p) + } } errs := validateVariables(steps, "params", parameterNames) - return errs.Also(validateArrayUsage(steps, "params", arrayParameterNames)) + errs = errs.Also(validateArrayUsage(steps, "params", arrayParameterNames)) + return errs.Also(validateObjectUsage(steps, objectParamSpecs)) } func validateTaskContextVariables(steps []Step) *apis.FieldError { @@ -320,6 +358,52 @@ func ValidateResourcesVariables(steps []Step, resources *TaskResources) *apis.Fi return validateVariables(steps, "resources.(?:inputs|outputs)", resourceNames) } +// TODO (@chuangw6): Make sure an object param is not used as a whole when providing values for strings. +// https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#variable-replacement-with-object-params +// "When providing values for strings, Task and Pipeline authors can access +// individual attributes of an object param; they cannot access the object +// as whole (we could add support for this later)." +func validateObjectUsage(steps []Step, params []ParamSpec) (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) + } + + if p.Default != nil && p.Default.ObjectVal != nil { + errs = errs.Also(validateObjectKeysInDefault(p.Default.ObjectVal, objectKeys, p.Name)) + } + + // check if the object's key names are referenced correctly i.e. param.objectParam.key1 + errs = errs.Also(validateVariables(steps, fmt.Sprintf("params\\.%s", p.Name), objectKeys)) + } + + return errs +} + +// validate if object keys defined in properties are all provided in default +func validateObjectKeysInDefault(defaultObject map[string]string, neededObjectKeys sets.String, paramName string) (errs *apis.FieldError) { + neededObjectKeysInSpec := neededObjectKeys.List() + providedObjectKeysInDefault := []string{} + for k := range defaultObject { + providedObjectKeysInDefault = append(providedObjectKeysInDefault, k) + } + + missingObjectKeys := list.DiffLeft(neededObjectKeysInSpec, providedObjectKeysInDefault) + if len(missingObjectKeys) != 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("Required key(s) %s for the parameter %s are not provided in default.", missingObjectKeys, paramName), + Paths: []string{fmt.Sprintf("%s.properties", paramName), fmt.Sprintf("%s.default", paramName)}, + } + } + return nil +} + func validateArrayUsage(steps []Step, prefix string, vars sets.String) (errs *apis.FieldError) { for idx, step := range steps { errs = errs.Also(validateStepArrayUsage(step, prefix, vars)).ViaFieldIndex("steps", idx) diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index bd50010f604..31f34452865 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -137,6 +137,26 @@ func TestTaskSpecValidate(t *testing.T) { Type: v1beta1.ParamTypeString, Description: "param", Default: v1beta1.NewArrayOrString("default"), + }, { + Name: "myobj", + Type: v1beta1.ParamTypeObject, + Description: "param", + Properties: map[string]v1beta1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + Default: v1beta1.NewObject(map[string]string{ + "key1": "var1", + "key2": "var2", + }), + }, { + Name: "myobjWithoutDefault", + Type: v1beta1.ParamTypeObject, + Description: "param", + Properties: map[string]v1beta1.PropertySpec{ + "key1": {}, + "key2": {}, + }, }}, Steps: validSteps, }, @@ -363,7 +383,7 @@ func TestTaskSpecValidate(t *testing.T) { Workspaces: tt.fields.Workspaces, Results: tt.fields.Results, } - ctx := context.Background() + ctx := getContextBasedOnFeatureFlag("alpha") ts.SetDefaults(ctx) if err := ts.Validate(ctx); err != nil { t.Errorf("TaskSpec.Validate() = %v", err) @@ -530,6 +550,90 @@ func TestTaskSpecValidateError(t *testing.T) { 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: []v1beta1.ParamSpec{{ + Name: "task", + Type: v1beta1.ParamTypeArray, + Description: "param", + Default: v1beta1.NewObject(map[string]string{ + "key1": "var1", + "key2": "var2", + }), + }}, + Steps: validSteps, + }, + 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: []v1beta1.ParamSpec{{ + Name: "task", + Type: v1beta1.ParamTypeObject, + Description: "param", + Properties: map[string]v1beta1.PropertySpec{"key1": {}}, + Default: v1beta1.NewArrayOrString("var"), + }}, + Steps: validSteps, + }, + expectedError: apis.FieldError{ + Message: `"object" type does not match default value's type: "string"`, + Paths: []string{"params.task.type", "params.task.default.type"}, + }, + }, { + name: "the spec of object type parameter misses the definition of properties", + fields: fields{ + Params: []v1beta1.ParamSpec{{ + Name: "task", + Type: v1beta1.ParamTypeObject, + Description: "param", + }}, + Steps: validSteps, + }, + expectedError: *apis.ErrMissingField(fmt.Sprintf("params.task.properties")), + }, { + name: "PropertySpec type is set with unsupported type", + fields: fields{ + Params: []v1beta1.ParamSpec{{ + Name: "task", + Type: v1beta1.ParamTypeObject, + Description: "param", + Properties: map[string]v1beta1.PropertySpec{ + "key1": {Type: "number"}, + "key2": {Type: "string"}, + }, + }}, + Steps: validSteps, + }, + expectedError: apis.FieldError{ + Message: fmt.Sprintf("The value type specified for these keys %v is invalid", []string{"key1"}), + Paths: []string{"params.task.properties"}, + }, + }, { + name: "keys defined in properties are missed in default", + fields: fields{ + Params: []v1beta1.ParamSpec{{ + Name: "myobjectParam", + Description: "param", + Properties: map[string]v1beta1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + Default: v1beta1.NewObject(map[string]string{ + "key1": "var1", + "key3": "var1", + }), + }}, + Steps: validSteps, + }, + expectedError: apis.FieldError{ + Message: fmt.Sprintf("Required key(s) %s for the parameter %s are not provided in default.", []string{"key2"}, "myobjectParam"), + Paths: []string{"myobjectParam.properties", "myobjectParam.default"}, + }, }, { name: "invalid step", fields: fields{ @@ -994,9 +1098,9 @@ func TestTaskSpecValidateError(t *testing.T) { Workspaces: tt.fields.Workspaces, Results: tt.fields.Results, } - ctx := context.Background() + ctx := getContextBasedOnFeatureFlag("alpha") ts.SetDefaults(ctx) - err := ts.Validate(context.Background()) + err := ts.Validate(ctx) if err == nil { t.Fatalf("Expected an error, got nothing for %v", ts) } diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index 36612306b7d..e4fa03228e7 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -56,7 +56,7 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(ts.TaskSpec.Validate(ctx).ViaField("taskSpec")) } - errs = errs.Also(validateParameters(ts.Params).ViaField("params")) + errs = errs.Also(validateParameters(ctx, ts.Params).ViaField("params")) errs = errs.Also(validateWorkspaceBindings(ctx, ts.Workspaces).ViaField("workspaces")) errs = errs.Also(ts.Resources.Validate(ctx).ViaField("resources")) if ts.Debug != nil { @@ -112,12 +112,17 @@ func validateWorkspaceBindings(ctx context.Context, wb []WorkspaceBinding) (errs return errs } -func validateParameters(params []Param) (errs *apis.FieldError) { +func validateParameters(ctx context.Context, params []Param) (errs *apis.FieldError) { var names []string for _, p := range params { + if p.Value.Type == ParamTypeObject { + // Object type parameter is an alpha feature and will fail validation if it's used in a taskrun spec + // when the enable-api-fields feature gate is not "alpha". + errs = errs.Also(ValidateEnabledAPIFields(ctx, "object type parameter", config.AlphaAPIFields)) + } names = append(names, p.Name) } - return validateNoDuplicateNames(names, false) + return errs.Also(validateNoDuplicateNames(names, false)) } func validateStepOverrides(overrides []TaskRunStepOverride) (errs *apis.FieldError) { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 569a7100729..89bfffe91ed 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -261,7 +261,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, wantErr: apis.ErrMultipleOneOf("params[myname].name"), }, { - name: "invalid params- same names but different case", + name: "invalid params - same names but different case", spec: v1beta1.TaskRunSpec{ Params: []v1beta1.Param{{ Name: "FOO", @@ -273,6 +273,20 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { TaskRef: &v1beta1.TaskRef{Name: "mytask"}, }, wantErr: apis.ErrMultipleOneOf("params[foo].name"), + }, { + name: "invalid params (object type) - same names but different case", + spec: v1beta1.TaskRunSpec{ + Params: []v1beta1.Param{{ + Name: "MYOBJECTPARAM", + Value: *v1beta1.NewObject(map[string]string{"key1": "val1", "key2": "val2"}), + }, { + Name: "myobjectparam", + Value: *v1beta1.NewObject(map[string]string{"key1": "val1", "key2": "val2"}), + }}, + TaskRef: &v1beta1.TaskRef{Name: "mytask"}, + }, + wantErr: apis.ErrMultipleOneOf("params[myobjectparam].name"), + wc: enableAlphaAPIFields, }, { name: "use of bundle without the feature flag set", spec: v1beta1.TaskRunSpec{ diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 50e3cdbc3a7..70b62ad2027 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -38,6 +38,13 @@ func (in *ArrayOrString) DeepCopyInto(out *ArrayOrString) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.ObjectVal != nil { + in, out := &in.ObjectVal, &out.ObjectVal + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } @@ -323,6 +330,13 @@ func (in *Param) DeepCopy() *Param { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParamSpec) DeepCopyInto(out *ParamSpec) { *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 + } + } if in.Default != nil { in, out := &in.Default, &out.Default *out = new(ArrayOrString) @@ -1241,6 +1255,22 @@ func (in *PipelineWorkspaceDeclaration) DeepCopy() *PipelineWorkspaceDeclaration return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PropertySpec) DeepCopyInto(out *PropertySpec) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PropertySpec. +func (in *PropertySpec) DeepCopy() *PropertySpec { + if in == nil { + return nil + } + out := new(PropertySpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResolverParam) DeepCopyInto(out *ResolverParam) { *out = *in diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index c16e48b8280..96d3a3589bb 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -81,6 +81,9 @@ func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params if wrongTypeParamNames := wrongTypeParamsNames(params, matrix, neededParamsTypes); len(wrongTypeParamNames) != 0 { return fmt.Errorf("param types don't match the user-specified type: %s", wrongTypeParamNames) } + if missingKeysObjectParamNames := missingKeysObjectParamNames(paramSpecs, params); len(missingKeysObjectParamNames) != 0 { + return fmt.Errorf("missing keys for these params which are required in ParamSpec's properties %v", missingKeysObjectParamNames) + } return nil } @@ -154,6 +157,51 @@ func wrongTypeParamsNames(params []v1beta1.Param, matrix []v1beta1.Param, needed return wrongTypeParamNames } +// missingKeysObjectParamNames checks if all required keys of object type params are provided in taskrun params. +// TODO (@chuangw6): This might be refactored out to support single pair validation. +func missingKeysObjectParamNames(paramSpecs []v1beta1.ParamSpec, params []v1beta1.Param) []string { + neededKeys := make(map[string][]string) + providedKeys := make(map[string][]string) + + // collect needed keys for object parameters + for _, spec := range paramSpecs { + if spec.Type == v1beta1.ParamTypeObject { + for key := range spec.Properties { + neededKeys[spec.Name] = append(neededKeys[spec.Name], key) + } + } + } + + // collect provided keys for object parameters + for _, p := range params { + if p.Value.Type == v1beta1.ParamTypeObject { + for key := range p.Value.ObjectVal { + providedKeys[p.Name] = append(providedKeys[p.Name], key) + } + } + } + + return validateObjectKeys(neededKeys, providedKeys) +} + +// validateObjectKeys checks if objects have missing keys in its provider (either taskrun value or result value) +func validateObjectKeys(neededObjectKeys, providedObjectKeys map[string][]string) []string { + missings := []string{} + for p, keys := range providedObjectKeys { + if _, ok := neededObjectKeys[p]; !ok { + // Ignore any missing objects - this happens when extra objects were + // passed that aren't being used. + continue + } + missedKeys := list.DiffLeft(neededObjectKeys[p], keys) + if len(missedKeys) != 0 { + missings = append(missings, p) + } + } + + return missings +} + // ValidateResolvedTaskResources validates task inputs, params and output matches taskrun func ValidateResolvedTaskResources(ctx context.Context, params []v1beta1.Param, matrix []v1beta1.Param, rtr *resources.ResolvedTaskResources) error { if err := validateParams(ctx, rtr.TaskSpec.Params, params, matrix); err != nil { diff --git a/pkg/reconciler/taskrun/validate_resources_test.go b/pkg/reconciler/taskrun/validate_resources_test.go index f16ca441dcc..6feae3f9044 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -141,6 +141,13 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { { Name: "zoo", Type: v1beta1.ParamTypeString, + }, { + Name: "myobj", + Type: v1beta1.ParamTypeObject, + Properties: map[string]v1beta1.PropertySpec{ + "key1": {}, + "key2": {}, + }, }, }, }, @@ -154,6 +161,13 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { }, { Name: "bar", Value: *v1beta1.NewArrayOrString("somethinggood"), + }, { + Name: "myobj", + Value: *v1beta1.NewObject(map[string]string{ + "key1": "val1", + "key2": "val2", + "extra_key": "val3", + }), }} m := []v1beta1.Param{{ Name: "zoo", @@ -196,6 +210,14 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { Name: "bar", Type: v1beta1.ParamTypeArray, }, + { + Name: "myobj", + Type: v1beta1.ParamTypeObject, + Properties: map[string]v1beta1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + }, }, }, } @@ -235,7 +257,20 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { Name: "bar", Value: *v1beta1.NewArrayOrString("bar", "foo"), }}, - }} + }, { + name: "missing object param keys", + rtr: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + params: []v1beta1.Param{{ + Name: "myobj", + Value: *v1beta1.NewObject(map[string]string{ + "key1": "val1", + "misskey": "val2", + }), + }}, + }, + } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { if err := ValidateResolvedTaskResources(ctx, tc.params, tc.matrix, tc.rtr); err == nil {