diff --git a/teps/0029-step-workspaces.md b/teps/0029-step-workspaces.md
index 5b7204dc6..75bb3b8b1 100644
--- a/teps/0029-step-workspaces.md
+++ b/teps/0029-step-workspaces.md
@@ -4,7 +4,7 @@ authors:
- "@sbwsg"
creation-date: 2020-10-02
last-updated: 2020-10-02
-status: proposed
+status: implementable
---
# TEP-0029: Step and Sidecar Workspaces
@@ -20,25 +20,38 @@ tags, and then generate with `hack/update-toc.sh`.
- [Motivation](#motivation)
- [Goals](#goals)
- [Requirements](#requirements)
+- [Proposal](#proposal)
+ - [Add workspaces
to Steps
](#add--to-)
+ - [Add workspaces
to Sidecars
](#add--to--1)
+ - [Allow workspaces
in `Steps` and `Sidecars` to have their own `mountPath`](#allow-workspaces-in-steps-and-sidecars-to-have-their-own-mountpath)
- [User Stories](#user-stories)
- [Story 1](#story-1)
- [Story 2](#story-2)
- [Story 3](#story-3)
- [Story 4](#story-4)
- [Story 5](#story-5)
+- [Design Details](#design-details)
+- [Drawbacks](#drawbacks)
+- [Alternatives](#alternatives)
+ - [Use an explicit volumeMount instead](#use-an-explicit-volumemount-instead)
+ - [Advantages](#advantages)
+ - [Drawbacks](#drawbacks-1)
+ - [Specify complete Workspace declarations in Steps](#specify-complete-workspace-declarations-in-steps)
+ - [Advantages](#advantages-1)
+ - [Disadvantages](#disadvantages)
+- [Upgrade & Migration Strategy (optional)](#upgrade--migration-strategy-optional)
+- [References (optional)](#references-optional)
## Summary
## Motivation
-This TEP is motivated by 3 major goals: First, to allow a Task to explicitly declare the
-Steps and Sidecars that have access to workspaces so that sensitive data can be isolated
-to only the containers that need it. Second, to add blessed support for Sidecars to access
-Workspaces without using a "hack" requiring the Task author to wire a Workspace's `volume`
-name to a `volumeMount` in the pod spec. Third, to make the behaviour of Workspaces uniform
-across Steps and Sidecars so that understanding the behaviour of one eases understanding
-the behaviour of the other.
+This TEP is motivated by 3 major goals:
+
+1. Limit access to sensitive credentials.
+2. Add blessed support for Sidecars to access workspaces.
+3. Make Workspace behaviour uniform across Steps and Sidecars.
### Goals
@@ -56,6 +69,127 @@ that has access to those contents as well.
- A Task author can use a Workspace from a `Sidecar`.
- A Task author can still use the volume "hack" to attach `Workspaces` to `Sidecars` in
combination with the feature proposed here.
+- A Task author can choose different `mountPaths` for each Step that receives the
+`Workspace`.
+
+## Proposal
+
+Add `workspaces` lists to Steps and Sidecars. Here's the existing YAML and behaviour:
+
+```yaml
+spec:
+ workspaces:
+ - name: foo
+ steps:
+ - image: ubuntu
+ script: # ... This Step automatically mounts "foo" workspace.
+ - image: ubuntu
+ script: # ... And so does this Step.
+```
+
+And here's a Task using the new Step Workspaces feature:
+
+```yaml
+spec:
+ workspaces:
+ - name: foo
+ steps:
+ - image: ubuntu
+ workspaces:
+ - name: foo
+ script: # ... This Step mounts "foo" workspace.
+ - image: ubuntu
+ script: # ... But this Step does not.
+```
+
+The existing YAML and behaviour will continue to be supported.
+
+### Add `workspaces` to `Steps`
+
+1. Add a `workspaces` list to `Steps`.
+2. Allow `workspaces` from the `Task` to be explicitly named in that list like this:
+
+ ```yaml
+ workspaces:
+ - name: my-sensitive-workspace
+ steps:
+ - name: foo
+ workspaces:
+ - name: my-sensitive-workspace
+ ```
+3. When a `workspace` is listed in a Step, it is no longer automatically mounted - either to
+`Steps` or `Sidecars` - unless they also have the `workspace` in their own `workspaces` list.
+
+Example YAML:
+
+```yaml
+# task spec
+spec:
+ workspaces:
+ - name: git-ssh-credentials
+ mountPath: /root/.ssh
+ steps:
+ - name: clone-repo
+ image: alpine/git:v2.26.2
+ workspaces:
+ - name: git-ssh-credentials
+ script: |
+ git clone $(params.repo-url) /workspace/source
+ - name: run-unit-tests
+ script: |
+ cd /workspace/source
+ go test ./...
+```
+
+In the above example only the `clone-repo` `Step` will receive access to the `git-ssh-credentials`
+`Workspace`. The `run-unit-tests` `Step` will not receive access to the Workspace volume. Importantly
+this also means that the user-supplied code does not have access to the credential files. Compromising
+the code for the unit tests does not also compromise the SSH credentials.
+
+### Add `workspaces` to `Sidecars`
+
+1. Automatically mount `workspaces` to `Sidecars` just as they're automatically mounted to `Steps` today.
+2. Add a `workspaces` list to `Sidecars`.
+3. Allow `workspaces` from the `Task` to be explicitly named in that list like this:
+
+ ```yaml
+ workspaces:
+ - name: my-workspace
+ sidecars:
+ - name: watch-workspace
+ workspaces:
+ - name: my-workspace
+ ```
+
+4. When a `workspace` is listed in a Sidecar, it is no longer automatically mounted - either to
+`Steps` or `Sidecars` - unless they also have the `workspace` in their own `workspaces` list.
+
+### Allow `workspaces` in `Steps` and `Sidecars` to have their own `mountPath`
+
+When declaring the Workspace in the Step or Sidecar a custom mountPath can also be specified.
+This allows for situations where different images may have different expectations for the location
+of files from a workspace. This mountPath overrides whatever mountPath is set on the Task Spec's
+`workspaces` entry.
+
+```yaml
+# Task Spec
+workspaces:
+- name: ws
+ mountPath: /workspaces/ws
+steps:
+- name: edit-files-1
+ workspaces:
+ - name: foo
+ mountPath: /foo # overrides mountPath
+- name: edit-files-2
+ workspaces:
+ - name: foo # no mountPath specified so will use /workspaces/ws
+sidecars:
+- name: watch-files-on-workspace
+ workspaces:
+ - name: foo
+ mountPath: /files # overrides mountPath
+```
### User Stories
@@ -101,3 +235,197 @@ to other `Steps` in the same Task performing ancillary work.
As a Pipeline author I want to be able to quickly audit that the `git-fetch` Task I am using in
my Pipeline is only exposing the git SSH key for my team's source repo in the single `Step` that
performs `git clone`, and not to any `Steps` in the same Task performing ancillary work.
+
+## Design Details
+
+Add a new `WorkspaceUsage` struct to the `task_types.go` file:
+
+```go
+// WorkspaceUsage declares that a Step or Sidecar utilizes a Task's Workspace.
+type WorkspaceUsage struct {
+ // Name is the name of the Task's WorkspaceDeclaration that this Step or Sidecar is utilizing. It is required.
+ Name string `json:"name"`
+ // MountPath is an optional path that overrides where the Workspace will be mounted in the Task's filesystem.
+ MountPath string `json:"mountPath"`
+}
+```
+
+Update the `Step` struct to include a slice of `WorkspaceUsage`:
+
+```go
+type Step struct {
+ corev1.Container `json:",inline"`
+ Script string `json:"script,omitempty"`
+
+ // Workspaces is a list of workspaces that this Step declares it will use in some way. The presence
+ // of a Workspace in this list prevents other Steps from automatically receiving the Workspace -
+ // they must also explicitly opt-in to receiving it as well in their own workspaces list.
+ Workspaces []WorkspaceUsage `json:"workspaces,omitempty"`
+}
+```
+
+Update the `Sidecar` struct to include a slice of `WorkspaceUsage`:
+
+```go
+// Sidecar embeds the Container type, which allows it to include fields not
+// provided by Container.
+type Sidecar struct {
+ corev1.Container `json:",inline"`
+ Script string `json:"script,omitempty"`
+
+ // Workspaces is a list of workspaces that this Sidecar declares it will use in some way.
+ Workspaces []WorkspaceUsage `json:"workspaces,omitempty"`
+}
+```
+
+## Drawbacks
+
+- If we were to pursue the idea of a shareable `Step` type then we would need to find some way to map from
+the workspaces a Task declares into those that a referenced `Step` declares. We'd similarly need to do this
+for params and results.
+- It takes the `Step` and `Sidecar` types further from being "pure" k8s Container. However we've already
+made moves in this direction by introducing our own `Script` field to `Steps` and `Sidecars`.
+- Automounting workspaces into sidecars could have unexpected side effects. If a user is already mounting
+a workspace to a specific location for their `Steps` but are relying on the fact that the sidecar does _not_
+mount to the same spot then they could potentially be broken by this change.
+
+## Alternatives
+
+### Use an explicit volumeMount instead
+
+Instead of adding a `workspaces` list to `Steps`, we could instead lean on the existing
+`volumeMounts` list like this:
+
+```yaml
+# task spec
+spec:
+ workspaces:
+ - name: git-ssh-credentials
+ steps:
+ - name: clone-repo
+ image: alpine/git:v2.26.2
+ volumeMounts:
+ - name: $(workspaces.git-ssh-credentials.volume)
+ script: |
+ git clone $(params.repo-url) /workspace/source
+ - name: run-unit-tests
+ script: |
+ cd /workspace/source
+ go test ./...
+```
+
+And then add a rule that says "if you use a volumeMount to mount a workspace to your Step
+then Tekton will not automatically add volumeMounts to any other Steps." We would also need
+to document this rule.
+
+#### Advantages
+
+Uses an existing Kubernetes container field that the user may already be familiar with.
+
+#### Drawbacks
+
+It's surprising. This approach overloads the meaning of `volumeMounts` with
+additional Tekton-specific context and behaviour (i.e. the additional constraint
+that adding a workspace volume to a `volumeMount` entry will prevent other Steps
+from receiving that `workspace` unless they too include it in their
+`volumeMounts` list).
+
+A further drawback is that `workspaces` provide a useful abstraction for API implementers.
+Leaning on `volumeMounts` would move a Kubernetes-specific field into Tekton's API. We'd have
+to decide which of the fields at https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#volumemount-v1-core
+to support.
+
+### Specify complete Workspace declarations in Steps
+
+Allow `Steps` to fully-specify Workspace declarations so that they're not also required
+at the top-level of the Task spec. Here's how that might look:
+
+```yaml
+steps:
+- name: foo
+ workspaces:
+ - name: my-sensitive-workspace
+```
+
+In contrast with the existing proposal:
+
+```yaml
+workspaces:
+- name: my-sensitive-workspace
+steps:
+- name: foo
+ workspaces:
+ - name: my-sensitive-workspace
+```
+
+#### Advantages
+
+- Shorter syntax for isolating `Workspaces` to single `Steps`.
+- Allows for immediate use of fields like `mountPath` as part of the `Workspace` entry in the `Step`.
+
+#### Disadvantages
+
+- What's the behaviour when a `Task` declares a `Workspace` and `Steps` declare a `Workspace` with the
+same name? Possibly the same behaviour as is being proposed by this document? Or a validation error?
+
+- Open question whether a Task author would be able to share a `Workspace` this way across
+multiple `Steps` if they share a common `Workspace` name:
+
+ ```yaml
+ steps:
+ - name: foo
+ workspaces:
+ - name: my-sensitive-workspace
+ - name: bar
+ workspaces:
+ - name: my-sensitive-workspace
+ ```
+
+ Here the `Task` would presumably only expose a single `Workspace` named "my-sensitive-workspace"
+ to be populated by a `TaskRun` / `PipelineRun`?
+
+ Validation might be made more difficult here - if two `Workspaces` in two `Steps` are named
+ almost the same thing the reconciler won't be able to tell if they're "supposed" to be the same or not.
+ Consider the following example:
+
+ ```yaml
+ steps:
+ - name: foo
+ workspaces:
+ - name: my-sensitive-workspace
+ - name: bar
+ workspaces:
+ - name: my-senistive-workspace
+ ```
+
+ Was this a mis-spelling by the `Task` author or intentionally separate `Workspace` declarations?
+
+ One further extension to this idea would be to allow `TaskRuns` or `PipelineRuns` to bind `Workspaces`
+ to specific `Steps` and `Sidecars`:
+
+ ```yaml
+ workspaces:
+ - name: my-workspace
+ emptyDir: {}
+ - name: sensitive-workspace
+ secretRef:
+ name: foo-secret
+ sidecars:
+ - name: do-something-not-secret
+ workspaces:
+ - name: my-workspace
+ steps:
+ - name: do-something-secret
+ workspaces:
+ - name: some-sensitive-workspace
+ ```
+
+## Upgrade & Migration Strategy (optional)
+
+The feature as proposed is entirely backwards-compatible. Omitting the
+`workspaces` field from `Steps` leaves the existing behaviour exactly as
+it works today - all Steps will receive all workspaces.
+
+## References (optional)
+
+- Original design part of the [Credentials UX](https://github.com/tektoncd/pipeline/issues/2343#issuecomment-611155667) issue.
diff --git a/teps/README.md b/teps/README.md
index 1027750c4..43d1e713a 100644
--- a/teps/README.md
+++ b/teps/README.md
@@ -141,7 +141,7 @@ This is the complete list of Tekton teps:
|[TEP-0026](0026-interceptor-plugins.md) | interceptor-plugins | implementable | 2020-10-08 |
|[TEP-0027](0027-https-connection-to-triggers-eventlistener.md) | HTTPS Connection to Triggers EventListener | implementable | 2020-11-01 |
|[TEP-0028](0028-task-execution-status-at-runtime.md) | task-exec-status-at-runtime | implementable | 2020-11-02 |
-|[TEP-0029](0029-step-workspaces.md) | step-and-sidecar-workspaces | proposed | 2020-10-02 |
+|[TEP-0029](0029-step-workspaces.md) | step-and-sidecar-workspaces | implementable | 2020-10-02 |
|[TEP-0030](0030-workspace-paths.md) | workspace-paths | proposed | 2020-10-18 |
|[TEP-0031](0031-tekton-bundles-cli.md) | tekton-bundles-cli | proposed | 2020-11-18 |
|[TEP-0032](0032-tekton-notifications.md) | Tekton Notifications | proposed | 2020-11-18 |