Remove deprecated PipelineRun.Spec.ServiceAccountNames field.

Finishing up tektoncd#2614 We had multiple ways to specify the `serviceAccountName` for `PipelineTask`s - the original `PipelineRun.Spec.ServiceAccountNames`, and the more general `PipelineRun.Spec.TaskRunSpecs`, which also allows specifying a pod template, metadata, and container overrides for individual steps and sidecars. Therefore, we deprecated `ServiceAccountNames`, and are now removing it. This has been scheduled for removal since May 2021. Signed-off-by: Andrew Bayer <andrew.bayer@gmail.com>
abayer · Jun 15, 2022 · 2a0ce9a · 2a0ce9a
1 parent fcbeb45
commit 2a0ce9a
Show file tree

Hide file tree

Showing 17 changed files with 103 additions and 537 deletions.
diff --git a/docs/deprecations.md b/docs/deprecations.md
@@ -19,7 +19,6 @@ being deprecated.
 
 | Feature Being Deprecated                                                                                                                                            | Deprecation Announcement                                             | [API Compatibility Policy](https://github.com/tektoncd/pipeline/tree/main/api_compatibility_policy.md) | Earliest Date or Release of Removal |
 |---------------------------------------------------------------------------------------------------------------------------------------------------------------------|----------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------|-------------------------------------|
-| [The `PipelineRun.Spec.ServiceAccountNames` field is deprecated and will be removed.](https://github.com/tektoncd/pipeline/issues/2614)                             | [v0.15.0](https://github.com/tektoncd/pipeline/releases/tag/v0.15.0) | Beta                                                                                                   | May 15 2021                         |
 | [`PipelineRunCancelled` is deprecated and will be removed](https://github.com/tektoncd/pipeline/issues/4611)                                                        | [v0.25.0](https://github.com/tektoncd/pipeline/releases/tag/v0.25.0) | Beta                                                                                                   | July 12 2022                       |
 | [`PipelineResources` are deprecated.](https://github.com/tektoncd/community/blob/main/teps/0074-deprecate-pipelineresources.md)                                     | [v0.30.0](https://github.com/tektoncd/pipeline/releases/tag/v0.30.0) | Alpha                                                                                                  | Dec 20 2021                         |
 | [The `PipelineRun.Status.TaskRuns` and `PipelineRun.Status.Runs` fields are deprecated and will be removed.](https://github.com/tektoncd/community/blob/main/teps/0100-embedded-taskruns-and-runs-status-in-pipelineruns.md) | v0.35.0                                             | Beta                                                                                                   | Jan 25, 2023                        |

diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md
@@ -71,8 +71,6 @@ A `PipelineRun` definition supports the following fields:
   - [`params`](#specifying-parameters) - Specifies the desired execution parameters for the `Pipeline`.
   - [`serviceAccountName`](#specifying-custom-serviceaccount-credentials) - Specifies a `ServiceAccount`
     object that supplies specific execution credentials for the `Pipeline`.
-  - [`serviceAccountNames`](#mapping-serviceaccount-credentials-to-tasks) - Maps specific `serviceAccountName` values
-    to `Tasks` in the `Pipeline`. This overrides the credentials set for the entire `Pipeline`.
   - [`status`](#cancelling-a-pipelinerun) - Specifies options for cancelling a `PipelineRun`. 
   - [`taskRunSpecs`](#specifying-taskrunspecs) - Specifies a list of `PipelineRunTaskSpec` which allows for setting `ServiceAccountName`, [`Pod` template](./podtemplates.md), and `Metadata` for each task. This overrides the `Pod` template set for the entire `Pipeline`.
   - [`timeout`](#configuring-a-failure-timeout) - Specifies the timeout before the `PipelineRun` fails. `timeout` is deprecated and will eventually be removed, so consider using `timeouts` instead.
@@ -645,7 +643,7 @@ Consult the documentation of the custom task that you are using to determine whe
 
 ### Mapping `ServiceAccount` credentials to `Tasks`
 
-If you require more granularity in specifying execution credentials, use the `serviceAccountNames` field to
+If you require more granularity in specifying execution credentials, use the `taskRunSpecs[].taskServiceAccountName` field to
 map a specific `serviceAccountName` value to a specific `Task` in the `Pipeline`. This overrides the global
 `serviceAccountName` you may have set for the `Pipeline` as described in the previous section.
 
@@ -654,9 +652,9 @@ For example, if you specify these mappings:
 ```yaml
 spec:
   serviceAccountName: sa-1
-  serviceAccountNames:
-    - taskName: build-task
-      serviceAccountName: sa-for-build
+  taskRunSpecs:
+    - pipelineTaskName: build-task
+      taskServiceAccountName: sa-for-build
 ```
 
 for this `Pipeline`:

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion.go
@@ -65,7 +65,6 @@ func (prs *PipelineRunSpec) ConvertTo(ctx context.Context, sink *v1beta1.Pipelin
 	sink.Resources = prs.Resources
 	sink.Params = prs.Params
 	sink.ServiceAccountName = prs.ServiceAccountName
-	sink.ServiceAccountNames = prs.ServiceAccountNames
 	sink.Status = prs.Status
 	sink.Timeout = prs.Timeout
 	sink.PodTemplate = prs.PodTemplate
@@ -107,7 +106,6 @@ func (prs *PipelineRunSpec) ConvertFrom(ctx context.Context, source *v1beta1.Pip
 	prs.Resources = source.Resources
 	prs.Params = source.Params
 	prs.ServiceAccountName = source.ServiceAccountName
-	prs.ServiceAccountNames = source.ServiceAccountNames
 	prs.Status = source.Status
 	prs.Timeout = source.Timeout
 	prs.PodTemplate = source.PodTemplate

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_conversion_test.go
@@ -61,11 +61,7 @@ func TestPipelineRunConversion(t *testing.T) {
 					Name: "pipeline",
 				},
 				ServiceAccountName: "sa",
-				ServiceAccountNames: []PipelineRunSpecServiceAccountName{{
-					TaskName:           "t1",
-					ServiceAccountName: "sa1",
-				}},
-				Timeout: &metav1.Duration{Duration: 1 * time.Minute},
+				Timeout:            &metav1.Duration{Duration: 1 * time.Minute},
 				PodTemplate: &PodTemplate{
 					NodeSelector: map[string]string{"foo": "bar"},
 				},
@@ -121,11 +117,7 @@ func TestPipelineRunConversion(t *testing.T) {
 					}},
 				},
 				ServiceAccountName: "sa",
-				ServiceAccountNames: []PipelineRunSpecServiceAccountName{{
-					TaskName:           "t1",
-					ServiceAccountName: "sa1",
-				}},
-				Timeout: &metav1.Duration{Duration: 1 * time.Minute},
+				Timeout:            &metav1.Duration{Duration: 1 * time.Minute},
 				PodTemplate: &PodTemplate{
 					NodeSelector: map[string]string{"foo": "bar"},
 				},

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
@@ -68,8 +68,6 @@ type PipelineRunSpec struct {
 	Params []Param `json:"params,omitempty"`
 	// +optional
 	ServiceAccountName string `json:"serviceAccountName,omitempty"`
-	// +optional
-	ServiceAccountNames []PipelineRunSpecServiceAccountName `json:"serviceAccountNames,omitempty"`
 	// Used for cancelling a pipelinerun (and maybe more later on)
 	// +optional
 	Status PipelineRunSpecStatus `json:"status,omitempty"`
@@ -115,10 +113,6 @@ type PipelineRunStatusFields = v1beta1.PipelineRunStatusFields
 // PipelineRunTaskRunStatus contains the name of the PipelineTask for this TaskRun and the TaskRun's Status
 type PipelineRunTaskRunStatus = v1beta1.PipelineRunTaskRunStatus
 
-// PipelineRunSpecServiceAccountName can be used to configure specific
-// ServiceAccountName for a concrete Task
-type PipelineRunSpecServiceAccountName = v1beta1.PipelineRunSpecServiceAccountName
-
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
 // PipelineRunList contains a list of PipelineRun
@@ -180,14 +174,8 @@ func (pr *PipelineRun) IsTimedOut(c clock.PassiveClock) bool {
 
 // GetServiceAccountName returns the service account name for a given
 // PipelineTask if configured, otherwise it returns the PipelineRun's serviceAccountName.
-func (pr *PipelineRun) GetServiceAccountName(pipelineTaskName string) string {
-	serviceAccountName := pr.Spec.ServiceAccountName
-	for _, sa := range pr.Spec.ServiceAccountNames {
-		if sa.TaskName == pipelineTaskName {
-			serviceAccountName = sa.ServiceAccountName
-		}
-	}
-	return serviceAccountName
+func (pr *PipelineRun) GetServiceAccountName() string {
+	return pr.Spec.ServiceAccountName
 }
 
 // HasVolumeClaimTemplate returns true if PipelineRun contains volumeClaimTemplates that is
@@ -211,7 +199,7 @@ type PipelineTaskRunSpec struct {
 // GetTaskRunSpecs returns the task specific spec for a given
 // PipelineTask if configured, otherwise it returns the PipelineRun's default.
 func (pr *PipelineRun) GetTaskRunSpecs(pipelineTaskName string) (string, *PodTemplate) {
-	serviceAccountName := pr.GetServiceAccountName(pipelineTaskName)
+	serviceAccountName := pr.GetServiceAccountName()
 	taskPodTemplate := pr.Spec.PodTemplate
 	for _, task := range pr.Spec.TaskRunSpecs {
 		if task.PipelineTaskName == pipelineTaskName {

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go
@@ -237,73 +237,6 @@ func TestPipelineRunHasTimedOut(t *testing.T) {
 	}
 }
 
-func TestPipelineRunGetServiceAccountName(t *testing.T) {
-	for _, tt := range []struct {
-		name    string
-		pr      *v1alpha1.PipelineRun
-		saNames map[string]string
-	}{
-		{
-			name: "default SA",
-			pr: &v1alpha1.PipelineRun{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "pr",
-				},
-				Spec: v1alpha1.PipelineRunSpec{
-					PipelineRef: &v1alpha1.PipelineRef{
-						Name: "prs",
-					},
-					ServiceAccountName: "defaultSA",
-					ServiceAccountNames: []v1alpha1.PipelineRunSpecServiceAccountName{{
-						TaskName:           "taskName",
-						ServiceAccountName: "taskSA",
-					}},
-				},
-			},
-			saNames: map[string]string{
-				"unknown":  "defaultSA",
-				"taskName": "taskSA",
-			},
-		},
-		{
-			name: "mixed default SA",
-			pr: &v1alpha1.PipelineRun{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "defaultSA",
-				},
-				Spec: v1alpha1.PipelineRunSpec{
-					PipelineRef: &v1alpha1.PipelineRef{
-						Name: "defaultSA",
-					},
-					ServiceAccountName: "defaultSA",
-					ServiceAccountNames: []v1alpha1.PipelineRunSpecServiceAccountName{
-						{
-							TaskName:           "task1",
-							ServiceAccountName: "task1SA",
-						},
-						{
-							TaskName:           "task2",
-							ServiceAccountName: "task2SA",
-						},
-					},
-				},
-			},
-			saNames: map[string]string{
-				"unknown": "defaultSA",
-				"task1":   "task1SA",
-				"task2":   "task2SA",
-			},
-		},
-	} {
-		for taskName, expected := range tt.saNames {
-			sa := tt.pr.GetServiceAccountName(taskName)
-			if expected != sa {
-				t.Errorf("%s: wrong service account: got: %v, want: %v", tt.name, sa, expected)
-			}
-		}
-	}
-}
-
 func TestPipelineRunGetPodSpecSABackcompatibility(t *testing.T) {
 	for _, tt := range []struct {
 		name        string
@@ -317,9 +250,6 @@ func TestPipelineRunGetPodSpecSABackcompatibility(t *testing.T) {
 				Spec: v1alpha1.PipelineRunSpec{
 					PipelineRef:        &v1alpha1.PipelineRef{Name: "prs"},
 					ServiceAccountName: "defaultSA",
-					ServiceAccountNames: []v1alpha1.PipelineRunSpecServiceAccountName{{
-						TaskName: "taskName", ServiceAccountName: "taskSA",
-					}},
 					TaskRunSpecs: []v1alpha1.PipelineTaskRunSpec{{
 						PipelineTaskName:       "taskName",
 						TaskServiceAccountName: "newTaskSA",

diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go