From 5b4cb0ca3099bbd4373f0a055565a8a1c532eee1 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Mon, 27 Jun 2022 14:02:18 +0000 Subject: [PATCH] Relax result type validation to avoid nightly build failure This commit fixes the string result type validation, PR #4779 adds result type and asumes that the default type should be string via mutating webhook. PR #4818 adds the validation for this. However, resources that did already exist in etcd didn't get the default. So this commit relaxes the validation for empty result type. --- .../pipeline/v1beta1/openapi_generated.go | 1 - pkg/apis/pipeline/v1beta1/result_types.go | 2 +- .../pipeline/v1beta1/result_validation.go | 6 +++ pkg/apis/pipeline/v1beta1/swagger.json | 3 +- .../pipeline/v1beta1/task_validation_test.go | 48 +++++++++++++++++++ 5 files changed, 56 insertions(+), 4 deletions(-) diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 5ae4d1a7b91..2f7b249752a 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -3977,7 +3977,6 @@ func schema_pkg_apis_pipeline_v1beta1_TaskResult(ref common.ReferenceCallback) c "description": { SchemaProps: spec.SchemaProps{ Description: "Description is a human-readable description of the result", - Default: "", Type: []string{"string"}, Format: "", }, diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index a4fd77b26da..21a4e1ae166 100644 --- a/pkg/apis/pipeline/v1beta1/result_types.go +++ b/pkg/apis/pipeline/v1beta1/result_types.go @@ -25,7 +25,7 @@ type TaskResult struct { // Description is a human-readable description of the result // +optional - Description string `json:"description"` + Description string `json:"description,omitempty"` } // TaskRunResult used to describe the results of a task diff --git a/pkg/apis/pipeline/v1beta1/result_validation.go b/pkg/apis/pipeline/v1beta1/result_validation.go index f8b31857280..4e72529e66d 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -31,6 +31,12 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { return errs.Also(ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields)) } + // Skip the validation for existing resources without specifying a type and not call the setDefaults + if tr.Type == "" { + return nil + } + + // By default the result type is string if tr.Type != ResultsTypeString { return apis.ErrInvalidValue(tr.Type, "type", fmt.Sprintf("type must be string")) } diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index c45f3b1faf1..0d55e1f7791 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2243,8 +2243,7 @@ "properties": { "description": { "description": "Description is a human-readable description of the result", - "type": "string", - "default": "" + "type": "string" }, "name": { "description": "Name the given name", diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index e08433eb336..ecf895b033e 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -436,6 +436,54 @@ func TestTaskSpecValidate(t *testing.T) { } } +func TestTaskSpecResultsValidate(t *testing.T) { + type fields struct { + Steps []v1beta1.Step + Results []v1beta1.TaskResult + } + tests := []struct { + name string + fields fields + }{{ + name: "valid result type empty", + fields: fields{ + Steps: []v1beta1.Step{{ + Image: "my-image", + Args: []string{"arg"}, + }}, + Results: []v1beta1.TaskResult{{ + Name: "MY-RESULT", + Description: "my great result", + }}, + }, + }, { + name: "valid result type string", + fields: fields{ + Steps: []v1beta1.Step{{ + Image: "my-image", + Args: []string{"arg"}, + }}, + Results: []v1beta1.TaskResult{{ + Name: "MY-RESULT", + Type: "string", + Description: "my great result", + }}, + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := &v1beta1.TaskSpec{ + Steps: tt.fields.Steps, + Results: tt.fields.Results, + } + ctx := getContextBasedOnFeatureFlag("stable") + if err := ts.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + func TestTaskSpecValidateError(t *testing.T) { type fields struct { Params []v1beta1.ParamSpec