From c55d311ffc4b277f5a64a3787341d68d0288ff00 Mon Sep 17 00:00:00 2001 From: Christie Wilson Date: Tue, 26 Nov 2019 14:55:00 -0500 Subject: [PATCH] Add workspace types for Task and TaskRun with validation This allows users to use Volumes with Tasks such that: - The actual volumes to use (or subdirectories on those volumes) are provided at runtime, not at Task authoring time - At Task authoring time you can declare that you expect a volume to be provided and control what path that volume should end up at - Validation will be provided that the volumes (workspaces) are actually provided at runtime Before this change, there were two ways to use Volumes with Tasks: - VolumeMounts were explicitly declared at the level of a step - Volumes were declared in Tasks, meaning the Task author controlled the name of the volume being used and it wasn't possible at runtime to use a subdir of the volume - Or the Volume could be provided via the podTemplate, if the user realized this was possible None of this was validated and could cause unexpected and hard to diagnose errors at runtime. It's possible folks might be specifying volumes already in the Task or via the stepTemplate that might collide with the names we are using for the workspaces; instead of validating this and making the Task author change these, we can instead randomize them! We have also limited (at least initially) the types of volume source being supported instead of expanding to all volume sources, tho we can expand it later if we want to and if users need it. This would reduce the API surface that a Tekton compliant system would need to conform to (once we actually define what conformance means!). Part of #1438 In future commits we will add support for workspaces to Pipelines and PipelineRuns as well; for now if a user tries to use a Pipeline with a Task that requires a Workspace, it will fail at runtime because it is not (yet) possible for the Pipeline and PipelineRun to provide workspaces. Co-authored-by: Scott --- docs/pipelineruns.md | 9 +- docs/pipelines.md | 7 + docs/taskruns.md | 43 +- docs/tasks.md | 72 ++- examples/taskruns/custom-volume.yaml | 2 +- examples/taskruns/workspace.yaml | 68 +++ pkg/apis/pipeline/paths.go | 22 + pkg/apis/pipeline/v1alpha1/git_resource.go | 5 +- .../v1alpha1/pull_request_resource.go | 3 +- .../v1alpha1/pull_request_resource_test.go | 5 +- pkg/apis/pipeline/v1alpha1/task_types.go | 3 + pkg/apis/pipeline/v1alpha1/task_validation.go | 43 ++ .../pipeline/v1alpha1/task_validation_test.go | 147 +++++- pkg/apis/pipeline/v1alpha1/taskrun_types.go | 5 + .../pipeline/v1alpha1/taskrun_validation.go | 21 + .../v1alpha1/taskrun_validation_test.go | 43 +- pkg/apis/pipeline/v1alpha1/workspace_types.go | 65 +++ .../pipeline/v1alpha1/workspace_validation.go | 49 ++ .../v1alpha1/workspace_validation_test.go | 88 ++++ .../v1alpha1/zz_generated.deepcopy.go | 54 +++ pkg/pod/pod.go | 7 +- pkg/pod/pod_test.go | 30 +- pkg/pod/workingdir_init.go | 3 +- pkg/pod/workingdir_init_test.go | 3 +- pkg/reconciler/taskrun/resources/apply.go | 17 + .../taskrun/resources/apply_test.go | 114 +++++ pkg/reconciler/taskrun/taskrun.go | 28 +- pkg/workspace/apply.go | 98 ++++ pkg/workspace/apply_test.go | 428 ++++++++++++++++++ pkg/workspace/validate.go | 50 ++ pkg/workspace/validate_test.go | 133 ++++++ test/builder/task.go | 35 ++ test/builder/task_test.go | 19 + 33 files changed, 1668 insertions(+), 51 deletions(-) create mode 100644 examples/taskruns/workspace.yaml create mode 100644 pkg/apis/pipeline/paths.go create mode 100644 pkg/apis/pipeline/v1alpha1/workspace_types.go create mode 100644 pkg/apis/pipeline/v1alpha1/workspace_validation.go create mode 100644 pkg/apis/pipeline/v1alpha1/workspace_validation_test.go create mode 100644 pkg/workspace/apply.go create mode 100644 pkg/workspace/apply_test.go create mode 100644 pkg/workspace/validate.go create mode 100644 pkg/workspace/validate_test.go 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{