Deprecate PipelineRun.Spec.ServiceAccountNames 🦌

`PipelineRun.Spec.TaskRunSpecs` already allows to specify a `ServiceAccountName` for a specific task. This marks `PipelineRun.Spec.ServiceAccountNames` as deprecated and to be removed in about 9 months. This also make sure `PipelineRun.Spec.ServiceAccountNames` are not ignored. Signed-off-by: Vincent Demeester <vincent@sbr.pm>
tektoncd · Jul 29, 2020 · 2994728 · 2994728
1 parent e6c91d2
commit 2994728
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 8 deletions.
diff --git a/docs/deprecations.md b/docs/deprecations.md
@@ -18,9 +18,10 @@ being deprecated.
 
 ## Deprecation Table
 
-| Feature Being Deprecated | Deprecation Announcement | [API Compatibility Policy](https://github.com/tektoncd/pipeline/tree/master/api_compatibility_policy.md) | Earliest Date or Release of Removal |
-| ------------------------ | ------------------------ | -------------------------------------------------------------------------------------------------------- | ------------------------ |
-| [`tekton.dev/task` label on ClusterTasks](https://github.com/tektoncd/pipeline/issues/2533) | [v0.12.0](https://github.com/tektoncd/pipeline/releases/tag/v0.12.0) | Beta | January 30 2021 |
-| [Step `$HOME` env var defaults to `/tekton/home`](https://github.com/tektoncd/pipeline/issues/2013) | [v0.11.0-rc1](https://github.com/tektoncd/pipeline/releases/tag/v0.11.0-rc1) | Beta | December 4 2020 |
-| [Step `workingDir` defaults to `/workspace`](https://github.com/tektoncd/pipeline/issues/1836) | [v0.11.0-rc1](https://github.com/tektoncd/pipeline/releases/tag/v0.11.0-rc1) | Beta | December 4 2020 |
-| [The `TaskRun.Status.ResourceResults.ResourceRef` field is deprecated and will be removed.](https://github.com/tektoncd/pipeline/issues/2694) | [v0.14.0](https://github.com/tektoncd/pipeline/releases/tag/v0.13.0) | Beta | April 30 2021 |
+| Feature Being Deprecated                                                                                                                      | Deprecation Announcement                                                     | [API Compatibility Policy](https://github.com/tektoncd/pipeline/tree/master/api_compatibility_policy.md) | Earliest Date or Release of Removal |
+| ------------------------                                                                                                                      | ------------------------                                                     | -------------------------------------------------------------------------------------------------------- | ------------------------            |
+| [`tekton.dev/task` label on ClusterTasks](https://github.com/tektoncd/pipeline/issues/2533)                                                   | [v0.12.0](https://github.com/tektoncd/pipeline/releases/tag/v0.12.0)         | Beta                                                                                                     | January 30 2021                     |
+| [Step `$HOME` env var defaults to `/tekton/home`](https://github.com/tektoncd/pipeline/issues/2013)                                           | [v0.11.0-rc1](https://github.com/tektoncd/pipeline/releases/tag/v0.11.0-rc1) | Beta                                                                                                     | December 4 2020                     |
+| [Step `workingDir` defaults to `/workspace`](https://github.com/tektoncd/pipeline/issues/1836)                                                | [v0.11.0-rc1](https://github.com/tektoncd/pipeline/releases/tag/v0.11.0-rc1) | Beta                                                                                                     | December 4 2020                     |
+| [The `TaskRun.Status.ResourceResults.ResourceRef` field is deprecated and will be removed.](https://github.com/tektoncd/pipeline/issues/2694) | [v0.14.0](https://github.com/tektoncd/pipeline/releases/tag/v0.14.0)         | Beta                                                                                                     | April 30 2021                       |
+| [The `PipelineRun.Spec.ServiceAccountNames` field is deprecated and will be removed.](https://github.com/tektoncd/pipeline/issues/xxxx)       | [v0.15.0](https://github.com/tektoncd/pipeline/releases/tag/v0.15.0)         | Beta                                                                                                     | May 15 2021                         |
diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go
@@ -164,6 +164,8 @@ type PipelineRunSpec struct {
 	Params []Param `json:"params,omitempty"`
 	// +optional
 	ServiceAccountName string `json:"serviceAccountName,omitempty"`
+
+	// Deprecated: use taskRunSpecs.ServiceAccountName instead
 	// +optional
 	ServiceAccountNames []PipelineRunSpecServiceAccountName `json:"serviceAccountNames,omitempty"`
 	// Used for cancelling a pipelinerun (and maybe more later on)

diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -366,14 +366,23 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err
 		return controller.NewPermanentError(err)
 	}
 
-	// Ensure that the ServiceAccountNames defined correct.
+	// Ensure that the ServiceAccountNames defined are correct.
+	// This is "deprecated".
 	if err := resources.ValidateServiceaccountMapping(pipelineSpec, pr); err != nil {
 		pr.Status.MarkFailed(ReasonInvalidServiceAccountMapping,
 			"PipelineRun %s/%s doesn't define ServiceAccountNames correctly: %s",
 			pr.Namespace, pr.Name, err)
 		return controller.NewPermanentError(err)
 	}
 
+	// Ensure that the TaskRunSpecs defined are correct.
+	if err := resources.ValidateTaskRunSpecs(pipelineSpec, pr); err != nil {
+		pr.Status.MarkFailed(ReasonInvalidServiceAccountMapping,
+			"PipelineRun %s/%s doesn't define taskRunSpecs correctly: %s",
+			pr.Namespace, pr.Name, err)
+		return controller.NewPermanentError(err)
+	}
+
 	// Apply parameter substitution from the PipelineRun
 	pipelineSpec = resources.ApplyParameters(pipelineSpec, pr)
 	pipelineSpec = resources.ApplyContexts(pipelineSpec, pipelineMeta.Name, pr)

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
@@ -374,6 +374,21 @@ func ValidateWorkspaceBindings(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun)
 	return nil
 }
 
+// ValidateTaskRunSpecs that the TaskRunSpecs defined by a PipelineRun are correct.
+func ValidateServiceaccountMapping(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) error {
+	pipelineTasks := make(map[string]string)
+	for _, task := range p.Tasks {
+		pipelineTasks[task.Name] = task.Name
+	}
+
+	for _, name := range pr.Spec.TaskRunSpecs {
+		if _, ok := pipelineTasks[name.PipelineTaskName]; !ok {
+			return fmt.Errorf("PipelineRun's taskrunSpecs defined wrong taskName: %q, does not exist in Pipeline", name.TaskName)
+		}
+	}
+	return nil
+}
+
 // ValidateServiceaccountMapping validates that the ServiceAccountNames defined by a PipelineRun are not correct.
 func ValidateServiceaccountMapping(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) error {
 	pipelineTasks := make(map[string]string)
@@ -383,7 +398,7 @@ func ValidateServiceaccountMapping(p *v1beta1.PipelineSpec, pr *v1beta1.Pipeline
 
 	for _, name := range pr.Spec.ServiceAccountNames {
 		if _, ok := pipelineTasks[name.TaskName]; !ok {
-			return fmt.Errorf("PipelineRun's ServiceAccountNames defined wrong taskName: %q, not existed in Pipeline", name.TaskName)
+			return fmt.Errorf("PipelineRun's ServiceAccountNames defined wrong taskName: %q, does not exist in Pipeline", name.TaskName)
 		}
 	}
 	return nil