diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 573e461d09f..fdb75ac2d64 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -16,6 +16,7 @@ Creation of a `PipelineRun` will trigger the creation of - [Service account](#service-account) - [Service accounts](#service-accounts) - [Pod Template](#pod-template) + - [Workspaces](#workspaces) - [Cancelling a PipelineRun](#cancelling-a-pipelinerun) - [Examples](https://github.com/tektoncd/pipeline/tree/master/examples/pipelineruns) - [Logs](logs.md) @@ -27,7 +28,7 @@ following fields: - Required: - [`apiVersion`][kubernetes-overview] - Specifies the API version, for example - `tekton.dev/v1alpha1`. + `tekton.dev/v1alpha1` - [`kind`][kubernetes-overview] - Specify the `PipelineRun` resource object. - [`metadata`][kubernetes-overview] - Specifies data to uniquely identify the `PipelineRun` resource object, for example a `name`. @@ -265,6 +266,12 @@ spec: claimName: my-volume-claim ``` +## Workspaces + +It is not yet possible to specify [workspaces](tasks.md#workspaces) via `Pipelines` +or `PipelineRuns`, so `Tasks` requiring `workspaces` cannot be used with them until +[#1438](https://github.com/tektoncd/pipeline/issues/1438) is completed. + ## Cancelling a PipelineRun In order to cancel a running pipeline (`PipelineRun`), you need to update its diff --git a/docs/pipelines.md b/docs/pipelines.md index 62e2f9694aa..0807f9416c2 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -6,6 +6,7 @@ This document defines `Pipelines` and their capabilities. - [Syntax](#syntax) - [Declared resources](#declared-resources) + - [Workspaces][#declared-workspaces] - [Parameters](#parameters) - [Pipeline Tasks](#pipeline-tasks) - [From](#from) @@ -72,6 +73,12 @@ spec: type: image ``` +### Declared Workspaces + +It is not yet possible to specify [workspaces](tasks.md#workspaces) via `Pipelines` +or `PipelineRuns`, so `Tasks` requiring `workspaces` cannot be used with them until +[#1438](https://github.com/tektoncd/pipeline/issues/1438) is completed. + ### Parameters `Pipeline`s can declare input parameters that must be supplied to the `Pipeline` diff --git a/docs/taskruns.md b/docs/taskruns.md index 01de3efc7ac..972df49ccde 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -18,6 +18,7 @@ A `TaskRun` runs until all `steps` have completed or until a failure occurs. - [Overriding where resources are copied from](#overriding-where-resources-are-copied-from) - [Service Account](#service-account) - [Pod Template](#pod-template) + - [Workspaces](#workspaces) - [Status](#status) - [Steps](#steps) - [Cancelling a TaskRun](#cancelling-a-taskrun) @@ -57,7 +58,9 @@ following fields: to configure the default timeout. - [`podTemplate`](#pod-template) - Specifies a subset of [`PodSpec`](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.15/#pod-v1-core) - configuration that will be used as the basis for the `Task` pod. + configuration that will be used as the basis for the `Task` pod. + - [`workspaces`](#workspaces) - Specify the actual volumes to use for the + [workspaces](tasks.md#workspaces) declared by a `Task` [kubernetes-overview]: https://kubernetes.io/docs/concepts/overview/working-with-objects/kubernetes-objects/#required-fields @@ -227,7 +230,45 @@ spec: claimName: my-volume-claim ``` +## Workspaces +For a `TaskRun` to execute [a `Task` that declares `workspaces`](tasks.md#workspaces), +at runtime you need to map the `workspaces` to actual physical volumes with +`workspaces`. Values in `workspaces` are +[`Volumes`](https://kubernetes.io/docs/tasks/configure-pod-container/configure-volume-storage/), however currently we only support a subset of `VolumeSources`: + +* [`emptyDir`](https://kubernetes.io/docs/concepts/storage/volumes/#emptydir) +* [`persistentVolumeClaim`](https://kubernetes.io/docs/concepts/storage/volumes/#persistentvolumeclaim) + +_If you need support for a `VolumeSource` not listed here +[please open an issue](https://github.com/tektoncd/pipeline/issues) or feel free to +[contribute a PR](https://github.com/tektoncd/pipeline/blob/master/CONTRIBUTING.md)._ + + +If the declared `workspaces` are not provided at runtime, the `TaskRun` will fail +with an error. + +For example to provide an existing PVC called `mypvc` for a `workspace` called +`myworkspace` declared by the `Pipeline`, using the `my-subdir` folder which already exists +on the PVC (there will be an error if it does not exist): + +```yaml +workspaces: +- name: myworkspace + persistentVolumeClaim: + claimName: mypvc + subPath: my-subdir +``` + +Or to use [`emptyDir`](https://kubernetes.io/docs/concepts/storage/volumes/#emptydir) for the same `workspace`: + +```yaml +workspaces: +- name: myworkspace + emptyDir: {} +``` + +_For a complete example see [workspace.yaml](../examples/taskruns/workspace.yaml)._ ## Status diff --git a/docs/tasks.md b/docs/tasks.md index 863521760c6..80679725f53 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -23,7 +23,7 @@ entire Kubernetes cluster. - [Outputs](#outputs) - [Controlling where resources are mounted](#controlling-where-resources-are-mounted) - [Volumes](#volumes) - - [Container Template **deprecated**](#step-template) + - [Workspaces](#workspaces) - [Step Template](#step-template) - [Variable Substitution](#variable-substitution) - [Examples](#examples) @@ -77,6 +77,8 @@ following fields: created by your `Task` - [`volumes`](#volumes) - Specifies one or more volumes that you want to make available to your `Task`'s steps. + - [`workspaces`](#workspaces) - Specifies paths at which you expect volumes to + be mounted and available - [`stepTemplate`](#step-template) - Specifies a `Container` step definition to use as the basis for all steps within your `Task`. - [`sidecars`](#sidecars) - Specifies sidecar containers to run alongside @@ -132,7 +134,6 @@ the body of a `Task`. If multiple `steps` are defined, they will be executed in the same order as they are defined, if the `Task` is invoked by a [`TaskRun`](taskruns.md). - Each `steps` in a `Task` must specify a container image that adheres to the [container contract](./container-contract.md). For each of the `steps` fields, or container images that you define: @@ -377,6 +378,44 @@ For example, use volumes to accomplish one of the following common tasks: unsafe_. Use [kaniko](https://github.com/GoogleContainerTools/kaniko) instead. This is used only for the purposes of demonstration. +### Workspaces + +`workspaces` are a way of declaring volumes you expect to be made available to your +executing `Task` and the path to make them available at. They are similar to +[`volumes`](#volumes) but allow you to enforce at runtime that the volumes have +been attached and [allow you to specify subpaths](taskruns.md#workspaces) in the volumes +to attach. + +The volume will be made available at `/workspace/myworkspace`, or you can override +this with `mountPath`. The value at `mountPath` can be anywhere on your pod's filesystem. +The path will be available via [variable substitution](#variable-substitution) with +`$(workspaces.myworkspace.path)`. + +The actual volumes must be provided at runtime +[in the `TaskRun`](taskruns.md#workspaces). +In a future iteration ([#1438](https://github.com/tektoncd/pipeline/issues/1438)) +it [will be possible to specify these in the `PipelineRun`](pipelineruns.md#workspaces) +as well. + +For example: + +```yaml +spec: + steps: + - name: write-message + image: ubuntu + script: | + #!/usr/bin/env bash + set -xe + echo hello! > $(workspaces.messages.path)/message + workspaces: + - name: messages + description: The folder where we write the message to + mountPath: /custom/path/relative/to/root +``` + +_For a complete example see [workspace.yaml](../examples/taskruns/workspace.yaml)._ + ### Step Template Specifies a [`Container`](https://kubernetes.io/docs/concepts/containers/) @@ -464,9 +503,17 @@ has been created to track this bug. ### Variable Substitution -`Tasks` support string replacement using values from all [`inputs`](#inputs) and -[`outputs`](#outputs). +`Tasks` support string replacement using values from: + +* [Inputs and Outputs](#input-and-output-substitution) + * [Array params](#variable-substitution-with-parameters-of-type-array) +* [`workspaces`](#variable-substitution-with-workspaces) +* [`volumes`](#variable-substitution-with-volumes) + +#### Input and Output substitution +[`inputs`](#inputs) and [`outputs`](#outputs) attributes can be used in replacements, +including [`params`](#params) and [`resources`](./resources.md#variable-substitution). Input parameters can be referenced in the `Task` spec using the variable substitution syntax below, where `` is the name of the parameter: @@ -477,7 +524,7 @@ $(inputs.params.) Param values from resources can also be accessed using [variable substitution](./resources.md#variable-substitution) -#### Variable Substitution with Parameters of Type `Array` +##### Variable Substitution with Parameters of Type `Array` Referenced parameters of type `array` will expand to insert the array elements in the reference string's spot. @@ -520,6 +567,21 @@ A valid reference to the `build-args` parameter is isolated and in an eligible f args: ["build", "$(inputs.params.build-args)", "additonalArg"] ``` +#### Variable Substitution with Workspaces + +Paths to a `Task's` declared [workspaces](#workspaces) can be substituted with: + +``` +$(workspaces.myworkspace.path) +``` + +Since the name of the `Volume` is not known until runtime and is randomized, you can also +substitute the volume name with: + +``` +$(workspaces.myworkspace.volume) +``` + #### Variable Substitution within Volumes Task volume names and different diff --git a/examples/taskruns/custom-volume.yaml b/examples/taskruns/custom-volume.yaml index 9729809b128..8ad01afa57d 100644 --- a/examples/taskruns/custom-volume.yaml +++ b/examples/taskruns/custom-volume.yaml @@ -17,7 +17,7 @@ spec: image: ubuntu script: | #!/usr/bin/env bash - cat /short/and/stout/file + cat /short/and/stout/file | grep stuff volumeMounts: - name: custom mountPath: /short/and/stout diff --git a/examples/taskruns/workspace.yaml b/examples/taskruns/workspace.yaml new file mode 100644 index 00000000000..2ff0ae0f7bf --- /dev/null +++ b/examples/taskruns/workspace.yaml @@ -0,0 +1,68 @@ +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: my-pvc +spec: + resources: + requests: + storage: 5Gi + volumeMode: Filesystem + accessModes: + - ReadWriteOnce +--- +apiVersion: tekton.dev/v1alpha1 +kind: TaskRun +metadata: + generateName: custom-volume- +spec: + workspaces: + - name: custom + persistentVolumeClaim: + claimName: my-pvc + subPath: my-subdir + - name: custom2 + persistentVolumeClaim: + claimName: my-pvc + - name: custom3 + emptyDir: {} + subPath: testing + taskSpec: + steps: + - name: write + image: ubuntu + script: | + #!/usr/bin/env bash + set -xe + echo $(workspaces.custom.volume) > $(workspaces.custom.path)/foo + - name: read + image: ubuntu + script: | + #!/usr/bin/env bash + set -xe + cat $(workspaces.custom.path)/foo | grep $(workspaces.custom.volume) + - name: write2 + image: ubuntu + script: | + #!/usr/bin/env bash + set -xe + echo $(workspaces.custom2.path) > $(workspaces.custom2.path)/foo + - name: read2 + image: ubuntu + script: | + #!/usr/bin/env bash + cat $(workspaces.custom2.path)/foo | grep $(workspaces.custom2.path) + - name: write3 + image: ubuntu + script: | + #!/usr/bin/env bash + echo $(workspaces.custom3.path) > $(workspaces.custom3.path)/foo + - name: read3 + image: ubuntu + script: | + #!/usr/bin/env bash + cat $(workspaces.custom3.path)/foo | grep $(workspaces.custom3.path) + workspaces: + - name: custom + - name: custom2 + mountPath: /foo/bar/baz + - name: custom3 \ No newline at end of file diff --git a/pkg/apis/pipeline/paths.go b/pkg/apis/pipeline/paths.go new file mode 100644 index 00000000000..1016faab10a --- /dev/null +++ b/pkg/apis/pipeline/paths.go @@ -0,0 +1,22 @@ +/* +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 pipeline + +const ( + // WorkspaceDir is the root directory used for PipelineResources and (by default) Workspaces + WorkspaceDir = "/workspace" +) diff --git a/pkg/apis/pipeline/v1alpha1/git_resource.go b/pkg/apis/pipeline/v1alpha1/git_resource.go index 0a5bf4bd526..745cdbe9ed3 100644 --- a/pkg/apis/pipeline/v1alpha1/git_resource.go +++ b/pkg/apis/pipeline/v1alpha1/git_resource.go @@ -21,12 +21,11 @@ import ( "strconv" "strings" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/names" corev1 "k8s.io/api/core/v1" ) -const WorkspaceDir = "/workspace" - var ( gitSource = "git-source" ) @@ -145,7 +144,7 @@ func (s *GitResource) GetInputTaskModifier(_ *TaskSpec, path string) (TaskModifi Image: s.GitImage, Command: []string{"/ko-app/git-init"}, Args: args, - WorkingDir: WorkspaceDir, + WorkingDir: pipeline.WorkspaceDir, // This is used to populate the ResourceResult status. Env: []corev1.EnvVar{{ Name: "TEKTON_RESOURCE_NAME", diff --git a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go index 012b9997a7b..288613eb136 100644 --- a/pkg/apis/pipeline/v1alpha1/pull_request_resource.go +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource.go @@ -20,6 +20,7 @@ import ( "fmt" "strings" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/names" corev1 "k8s.io/api/core/v1" ) @@ -138,7 +139,7 @@ func (s *PullRequestResource) getSteps(mode string, sourcePath string) []Step { Image: s.PRImage, Command: []string{"/ko-app/pullrequest-init"}, Args: args, - WorkingDir: WorkspaceDir, + WorkingDir: pipeline.WorkspaceDir, Env: evs, }}} } diff --git a/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go b/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go index 07e48055b9c..0c11219b453 100644 --- a/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go +++ b/pkg/apis/pipeline/v1alpha1/pull_request_resource_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" tb "github.com/tektoncd/pipeline/test/builder" "github.com/tektoncd/pipeline/test/names" @@ -76,7 +77,7 @@ func containerTestCases(mode string) []testcase { out: []v1alpha1.Step{{Container: corev1.Container{ Name: "pr-source-nocreds-9l9zj", Image: "override-with-pr:latest", - WorkingDir: v1alpha1.WorkspaceDir, + WorkingDir: pipeline.WorkspaceDir, Command: []string{"/ko-app/pullrequest-init"}, Args: []string{"-url", "https://example.com", "-path", workspace, "-mode", mode}, Env: []corev1.EnvVar{}, @@ -96,7 +97,7 @@ func containerTestCases(mode string) []testcase { out: []v1alpha1.Step{{Container: corev1.Container{ Name: "pr-source-creds-mz4c7", Image: "override-with-pr:latest", - WorkingDir: v1alpha1.WorkspaceDir, + WorkingDir: pipeline.WorkspaceDir, Command: []string{"/ko-app/pullrequest-init"}, Args: []string{"-url", "https://example.com", "-path", "/workspace", "-mode", mode, "-provider", "github"}, Env: []corev1.EnvVar{{ diff --git a/pkg/apis/pipeline/v1alpha1/task_types.go b/pkg/apis/pipeline/v1alpha1/task_types.go index 5c8f5b20d7f..30b5acab491 100644 --- a/pkg/apis/pipeline/v1alpha1/task_types.go +++ b/pkg/apis/pipeline/v1alpha1/task_types.go @@ -61,6 +61,9 @@ type TaskSpec struct { // Sidecars are run alongside the Task's step containers. They begin before // the steps start and end after the steps complete. Sidecars []corev1.Container `json:"sidecars,omitempty"` + + // Workspaces are the volumes that this Task requires. + Workspaces []WorkspaceDeclaration } // Step embeds the Container type, which allows it to include fields not diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index b3de508302b..240f306e523 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation.go @@ -19,6 +19,7 @@ package v1alpha1 import ( "context" "fmt" + "path/filepath" "strings" "github.com/tektoncd/pipeline/pkg/apis/validate" @@ -49,6 +50,9 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError { if err := ValidateVolumes(ts.Volumes).ViaField("volumes"); err != nil { return err } + if err := ValidateDeclaredWorkspaces(ts.Workspaces, ts.Steps, ts.StepTemplate); err != nil { + return err + } mergedSteps, err := MergeStepsWithStepTemplate(ts.StepTemplate, ts.Steps) if err != nil { return &apis.FieldError{ @@ -108,6 +112,45 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError { return nil } +// ValidateDeclaredWorkspaces will make sure that the declared workspaces do not try to use +// a mount path which conflicts with any other declared workspaces, with the explicitly +// declared volume mounts, or with the stepTemplate. The names must also be unique. +func ValidateDeclaredWorkspaces(workspaces []WorkspaceDeclaration, steps []Step, stepTemplate *corev1.Container) *apis.FieldError { + mountPaths := map[string]struct{}{} + for _, step := range steps { + for _, vm := range step.VolumeMounts { + mountPaths[filepath.Clean(vm.MountPath)] = struct{}{} + } + } + if stepTemplate != nil { + for _, vm := range stepTemplate.VolumeMounts { + mountPaths[filepath.Clean(vm.MountPath)] = struct{}{} + } + } + + wsNames := map[string]struct{}{} + for _, w := range workspaces { + // Workspace names must be unique + if _, ok := wsNames[w.Name]; ok { + return &apis.FieldError{ + Message: fmt.Sprintf("workspace name %q must be unique", w.Name), + Paths: []string{"workspaces.name"}, + } + } + wsNames[w.Name] = struct{}{} + // Workspaces must not try to use mount paths that are already used + mountPath := filepath.Clean(w.GetMountPath()) + if _, ok := mountPaths[mountPath]; ok { + return &apis.FieldError{ + Message: fmt.Sprintf("workspace mount path %q must be unique", mountPath), + Paths: []string{"workspaces.mountpath"}, + } + } + mountPaths[mountPath] = struct{}{} + } + return nil +} + func ValidateVolumes(volumes []corev1.Volume) *apis.FieldError { // Task must not have duplicate volume names. vols := map[string]struct{}{} diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go index 9c7b3b1c84e..32dc449ae3b 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation_test.go @@ -81,6 +81,7 @@ func TestTaskSpecValidate(t *testing.T) { Outputs *v1alpha1.Outputs Steps []v1alpha1.Step StepTemplate *corev1.Container + Workspaces []v1alpha1.WorkspaceDeclaration } tests := []struct { name string @@ -276,6 +277,21 @@ func TestTaskSpecValidate(t *testing.T) { }}, }}}, }, + }, { + name: "valid workspace", + fields: fields{ + Steps: []v1alpha1.Step{{ + Container: corev1.Container{ + Image: "my-image", + Args: []string{"arg"}, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "foo-workspace", + Description: "my great workspace", + MountPath: "some/path", + }}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -284,6 +300,7 @@ func TestTaskSpecValidate(t *testing.T) { Outputs: tt.fields.Outputs, Steps: tt.fields.Steps, StepTemplate: tt.fields.StepTemplate, + Workspaces: tt.fields.Workspaces, } ctx := context.Background() ts.SetDefaults(ctx) @@ -296,10 +313,12 @@ func TestTaskSpecValidate(t *testing.T) { func TestTaskSpecValidateError(t *testing.T) { type fields struct { - Inputs *v1alpha1.Inputs - Outputs *v1alpha1.Outputs - Steps []v1alpha1.Step - Volumes []corev1.Volume + Inputs *v1alpha1.Inputs + Outputs *v1alpha1.Outputs + Steps []v1alpha1.Step + Volumes []corev1.Volume + StepTemplate *corev1.Container + Workspaces []v1alpha1.WorkspaceDeclaration } tests := []struct { name string @@ -735,14 +754,126 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `step 0 volumeMount name "tekton-internal-foo" cannot start with "tekton-internal-"`, Paths: []string{"steps.volumeMounts.name"}, }, + }, { + name: "declared workspaces names are not unique", + fields: fields{ + Steps: validSteps, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "same-workspace", + }, { + Name: "same-workspace", + }}, + }, + expectedError: apis.FieldError{ + Message: "workspace name \"same-workspace\" must be unique", + Paths: []string{"workspaces.name"}, + }, + }, { + name: "declared workspaces clash with each other", + fields: fields{ + Steps: validSteps, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "some-workspace", + MountPath: "/foo", + }, { + Name: "another-workspace", + MountPath: "/foo", + }}, + }, + expectedError: apis.FieldError{ + Message: "workspace mount path \"/foo\" must be unique", + Paths: []string{"workspaces.mountpath"}, + }, + }, { + name: "workspace mount path already in volumeMounts", + fields: fields{ + Steps: []v1alpha1.Step{{ + Container: corev1.Container{ + Image: "myimage", + Command: []string{"command"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "my-mount", + MountPath: "/foo", + }}, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "some-workspace", + MountPath: "/foo", + }}, + }, + expectedError: apis.FieldError{ + Message: "workspace mount path \"/foo\" must be unique", + Paths: []string{"workspaces.mountpath"}, + }, + }, { + name: "workspace default mount path already in volumeMounts", + fields: fields{ + Steps: []v1alpha1.Step{{ + Container: corev1.Container{ + Image: "myimage", + Command: []string{"command"}, + VolumeMounts: []corev1.VolumeMount{{ + Name: "my-mount", + MountPath: "/workspace/some-workspace/", + }}, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "some-workspace", + }}, + }, + expectedError: apis.FieldError{ + Message: "workspace mount path \"/workspace/some-workspace\" must be unique", + Paths: []string{"workspaces.mountpath"}, + }, + }, { + name: "workspace mount path already in stepTemplate", + fields: fields{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "my-mount", + MountPath: "/foo", + }}, + }, + Steps: validSteps, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "some-workspace", + MountPath: "/foo", + }}, + }, + expectedError: apis.FieldError{ + Message: "workspace mount path \"/foo\" must be unique", + Paths: []string{"workspaces.mountpath"}, + }, + }, { + name: "workspace default mount path already in stepTemplate", + fields: fields{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "my-mount", + MountPath: "/workspace/some-workspace", + }}, + }, + Steps: validSteps, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "some-workspace", + }}, + }, + expectedError: apis.FieldError{ + Message: "workspace mount path \"/workspace/some-workspace\" must be unique", + Paths: []string{"workspaces.mountpath"}, + }, }} 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, + Inputs: tt.fields.Inputs, + Outputs: tt.fields.Outputs, + Steps: tt.fields.Steps, + Volumes: tt.fields.Volumes, + StepTemplate: tt.fields.StepTemplate, + Workspaces: tt.fields.Workspaces, } ctx := context.Background() ts.SetDefaults(ctx) diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go index 6bfb65c8acb..2915b77d9dd 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_types.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_types.go @@ -50,7 +50,12 @@ type TaskRunSpec struct { Timeout *metav1.Duration `json:"timeout,omitempty"` // PodTemplate holds pod specific configuration + // +optional PodTemplate PodTemplate `json:"podTemplate,omitempty"` + + // Workspaces is a list of WorkspaceBindings from volumes to workspaces. + // +optional + Workspaces []WorkspaceBinding `json:"workspaces,omitempty"` } // TaskRunSpecStatus defines the taskrun spec status the user can provide diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation.go index 4b0112df67e..d95d1098bfc 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_validation.go @@ -69,6 +69,10 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) *apis.FieldError { return err } + if err := ValidateWorkspaceBindings(ctx, ts.Workspaces); err != nil { + return err + } + if ts.Timeout != nil { // timeout should be a valid duration of at least 0. if ts.Timeout.Duration < 0 { @@ -90,6 +94,23 @@ func (o TaskRunOutputs) Validate(ctx context.Context, path string) *apis.FieldEr return validatePipelineResources(ctx, o.Resources, fmt.Sprintf("%s.Resources.Name", path)) } +// ValidateWorkspaceBindings makes sure the volumes provided for the Task's declared workspaces make sense. +func ValidateWorkspaceBindings(ctx context.Context, wb []WorkspaceBinding) *apis.FieldError { + seen := map[string]struct{}{} + for _, w := range wb { + if _, ok := seen[w.Name]; ok { + return apis.ErrMultipleOneOf("spec.workspaces.name") + } + seen[w.Name] = struct{}{} + + if err := w.Validate(ctx); err != nil { + return err + } + } + + return nil +} + // validatePipelineResources validates that // 1. resource is not declared more than once // 2. if both resource reference and resource spec is defined at the same time diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go index cf69907e7de..73d46dbdde6 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go @@ -30,7 +30,7 @@ import ( "knative.dev/pkg/apis" ) -func TestTaskRun_Invalidate(t *testing.T) { +func TestTaskRun_Invalid(t *testing.T) { tests := []struct { name string task *v1alpha1.TaskRun @@ -66,7 +66,42 @@ func TestTaskRun_Validate(t *testing.T) { } } -func TestTaskRunSpec_Invalidate(t *testing.T) { +func Test_TaskRun_Workspaces_Invalid(t *testing.T) { + tests := []struct { + name string + tr *v1alpha1.TaskRun + wantErr *apis.FieldError + }{{ + name: "make sure WorkspaceBinding validation invoked", + tr: tb.TaskRun("taskname", "default", tb.TaskRunSpec( + tb.TaskRunTaskRef("task"), + // When using PVC it's required that you provide a volume name + tb.TaskRunWorkspacePVC("workspace", "", ""), + )), + wantErr: apis.ErrMissingField("workspace.persistentvolumeclaim.claimname"), + }, { + name: "bind same workspace twice", + tr: tb.TaskRun("taskname", "default", tb.TaskRunSpec( + tb.TaskRunTaskRef("task"), + tb.TaskRunWorkspaceEmptyDir("workspace", ""), + tb.TaskRunWorkspaceEmptyDir("workspace", ""), + )), + wantErr: apis.ErrMultipleOneOf("spec.workspaces.name"), + }} + for _, ts := range tests { + t.Run(ts.name, func(t *testing.T) { + err := ts.tr.Validate(context.Background()) + if err == nil { + t.Errorf("Expected error for invalid TaskRun but got none") + } + if d := cmp.Diff(ts.wantErr.Error(), err.Error()); d != "" { + t.Errorf("TaskRunSpec.Validate/%s (-want, +got) = %v", ts.name, d) + } + }) + } +} + +func TestTaskRunSpec_Invalid(t *testing.T) { tests := []struct { name string spec v1alpha1.TaskRunSpec @@ -185,7 +220,7 @@ func TestInput_Validate(t *testing.T) { } } -func TestInput_Invalidate(t *testing.T) { +func TestInput_Invalid(t *testing.T) { tests := []struct { name string inputs v1alpha1.TaskRunInputs @@ -295,7 +330,7 @@ func TestOutput_Validate(t *testing.T) { t.Errorf("TaskRunOutputs.Validate() error = %v", err) } } -func TestOutput_Invalidate(t *testing.T) { +func TestOutput_Invalid(t *testing.T) { tests := []struct { name string outputs v1alpha1.TaskRunOutputs diff --git a/pkg/apis/pipeline/v1alpha1/workspace_types.go b/pkg/apis/pipeline/v1alpha1/workspace_types.go new file mode 100644 index 00000000000..6c61dae2997 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/workspace_types.go @@ -0,0 +1,65 @@ +/* +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 v1alpha1 + +import ( + "path/filepath" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline" + corev1 "k8s.io/api/core/v1" +) + +// WorkspaceDeclaration is a declaration of a volume that a Task requires. +type WorkspaceDeclaration struct { + // Name is the name by which you can bind the volume at runtime. + Name string `json:"name"` + // Description is an optional human readable description of this volume. + // +optional + Description string `json:"description,omitempty"` + // MountPath overrides the directory that the volume will be made available at. + // +optional + MountPath string `json:"mountPath,omitempty"` +} + +// GetMountPath returns the mountPath for w which is the MountPath if provided or the +// default if not. +func (w *WorkspaceDeclaration) GetMountPath() string { + if w.MountPath != "" { + return w.MountPath + } + return filepath.Join(pipeline.WorkspaceDir, w.Name) +} + +// WorkspaceBinding maps a Task's declared workspace to a Volume. +// Currently we only support PersistentVolumeClaims and EmptyDir. +type WorkspaceBinding struct { + // Name is the name of the workspace populated by the volume. + Name string `json:"name"` + // SubPath is optionally a directory on the volume which should be used + // for this binding (i.e. the volume will be mounted at this sub directory). + // +optional + SubPath string `json:"subPath,omitempty"` + // PersistentVolumeClaimVolumeSource represents a reference to a + // PersistentVolumeClaim in the same namespace. Either this OR EmptyDir can be used. + // +optional + PersistentVolumeClaim *corev1.PersistentVolumeClaimVolumeSource `json:"persistentVolumeClaim,omitempty"` + // EmptyDir represents a temporary directory that shares a Task's lifetime. + // More info: https://kubernetes.io/docs/concepts/storage/volumes#emptydir + // Either this OR PersistentVolumeClaim can be used. + // +optional + EmptyDir *corev1.EmptyDirVolumeSource `json:"emptyDir,omitempty"` +} diff --git a/pkg/apis/pipeline/v1alpha1/workspace_validation.go b/pkg/apis/pipeline/v1alpha1/workspace_validation.go new file mode 100644 index 00000000000..1ca6fe0527d --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/workspace_validation.go @@ -0,0 +1,49 @@ +/* +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 v1alpha1 + +import ( + "context" + + "k8s.io/apimachinery/pkg/api/equality" + "knative.dev/pkg/apis" +) + +// Validate looks at the Volume provided in wb and makes sure that it is valid. +// This means that only one VolumeSource can be specified, and also that the +// supported VolumeSource is itself valid. +func (b *WorkspaceBinding) Validate(ctx context.Context) *apis.FieldError { + if equality.Semantic.DeepEqual(b, &WorkspaceBinding{}) || b == nil { + return apis.ErrMissingField(apis.CurrentField) + } + + // Users should only provide one supported VolumeSource. + if b.PersistentVolumeClaim != nil && b.EmptyDir != nil { + return apis.ErrMultipleOneOf("workspace.persistentvolumeclaim", "workspace.emptydir") + } + + // Users must provide at least one supported VolumeSource. + if b.PersistentVolumeClaim == nil && b.EmptyDir == nil { + return apis.ErrMissingOneOf("workspace.persistentvolumeclaim", "workspace.emptydir") + } + + // For a PersistentVolumeClaim to work, you must at least provide the name of the PVC to use. + if b.PersistentVolumeClaim != nil && b.PersistentVolumeClaim.ClaimName == "" { + return apis.ErrMissingField("workspace.persistentvolumeclaim.claimname") + } + return nil +} diff --git a/pkg/apis/pipeline/v1alpha1/workspace_validation_test.go b/pkg/apis/pipeline/v1alpha1/workspace_validation_test.go new file mode 100644 index 00000000000..5866305784a --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/workspace_validation_test.go @@ -0,0 +1,88 @@ +/* +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 v1alpha1 + +import ( + "context" + "testing" + + corev1 "k8s.io/api/core/v1" +) + +func TestWorkspaceBindingValidateValid(t *testing.T) { + for _, tc := range []struct { + name string + binding *WorkspaceBinding + }{{ + name: "Valid PVC", + binding: &WorkspaceBinding{ + Name: "beth", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pool-party", + }, + }, + }, { + name: "Valid emptyDir", + binding: &WorkspaceBinding{ + Name: "beth", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }} { + t.Run(tc.name, func(t *testing.T) { + if err := tc.binding.Validate(context.Background()); err != nil { + t.Errorf("didnt expect error for valid binding but got: %v", err) + } + }) + } + +} + +func TestWorkspaceBindingValidateInvalid(t *testing.T) { + for _, tc := range []struct { + name string + binding *WorkspaceBinding + }{{ + name: "no binding provided", + binding: nil, + }, { + name: "Provided both pvc and emptydir", + binding: &WorkspaceBinding{ + Name: "beth", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pool-party", + }, + }, + }, { + name: "Provided neither pvc nor emptydir", + binding: &WorkspaceBinding{ + Name: "beth", + }, + }, { + name: "Provided pvc without claim name", + binding: &WorkspaceBinding{ + Name: "beth", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{}, + }, + }} { + t.Run(tc.name, func(t *testing.T) { + if err := tc.binding.Validate(context.Background()); err == nil { + t.Errorf("expected error for invalid binding but didn't get any!") + } + }) + } +} diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 8fb1ba6b550..64ca02d8adf 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -1556,6 +1556,13 @@ func (in *TaskRunSpec) DeepCopyInto(out *TaskRunSpec) { **out = **in } in.PodTemplate.DeepCopyInto(&out.PodTemplate) + if in.Workspaces != nil { + in, out := &in.Workspaces, &out.Workspaces + *out = make([]WorkspaceBinding, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } @@ -1681,6 +1688,11 @@ func (in *TaskSpec) DeepCopyInto(out *TaskSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Workspaces != nil { + in, out := &in.Workspaces, &out.Workspaces + *out = make([]WorkspaceDeclaration, len(*in)) + copy(*out, *in) + } return } @@ -1709,3 +1721,45 @@ func (in *TestResult) DeepCopy() *TestResult { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WorkspaceBinding) DeepCopyInto(out *WorkspaceBinding) { + *out = *in + if in.PersistentVolumeClaim != nil { + in, out := &in.PersistentVolumeClaim, &out.PersistentVolumeClaim + *out = new(v1.PersistentVolumeClaimVolumeSource) + **out = **in + } + if in.EmptyDir != nil { + in, out := &in.EmptyDir, &out.EmptyDir + *out = new(v1.EmptyDirVolumeSource) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkspaceBinding. +func (in *WorkspaceBinding) DeepCopy() *WorkspaceBinding { + if in == nil { + return nil + } + out := new(WorkspaceBinding) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WorkspaceDeclaration) DeepCopyInto(out *WorkspaceDeclaration) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WorkspaceDeclaration. +func (in *WorkspaceDeclaration) DeepCopy() *WorkspaceDeclaration { + if in == nil { + return nil + } + out := new(WorkspaceDeclaration) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 1a1f5c5bdb2..2ce67295d9b 100644 --- a/pkg/pod/pod.go +++ b/pkg/pod/pod.go @@ -30,8 +30,7 @@ import ( ) const ( - workspaceDir = "/workspace" - homeDir = "/tekton/home" + homeDir = "/tekton/home" taskRunLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey ManagedByLabelKey = "app.kubernetes.io/managed-by" @@ -52,7 +51,7 @@ var ( }} implicitVolumeMounts = []corev1.VolumeMount{{ Name: "tekton-internal-workspace", - MountPath: workspaceDir, + MountPath: pipeline.WorkspaceDir, }, { Name: "tekton-internal-home", MountPath: homeDir, @@ -153,7 +152,7 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha // isolation. for i, s := range stepContainers { if s.WorkingDir == "" { - stepContainers[i].WorkingDir = workspaceDir + stepContainers[i].WorkingDir = pipeline.WorkspaceDir } if s.Name == "" { stepContainers[i].Name = names.SimpleNameGenerator.RestrictLength(fmt.Sprintf("%sunnamed-%d", stepPrefix, i)) diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go index 78bf091578a..dfde196cb82 100644 --- a/pkg/pod/pod_test.go +++ b/pkg/pod/pod_test.go @@ -95,7 +95,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }}, Volumes: append(implicitVolumes, toolsVolume, downwardVolume), @@ -146,7 +146,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }}, Volumes: append(implicitVolumes, secretsVolume, toolsVolume, downwardVolume), @@ -189,7 +189,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }}, Volumes: append(implicitVolumes, toolsVolume, downwardVolume), @@ -228,7 +228,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }}, Volumes: append(implicitVolumes, toolsVolume, downwardVolume), @@ -261,7 +261,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }}, Volumes: append(implicitVolumes, toolsVolume, downwardVolume), @@ -273,7 +273,7 @@ func TestMakePod(t *testing.T) { Name: "name", Image: "image", Command: []string{"cmd"}, // avoid entrypoint lookup. - WorkingDir: filepath.Join(workspaceDir, "test"), + WorkingDir: filepath.Join(pipeline.WorkspaceDir, "test"), }}}, }, want: &corev1.PodSpec{ @@ -282,8 +282,8 @@ func TestMakePod(t *testing.T) { Name: "working-dir-initializer", Image: images.ShellImage, Command: []string{"sh"}, - Args: []string{"-c", fmt.Sprintf("mkdir -p %s", filepath.Join(workspaceDir, "test"))}, - WorkingDir: workspaceDir, + Args: []string{"-c", fmt.Sprintf("mkdir -p %s", filepath.Join(pipeline.WorkspaceDir, "test"))}, + WorkingDir: pipeline.WorkspaceDir, VolumeMounts: implicitVolumeMounts, }, placeToolsInit, @@ -304,7 +304,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: filepath.Join(workspaceDir, "test"), + WorkingDir: filepath.Join(pipeline.WorkspaceDir, "test"), Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }}, Volumes: append(implicitVolumes, toolsVolume, downwardVolume), @@ -342,7 +342,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }, { Name: "sidecar-sc-name", @@ -395,7 +395,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }, { Name: "step-unnamed-1", @@ -412,7 +412,7 @@ func TestMakePod(t *testing.T) { }, Env: implicitEnvVars, VolumeMounts: append([]corev1.VolumeMount{toolsMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{ Requests: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("8"), @@ -497,7 +497,7 @@ script-heredoc-randomly-generated-78c5n }, Env: append(implicitEnvVars, corev1.EnvVar{Name: "FOO", Value: "bar"}), VolumeMounts: append([]corev1.VolumeMount{scriptsVolumeMount, toolsMount, downwardMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }, { Name: "step-two", @@ -516,7 +516,7 @@ script-heredoc-randomly-generated-78c5n }, Env: append(implicitEnvVars, corev1.EnvVar{Name: "FOO", Value: "bar"}), VolumeMounts: append([]corev1.VolumeMount{{Name: "i-have-a-volume-mount"}, scriptsVolumeMount, toolsMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }, { Name: "step-regular-step", @@ -536,7 +536,7 @@ script-heredoc-randomly-generated-78c5n }, Env: append(implicitEnvVars, corev1.EnvVar{Name: "FOO", Value: "bar"}), VolumeMounts: append([]corev1.VolumeMount{toolsMount}, implicitVolumeMounts...), - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, Resources: corev1.ResourceRequirements{Requests: allZeroQty()}, }}, Volumes: append(implicitVolumes, scriptsVolume, toolsVolume, downwardVolume), diff --git a/pkg/pod/workingdir_init.go b/pkg/pod/workingdir_init.go index dcadf335272..62220437dab 100644 --- a/pkg/pod/workingdir_init.go +++ b/pkg/pod/workingdir_init.go @@ -21,6 +21,7 @@ import ( "sort" "strings" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" corev1 "k8s.io/api/core/v1" ) @@ -68,7 +69,7 @@ func workingDirInit(shellImage string, stepContainers []corev1.Container) *corev Image: shellImage, Command: []string{"sh"}, Args: []string{"-c", "mkdir -p " + strings.Join(relativeDirs, " ")}, - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, VolumeMounts: implicitVolumeMounts, } } diff --git a/pkg/pod/workingdir_init_test.go b/pkg/pod/workingdir_init_test.go index 21fb2586e1b..7752b8aea9a 100644 --- a/pkg/pod/workingdir_init_test.go +++ b/pkg/pod/workingdir_init_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" ) @@ -58,7 +59,7 @@ func TestWorkingDirInit(t *testing.T) { Image: images.ShellImage, Command: []string{"sh"}, Args: []string{"-c", "mkdir -p /workspace/bbb aaa zzz"}, - WorkingDir: workspaceDir, + WorkingDir: pipeline.WorkspaceDir, VolumeMounts: implicitVolumeMounts, }, }} { diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index d69bc81a0c0..de15d1fb678 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -19,6 +19,8 @@ package resources import ( "fmt" + "github.com/tektoncd/pipeline/pkg/workspace" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/substitution" ) @@ -79,6 +81,21 @@ func ApplyResources(spec *v1alpha1.TaskSpec, resolvedResources map[string]v1alph return ApplyReplacements(spec, replacements, map[string][]string{}) } +// ApplyWorkspaces applies the substitution from paths that the workspaces in w are mounted to and the +// volumes that wb are realized with in the task spec ts. +func ApplyWorkspaces(spec *v1alpha1.TaskSpec, w []v1alpha1.WorkspaceDeclaration, wb []v1alpha1.WorkspaceBinding) *v1alpha1.TaskSpec { + stringReplacements := map[string]string{} + + for _, ww := range w { + stringReplacements[fmt.Sprintf("workspaces.%s.path", ww.Name)] = ww.GetMountPath() + } + v := workspace.GetVolumes(wb) + for name, vv := range v { + stringReplacements[fmt.Sprintf("workspaces.%s.volume", name)] = vv.Name + } + return ApplyReplacements(spec, stringReplacements, map[string][]string{}) +} + // ApplyReplacements replaces placeholders for declared parameters with the specified replacements. func ApplyReplacements(spec *v1alpha1.TaskSpec, stringReplacements map[string]string, arrayReplacements map[string][]string) *v1alpha1.TaskSpec { spec = spec.DeepCopy() diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index f27432ac8b7..2809ffcac29 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -24,6 +24,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/test/builder" + "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -453,6 +454,7 @@ func TestApplyArrayParameters(t *testing.T) { }) } } + func TestApplyParameters(t *testing.T) { tr := &v1alpha1.TaskRun{ Spec: v1alpha1.TaskRunSpec{ @@ -576,3 +578,115 @@ func TestApplyResources(t *testing.T) { }) } } + +func TestApplyWorkspaces(t *testing.T) { + names.TestingSeed() + ts := &v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + Env: []corev1.EnvVar{{ + Name: "template-var", + Value: "$(workspaces.myws.volume)", + }}, + }, + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "$(workspaces.myws.volume)", + Image: "$(workspaces.otherws.volume)", + WorkingDir: "$(workspaces.otherws.volume)", + Args: []string{"$(workspaces.myws.path)"}, + }}, {Container: corev1.Container{ + Name: "foo", + Image: "bar", + VolumeMounts: []corev1.VolumeMount{{ + Name: "$(workspaces.myws.volume)", + MountPath: "path/to/$(workspaces.otherws.path)", + SubPath: "$(workspaces.myws.volume)", + }}, + }}, {Container: corev1.Container{ + Name: "foo", + Image: "bar", + Env: []corev1.EnvVar{{ + Name: "foo", + Value: "$(workspaces.myws.volume)", + }, { + Name: "baz", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "$(workspaces.myws.volume)"}, + Key: "$(workspaces.myws.volume)", + }, + }, + }}, + EnvFrom: []corev1.EnvFromSource{{ + Prefix: "$(workspaces.myws.volume)", + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: "$(workspaces.myws.volume)"}, + }, + }}, + }}}, + Volumes: []corev1.Volume{{ + Name: "$(workspaces.myws.volume)", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(workspaces.myws.volume)", + }, + }, + }}, { + Name: "some-secret", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "$(workspaces.myws.volume)", + }, + }}, { + Name: "some-pvc", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "$(workspaces.myws.volume)", + }, + }, + }, + }, + } + want := applyMutation(ts, func(spec *v1alpha1.TaskSpec) { + spec.StepTemplate.Env[0].Value = "ws-9l9zj" + + spec.Steps[0].Name = "ws-9l9zj" + spec.Steps[0].Image = "ws-mz4c7" + spec.Steps[0].WorkingDir = "ws-mz4c7" + spec.Steps[0].Args = []string{"/workspace/myws"} + + spec.Steps[1].VolumeMounts[0].Name = "ws-9l9zj" + spec.Steps[1].VolumeMounts[0].MountPath = "path/to//foo" + spec.Steps[1].VolumeMounts[0].SubPath = "ws-9l9zj" + + spec.Steps[2].Env[0].Value = "ws-9l9zj" + spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.LocalObjectReference.Name = "ws-9l9zj" + spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.Key = "ws-9l9zj" + spec.Steps[2].EnvFrom[0].Prefix = "ws-9l9zj" + spec.Steps[2].EnvFrom[0].ConfigMapRef.LocalObjectReference.Name = "ws-9l9zj" + + spec.Volumes[0].Name = "ws-9l9zj" + spec.Volumes[0].VolumeSource.ConfigMap.LocalObjectReference.Name = "ws-9l9zj" + spec.Volumes[1].VolumeSource.Secret.SecretName = "ws-9l9zj" + spec.Volumes[2].VolumeSource.PersistentVolumeClaim.ClaimName = "ws-9l9zj" + }) + w := []v1alpha1.WorkspaceDeclaration{{ + Name: "myws", + }, { + Name: "otherws", + MountPath: "/foo", + }} + wb := []v1alpha1.WorkspaceBinding{{ + Name: "myws", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "foo", + }, + }, { + Name: "otherws", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }} + got := resources.ApplyWorkspaces(ts, w, wb) + if d := cmp.Diff(got, want); d != "" { + t.Errorf("ApplyParameters() got diff %s", d) + } +} diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index ca182ec1a24..4014cd6712f 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -35,6 +35,7 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources/cloudevent" + "github.com/tektoncd/pipeline/pkg/workspace" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -292,7 +293,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error } if err := ValidateResolvedTaskResources(tr.Spec.Inputs.Params, rtr); err != nil { - c.Logger.Errorf("Failed to validate taskrun %q: %v", tr.Name, err) + c.Logger.Errorf("TaskRun %q resources are invalid: %v", tr.Name, err) tr.Status.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, @@ -302,6 +303,16 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error return nil } + if err := workspace.ValidateBindings(taskSpec.Workspaces, tr.Spec.Workspaces); err != nil { + c.Logger.Errorf("TaskRun %q workspaces are invalid: %v", tr.Name, err) + tr.Status.SetCondition(&apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: podconvert.ReasonFailedValidation, + Message: err.Error(), + }) + } + // Initialize the cloud events if at least a CloudEventResource is defined // and they have not been initialized yet. // FIXME(afrittoli) This resource specific logic will have to be replaced @@ -455,19 +466,19 @@ func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTask err = resources.AddOutputImageDigestExporter(c.Images.ImageDigestExporterImage, tr, ts, c.resourceLister.PipelineResources(tr.Namespace).Get) if err != nil { - c.Logger.Errorf("Failed to create a build for taskrun: %s due to output image resource error %v", tr.Name, err) + c.Logger.Errorf("Failed to create a pod for taskrun: %s due to output image resource error %v", tr.Name, err) return nil, err } ts, err = resources.AddInputResource(c.KubeClientSet, c.Images, rtr.TaskName, ts, tr, inputResources, c.Logger) if err != nil { - c.Logger.Errorf("Failed to create a build for taskrun: %s due to input resource error %v", tr.Name, err) + c.Logger.Errorf("Failed to create a pod for taskrun: %s due to input resource error %v", tr.Name, err) return nil, err } ts, err = resources.AddOutputResources(c.KubeClientSet, c.Images, rtr.TaskName, ts, tr, outputResources, c.Logger) if err != nil { - c.Logger.Errorf("Failed to create a build for taskrun: %s due to output resource error %v", tr.Name, err) + c.Logger.Errorf("Failed to create a pod for taskrun: %s due to output resource error %v", tr.Name, err) return nil, err } @@ -482,6 +493,15 @@ func (c *Reconciler) createPod(tr *v1alpha1.TaskRun, rtr *resources.ResolvedTask ts = resources.ApplyResources(ts, inputResources, "inputs") ts = resources.ApplyResources(ts, outputResources, "outputs") + // Apply workspace resource substitution + ts = resources.ApplyWorkspaces(ts, ts.Workspaces, tr.Spec.Workspaces) + + ts, err = workspace.Apply(*ts, tr.Spec.Workspaces) + if err != nil { + c.Logger.Errorf("Failed to create a pod for taskrun: %s due to workspace error %v", tr.Name, err) + return nil, err + } + pod, err := podconvert.MakePod(c.Images, tr, *ts, c.KubeClientSet, c.entrypointCache) if err != nil { return nil, fmt.Errorf("translating Build to Pod: %w", err) diff --git a/pkg/workspace/apply.go b/pkg/workspace/apply.go new file mode 100644 index 00000000000..f2df9f619a9 --- /dev/null +++ b/pkg/workspace/apply.go @@ -0,0 +1,98 @@ +package workspace + +import ( + "fmt" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/names" + corev1 "k8s.io/api/core/v1" +) + +const ( + volumeNameBase = "ws" +) + +// GetVolumes will return a dictionary where the keys are the names fo the workspaces bound in +// wb and the value is the Volume to use. If the same Volume is bound twice, the resulting volumes +// will both have the same name to prevent the same Volume from being attached to pod twice. +func GetVolumes(wb []v1alpha1.WorkspaceBinding) map[string]corev1.Volume { + pvcs := map[string]corev1.Volume{} + v := map[string]corev1.Volume{} + for _, w := range wb { + name := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(volumeNameBase) + if w.PersistentVolumeClaim != nil { + // If it's a PVC, we need to check if we've encountered it before so we avoid mounting it twice + if vv, ok := pvcs[w.PersistentVolumeClaim.ClaimName]; ok { + v[w.Name] = vv + } else { + pvc := *w.PersistentVolumeClaim + v[w.Name] = corev1.Volume{ + Name: name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &pvc, + }, + } + pvcs[pvc.ClaimName] = v[w.Name] + } + } else if w.EmptyDir != nil { + ed := *w.EmptyDir + v[w.Name] = corev1.Volume{ + Name: name, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &ed, + }, + } + } + } + return v +} + +func getDeclaredWorkspace(name string, w []v1alpha1.WorkspaceDeclaration) (*v1alpha1.WorkspaceDeclaration, error) { + for _, workspace := range w { + if workspace.Name == name { + return &workspace, nil + } + } + // Trusting validation to ensure + return nil, fmt.Errorf("even though validation should have caught it, bound workspace %s did not exist in declared workspaces", name) +} + +// Apply will update the StepTemplate and Volumes declaration in ts so that the workspaces +// specified through wb combined with the declared workspaces in ts will be available for +// all containers in the resulting pod. +func Apply(ts v1alpha1.TaskSpec, wb []v1alpha1.WorkspaceBinding) (*v1alpha1.TaskSpec, error) { + // If there are no bound workspaces, we don't need to do anything + if len(wb) == 0 { + return &ts, nil + } + + v := GetVolumes(wb) + addedVolumes := map[string]struct{}{} + + // Initialize StepTemplate if it hasn't been already + if ts.StepTemplate == nil { + ts.StepTemplate = &corev1.Container{} + } + + for i := range wb { + w, err := getDeclaredWorkspace(wb[i].Name, ts.Workspaces) + if err != nil { + return nil, err + } + // Get the volume we should be using for this binding + vv := v[wb[i].Name] + + ts.StepTemplate.VolumeMounts = append(ts.StepTemplate.VolumeMounts, corev1.VolumeMount{ + Name: vv.Name, + MountPath: w.GetMountPath(), + SubPath: wb[i].SubPath, + }) + + // Only add this volume if it hasn't already been added + if _, ok := addedVolumes[vv.Name]; !ok { + ts.Volumes = append(ts.Volumes, vv) + addedVolumes[vv.Name] = struct{}{} + } + } + return &ts, nil +} diff --git a/pkg/workspace/apply_test.go b/pkg/workspace/apply_test.go new file mode 100644 index 00000000000..9fd1f4057e7 --- /dev/null +++ b/pkg/workspace/apply_test.go @@ -0,0 +1,428 @@ +package workspace_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/workspace" + "github.com/tektoncd/pipeline/test/names" + corev1 "k8s.io/api/core/v1" +) + +func TestGetVolumes(t *testing.T) { + names.TestingSeed() + for _, tc := range []struct { + name string + workspaces []v1alpha1.WorkspaceBinding + expectedVolumes map[string]corev1.Volume + }{{ + name: "binding a single workspace with a PVC", + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/foo/bar/baz", + }}, + expectedVolumes: map[string]corev1.Volume{ + "custom": { + Name: "ws-9l9zj", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }, + }, + }, { + name: "binding a single workspace with emptyDir", + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + }, + SubPath: "/foo/bar/baz", + }}, + expectedVolumes: map[string]corev1.Volume{ + "custom": { + Name: "ws-mz4c7", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + }, + }, + }, + }, + }, { + name: "0 workspace bindings", + workspaces: []v1alpha1.WorkspaceBinding{}, + expectedVolumes: map[string]corev1.Volume{}, + }, { + name: "binding multiple workspaces", + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/foo/bar/baz", + }, { + Name: "even-more-custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myotherpvc", + }, + }}, + expectedVolumes: map[string]corev1.Volume{ + "custom": { + Name: "ws-mssqb", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }, + "even-more-custom": { + Name: "ws-78c5n", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myotherpvc", + }, + }, + }, + }, + }, { + name: "multiple workspaces binding to the same volume with diff subpaths doesnt duplicate", + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/foo/bar/baz", + }, { + Name: "custom2", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/very/professional/work/space", + }}, + expectedVolumes: map[string]corev1.Volume{ + "custom": { + Name: "ws-6nl7g", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }, + "custom2": { + // Since it is the same PVC source, it can't be added twice with two different names + Name: "ws-6nl7g", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }, + }, + }} { + t.Run(tc.name, func(t *testing.T) { + v := workspace.GetVolumes(tc.workspaces) + if d := cmp.Diff(tc.expectedVolumes, v); d != "" { + t.Errorf("Didn't get expected volumes from bindings (-want, +got): %s", d) + } + }) + } +} + +func TestApply(t *testing.T) { + names.TestingSeed() + for _, tc := range []struct { + name string + ts v1alpha1.TaskSpec + workspaces []v1alpha1.WorkspaceBinding + expectedTaskSpec v1alpha1.TaskSpec + }{{ + name: "binding a single workspace with a PVC", + ts: v1alpha1.TaskSpec{ + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }}, + }, + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/foo/bar/baz", + }}, + expectedTaskSpec: v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "ws-9l9zj", + MountPath: "/workspace/custom", + SubPath: "/foo/bar/baz", + }}, + }, + Volumes: []corev1.Volume{{ + Name: "ws-9l9zj", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }}, + }, + }, { + name: "binding a single workspace with emptyDir", + ts: v1alpha1.TaskSpec{ + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }}, + }, + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + }, + SubPath: "/foo/bar/baz", + }}, + expectedTaskSpec: v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "ws-mz4c7", + MountPath: "/workspace/custom", + SubPath: "/foo/bar/baz", // TODO: what happens when you use subPath with emptyDir + }}, + }, + Volumes: []corev1.Volume{{ + Name: "ws-mz4c7", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + }, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }}, + }, + }, { + name: "task spec already has volumes and stepTemplate", + ts: v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "awesome-volume", + MountPath: "/", + }}, + }, + Volumes: []corev1.Volume{{ + Name: "awesome-volume", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }}, + }, + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + }, + SubPath: "/foo/bar/baz", + }}, + expectedTaskSpec: v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "awesome-volume", + MountPath: "/", + }, { + Name: "ws-mssqb", + MountPath: "/workspace/custom", + SubPath: "/foo/bar/baz", // TODO: what happens when you use subPath with emptyDir + }}, + }, + Volumes: []corev1.Volume{{ + Name: "awesome-volume", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, { + Name: "ws-mssqb", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + Medium: corev1.StorageMediumMemory, + }, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }}, + }, + }, { + name: "0 workspace bindings", + ts: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{ + Container: corev1.Container{ + Name: "foo", + }}}, + }, + workspaces: []v1alpha1.WorkspaceBinding{}, + expectedTaskSpec: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{ + Container: corev1.Container{ + Name: "foo", + }}}, + }, + }, { + name: "binding multiple workspaces", + ts: v1alpha1.TaskSpec{ + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }, { + Name: "even-more-custom", + }}, + }, + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/foo/bar/baz", + }, { + Name: "even-more-custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myotherpvc", + }, + }}, + expectedTaskSpec: v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "ws-78c5n", + MountPath: "/workspace/custom", + SubPath: "/foo/bar/baz", + }, { + Name: "ws-6nl7g", + MountPath: "/workspace/even-more-custom", + SubPath: "", + }}, + }, + Volumes: []corev1.Volume{{ + Name: "ws-78c5n", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }, { + Name: "ws-6nl7g", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myotherpvc", + }, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom"}, { + Name: "even-more-custom", + }}, + }, + }, { + name: "multiple workspaces binding to the same volume with diff subpaths doesnt duplicate", + ts: v1alpha1.TaskSpec{ + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }, { + Name: "custom2", + }}, + }, + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/foo/bar/baz", + }, { + Name: "custom2", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + SubPath: "/very/professional/work/space", + }}, + expectedTaskSpec: v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "ws-j2tds", + MountPath: "/workspace/custom", + SubPath: "/foo/bar/baz", + }, { + Name: "ws-j2tds", + MountPath: "/workspace/custom2", + SubPath: "/very/professional/work/space", + }}, + }, + Volumes: []corev1.Volume{{ + Name: "ws-j2tds", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + }, { + Name: "custom2", + }}, + }, + }, { + name: "non default mount path", + ts: v1alpha1.TaskSpec{ + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + MountPath: "/my/fancy/mount/path", + }}, + }, + workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "custom", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }}, + expectedTaskSpec: v1alpha1.TaskSpec{ + StepTemplate: &corev1.Container{ + VolumeMounts: []corev1.VolumeMount{{ + Name: "ws-l22wn", + MountPath: "/my/fancy/mount/path", + }}, + }, + Volumes: []corev1.Volume{{ + Name: "ws-l22wn", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "mypvc", + }, + }, + }}, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "custom", + MountPath: "/my/fancy/mount/path", + }}, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ts, err := workspace.Apply(tc.ts, tc.workspaces) + if err != nil { + t.Fatalf("Did not expect error but got %v", err) + } + if d := cmp.Diff(tc.expectedTaskSpec, *ts); d != "" { + t.Errorf("Didn't get expected TaskSpec modifications (-want, +got): %s", d) + } + }) + } +} diff --git a/pkg/workspace/validate.go b/pkg/workspace/validate.go new file mode 100644 index 00000000000..d59542bbbbc --- /dev/null +++ b/pkg/workspace/validate.go @@ -0,0 +1,50 @@ +/* +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 workspace + +import ( + "context" + "fmt" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/list" +) + +// ValidateBindings will return an error if the bound workspaces in wb satisfy the declared +// workspaces in w. +func ValidateBindings(w []v1alpha1.WorkspaceDeclaration, wb []v1alpha1.WorkspaceBinding) error { + // This will also be validated at webhook time but in case the webhook isn't invoked for some + // reason we'll invoke the same validation here. + for _, b := range wb { + if err := b.Validate(context.Background()); err != nil { + return fmt.Errorf("binding %q is invalid: %v", b.Name, err) + } + } + + declNames := make([]string, len(w)) + for i := range w { + declNames[i] = w[i].Name + } + bindNames := make([]string, len(wb)) + for i := range wb { + bindNames[i] = wb[i].Name + } + if err := list.IsSame(declNames, bindNames); err != nil { + return fmt.Errorf("bound workspaces did not match declared workspaces: %v", err) + } + return nil +} diff --git a/pkg/workspace/validate_test.go b/pkg/workspace/validate_test.go new file mode 100644 index 00000000000..e79d104a509 --- /dev/null +++ b/pkg/workspace/validate_test.go @@ -0,0 +1,133 @@ +/* +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 workspace + +import ( + "testing" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + corev1 "k8s.io/api/core/v1" +) + +func TestValidateBindingsValid(t *testing.T) { + for _, tc := range []struct { + name string + declarations []v1alpha1.WorkspaceDeclaration + bindings []v1alpha1.WorkspaceBinding + }{{ + name: "no bindings provided or required", + declarations: nil, + bindings: nil, + }, { + name: "empty list of bindings provided and required", + declarations: []v1alpha1.WorkspaceDeclaration{}, + bindings: []v1alpha1.WorkspaceBinding{}, + }, { + name: "Successfully bound with PVC", + declarations: []v1alpha1.WorkspaceDeclaration{{ + Name: "beth", + }}, + bindings: []v1alpha1.WorkspaceBinding{{ + Name: "beth", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pool-party", + }, + }}, + }, { + name: "Successfully bound with emptyDir", + declarations: []v1alpha1.WorkspaceDeclaration{{ + Name: "beth", + }}, + bindings: []v1alpha1.WorkspaceBinding{{ + Name: "beth", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }}, + }} { + t.Run(tc.name, func(t *testing.T) { + if err := ValidateBindings(tc.declarations, tc.bindings); err != nil { + t.Errorf("didnt expect error for valid bindings but got: %v", err) + } + }) + } + +} + +func TestValidateBindingsInvalid(t *testing.T) { + for _, tc := range []struct { + name string + declarations []v1alpha1.WorkspaceDeclaration + bindings []v1alpha1.WorkspaceBinding + }{{ + name: "Didn't provide binding matching declared workspace", + declarations: []v1alpha1.WorkspaceDeclaration{{ + Name: "beth", + }}, + bindings: []v1alpha1.WorkspaceBinding{{ + Name: "kate", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, { + Name: "beth", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }}, + }, { + name: "Provided a binding that wasn't needed", + declarations: []v1alpha1.WorkspaceDeclaration{{ + Name: "randall", + }, { + Name: "beth", + }}, + bindings: []v1alpha1.WorkspaceBinding{{ + Name: "beth", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }}, + }, { + name: "Provided both pvc and emptydir", + declarations: []v1alpha1.WorkspaceDeclaration{{ + Name: "beth", + }}, + bindings: []v1alpha1.WorkspaceBinding{{ + Name: "beth", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pool-party", + }, + }}, + }, { + name: "Provided neither pvc nor emptydir", + declarations: []v1alpha1.WorkspaceDeclaration{{ + Name: "beth", + }}, + bindings: []v1alpha1.WorkspaceBinding{{ + Name: "beth", + }}, + }, { + name: "Provided pvc without claim name", + declarations: []v1alpha1.WorkspaceDeclaration{{ + Name: "beth", + }}, + bindings: []v1alpha1.WorkspaceBinding{{ + Name: "beth", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{}, + }}, + }} { + t.Run(tc.name, func(t *testing.T) { + if err := ValidateBindings(tc.declarations, tc.bindings); err == nil { + t.Errorf("expected error for invalid bindings but didn't get any!") + } + }) + } +} diff --git a/test/builder/task.go b/test/builder/task.go index 070c9a8c3ff..5586c4c72f1 100644 --- a/test/builder/task.go +++ b/test/builder/task.go @@ -167,6 +167,17 @@ func Sidecar(name, image string, ops ...ContainerOp) TaskSpecOp { } } +// TaskWorkspace adds a workspace declaration. +func TaskWorkspace(name, desc, mountPath string) TaskSpecOp { + return func(spec *v1alpha1.TaskSpec) { + spec.Workspaces = append(spec.Workspaces, v1alpha1.WorkspaceDeclaration{ + Name: name, + Description: desc, + MountPath: mountPath, + }) + } +} + // TaskStepTemplate adds a base container for all steps in the task. func TaskStepTemplate(ops ...ContainerOp) TaskSpecOp { return func(spec *v1alpha1.TaskSpec) { @@ -662,6 +673,30 @@ func TaskRunOutputsResource(name string, ops ...TaskResourceBindingOp) TaskRunOu } } +// TaskRunWorkspaceEmptyDir adds a workspace binding to an empty dir volume source. +func TaskRunWorkspaceEmptyDir(name, subPath string) TaskRunSpecOp { + return func(spec *v1alpha1.TaskRunSpec) { + spec.Workspaces = append(spec.Workspaces, v1alpha1.WorkspaceBinding{ + Name: name, + SubPath: subPath, + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }) + } +} + +// TaskRunWorkspacePVC adds a workspace binding to a PVC volume source. +func TaskRunWorkspacePVC(name, subPath, claimName string) TaskRunSpecOp { + return func(spec *v1alpha1.TaskRunSpec) { + spec.Workspaces = append(spec.Workspaces, v1alpha1.WorkspaceBinding{ + Name: name, + SubPath: subPath, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: claimName, + }, + }) + } +} + // ResolvedTaskResources creates a ResolvedTaskResources with default values. // Any number of ResolvedTaskResources modifier can be passed to transform it. func ResolvedTaskResources(ops ...ResolvedTaskResourcesOp) *resources.ResolvedTaskResources { diff --git a/test/builder/task_test.go b/test/builder/task_test.go index b892b38819b..b57d3da267b 100644 --- a/test/builder/task_test.go +++ b/test/builder/task_test.go @@ -61,6 +61,7 @@ func TestTask(t *testing.T) { tb.TaskStepTemplate( tb.EnvVar("FRUIT", "BANANA"), ), + tb.TaskWorkspace("bread", "kind of bread", "/bread/path"), )) expectedTask := &v1alpha1.Task{ ObjectMeta: metav1.ObjectMeta{Name: "test-task", Namespace: "foo"}, @@ -120,6 +121,11 @@ func TestTask(t *testing.T) { Value: "BANANA", }}, }, + Workspaces: []v1alpha1.WorkspaceDeclaration{{ + Name: "bread", + Description: "kind of bread", + MountPath: "/bread/path", + }}, }, } if d := cmp.Diff(expectedTask, task); d != "" { @@ -181,6 +187,8 @@ func TestTaskRunWithTaskRef(t *testing.T) { tb.TaskResourceBindingPaths("output-folder"), ), ), + tb.TaskRunWorkspaceEmptyDir("bread", "path"), + tb.TaskRunWorkspacePVC("pizza", "", "pool-party"), ), tb.TaskRunStatus( tb.PodName("my-pod-name"), @@ -244,6 +252,17 @@ func TestTaskRunWithTaskRef(t *testing.T) { Kind: v1alpha1.ClusterTaskKind, APIVersion: "a1", }, + Workspaces: []v1alpha1.WorkspaceBinding{{ + Name: "bread", + SubPath: "path", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, { + Name: "pizza", + SubPath: "", + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "pool-party", + }, + }}, }, Status: v1alpha1.TaskRunStatus{ Status: duckv1beta1.Status{