diff --git a/pkg/apis/pipeline/v1/container_validation_test.go b/pkg/apis/pipeline/v1/container_validation_test.go index 5c378d8c29e..4c5fdd2801e 100644 --- a/pkg/apis/pipeline/v1/container_validation_test.go +++ b/pkg/apis/pipeline/v1/container_validation_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -28,6 +29,8 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) @@ -171,6 +174,136 @@ func TestRef_Invalid(t *testing.T) { } } +func TestStepValidate(t *testing.T) { + tests := []struct { + name string + Step v1.Step + }{{ + name: "unnamed step", + Step: v1.Step{ + Image: "myimage", + }, + }, { + name: "valid step with script", + Step: v1.Step{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + hello world`, + }, + }, { + name: "valid step with script and args", + Step: v1.Step{ + Image: "my-image", + Args: []string{"arg"}, + Script: ` + #!/usr/bin/env bash + hello $1`, + }, + }, { + name: "valid step with volumeMount under /tekton/home", + Step: v1.Step{ + Image: "myimage", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/tekton/home", + }}, + }, + }} + for _, st := range tests { + t.Run(st.name, func(t *testing.T) { + ctx := cfgtesting.EnableAlphaAPIFields(context.Background()) + if err := st.Step.Validate(ctx); err != nil { + t.Errorf("Step.Validate() = %v", err) + } + }) + } +} + +func TestStepValidateError(t *testing.T) { + tests := []struct { + name string + Step v1.Step + expectedError apis.FieldError + }{{ + name: "invalid step with missing Image", + Step: v1.Step{}, + expectedError: apis.FieldError{ + Message: "missing field(s)", + Paths: []string{"Image"}, + }, + }, { + name: "invalid step name", + Step: v1.Step{ + Name: "replaceImage", + Image: "myimage", + }, + expectedError: apis.FieldError{ + Message: `invalid value "replaceImage"`, + Paths: []string{"name"}, + Details: "Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", + }, + }, { + name: "step with script and command", + Step: v1.Step{ + Image: "myimage", + Command: []string{"command"}, + Script: "script", + }, + expectedError: apis.FieldError{ + Message: "script cannot be used with command", + Paths: []string{"script"}, + }, + }, { + name: "step volume mounts under /tekton/", + Step: v1.Step{ + Image: "myimage", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/tekton/foo", + }}, + }, + expectedError: apis.FieldError{ + Message: `volumeMount cannot be mounted under /tekton/ (volumeMount "foo" mounted at "/tekton/foo")`, + Paths: []string{"volumeMounts[0].mountPath"}, + }, + }, { + name: "step volume mount name starts with tekton-internal-", + Step: v1.Step{ + Image: "myimage", + VolumeMounts: []corev1.VolumeMount{{ + Name: "tekton-internal-foo", + MountPath: "/this/is/fine", + }}, + }, + expectedError: apis.FieldError{ + Message: `volumeMount name "tekton-internal-foo" cannot start with "tekton-internal-"`, + Paths: []string{"volumeMounts[0].name"}, + }, + }, { + name: "negative timeout string", + Step: v1.Step{ + Timeout: &metav1.Duration{Duration: -10 * time.Second}, + }, + expectedError: apis.FieldError{ + Message: "invalid value: -10s", + Paths: []string{"negative timeout"}, + }, + }} + for _, st := range tests { + t.Run(st.name, func(t *testing.T) { + ctx := cfgtesting.EnableAlphaAPIFields(context.Background()) + err := st.Step.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", st.Step) + } + if d := cmp.Diff(st.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("Step.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestSidecarValidate(t *testing.T) { tests := []struct { name string diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 254e11c2d62..dc025934da3 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "testing" - "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -38,11 +37,6 @@ var validSteps = []v1.Step{{ Image: "myimage", }} -var invalidSteps = []v1.Step{{ - Name: "replaceImage", - Image: "myimage", -}} - func TestTaskValidate(t *testing.T) { tests := []struct { name string @@ -161,15 +155,6 @@ func TestTaskSpecValidate(t *testing.T) { name string fields fields }{{ - name: "unnamed steps", - fields: fields{ - Steps: []v1.Step{{ - Image: "myimage", - }, { - Image: "myotherimage", - }}, - }, - }, { name: "valid params type implied", fields: fields{ Params: []v1.ParamSpec{{ @@ -323,16 +308,6 @@ func TestTaskSpecValidate(t *testing.T) { Image: "some-image", }, }, - }, { - name: "valid step with script", - fields: fields{ - Steps: []v1.Step{{ - Image: "my-image", - Script: ` - #!/usr/bin/env bash - hello world`, - }}, - }, }, { name: "step template included in validation with stepaction", fields: fields{ @@ -383,28 +358,6 @@ func TestTaskSpecValidate(t *testing.T) { hello $(params.baz)`, }}, }, - }, { - name: "valid step with script and args", - fields: fields{ - Steps: []v1.Step{{ - Image: "my-image", - Args: []string{"arg"}, - Script: ` - #!/usr/bin/env bash - hello $1`, - }}, - }, - }, { - name: "valid step with volumeMount under /tekton/home", - fields: fields{ - Steps: []v1.Step{{ - Image: "myimage", - VolumeMounts: []corev1.VolumeMount{{ - Name: "foo", - MountPath: "/tekton/home", - }}, - }}, - }, }, { name: "valid workspace", fields: fields{ @@ -1037,16 +990,6 @@ func TestTaskSpecValidateError(t *testing.T) { Message: "missing field(s)", Paths: []string{"steps"}, }, - }, { - name: "invalid step name", - fields: fields{ - Steps: invalidSteps, - }, - expectedError: apis.FieldError{ - Message: `invalid value "replaceImage"`, - Paths: []string{"steps[0].name"}, - Details: "Task step name must be a valid DNS Label, For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names", - }, }, { name: "duplicate step name", fields: fields{ @@ -1245,49 +1188,6 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `multiple volumes with same name "workspace"`, Paths: []string{"volumes[1].name"}, }, - }, { - name: "step with script and command", - fields: fields{ - Steps: []v1.Step{{ - Image: "myimage", - Command: []string{"command"}, - Script: "script", - }}, - }, - expectedError: apis.FieldError{ - Message: "script cannot be used with command", - Paths: []string{"steps[0].script"}, - }, - }, { - name: "step volume mounts under /tekton/", - fields: fields{ - Steps: []v1.Step{{ - Image: "myimage", - VolumeMounts: []corev1.VolumeMount{{ - Name: "foo", - MountPath: "/tekton/foo", - }}, - }}, - }, - expectedError: apis.FieldError{ - Message: `volumeMount cannot be mounted under /tekton/ (volumeMount "foo" mounted at "/tekton/foo")`, - Paths: []string{"steps[0].volumeMounts[0].mountPath"}, - }, - }, { - name: "step volume mount name starts with tekton-internal-", - fields: fields{ - Steps: []v1.Step{{ - Image: "myimage", - VolumeMounts: []corev1.VolumeMount{{ - Name: "tekton-internal-foo", - MountPath: "/this/is/fine", - }}, - }}, - }, - expectedError: apis.FieldError{ - Message: `volumeMount name "tekton-internal-foo" cannot start with "tekton-internal-"`, - Paths: []string{"steps[0].volumeMounts[0].name"}, - }, }, { name: "declared workspaces names are not unique", fields: fields{ @@ -1397,7 +1297,7 @@ func TestTaskSpecValidateError(t *testing.T) { Paths: []string{"workspaces[0].mountpath"}, }, }, { - name: "result name not validate", + name: "result name not valid", fields: fields{ Steps: validSteps, Results: []v1.TaskResult{{ @@ -1411,7 +1311,7 @@ func TestTaskSpecValidateError(t *testing.T) { Details: "Name must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$')", }, }, { - name: "result type not validate", + name: "result type not valid", fields: fields{ Steps: validSteps, Results: []v1.TaskResult{{ @@ -1426,7 +1326,7 @@ func TestTaskSpecValidateError(t *testing.T) { Details: "type must be string", }, }, { - name: "context not validate", + name: "context not valid", fields: fields{ Steps: []v1.Step{{ Image: "my-image", @@ -1440,17 +1340,6 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `non-existent variable in "\n\t\t\t\t#!/usr/bin/env bash\n\t\t\t\thello \"$(context.task.missing)\""`, Paths: []string{"steps[0].script"}, }, - }, { - name: "negative timeout string", - fields: fields{ - Steps: []v1.Step{{ - Timeout: &metav1.Duration{Duration: -10 * time.Second}, - }}, - }, - expectedError: apis.FieldError{ - Message: "invalid value: -10s", - Paths: []string{"steps[0].negative timeout"}, - }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {