From 581f32cc10228db3bc22d0bc48116886a4aac58b Mon Sep 17 00:00:00 2001 From: Andrea Frittoli Date: Wed, 12 Jul 2023 14:07:49 +0100 Subject: [PATCH] [TEP-0029] Isolated steps and sidecar workspaces to beta This feature has been implemented in Tekton Pipelines v0.24.0. Isolated workspaces gives our user an option for improved security, which is why it's important to work towards making it available to our users by default. No outstanding issue exists for this feature, except for the proposal to promote it to beta. Fixes #6109 Signed-off-by: Andrea Frittoli --- docs/additional-configs.md | 6 +-- docs/workspaces.md | 10 ++--- .../{alpha => beta}/isolated-workspaces.yaml | 0 .../authenticating-git-commands.yaml | 0 .../{alpha => beta}/workspace-in-sidecar.yaml | 0 .../{alpha => beta}/workspace-isolation.yaml | 0 pkg/apis/pipeline/v1/task_validation.go | 4 +- pkg/apis/pipeline/v1/task_validation_test.go | 39 ++++++++++++++---- pkg/apis/pipeline/v1beta1/task_validation.go | 10 +---- .../pipeline/v1beta1/task_validation_test.go | 40 ------------------- pkg/reconciler/taskrun/resources/apply.go | 4 +- .../taskrun/resources/apply_test.go | 2 +- 12 files changed, 47 insertions(+), 68 deletions(-) rename examples/v1beta1/pipelineruns/{alpha => beta}/isolated-workspaces.yaml (100%) rename examples/v1beta1/taskruns/{alpha => beta}/authenticating-git-commands.yaml (100%) rename examples/v1beta1/taskruns/{alpha => beta}/workspace-in-sidecar.yaml (100%) rename examples/v1beta1/taskruns/{alpha => beta}/workspace-isolation.yaml (100%) diff --git a/docs/additional-configs.md b/docs/additional-configs.md index dbe4422668d..cbc78100bbe 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -298,7 +298,6 @@ Features currently in "alpha" are: | Feature | Proposal | Release | Individual Flag | |:----------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------|:------------------------------| | [Bundles ](./pipelineruns.md#tekton-bundles) | [TEP-0005](https://github.com/tektoncd/community/blob/main/teps/0005-tekton-oci-bundles.md) | [v0.18.0](https://github.com/tektoncd/pipeline/releases/tag/v0.18.0) | `enable-tekton-oci-bundles` | -| [Isolated `Step` & `Sidecar` `Workspaces`](./workspaces.md#isolated-workspaces) | [TEP-0029](https://github.com/tektoncd/community/blob/main/teps/0029-step-workspaces.md) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | | | [Hermetic Execution Mode](./hermetic.md) | [TEP-0025](https://github.com/tektoncd/community/blob/main/teps/0025-hermekton.md) | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | | | [Windows Scripts](./tasks.md#windows-scripts) | [TEP-0057](https://github.com/tektoncd/community/blob/main/teps/0057-windows-support.md) | [v0.28.0](https://github.com/tektoncd/pipeline/releases/tag/v0.28.0) | | | [Debug](./debug.md) | [TEP-0042](https://github.com/tektoncd/community/blob/main/teps/0042-taskrun-breakpoint-on-failure.md) | [v0.26.0](https://github.com/tektoncd/pipeline/releases/tag/v0.26.0) | | @@ -323,11 +322,12 @@ Features currently in "beta" are: | Feature | Proposal | Alpha Release | Beta Release | Individual Flag | |:-------------------------------------------------------------------|:------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------|:---------------------------------------------------------------------|:----------------| | [Array Results and Array Indexing](pipelineruns.md#specifying-parameters) | [TEP-0076](https://github.com/tektoncd/community/blob/main/teps/0076-array-result-types.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | [v0.45.0](https://github.com/tektoncd/pipeline/releases/tag/v0.45.0) | | -| [Object Parameters and Results](pipelineruns.md#specifying-parameters) | [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md) | [v0.46.0](https://github.com/tektoncd/pipeline/releases/tag/v0.46.0) | -| [Remote Tasks](./taskruns.md#remote-tasks) and [Remote Pipelines](./pipelineruns.md#remote-pipelines) | [TEP-0060](https://github.com/tektoncd/community/blob/main/teps/0060-remote-resolution.md) | [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0) | +| [Object Parameters and Results](pipelineruns.md#specifying-parameters) | [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md) | | [v0.46.0](https://github.com/tektoncd/pipeline/releases/tag/v0.46.0) | +| [Remote Tasks](./taskruns.md#remote-tasks) and [Remote Pipelines](./pipelineruns.md#remote-pipelines) | [TEP-0060](https://github.com/tektoncd/community/blob/main/teps/0060-remote-resolution.md) | | [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0) | | [`Provenance` field in Status](pipeline-api.md#provenance)| [issue#5550](https://github.com/tektoncd/pipeline/issues/5550)| [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0)| [v0.48.0](https://github.com/tektoncd/pipeline/releases/tag/v0.48.0) | `enable-provenance-in-status`| | [CSI workspaces](workspaces.md#csi)| [issue#4446](https://github.com/tektoncd/pipeline/issues/4446)| [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0)| [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0) | | | [Projected volume workspaces](workspaces.md#projected)| [issue#5075](https://github.com/tektoncd/pipeline/issues/5075)| [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0)| [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0) | | +| [Isolated `Step` & `Sidecar` `Workspaces`](./workspaces.md#isolated-workspaces) | [TEP-0029](https://github.com/tektoncd/community/blob/main/teps/0029-step-workspaces.md) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | [v0.50.0](https://github.com/tektoncd/pipeline/releases/tag/v0.50.0) | ## Enabling larger results using sidecar logs diff --git a/docs/workspaces.md b/docs/workspaces.md index 15b4619ed1e..100c3bbf9eb 100644 --- a/docs/workspaces.md +++ b/docs/workspaces.md @@ -67,7 +67,7 @@ information in the `TaskRun` changes. configuration involved to add the required `volumeMount`. This allows for a long-running process in a `Sidecar` to share data with the executing `Steps` of a `Task`. -**Note**: If the `enable-api-fields` feature-flag is set to `"alpha"` then workspaces +**Note**: If the `enable-api-fields` feature-flag is set to `"beta"` then workspaces will automatically be available to `Sidecars` too! ### `Workspaces` in `Pipelines` and `PipelineRuns` @@ -101,7 +101,7 @@ the `optional` field. ### Isolated `Workspaces` -This is an alpha feature. The `enable-api-fields` feature flag [must be set to `"alpha"`](./install.md) +This is a beta feature. The `enable-api-fields` feature flag [must be set to `"beta"`](./install.md) for Isolated Workspaces to function. Certain kinds of data are more sensitive than others. To reduce exposure of sensitive data Task @@ -187,8 +187,8 @@ spec: touch "$(workspaces.signals.path)/ready" ``` -**Note:** Starting in Pipelines v0.24.0 `Sidecars` automatically get access to `Workspaces`.This is an -alpha feature and requires Pipelines to have [the "alpha" feature gate enabled](./install.md#alpha-features). +**Note:** Starting in Pipelines v0.24.0 `Sidecars` automatically get access to `Workspaces`. This is a +beta feature and requires Pipelines to have [the "beta" feature gate enabled](./install.md#beta-features). If a Sidecar already has a `volumeMount` at the location expected for a `workspace` then that `workspace` is not bound to the Sidecar. This preserves backwards-compatibility with any existing uses of the `volumeMount` @@ -196,7 +196,7 @@ trick described above. #### Isolating `Workspaces` to Specific `Steps` or `Sidecars` -This is an alpha feature. The `enable-api-fields` feature flag [must be set to `"alpha"`](./install.md) +This is a beta feature. The `enable-api-fields` feature flag [must be set to `"beta"`](./install.md#beta-features) for Isolated Workspaces to function. To limit access to a `Workspace` from a subset of a `Task's` `Steps` or `Sidecars` requires diff --git a/examples/v1beta1/pipelineruns/alpha/isolated-workspaces.yaml b/examples/v1beta1/pipelineruns/beta/isolated-workspaces.yaml similarity index 100% rename from examples/v1beta1/pipelineruns/alpha/isolated-workspaces.yaml rename to examples/v1beta1/pipelineruns/beta/isolated-workspaces.yaml diff --git a/examples/v1beta1/taskruns/alpha/authenticating-git-commands.yaml b/examples/v1beta1/taskruns/beta/authenticating-git-commands.yaml similarity index 100% rename from examples/v1beta1/taskruns/alpha/authenticating-git-commands.yaml rename to examples/v1beta1/taskruns/beta/authenticating-git-commands.yaml diff --git a/examples/v1beta1/taskruns/alpha/workspace-in-sidecar.yaml b/examples/v1beta1/taskruns/beta/workspace-in-sidecar.yaml similarity index 100% rename from examples/v1beta1/taskruns/alpha/workspace-in-sidecar.yaml rename to examples/v1beta1/taskruns/beta/workspace-in-sidecar.yaml diff --git a/examples/v1beta1/taskruns/alpha/workspace-isolation.yaml b/examples/v1beta1/taskruns/beta/workspace-isolation.yaml similarity index 100% rename from examples/v1beta1/taskruns/alpha/workspace-isolation.yaml rename to examples/v1beta1/taskruns/beta/workspace-isolation.yaml diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 9b56bf6789a..41a93bc5153 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -222,7 +222,7 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel for stepIdx, step := range steps { if len(step.Workspaces) != 0 { - errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step workspaces", config.AlphaAPIFields).ViaIndex(stepIdx).ViaField("steps")) + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step workspaces", config.BetaAPIFields).ViaIndex(stepIdx).ViaField("steps")) } for workspaceIdx, w := range step.Workspaces { if !wsNames.Has(w.Name) { @@ -233,7 +233,7 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel for sidecarIdx, sidecar := range sidecars { if len(sidecar.Workspaces) != 0 { - errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "sidecar workspaces", config.AlphaAPIFields).ViaIndex(sidecarIdx).ViaField("sidecars")) + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "sidecar workspaces", config.BetaAPIFields).ViaIndex(sidecarIdx).ViaField("sidecars")) } for workspaceIdx, w := range sidecar.Workspaces { if !wsNames.Has(w.Name) { diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 6abf86f4f0d..8e82e87aeed 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -1580,13 +1580,30 @@ func TestStepOnError(t *testing.T) { // TestIncompatibleAPIVersions exercises validation of fields that // require a specific feature gate version in order to work. func TestIncompatibleAPIVersions(t *testing.T) { + + isStricterThen := func(first, second string) bool { + // assume values are in order alpha (less strict), beta, stable (strictest) + // return true if first is stricter then second + if first == second { + return false + } + if first == "alpha" { + return false + } + if first == "stable" { + return true + } + // first is beta, true is second is alpha, false is second is stable + return second == "alpha" + } + tests := []struct { name string requiredVersion string spec v1.TaskSpec }{{ - name: "step workspace requires alpha", - requiredVersion: "alpha", + name: "step workspace requires beta", + requiredVersion: "beta", spec: v1.TaskSpec{ Workspaces: []v1.WorkspaceDeclaration{{ Name: "foo", @@ -1599,8 +1616,8 @@ func TestIncompatibleAPIVersions(t *testing.T) { }}, }, }, { - name: "sidecar workspace requires alpha", - requiredVersion: "alpha", + name: "sidecar workspace requires beta", + requiredVersion: "beta", spec: v1.TaskSpec{ Workspaces: []v1.WorkspaceDeclaration{{ Name: "foo", @@ -1649,7 +1666,7 @@ func TestIncompatibleAPIVersions(t *testing.T) { }}, }}, } - versions := []string{"alpha", "stable"} + versions := []string{"alpha", "beta", "stable"} for _, tt := range tests { for _, version := range versions { testName := fmt.Sprintf("(using %s) %s", version, tt.name) @@ -1659,14 +1676,22 @@ func TestIncompatibleAPIVersions(t *testing.T) { if version == "alpha" { ctx = config.EnableAlphaAPIFields(ctx) } + if version == "beta" { + ctx = config.EnableBetaAPIFields(ctx) + } + if version == "stable" { + ctx = config.EnableStableAPIFields(ctx) + } ts.SetDefaults(ctx) err := ts.Validate(ctx) - if tt.requiredVersion != version && err == nil { + // If the configured version is stricter than the required one, we expect an error + if isStricterThen(version, tt.requiredVersion) && err == nil { t.Fatalf("no error received even though version required is %q while feature gate is %q", tt.requiredVersion, version) } - if tt.requiredVersion == version && err != nil { + // If the configured version is more permissive than the required one, we expect no error + if isStricterThen(tt.requiredVersion, version) && err != nil { t.Fatalf("error received despite required version and feature gate matching %q: %v", version, err) } }) diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index e9e6d2832df..36b97d38238 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -176,8 +176,8 @@ func validateDeclaredWorkspaces(workspaces []WorkspaceDeclaration, steps []Step, // validateWorkspaceUsages checks that all WorkspaceUsage objects in Steps // refer to workspaces that are defined in the Task. // -// This is an alpha feature and will fail validation if it's used by a step -// or sidecar when the enable-api-fields feature gate is anything but "alpha". +// This is an beta feature and, until TEP-0138 https://github.com/tektoncd/community/pull/1034 +// is implemented, it will pass validation on v1beta1 resources func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.FieldError) { workspaces := ts.Workspaces steps := ts.Steps @@ -189,9 +189,6 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel } for stepIdx, step := range steps { - if len(step.Workspaces) != 0 { - errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step workspaces", config.AlphaAPIFields).ViaIndex(stepIdx).ViaField("steps")) - } for workspaceIdx, w := range step.Workspaces { if !wsNames.Has(w.Name) { errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("undefined workspace %q", w.Name), "name").ViaIndex(workspaceIdx).ViaField("workspaces").ViaIndex(stepIdx).ViaField("steps")) @@ -200,9 +197,6 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel } for sidecarIdx, sidecar := range sidecars { - if len(sidecar.Workspaces) != 0 { - errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "sidecar workspaces", config.AlphaAPIFields).ViaIndex(sidecarIdx).ViaField("sidecars")) - } for workspaceIdx, w := range sidecar.Workspaces { if !wsNames.Has(w.Name) { errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("undefined workspace %q", w.Name), "name").ViaIndex(workspaceIdx).ViaField("workspaces").ViaIndex(sidecarIdx).ViaField("sidecars")) diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 3272a8739e4..16ff8bd3dc9 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -1597,37 +1597,6 @@ func TestIncompatibleAPIVersions(t *testing.T) { requiredVersion string spec v1beta1.TaskSpec }{{ - name: "step workspace requires alpha", - requiredVersion: "alpha", - spec: v1beta1.TaskSpec{ - Workspaces: []v1beta1.WorkspaceDeclaration{{ - Name: "foo", - }}, - Steps: []v1beta1.Step{{ - Image: "foo", - Workspaces: []v1beta1.WorkspaceUsage{{ - Name: "foo", - }}, - }}, - }, - }, { - name: "sidecar workspace requires alpha", - requiredVersion: "alpha", - spec: v1beta1.TaskSpec{ - Workspaces: []v1beta1.WorkspaceDeclaration{{ - Name: "foo", - }}, - Steps: []v1beta1.Step{{ - Image: "foo", - }}, - Sidecars: []v1beta1.Sidecar{{ - Image: "foo", - Workspaces: []v1beta1.WorkspaceUsage{{ - Name: "foo", - }}, - }}, - }, - }, { name: "windows script support requires alpha", requiredVersion: "alpha", spec: v1beta1.TaskSpec{ @@ -1687,15 +1656,6 @@ func TestIncompatibleAPIVersions(t *testing.T) { } func TestGetArrayIndexParamRefs(t *testing.T) { - stepsReferences := []string{} - for i := 10; i <= 26; i++ { - stepsReferences = append(stepsReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - volumesReferences := []string{} - for i := 10; i <= 22; i++ { - volumesReferences = append(volumesReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - tcs := []struct { name string taskspec *v1beta1.TaskSpec diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index 8fb537f15aa..78dda27fb2f 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -157,8 +157,8 @@ func ApplyWorkspaces(ctx context.Context, spec *v1.TaskSpec, declarations []v1.W stringReplacements[prefix+"path"] = "" } else { stringReplacements[prefix+"bound"] = "true" - alphaAPIEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.AlphaAPIFields - if alphaAPIEnabled { + betaAPIEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == config.BetaAPIFields + if betaAPIEnabled { spec = applyWorkspaceMountPath(prefix+"path", spec, declaration) } else { stringReplacements[prefix+"path"] = declaration.GetMountPath() diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index 91f8c9cb7c8..db194742b92 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -1157,7 +1157,7 @@ func TestApplyWorkspaces_IsolatedWorkspaces(t *testing.T) { t.Run(tc.name, func(t *testing.T) { ctx := config.ToContext(context.Background(), &config.Config{ FeatureFlags: &config.FeatureFlags{ - EnableAPIFields: "alpha", + EnableAPIFields: "beta", }, }) vols := workspace.CreateVolumes(tc.binds)