diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 232fcbbe03f..d68e1a936e0 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), @@ -405,8 +406,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"}, }, }, } @@ -1119,7 +1135,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: "", }, @@ -1131,6 +1147,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.", @@ -1142,7 +1173,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"}, } } @@ -2893,6 +2924,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..b07406202ea 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 @@ -135,11 +139,20 @@ func getContextParams(ctx context.Context, overlays ...Param) []Param { Type: ParamTypeString, StringVal: fmt.Sprintf("$(params.%s)", ps.Name), } - } else { + } else if ps.Type == ParamTypeArray { p.Value = ArrayOrString{ Type: ParamTypeArray, ArrayVal: []string{fmt.Sprintf("$(params.%s[*])", ps.Name)}, } + } else { + objVal := make(map[string]string) + for k := range ps.Properties { + objVal[fmt.Sprintf("$(params.%s.%s)", p.Name, k)] = "" + } + p.Value = ArrayOrString{ + Type: ParamTypeObject, + ObjectVal: objVal, + } } out = append(out, p) } @@ -161,6 +174,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..2f03a81e8fa 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{"$(params.c.key1)": ""}, + }, + }, }, }, { @@ -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..83dda5ff4e9 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,26 +55,44 @@ 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 + if pp == nil || pp.Type != "" { + return + } + + // if the properties section is not empty, set object as the parameter type + if pp.Properties != nil { + pp.Type = ParamTypeObject + for _, property := range pp.Properties { + property.Type = ParamTypeString + } + return + } + + 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 if pp.Default.ObjectVal != nil { + pp.Type = ParamTypeObject } 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 - } + pp.Type = ParamTypeString } - } else { - // ParamTypeString is the default value (when no type can be inferred from the default value) - pp.Type = ParamTypeString } + } else { + // ParamTypeString is the default value (when no type can be inferred from the default value) + pp.Type = ParamTypeString } } @@ -93,10 +114,11 @@ 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: @@ -107,17 +129,23 @@ 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 = ParamTypeString return json.Unmarshal(value, &arrayOrString.StringVal) + case '[': + arrayOrString.Type = ParamTypeArray + return json.Unmarshal(value, &arrayOrString.ArrayVal) + default: + arrayOrString.Type = ParamTypeObject + return json.Unmarshal(value, &arrayOrString.ObjectVal) } - arrayOrString.Type = ParamTypeArray - return json.Unmarshal(value, &arrayOrString.ArrayVal) } // MarshalJSON implements the json.Marshaller interface. @@ -127,6 +155,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 +190,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 d706cacaa8a..dffd4a14577 100644 --- a/pkg/apis/pipeline/v1beta1/param_types_test.go +++ b/pkg/apis/pipeline/v1beta1/param_types_test.go @@ -72,7 +72,33 @@ 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 - object", + 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": {}}, + }, + }, { + name: "fully defined ParamSpec - array", before: &v1beta1.ParamSpec{ Name: "parametername", Type: v1beta1.ParamTypeArray, @@ -89,6 +115,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) { @@ -176,6 +220,7 @@ func TestArrayOrString_UnmarshalJSON(t *testing.T) { {"{\"val\":[]}", v1beta1.ArrayOrString{Type: v1beta1.ParamTypeArray, ArrayVal: []string{}}}, {"{\"val\":[\"oneelement\"]}", v1beta1.ArrayOrString{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"oneelement"}}}, {"{\"val\":[\"multiple\", \"elements\"]}", *v1beta1.NewArrayOrString("multiple", "elements")}, + {"{\"val\":{\"key1\":\"var1\", \"key2\":\"var2\"}}", *v1beta1.NewObject(map[string]string{"key1": "var1", "key2": "var2"})}, } for _, c := range cases { @@ -197,6 +242,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 794007f7b02..96dcf25374a 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -304,7 +304,8 @@ "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 2a9435fd0b6..b23b7d850e2 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" @@ -257,6 +258,16 @@ func (p ParamSpec) ValidateType() *apis.FieldError { return apis.ErrInvalidValue(p.Type, fmt.Sprintf("%s.type", p.Name)) } + if p.Type == ParamTypeObject && p.Properties == nil { + return &apis.FieldError{ + Message: "The parameter of object type misses the definition of `properties`", + Paths: []string{ + fmt.Sprintf("%s.type", p.Name), + fmt.Sprintf("%s.default.type", p.Name), + }, + } + } + // If a default value is provided, ensure its type matches param's declared type. if (p.Default != nil) && (p.Default.Type != p.Type) { return &apis.FieldError{ @@ -275,16 +286,37 @@ func (p ParamSpec) ValidateType() *apis.FieldError { func ValidateParameterVariables(steps []Step, params []ParamSpec) *apis.FieldError { parameterNames := sets.NewString() arrayParameterNames := sets.NewString() + objectParameterNames := sets.NewString() for _, p := range params { parameterNames.Insert(p.Name) if p.Type == ParamTypeArray { arrayParameterNames.Insert(p.Name) } + if p.Type == ParamTypeObject { + // collect all object parameter names + objectParameterNames.Insert(p.Name) + + // collect all keys for this object param + objectKeys := sets.NewString() + for key := range p.Properties { + objectKeys.Insert(key) + } + + // check if object keys set in default are specified in object properties. + errs := validateObjectKeysInDefault(p.Default.ObjectVal, objectKeys, p.Name) + + // check if the object's keys are referenced correctly + errs = errs.Also(validateArrayORObjectUsage(steps, fmt.Sprintf("params.%s", p.Name), objectKeys)) + if errs != nil { + return errs + } + } } errs := validateVariables(steps, "params", parameterNames) - return errs.Also(validateArrayUsage(steps, "params", arrayParameterNames)) + errs = errs.Also(validateArrayORObjectUsage(steps, "params", arrayParameterNames)) + return errs.Also(validateArrayORObjectUsage(steps, "params", objectParameterNames)) } func validateTaskContextVariables(steps []Step) *apis.FieldError { @@ -320,14 +352,32 @@ func ValidateResourcesVariables(steps []Step, resources *TaskResources) *apis.Fi return validateVariables(steps, "resources.(?:inputs|outputs)", resourceNames) } -func validateArrayUsage(steps []Step, prefix string, vars sets.String) (errs *apis.FieldError) { +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 keys %v declared in properties are not provided in default", missingObjectKeys), + // Empty path is required to make the `ViaField`, … work + Paths: []string{""}, + } + } + return nil +} + +func validateArrayORObjectUsage(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) + errs = errs.Also(validateStepArrayORObjectUsage(step, prefix, vars)).ViaFieldIndex("steps", idx) } return errs } -func validateStepArrayUsage(step Step, prefix string, vars sets.String) *apis.FieldError { +func validateStepArrayORObjectUsage(step Step, prefix string, vars sets.String) *apis.FieldError { errs := validateTaskNoArrayReferenced(step.Name, prefix, vars).ViaField("name") errs = errs.Also(validateTaskNoArrayReferenced(step.Image, prefix, vars).ViaField("image")) errs = errs.Also(validateTaskNoArrayReferenced(step.WorkingDir, prefix, vars).ViaField("workingDir")) diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 6cb3391e71b..50d6e6a7b5c 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -137,6 +137,18 @@ 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", + }), }}, Steps: validSteps, }, @@ -552,6 +564,46 @@ 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: "object param mismatching property keys in default", + fields: fields{ + Params: []v1beta1.ParamSpec{{ + Name: "task", + Type: v1beta1.ParamTypeObject, + 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 keys %v declared in properties are not provided in default", []string{"key2"}), + Paths: []string{""}, + }, }, { name: "invalid step", fields: fields{ diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 983a234151b..52e956ffbf1 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/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 4187b915a84..3d59d4a713e 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -35,6 +35,7 @@ func ApplyParameters(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1. // that need to be further processed. stringReplacements := map[string]string{} arrayReplacements := map[string][]string{} + objectReplacements := map[string]map[string]string{} patterns := []string{ "params.%s", @@ -49,10 +50,14 @@ func ApplyParameters(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1. for _, pattern := range patterns { stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.StringVal } - } else { + } else if p.Default.Type == v1beta1.ParamTypeArray { for _, pattern := range patterns { arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ArrayVal } + } else { + for _, pattern := range patterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ObjectVal + } } } } @@ -62,10 +67,14 @@ func ApplyParameters(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1. for _, pattern := range patterns { stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.StringVal } - } else { + } else if p.Value.Type == v1beta1.ParamTypeArray { for _, pattern := range patterns { arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ArrayVal } + } else { + for _, pattern := range patterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ObjectVal + } } } diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index c8dcb75320d..3a15082515f 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -40,6 +40,7 @@ func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1 // that need to be further processed. stringReplacements := map[string]string{} arrayReplacements := map[string][]string{} + objectReplacements := map[string]map[string]string{} patterns := []string{ "params.%s", @@ -56,10 +57,14 @@ func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1 for _, pattern := range patterns { stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.StringVal } - } else { + } else if p.Default.Type == v1beta1.ParamTypeArray { for _, pattern := range patterns { arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ArrayVal } + } else { + for _, pattern := range patterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ObjectVal + } } } } @@ -69,10 +74,14 @@ func ApplyParameters(spec *v1beta1.TaskSpec, tr *v1beta1.TaskRun, defaults ...v1 for _, pattern := range patterns { stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.StringVal } - } else { + } else if p.Value.Type == v1beta1.ParamTypeArray { for _, pattern := range patterns { arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ArrayVal } + } else { + for _, pattern := range patterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ObjectVal + } } } return ApplyReplacements(spec, stringReplacements, arrayReplacements) diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index 182f2bf4c7a..bff4457bc68 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -73,13 +73,29 @@ func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params var neededParams []string paramTypes := make(map[string]v1beta1.ParamType) neededParams = make([]string, 0, len(paramSpecs)) + neededObjectParamsKeys := make(map[string][]string) for _, inputResourceParam := range paramSpecs { neededParams = append(neededParams, inputResourceParam.Name) paramTypes[inputResourceParam.Name] = inputResourceParam.Type + + // collect needed keys for each parameter of type Object + if inputResourceParam.Type == v1beta1.ParamTypeObject { + for propertyKey := range inputResourceParam.Properties { + neededObjectParamsKeys[inputResourceParam.Name] = append(neededObjectParamsKeys[inputResourceParam.Name], propertyKey) + } + } } providedParams := make([]string, 0, len(params)) + providedObjectParamsKeys := make(map[string][]string) for _, param := range params { providedParams = append(providedParams, param.Name) + + // collect provided keys for each parameter of type Object + if param.Value.Type == v1beta1.ParamTypeObject { + for propertyKey := range param.Value.ObjectVal { + providedObjectParamsKeys[param.Name] = append(providedObjectParamsKeys[param.Name], propertyKey) + } + } } missingParams := list.DiffLeft(neededParams, providedParams) var missingParamsNoDefaults []string @@ -122,6 +138,25 @@ func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params return fmt.Errorf("param types don't match the user-specified type: %s", wrongTypeParamNames) } + // At this point, we have checked against missing/extra params and param type alignment + // We now want to make sure all keys of object type params are provided in taskrun params. + var missingObjectKeysParamNames []string + for _, param := range params { + if _, ok := neededObjectParamsKeys[param.Name]; !ok { + // Ignore any missing params - this happens when extra params were + // passed to the task that aren't being used. + continue + } + miss := list.DiffLeft(neededObjectParamsKeys[param.Name], providedObjectParamsKeys[param.Name]) + if len(miss) != 0 { + missingObjectKeysParamNames = append(missingObjectKeysParamNames, param.Name) + } + } + + if len(missingObjectKeysParamNames) != 0 { + return fmt.Errorf("taskrun params miss keys for these params of type object specified in task params %s", missingObjectKeysParamNames) + } + return nil } diff --git a/pkg/reconciler/taskrun/validate_resources_test.go b/pkg/reconciler/taskrun/validate_resources_test.go index c5e84be8338..b1f9cdda751 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -143,6 +143,14 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { Name: "bar", Type: v1beta1.ParamTypeString, }, + { + Name: "myobj", + Type: v1beta1.ParamTypeObject, + Properties: map[string]v1beta1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + }, }, }, } @@ -155,6 +163,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", + }), }} if err := ValidateResolvedTaskResources(ctx, p, rtr); err != nil { t.Fatalf("Did not expect to see error when validating TaskRun with correct params but saw %v", err) @@ -188,6 +203,14 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { Name: "foo", Type: v1beta1.ParamTypeString, }, + { + Name: "myobj", + Type: v1beta1.ParamTypeObject, + Properties: map[string]v1beta1.PropertySpec{ + "key1": {}, + "key2": {}, + }, + }, }, }, } @@ -195,16 +218,36 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { name string rtr *resources.ResolvedTaskResources params []v1beta1.Param - }{{ - name: "missing-params", - rtr: &resources.ResolvedTaskResources{ - TaskSpec: &task.Spec, + }{ + { + name: "missing all params", + rtr: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + params: []v1beta1.Param{ + { + Name: "foobar", + Value: *v1beta1.NewArrayOrString("somethingfun"), + }, + }, }, - params: []v1beta1.Param{{ - Name: "foobar", - Value: *v1beta1.NewArrayOrString("somethingfun"), - }}, - }} + { + name: "missing object param keys", + rtr: &resources.ResolvedTaskResources{ + TaskSpec: &task.Spec, + }, + params: []v1beta1.Param{{ + Name: "foo", + Value: *v1beta1.NewArrayOrString("somethingfun"), + }, { + 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.rtr); err == nil {