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.
Yongxuanzhang · Aug 2, 2022 · e8ee879 · e8ee879
1 parent e4dd1e2
commit e8ee879
Show file tree

Hide file tree

Showing 6 changed files with 124 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"},
+				},
 			}},
 		},
 	}, {