Skip to content

Commit

Permalink
Helpful error message when multiple volumes share name
Browse files Browse the repository at this point in the history
A Task is not allowed to have multiple volumes that share a name. Prior
to this commit the error message printed when a name collision occurred
was "expected exactly one, got both: name" which doesn't describe the
problem clearly.

After this commit the error message is updated to 'multiple volumes with
same name "foo"'. A test case has been added to exercise the new message.
  • Loading branch information
Scott authored and tekton-robot committed Oct 9, 2019
1 parent d36e719 commit 5824fd5
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
5 changes: 4 additions & 1 deletion pkg/apis/pipeline/v1alpha1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ func ValidateVolumes(volumes []corev1.Volume) *apis.FieldError {
vols := map[string]struct{}{}
for _, v := range volumes {
if _, ok := vols[v.Name]; ok {
return apis.ErrMultipleOneOf("name")
return &apis.FieldError{
Message: fmt.Sprintf("multiple volumes with same name %q", v.Name),
Paths: []string{"name"},
}
}
vols[v.Name] = struct{}{}
}
Expand Down
19 changes: 18 additions & 1 deletion pkg/apis/pipeline/v1alpha1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ func TestTaskSpecValidateError(t *testing.T) {
Inputs *v1alpha1.Inputs
Outputs *v1alpha1.Outputs
Steps []v1alpha1.Step
Volumes []corev1.Volume
}
tests := []struct {
name string
Expand Down Expand Up @@ -597,13 +598,29 @@ func TestTaskSpecValidateError(t *testing.T) {
Message: `non-existent variable in "$(inputs.params.foo) && $(inputs.params.inexistent)" for step arg[0]`,
Paths: []string{"taskspec.steps.arg[0]"},
},
}}
},
{
name: "Multiple volumes with same name",
fields: fields{
Steps: validSteps,
Volumes: []corev1.Volume{{
Name: "workspace",
}, {
Name: "workspace",
}},
},
expectedError: apis.FieldError{
Message: `multiple volumes with same name "workspace"`,
Paths: []string{"volumes.name"},
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ts := &v1alpha1.TaskSpec{
Inputs: tt.fields.Inputs,
Outputs: tt.fields.Outputs,
Steps: tt.fields.Steps,
Volumes: tt.fields.Volumes,
}
ctx := context.Background()
ts.SetDefaults(ctx)
Expand Down

0 comments on commit 5824fd5

Please sign in to comment.