From 72930ce37b53a67c0cebf78db711f40c2bf964f1 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 | 7 + .../v1beta1/result_validation_test.go | 147 ++++++++++++++++++ pkg/apis/pipeline/v1beta1/swagger.json | 3 +- 5 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 pkg/apis/pipeline/v1beta1/result_validation_test.go diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index b7ecf823187..1493d23ebd6 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -4373,7 +4373,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..6df43149644 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -31,6 +31,13 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { return errs.Also(ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields)) } + // Resources created before the result. Type was introduced may not have Type set + // and should be considered valid + 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/result_validation_test.go b/pkg/apis/pipeline/v1beta1/result_validation_test.go new file mode 100644 index 00000000000..728e84dcfbe --- /dev/null +++ b/pkg/apis/pipeline/v1beta1/result_validation_test.go @@ -0,0 +1,147 @@ +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/test/diff" + "knative.dev/pkg/apis" +) + +func TestResultsValidate(t *testing.T) { + tests := []struct { + name string + Result v1beta1.TaskResult + apiFields string + }{{ + name: "valid result type empty", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Description: "my great result", + }, + apiFields: "stable", + }, { + name: "valid result type string", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: "string", + Description: "my great result", + }, + + apiFields: "stable", + }, { + name: "valid result type array", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: "array", + Description: "my great result", + }, + + apiFields: "alpha", + }, { + name: "valid result type object", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: "array", + Description: "my great result", + }, + + apiFields: "alpha", + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := getContextBasedOnFeatureFlag(tt.apiFields) + if err := tt.Result.Validate(ctx); err != nil { + t.Errorf("TaskSpec.Validate() = %v", err) + } + }) + } +} + +func TestResultsValidateError(t *testing.T) { + tests := []struct { + name string + Result v1beta1.TaskResult + apiFields string + expectedError apis.FieldError + }{{ + name: "invalid result type in stable", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: "wrong", + Description: "my great result", + }, + apiFields: "stable", + expectedError: apis.FieldError{ + Message: `invalid value: wrong`, + Paths: []string{"type"}, + Details: "type must be string", + }, + }, { + name: "invalid result type in alpha", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: "wrong", + Description: "my great result", + }, + apiFields: "alpha", + expectedError: apis.FieldError{ + Message: `invalid value: wrong`, + Paths: []string{"type"}, + Details: "type must be string", + }, + }, { + name: "invalid array result type in stable", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: "array", + Description: "my great result", + }, + apiFields: "stable", + expectedError: apis.FieldError{ + Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"", + }, + }, { + name: "invalid object result type in stable", + Result: v1beta1.TaskResult{ + Name: "MY-RESULT", + Type: "object", + Description: "my great result", + }, + apiFields: "stable", + expectedError: apis.FieldError{ + Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"", + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := getContextBasedOnFeatureFlag(tt.apiFields) + err := tt.Result.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", tt.Result) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 6e7d2c66b2d..dcc4490d42c 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2447,8 +2447,7 @@ "properties": { "description": { "description": "Description is a human-readable description of the result", - "type": "string", - "default": "" + "type": "string" }, "name": { "description": "Name the given name",