Migrate Propagated Workspaces to Stable

chitrangpatel · tekton-robot · commit ecb11fc82a64 · 2023-04-04T15:01:11.000+01:00
[Issue 6107](#6107) Migrate Propagated Workspaces to Stable Prior to this, propagated workspaces was hidden behind the beta feature gate. This feature is now a stable feature.
diff --git a/docs/additional-configs.md b/docs/additional-configs.md
@@ -307,7 +307,6 @@ Features currently in "beta" are:
 
 | Feature                                                            | Proposal                                                                                        | Alpha Release                                                        | Beta Release                                                         | Individual Flag |
 |:-------------------------------------------------------------------|:------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------|:---------------------------------------------------------------------|:----------------|
-| [Propagated `Workspaces`](./pipelineruns.md#propagated-workspaces) | [TEP-0111](https://github.com/tektoncd/community/blob/main/teps/0111-propagating-workspaces.md) | [v0.40.0](https://github.com/tektoncd/pipeline/releases/tag/v0.40.0) | [v0.45.0](https://github.com/tektoncd/pipeline/releases/tag/v0.45.0) |                 |
 | [Array Results](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) |                 |
 
 ## Enabling larger results using sidecar logs
diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md
@@ -855,8 +855,6 @@ Consult the documentation of the custom task that you are using to determine whe
 
 #### Propagated Workspaces
 
-**[beta](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#beta-features))**
-
 When using an embedded spec, workspaces from the parent `PipelineRun` will be
 propagated to any inlined specs without needing to be explicitly defined. This
 allows authors to simplify specs by automatically propagating top-level
diff --git a/docs/taskruns.md b/docs/taskruns.md
@@ -453,8 +453,6 @@ For more information, see the following topics:
 
 #### Propagated Workspaces
 
-**[beta](https://github.com/tektoncd/pipeline/blob/main/docs/install.md#beta-features))**
-
 When using an embedded spec, workspaces from the parent `TaskRun` will be
 propagated to any inlined specs without needing to be explicitly defined. This
 allows authors to simplify specs by automatically propagating top-level
diff --git a/examples/v1beta1/pipelineruns/propagating-workspaces.yaml b/examples/v1beta1/pipelineruns/propagating-workspaces.yaml
diff --git a/examples/v1beta1/pipelineruns/propagating_workspaces_with_referenced_resources.yaml b/examples/v1beta1/pipelineruns/propagating_workspaces_with_referenced_resources.yaml
diff --git a/examples/v1beta1/taskruns/propagating_workspaces.yaml b/examples/v1beta1/taskruns/propagating_workspaces.yaml
diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -1033,14 +1033,12 @@ func getTaskrunWorkspaces(ctx context.Context, pr *v1beta1.PipelineRun, rpt *res
 		pipelineRunWorkspaces[binding.Name] = binding
 	}
 
-	if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields {
-		// Propagate required workspaces from pipelineRun to the pipelineTasks
-		if rpt.PipelineTask.TaskSpec != nil {
-			rpt, err = propagateWorkspaces(rpt)
-			if err != nil {
-				// This error cannot be recovered without modifying the TaskSpec
-				return nil, "", controller.NewPermanentError(err)
-			}
+	// Propagate required workspaces from pipelineRun to the pipelineTasks
+	if rpt.PipelineTask.TaskSpec != nil {
+		rpt, err = propagateWorkspaces(rpt)
+		if err != nil {
+			// This error cannot be recovered without modifying the TaskSpec
+			return nil, "", controller.NewPermanentError(err)
 		}
 	}
 
diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -7362,10 +7362,9 @@ spec:
 			},
 		},
 	}
-	ctx := config.EnableBetaAPIFields(context.Background())
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, _, err := getTaskrunWorkspaces(ctx, tt.pr, tt.rprt)
+			_, _, err := getTaskrunWorkspaces(context.Background(), tt.pr, tt.rprt)
 
 			if err != nil {
 				t.Errorf("Pipeline.getTaskrunWorkspaces() returned error for valid pipeline: %v", err)
diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go
@@ -385,10 +385,8 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1
 	var workspaceDeclarations []v1beta1.WorkspaceDeclaration
 	// Propagating workspaces allows users to skip declarations
 	// In order to validate the workspace bindings we create declarations based on
-	// the workspaces provided in the task run spec. This logic is hidden behind the
-	// alpha/beta feature gate since propagating workspaces is behind the beta feature gate.
-	// In addition, we only allow this feature for embedded taskSpec.
-	if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields && tr.Spec.TaskSpec != nil {
+	// the workspaces provided in the task run spec. We only allow this feature for embedded taskSpec.
+	if tr.Spec.TaskSpec != nil {
 		for _, ws := range tr.Spec.Workspaces {
 			wspaceDeclaration := v1beta1.WorkspaceDeclaration{Name: ws.Name}
 			workspaceDeclarations = append(workspaceDeclarations, wspaceDeclaration)
@@ -767,25 +765,23 @@ func applyParamsContextsResultsAndWorkspaces(ctx context.Context, tr *v1beta1.Ta
 	ts = resources.ApplyStepExitCodePath(ts)
 
 	// Apply workspace resource substitution
-	if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields {
-		// propagate workspaces from taskrun to task.
-		twn := []string{}
-		for _, tw := range ts.Workspaces {
-			twn = append(twn, tw.Name)
-		}
-
-		for _, trw := range tr.Spec.Workspaces {
-			skip := false
-			for _, tw := range twn {
-				if tw == trw.Name {
-					skip = true
-					break
-				}
-			}
-			if !skip {
-				ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: trw.Name})
+	// propagate workspaces from taskrun to task.
+	twn := []string{}
+	for _, tw := range ts.Workspaces {
+		twn = append(twn, tw.Name)
+	}
+
+	for _, trw := range tr.Spec.Workspaces {
+		skip := false
+		for _, tw := range twn {
+			if tw == trw.Name {
+				skip = true
+				break
 			}
 		}
+		if !skip {
+			ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: trw.Name})
+		}
 	}
 	ts = resources.ApplyWorkspaces(ctx, ts, ts.Workspaces, tr.Spec.Workspaces, workspaceVolumes)
 
diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
@@ -2532,31 +2532,23 @@ spec:
   - emptyDir: {}
     name: tr-workspace
 `)
-	for _, apiField := range []string{config.BetaAPIFields, config.AlphaAPIFields} {
-		d := test.Data{
-			TaskRuns: []*v1beta1.TaskRun{taskRun},
-		}
-		d.ConfigMaps = []*corev1.ConfigMap{{
-			ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()},
-			Data: map[string]string{
-				"enable-api-fields": apiField,
-			},
-		}}
-		testAssets, cancel := getTaskRunController(t, d)
-		defer cancel()
-		createServiceAccount(t, testAssets, "default", taskRun.Namespace)
-		c := testAssets.Controller
-		if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err == nil {
-			t.Fatalf("Could not reconcile the taskrun: %v", err)
-		}
-		getTaskRun, _ := testAssets.Clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(testAssets.Ctx, taskRun.Name, metav1.GetOptions{})
+	d := test.Data{
+		TaskRuns: []*v1beta1.TaskRun{taskRun},
+	}
+	testAssets, cancel := getTaskRunController(t, d)
+	defer cancel()
+	createServiceAccount(t, testAssets, "default", taskRun.Namespace)
+	c := testAssets.Controller
+	if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRun)); err == nil {
+		t.Fatalf("Could not reconcile the taskrun: %v", err)
+	}
+	getTaskRun, _ := testAssets.Clients.Pipeline.TektonV1beta1().TaskRuns(taskRun.Namespace).Get(testAssets.Ctx, taskRun.Name, metav1.GetOptions{})
 
-		want := []v1beta1.WorkspaceDeclaration{{
-			Name: "tr-workspace",
-		}}
-		if c := cmp.Diff(want, getTaskRun.Status.TaskSpec.Workspaces); c != "" {
-			t.Errorf("TestPropagatedWorkspaces errored with: %s", diff.PrintWantGot(c))
-		}
+	want := []v1beta1.WorkspaceDeclaration{{
+		Name: "tr-workspace",
+	}}
+	if c := cmp.Diff(want, getTaskRun.Status.TaskSpec.Workspaces); c != "" {
+		t.Errorf("TestPropagatedWorkspaces errored with: %s", diff.PrintWantGot(c))
 	}
 }
 
diff --git a/pkg/workspace/apply.go b/pkg/workspace/apply.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/names"
 	"github.com/tektoncd/pipeline/pkg/substitution"
@@ -110,35 +109,29 @@ func Apply(ctx context.Context, ts v1beta1.TaskSpec, wb []v1beta1.WorkspaceBindi
 
 	isolatedWorkspaces := sets.NewString()
 
-	alphaOrBetaEnabled := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields
-
-	if alphaOrBetaEnabled {
-		for _, step := range ts.Steps {
-			for _, workspaceUsage := range step.Workspaces {
-				isolatedWorkspaces.Insert(workspaceUsage.Name)
-			}
+	for _, step := range ts.Steps {
+		for _, workspaceUsage := range step.Workspaces {
+			isolatedWorkspaces.Insert(workspaceUsage.Name)
 		}
-		for _, sidecar := range ts.Sidecars {
-			for _, workspaceUsage := range sidecar.Workspaces {
-				isolatedWorkspaces.Insert(workspaceUsage.Name)
-			}
+	}
+	for _, sidecar := range ts.Sidecars {
+		for _, workspaceUsage := range sidecar.Workspaces {
+			isolatedWorkspaces.Insert(workspaceUsage.Name)
 		}
 	}
 
 	for i := range wb {
-		if alphaOrBetaEnabled {
-			// Propagate missing Workspaces
-			addWorkspace := true
-			for _, ws := range ts.Workspaces {
-				if ws.Name == wb[i].Name {
-					addWorkspace = false
-					break
-				}
-			}
-			if addWorkspace {
-				ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: wb[i].Name})
+		// Propagate missing Workspaces
+		addWorkspace := true
+		for _, ws := range ts.Workspaces {
+			if ws.Name == wb[i].Name {
+				addWorkspace = false
+				break
 			}
 		}
+		if addWorkspace {
+			ts.Workspaces = append(ts.Workspaces, v1beta1.WorkspaceDeclaration{Name: wb[i].Name})
+		}
 		w, err := getDeclaredWorkspace(wb[i].Name, ts.Workspaces)
 		if err != nil {
 			return nil, err
@@ -153,15 +146,10 @@ func Apply(ctx context.Context, ts v1beta1.TaskSpec, wb []v1beta1.WorkspaceBindi
 			ReadOnly:  w.ReadOnly,
 		}
 
-		if alphaOrBetaEnabled {
-			if isolatedWorkspaces.Has(w.Name) {
-				mountAsIsolatedWorkspace(ts, w.Name, volumeMount)
-			} else {
-				mountAsSharedWorkspace(ts, volumeMount)
-			}
+		if isolatedWorkspaces.Has(w.Name) {
+			mountAsIsolatedWorkspace(ts, w.Name, volumeMount)
 		} else {
-			// Prior to the alpha feature gate only Steps may receive workspaces.
-			ts.StepTemplate.VolumeMounts = append(ts.StepTemplate.VolumeMounts, volumeMount)
+			mountAsSharedWorkspace(ts, volumeMount)
 		}
 
 		// Only add this volume if it hasn't already been added
diff --git a/pkg/workspace/apply_test.go b/pkg/workspace/apply_test.go
@@ -706,7 +706,7 @@ func TestApply_PropagatedWorkspacesFromWorkspaceBindingToDeclarations(t *testing
 		expectedTaskSpec v1beta1.TaskSpec
 		apiField         string
 	}{{
-		name: "propagate workspaces alpha enabled",
+		name: "propagate workspaces",
 		ts: v1beta1.TaskSpec{
 			Workspaces: []v1beta1.WorkspaceDeclaration{{
 				Name: "workspace1",
@@ -740,52 +740,10 @@ func TestApply_PropagatedWorkspacesFromWorkspaceBindingToDeclarations(t *testing
 				VolumeMounts: []v1.VolumeMount{{Name: "ws-9l9zj", MountPath: "/workspace/workspace2"}},
 			},
 		},
-		apiField: config.AlphaAPIFields,
-	}, {
-		name: "propagate workspaces beta enabled",
-		ts: v1beta1.TaskSpec{
-			Workspaces: []v1beta1.WorkspaceDeclaration{{
-				Name: "workspace1",
-			}},
-		},
-		workspaces: []v1beta1.WorkspaceBinding{{
-			Name: "workspace2",
-			PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
-				ClaimName: "mypvc",
-			},
-		}},
-		expectedTaskSpec: v1beta1.TaskSpec{
-			Volumes: []corev1.Volume{{
-				Name: "ws-mz4c7",
-				VolumeSource: corev1.VolumeSource{
-					PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
-						ClaimName: "mypvc",
-					},
-				},
-			}},
-			Workspaces: []v1beta1.WorkspaceDeclaration{{
-				Name:      "workspace1",
-				MountPath: "",
-				ReadOnly:  false,
-			}, {
-				Name:      "workspace2",
-				MountPath: "",
-				ReadOnly:  false,
-			}},
-			StepTemplate: &v1beta1.StepTemplate{
-				VolumeMounts: []v1.VolumeMount{{Name: "ws-mz4c7", MountPath: "/workspace/workspace2"}},
-			},
-		},
-		apiField: config.BetaAPIFields,
 	}} {
 		t.Run(tc.name, func(t *testing.T) {
-			ctx := config.ToContext(context.Background(), &config.Config{
-				FeatureFlags: &config.FeatureFlags{
-					EnableAPIFields: tc.apiField,
-				},
-			})
 			vols := workspace.CreateVolumes(tc.workspaces)
-			ts, err := workspace.Apply(ctx, tc.ts, tc.workspaces, vols)
+			ts, err := workspace.Apply(context.Background(), tc.ts, tc.workspaces, vols)
 			if err != nil {
 				t.Fatalf("Did not expect error but got %v", err)
 			}
@@ -1135,8 +1093,8 @@ func TestApplyWithMissingWorkspaceDeclaration(t *testing.T) {
 		},
 	}}
 	vols := workspace.CreateVolumes(bindings)
-	if _, err := workspace.Apply(context.Background(), ts, bindings, vols); err == nil {
-		t.Errorf("Expected error because workspace doesnt exist.")
+	if _, err := workspace.Apply(context.Background(), ts, bindings, vols); err != nil {
+		t.Errorf("Did not expect error because of workspace propagation but got %v", err)
 	}
 }
 

Original file line number	Diff line number	Diff line change
`@@ -1033,14 +1033,12 @@ func getTaskrunWorkspaces(ctx context.Context, pr v1beta1.PipelineRun, rpt res`
`1033`	`1033`	`pipelineRunWorkspaces[binding.Name] = binding`
`1034`	`1034`	`}`
`1035`	`1035`
`1036`		`- if config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields != config.StableAPIFields {`
`1037`		`- // Propagate required workspaces from pipelineRun to the pipelineTasks`
`1038`		`- if rpt.PipelineTask.TaskSpec != nil {`
`1039`		`- rpt, err = propagateWorkspaces(rpt)`
`1040`		`- if err != nil {`
`1041`		`- // This error cannot be recovered without modifying the TaskSpec`
`1042`		`- return nil, "", controller.NewPermanentError(err)`
`1043`		`- }`
	`1036`	`+ // Propagate required workspaces from pipelineRun to the pipelineTasks`
	`1037`	`+ if rpt.PipelineTask.TaskSpec != nil {`
	`1038`	`+ rpt, err = propagateWorkspaces(rpt)`
	`1039`	`+ if err != nil {`
	`1040`	`+ // This error cannot be recovered without modifying the TaskSpec`
	`1041`	`+ return nil, "", controller.NewPermanentError(err)`
`1044`	`1042`	`}`
`1045`	`1043`	`}`
`1046`	`1044`