Correct missing field(s) error

Fixes #2235 Fixes #1844
tektoncd · Mar 27, 2020 · 543f5a5 · 543f5a5
1 parent 0910814
commit 543f5a5
Show file tree

Hide file tree

Showing 10 changed files with 21 additions and 13 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go
@@ -154,7 +154,7 @@ func validateParamResults(tasks []PipelineTask) error {
 // of Tasks expressed in the Pipeline makes sense.
 func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError {
 	if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) {
-		return apis.ErrMissingField(apis.CurrentField)
+		return apis.ErrGeneric("expected at least one, got none", "spec.description", "spec.params", "spec.resources", "spec.tasks", "spec.workspaces")
 	}
 
 	// Names cannot be duplicated

diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
@@ -49,6 +49,11 @@ func TestPipeline_Validate(t *testing.T) {
 			tb.PipelineTask("foo", "foo-task"),
 		)),
 		failureExpected: true,
+	}, {
+		name: "pipeline spec missing",
+		p: tb.Pipeline("pipeline", "namespace",
+		),
+		failureExpected: true,
 	}, {
 		name: "pipeline spec invalid (duplicate tasks)",
 		p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(

diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go
@@ -26,7 +26,6 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 )
@@ -41,9 +40,6 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError {
 }
 
 func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError {
-	if equality.Semantic.DeepEqual(ts, &TaskSpec{}) {
-		return apis.ErrMissingField(apis.CurrentField)
-	}
 
 	if len(ts.Steps) == 0 {
 		return apis.ErrMissingField("steps")

diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go
@@ -368,7 +368,7 @@ func TestTaskSpecValidateError(t *testing.T) {
 		name: "nil",
 		expectedError: apis.FieldError{
 			Message: `missing field(s)`,
-			Paths:   []string{""},
+			Paths:   []string{"steps"},
 		},
 	}, {
 		name: "no build",

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go
@@ -138,7 +138,7 @@ func validateGraph(tasks []PipelineTask) error {
 // of Tasks expressed in the Pipeline makes sense.
 func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError {
 	if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) {
-		return apis.ErrMissingField(apis.CurrentField)
+		return apis.ErrGeneric("expected at least one, got none", "spec.description", "spec.params", "spec.resources", "spec.tasks", "spec.workspaces")
 	}
 
 	// Names cannot be duplicated

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
@@ -99,6 +99,12 @@ func TestPipeline_Validate(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{Name: "asdf123456789012345678901234567890123456789012345678901234567890"},
 		},
 		failureExpected: true,
+	}, {
+		name: "pipeline spec missing",
+		p: &v1beta1.Pipeline{
+			ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
+		},
+		failureExpected: true,
 	}, {
 		name: "pipeline spec missing taskref and taskspec",
 		p: &v1beta1.Pipeline{

diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go
@@ -25,7 +25,6 @@ import (
 	"github.com/tektoncd/pipeline/pkg/apis/validate"
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 )
@@ -40,9 +39,6 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError {
 }
 
 func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError {
-	if equality.Semantic.DeepEqual(ts, &TaskSpec{}) {
-		return apis.ErrMissingField(apis.CurrentField)
-	}
 
 	if len(ts.Steps) == 0 {
 		return apis.ErrMissingField("steps")

diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go
@@ -285,7 +285,7 @@ func TestTaskSpecValidateError(t *testing.T) {
 		name: "nil",
 		expectedError: apis.FieldError{
 			Message: `missing field(s)`,
-			Paths:   []string{""},
+			Paths:   []string{"steps"},
 		},
 	}, {
 		name: "no step",

diff --git a/pkg/apis/resource/v1alpha1/pipelineresource_validation.go b/pkg/apis/resource/v1alpha1/pipelineresource_validation.go
@@ -39,7 +39,7 @@ func (r *PipelineResource) Validate(ctx context.Context) *apis.FieldError {
 
 func (rs *PipelineResourceSpec) Validate(ctx context.Context) *apis.FieldError {
 	if equality.Semantic.DeepEqual(rs, &PipelineResourceSpec{}) {
-		return apis.ErrMissingField(apis.CurrentField)
+		return apis.ErrMissingField("spec.type")
 	}
 	if rs.Type == PipelineResourceTypeCluster {
 		var authFound, cadataFound, isInsecure bool

diff --git a/pkg/apis/resource/v1alpha1/pipelineresource_validation_test.go b/pkg/apis/resource/v1alpha1/pipelineresource_validation_test.go
@@ -135,6 +135,11 @@ func TestResourceValidation_Invalid(t *testing.T) {
 				},
 			},
 			want: apis.ErrInvalidValue("spec.type", "not-supported"),
+		}, {
+			name: "missing spec",
+			res: &v1alpha1.PipelineResource{
+			},
+			want: apis.ErrMissingField("spec.type"),
 		},
 	}
 	for _, tt := range tests {