From e8ee8798a6758bd4bfdd41bb23ec63eff1fb7aec Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Thu, 14 Jul 2022 22:08:55 +0000 Subject: [PATCH] Add validation for results object properties types This commit adds the validation for results object properties types. Right now we only support string types, so if the type is not string we should validate and return err for this. --- pkg/apis/pipeline/v1/result_validation.go | 27 ++++++++++++- .../pipeline/v1/result_validation_test.go | 38 ++++++++++++++++--- pkg/apis/pipeline/v1/task_validation_test.go | 4 ++ .../pipeline/v1beta1/result_validation.go | 27 ++++++++++++- .../v1beta1/result_validation_test.go | 38 ++++++++++++++++--- .../pipeline/v1beta1/task_validation_test.go | 4 ++ 6 files changed, 124 insertions(+), 14 deletions(-) diff --git a/pkg/apis/pipeline/v1/result_validation.go b/pkg/apis/pipeline/v1/result_validation.go index de17c847182..4d227cb6b1c 100644 --- a/pkg/apis/pipeline/v1/result_validation.go +++ b/pkg/apis/pipeline/v1/result_validation.go @@ -35,7 +35,9 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { } // Array and Object are alpha features if tr.Type == ResultsTypeArray || tr.Type == ResultsTypeObject { - return errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields)) + errs := validateObjectResult(tr) + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields)) + return errs } // Resources created before the result. Type was introduced may not have Type set @@ -51,3 +53,26 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { 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(tr TaskResult) (errs *apis.FieldError) { + if ParamType(tr.Type) == ParamTypeObject && tr.Properties == nil { + return apis.ErrMissingField(fmt.Sprintf("%s.properties", tr.Name)) + } + + invalidKeys := []string{} + for key, propertySpec := range tr.Properties { + if propertySpec.Type != ParamTypeString { + invalidKeys = append(invalidKeys, key) + } + } + + if len(invalidKeys) != 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("The value type specified for these keys %v is invalid, the type must be string", invalidKeys), + Paths: []string{fmt.Sprintf("%s.properties", tr.Name)}, + } + } + return nil +} diff --git a/pkg/apis/pipeline/v1/result_validation_test.go b/pkg/apis/pipeline/v1/result_validation_test.go index 366143bb42f..b7d8a41348a 100644 --- a/pkg/apis/pipeline/v1/result_validation_test.go +++ b/pkg/apis/pipeline/v1/result_validation_test.go @@ -44,7 +44,7 @@ func TestResultsValidate(t *testing.T) { name: "valid result type string", Result: v1.TaskResult{ Name: "MY-RESULT", - Type: "string", + Type: v1.ResultsTypeString, Description: "my great result", }, @@ -53,7 +53,7 @@ func TestResultsValidate(t *testing.T) { name: "valid result type array", Result: v1.TaskResult{ Name: "MY-RESULT", - Type: "array", + Type: v1.ResultsTypeArray, Description: "my great result", }, @@ -62,10 +62,10 @@ func TestResultsValidate(t *testing.T) { name: "valid result type object", Result: v1.TaskResult{ Name: "MY-RESULT", - Type: "array", + Type: v1.ResultsTypeObject, Description: "my great result", + Properties: map[string]v1.PropertySpec{"hello": {Type: v1.ParamTypeString}}, }, - apiFields: "alpha", }} for _, tt := range tests { @@ -117,7 +117,7 @@ func TestResultsValidateError(t *testing.T) { name: "invalid array result type in stable", Result: v1.TaskResult{ Name: "MY-RESULT", - Type: "array", + Type: v1.ResultsTypeArray, Description: "my great result", }, apiFields: "stable", @@ -128,13 +128,39 @@ func TestResultsValidateError(t *testing.T) { name: "invalid object result type in stable", Result: v1.TaskResult{ Name: "MY-RESULT", - Type: "object", + Type: v1.ResultsTypeObject, Description: "my great result", + Properties: map[string]v1.PropertySpec{"hello": {Type: v1.ParamTypeString}}, }, apiFields: "stable", expectedError: apis.FieldError{ Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"", }, + }, { + name: "invalid object properties type", + Result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1.PropertySpec{"hello": {Type: "wrong type"}}, + }, + apiFields: "alpha", + expectedError: apis.FieldError{ + Message: "The value type specified for these keys [hello] is invalid, the type must be string", + Paths: []string{"MY-RESULT.properties"}, + }, + }, { + name: "invalid object properties empty", + Result: v1.TaskResult{ + Name: "MY-RESULT", + Type: v1.ResultsTypeObject, + Description: "my great result", + }, + apiFields: "alpha", + expectedError: apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"MY-RESULT.properties"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 000540d7600..c35314b3cd1 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -328,6 +328,10 @@ func TestTaskSpecValidate(t *testing.T) { Name: "MY-RESULT", Type: v1.ResultsTypeObject, Description: "my great result", + Properties: map[string]v1.PropertySpec{ + "url": {"string"}, + "commit": {"string"}, + }, }}, }, }, { diff --git a/pkg/apis/pipeline/v1beta1/result_validation.go b/pkg/apis/pipeline/v1beta1/result_validation.go index 3d0dd4ee045..4232b174fad 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -29,7 +29,9 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { } // Array and Object is alpha feature if tr.Type == ResultsTypeArray || tr.Type == ResultsTypeObject { - return errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields)) + errs := validateObjectResult(tr) + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields)) + return errs } // Resources created before the result. Type was introduced may not have Type set @@ -45,3 +47,26 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { 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(tr TaskResult) (errs *apis.FieldError) { + if ParamType(tr.Type) == ParamTypeObject && tr.Properties == nil { + return apis.ErrMissingField(fmt.Sprintf("%s.properties", tr.Name)) + } + + invalidKeys := []string{} + for key, propertySpec := range tr.Properties { + if propertySpec.Type != ParamTypeString { + invalidKeys = append(invalidKeys, key) + } + } + + if len(invalidKeys) != 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("The value type specified for these keys %v is invalid, the type must be string", invalidKeys), + Paths: []string{fmt.Sprintf("%s.properties", tr.Name)}, + } + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/result_validation_test.go b/pkg/apis/pipeline/v1beta1/result_validation_test.go index 87c60f04646..57019c02143 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/result_validation_test.go @@ -44,7 +44,7 @@ func TestResultsValidate(t *testing.T) { name: "valid result type string", Result: v1beta1.TaskResult{ Name: "MY-RESULT", - Type: "string", + Type: v1beta1.ResultsTypeString, Description: "my great result", }, @@ -53,7 +53,7 @@ func TestResultsValidate(t *testing.T) { name: "valid result type array", Result: v1beta1.TaskResult{ Name: "MY-RESULT", - Type: "array", + Type: v1beta1.ResultsTypeArray, Description: "my great result", }, @@ -62,10 +62,10 @@ func TestResultsValidate(t *testing.T) { name: "valid result type object", Result: v1beta1.TaskResult{ Name: "MY-RESULT", - Type: "array", + Type: v1beta1.ResultsTypeObject, Description: "my great result", + Properties: map[string]v1beta1.PropertySpec{"hello": {Type: v1beta1.ParamTypeString}}, }, - apiFields: "alpha", }} for _, tt := range tests { @@ -117,7 +117,7 @@ func TestResultsValidateError(t *testing.T) { name: "invalid array result type in stable", Result: v1beta1.TaskResult{ Name: "MY-RESULT", - Type: "array", + Type: v1beta1.ResultsTypeArray, Description: "my great result", }, apiFields: "stable", @@ -128,13 +128,39 @@ func TestResultsValidateError(t *testing.T) { name: "invalid object result type in stable", Result: v1beta1.TaskResult{ Name: "MY-RESULT", - Type: "object", + Type: v1beta1.ResultsTypeObject, Description: "my great result", + Properties: map[string]v1beta1.PropertySpec{"hello": {Type: v1beta1.ParamTypeString}}, }, apiFields: "stable", expectedError: apis.FieldError{ Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"", }, + }, { + name: "invalid object properties type", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeObject, + Description: "my great result", + Properties: map[string]v1beta1.PropertySpec{"hello": {Type: "wrong type"}}, + }, + apiFields: "alpha", + expectedError: apis.FieldError{ + Message: "The value type specified for these keys [hello] is invalid, the type must be string", + Paths: []string{"MY-RESULT.properties"}, + }, + }, { + name: "invalid object properties empty", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeObject, + Description: "my great result", + }, + apiFields: "alpha", + expectedError: apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"MY-RESULT.properties"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 1bb722ee4c9..3d69ad6ae00 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -359,6 +359,10 @@ func TestTaskSpecValidate(t *testing.T) { Name: "MY-RESULT", Type: v1beta1.ResultsTypeObject, Description: "my great result", + Properties: map[string]v1beta1.PropertySpec{ + "url": {"string"}, + "commit": {"string"}, + }, }}, }, }, {