Skip to content

Commit

Permalink
Validate param type
Browse files Browse the repository at this point in the history
  • Loading branch information
chuangw6 committed Apr 30, 2022
1 parent be2448f commit 7f61593
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 11 deletions.
5 changes: 3 additions & 2 deletions pkg/apis/pipeline/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1beta1/param_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,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
Expand Down Expand Up @@ -161,6 +162,7 @@ func getContextParamSpecs(ctx context.Context) []ParamSpec {
Name: ps.Name,
Type: ps.Type,
Description: ps.Description,
Properties: ps.Properties,
Default: ps.Default,
})
}
Expand Down
26 changes: 18 additions & 8 deletions pkg/apis/pipeline/v1beta1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ 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
Expand All @@ -55,14 +55,20 @@ 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 {
if pp.Properties != nil {
pp.Type = ParamTypeObject
for _, property := range pp.Properties {
property.Type = ParamTypeString
}
} else 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
Expand All @@ -73,9 +79,6 @@ func (pp *ParamSpec) SetDefaults(ctx context.Context) {
pp.Type = ParamTypeArray
} else if pp.Default.ObjectVal != nil {
pp.Type = ParamTypeObject
for _, property := range pp.Properties {
property.Type = ParamTypeString
}
} else {
pp.Type = ParamTypeString
}
Expand Down Expand Up @@ -126,12 +129,17 @@ type ArrayOrString struct {

// 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.
Expand All @@ -141,6 +149,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)
}
Expand Down
13 changes: 13 additions & 0 deletions pkg/apis/pipeline/v1beta1/param_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ func TestParamSpec_SetDefaults(t *testing.T) {
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{
Expand Down Expand Up @@ -209,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 {
Expand All @@ -230,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 {
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/pipeline/v1beta1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@
}
},
"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"
}
}
Expand Down Expand Up @@ -1659,6 +1659,7 @@
}
},
"v1beta1.PropertySpec": {
"description": "PropertySpec defines the struct for object keys",
"type": "object",
"properties": {
"type": {
Expand Down
10 changes: 10 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,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{
Expand Down

0 comments on commit 7f61593

Please sign in to comment.