From ef8a6b2259724f0e29f2bfb87591564289fbf7b5 Mon Sep 17 00:00:00 2001 From: Michael Fraenkel Date: Fri, 6 Sep 2019 23:11:48 -0400 Subject: [PATCH] ServiceAccountName(s) replaces ServiceAccount(s) Following in the k8s footsteps, deprecate ServiceAccount and ServiceAccounts, replace them with ServiceAccountName and ServiceAccountNames respectively. ServiceAccountName and ServiceAccountNames will always take precedence over ServiceAccount and ServiceAccounts respectively. If ServiceAccountName is not set, the value provided by ServiceAccount will be used instead. ServiceAccountNames will always take precedence over ServiceAccounts. --- docs/auth.md | 14 +- docs/pipelineruns.md | 18 +- docs/taskruns.md | 16 +- .../pipeline/v1alpha1/pipelinerun_defaults.go | 4 +- .../v1alpha1/pipelinerun_defaults_test.go | 6 +- .../pipeline/v1alpha1/pipelinerun_types.go | 49 ++++- .../v1alpha1/pipelinerun_types_test.go | 54 ++++++ .../v1alpha1/pipelinerun_validation_test.go | 2 +- .../pipeline/v1alpha1/taskrun_defaults.go | 4 +- .../v1alpha1/taskrun_defaults_test.go | 6 +- pkg/apis/pipeline/v1alpha1/taskrun_types.go | 14 +- .../pipeline/v1alpha1/taskrun_types_test.go | 27 +++ .../v1alpha1/zz_generated.deepcopy.go | 35 +++- pkg/reconciler/pipelinerun/pipelinerun.go | 21 +-- .../pipelinerun/pipelinerun_test.go | 174 +++++++++++++++--- pkg/reconciler/reconciler_test.go | 2 +- .../taskrun/entrypoint/entrypoint.go | 2 +- .../taskrun/entrypoint/entrypoint_test.go | 68 ++++--- pkg/reconciler/taskrun/resources/pod.go | 4 +- pkg/reconciler/taskrun/resources/pod_test.go | 46 ++++- pkg/reconciler/taskrun/taskrun_test.go | 50 ++++- pkg/reconciler/timeout_handler_test.go | 4 +- test/builder/examples_test.go | 2 +- test/builder/pipeline.go | 30 ++- test/builder/pipeline_test.go | 20 +- test/builder/task.go | 12 +- test/builder/task_test.go | 8 +- test/entrypoint_test.go | 2 +- test/pipelinerun_test.go | 2 +- test/status_test.go | 4 +- 30 files changed, 545 insertions(+), 155 deletions(-) diff --git a/docs/auth.md b/docs/auth.md index 0c822007eec..30cc1d16e3d 100644 --- a/docs/auth.md +++ b/docs/auth.md @@ -73,7 +73,7 @@ kind: TaskRun metadata: name: build-push-task-run-2 spec: - serviceAccount: build-bot + serviceAccountName: build-bot taskRef: name: build-push ``` @@ -87,7 +87,7 @@ spec: name: demo-pipeline namespace: default spec: - serviceAccount: build-bot + serviceAccountName: build-bot pipelineRef: name: demo-pipeline ``` @@ -145,7 +145,7 @@ to authenticate when retrieving any `PipelineResources`. metadata: name: build-push-task-run-2 spec: - serviceAccount: build-bot + serviceAccountName: build-bot taskRef: name: build-push ``` @@ -159,7 +159,7 @@ to authenticate when retrieving any `PipelineResources`. name: demo-pipeline namespace: default spec: - serviceAccount: build-bot + serviceAccountName: build-bot pipelineRef: name: demo-pipeline ``` @@ -218,7 +218,7 @@ kind: TaskRun metadata: name: build-push-task-run-2 spec: - serviceAccount: build-bot + serviceAccountName: build-bot taskRef: name: build-push ``` @@ -232,7 +232,7 @@ spec: name: demo-pipeline namespace: default spec: - serviceAccount: build-bot + serviceAccountName: build-bot pipelineRef: name: demo-pipeline ``` @@ -283,7 +283,7 @@ addition to the one described above. metadata: name: build-with-basic-auth spec: - serviceAccount: build-bot + serviceAccountName: build-bot steps: ... ``` diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index bb8757b661b..72b88d445f9 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -38,11 +38,11 @@ following fields: - [`resources`](#resources) - Specifies which [`PipelineResources`](resources.md) to use for this `PipelineRun`. - - [`serviceAccount`](#service-account) - Specifies a `ServiceAccount` resource + - [`serviceAccountName`](#service-account) - Specifies a `ServiceAccount` resource object that enables your build to run with the defined authentication information. When a `ServiceAccount` isn't specified, the `default-service-account` specified in the configmap - config-defaults will be applied. - - [`serviceAccounts`](#service-accounts) - Specifies a list of `ServiceAccount` + - [`serviceAccountNames`](#service-accounts) - Specifies a list of `ServiceAccountName` and `PipelineTask` pairs that enable you to overwrite `ServiceAccount` for concrete `PipelineTask`. - [`timeout`] - Specifies timeout after which the `PipelineRun` will fail. If the value of `timeout` is empty, the default timeout will be applied. If the value is set to 0, @@ -159,8 +159,8 @@ spec: ### Service Account Specifies the `name` of a `ServiceAccount` resource object. Use the -`serviceAccount` field to run your `Pipeline` with the privileges of the -specified service account. If no `serviceAccount` field is specified, your +`serviceAccountName` field to run your `Pipeline` with the privileges of the +specified service account. If no `serviceAccountName` field is specified, your resulting `TaskRuns` run using the service account specified in the ConfigMap `configmap-defaults` which if absent will default to [`default` service account](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#use-the-default-service-account-to-access-the-api-server) @@ -172,16 +172,16 @@ For examples and more information about specifying service accounts, see the ### Service Accounts -Specifies the list of `ServiceAccount` and `PipelineTask` pairs. Specified +Specifies the list of `ServiceAccountName` and `PipelineTask` pairs. Specified `PipelineTask` will be run with configured `ServiceAccount`, -overwriting [`serviceAccount`](#service-account) configuration, for example: +overwriting [`serviceAccountName`](#service-account) configuration, for example: ```yaml spec: - serviceAccount: sa-1 - serviceAccounts: + serviceAccountName: sa-1 + serviceAccountNames: - taskName: build-task - serviceAccount: sa-for-build + serviceAccountName: sa-for-build ``` If used with this `Pipeline`, `test-task` will use the `ServiceAccount` `sa-1`, while `build-task` will use `sa-for-build`. diff --git a/docs/taskruns.md b/docs/taskruns.md index afaa2b5413b..8027539224a 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -44,7 +44,7 @@ following fields: the [`Task`](tasks.md) you want to run - Optional: - - [`serviceAccount`](#service-account) - Specifies a `ServiceAccount` resource + - [`serviceAccountName`](#service-account) - Specifies a `ServiceAccount` resource object that enables your build to run with the defined authentication information. When a `ServiceAccount` isn't specified, the `default-service-account` specified in the configmap - config-defaults will be applied. @@ -156,9 +156,9 @@ default, if `default-timeout-minutes` is set to 0. ### Service Account Specifies the `name` of a `ServiceAccount` resource object. Use the -`serviceAccount` field to run your `Task` with the privileges of the specified -service account. If no `serviceAccount` field is specified, your `Task` runs -using the service account specified in the ConfigMap `configmap-defaults` +`serviceAccountName` field to run your `Task` with the privileges of the specified +service account. If no `serviceAccountName` field is specified, your `Task` runs +using the service account specified in the ConfigMap `configmap-defaults` which if absent will default to [`default` service account](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#use-the-default-service-account-to-access-the-api-server) that is in the [namespace](https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/) @@ -520,7 +520,7 @@ kind: TaskRun metadata: name: test-task-with-serviceaccount-git-ssh spec: - serviceAccount: test-task-robot-git-ssh + serviceAccountName: test-task-robot-git-ssh inputs: resources: - name: workspace @@ -532,7 +532,7 @@ spec: args: ["-c", "cat README.md"] ``` -Where `serviceAccount: test-build-robot-git-ssh` references the following +Where `serviceAccountName: test-build-robot-git-ssh` references the following `ServiceAccount`: ```yaml @@ -564,8 +564,8 @@ data: ``` Specifies the `name` of a `ServiceAccount` resource object. Use the -`serviceAccount` field to run your `Task` with the privileges of the specified -service account. If no `serviceAccount` field is specified, your `Task` runs +`serviceAccountName` field to run your `Task` with the privileges of the specified +service account. If no `serviceAccountName` field is specified, your `Task` runs using the [`default` service account](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#use-the-default-service-account-to-access-the-api-server) that is in the diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go index cafb2f33c54..2467461afd8 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go @@ -44,7 +44,7 @@ func (prs *PipelineRunSpec) SetDefaults(ctx context.Context) { } defaultSA := cfg.Defaults.DefaultServiceAccount - if prs.ServiceAccount == "" && defaultSA != "" { - prs.ServiceAccount = defaultSA + if prs.ServiceAccountName == "" && defaultSA != "" { + prs.ServiceAccountName = defaultSA } } diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go index fcc4bae3a27..874c1d20127 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go @@ -132,9 +132,9 @@ func TestPipelineRunDefaulting(t *testing.T) { }, want: &v1alpha1.PipelineRun{ Spec: v1alpha1.PipelineRunSpec{ - PipelineRef: v1alpha1.PipelineRef{Name: "foo"}, - Timeout: &metav1.Duration{Duration: 5 * time.Minute}, - ServiceAccount: "tekton", + PipelineRef: v1alpha1.PipelineRef{Name: "foo"}, + Timeout: &metav1.Duration{Duration: 5 * time.Minute}, + ServiceAccountName: "tekton", }, }, wc: func(ctx context.Context) context.Context { diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index 9615b79e0b9..898b9f2197e 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go @@ -55,9 +55,15 @@ type PipelineRunSpec struct { // Params is a list of parameter names and values. Params []Param `json:"params,omitempty"` // +optional - ServiceAccount string `json:"serviceAccount"` + ServiceAccountName string `json:"serviceAccountName,omitempty"` + // DeprecatedServiceAccount is a depreciated alias for ServiceAccountName. + // Deprecated: Use serviceAccountName instead. // +optional - ServiceAccounts []PipelineRunSpecServiceAccount `json:"serviceAccounts,omitempty"` + DeprecatedServiceAccount string `json:"serviceAccount,omitempty"` + // +optional + DeprecatedServiceAccounts []DeprecatedPipelineRunSpecServiceAccount `json:"serviceAccounts,omitempty"` + // +optional + ServiceAccountNames []PipelineRunSpecServiceAccountName `json:"serviceAccountNames,omitempty"` // Deprecation Notice: The field Results will be removed in v0.8.0 // and should not be used. Plan to have this field removed before upgradring // to v0.8.0. @@ -165,10 +171,21 @@ func (pr *PipelineRunStatus) InitializeConditions() { pipelineRunCondSet.Manage(pr).InitializeConditions() } -// PipelineRunSpecServiceAccount can be used to configure specific ServiceAccount for a concrete Task -type PipelineRunSpecServiceAccount struct { - TaskName string `json:"taskName,omitempty"` - ServiceAccount string `json:"serviceAccount,omitempty"` +// DeprecatedPipelineRunSpecServiceAccount can be used to configure specific +// ServiceAccount for a concrete Task +// Deprecated: Use pipelineRunSpecServiceAccountName instead. +type DeprecatedPipelineRunSpecServiceAccount struct { + TaskName string `json:"taskName,omitempty"` + // DeprecatedServiceAccount is a depreciated alias for ServiceAccountName. + // Deprecated: Use serviceAccountName instead. + DeprecatedServiceAccount string `json:"serviceAccount,omitempty"` +} + +// PipelineRunSpecServiceAccountName can be used to configure specific +// ServiceAccountName for a concrete Task +type PipelineRunSpecServiceAccountName struct { + TaskName string `json:"taskName,omitempty"` + ServiceAccountName string `json:"serviceAccountName,omitempty"` } // SetCondition sets the condition, unsetting previous conditions with the same @@ -272,3 +289,23 @@ func (pr *PipelineRun) IsTimedOut() bool { } return false } + +// 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 + if serviceAccountName == "" { + serviceAccountName = pr.Spec.DeprecatedServiceAccount + } + for _, sa := range pr.Spec.DeprecatedServiceAccounts { + if sa.TaskName == pipelineTaskName { + serviceAccountName = sa.DeprecatedServiceAccount + } + } + for _, sa := range pr.Spec.ServiceAccountNames { + if sa.TaskName == pipelineTaskName { + serviceAccountName = sa.ServiceAccountName + } + } + return serviceAccountName +} diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go index 531c6f8166c..737b33a1e21 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types_test.go @@ -217,3 +217,57 @@ func TestPipelineRunHasTimedOut(t *testing.T) { }) } } + +func TestPipelineRunGetServiceAccountName(t *testing.T) { + for _, tt := range []struct { + name string + pr *v1alpha1.PipelineRun + saNames map[string]string + }{ + { + "default SA", + tb.PipelineRun("pr", "ns", + tb.PipelineRunSpec("prs", + tb.PipelineRunServiceAccountName("defaultSA"), + tb.PipelineRunServiceAccountNameTask("taskName", "taskSA"))), + map[string]string{ + "unknown": "defaultSA", + "taskName": "taskSA", + }, + }, + { + "deprecated default SA", + tb.PipelineRun("pr", "ns", + tb.PipelineRunSpec("prs", + tb.PipelineRunDeprecatedServiceAccountName("", "deprecatedSA"), + tb.PipelineRunDeprecatedServiceAccountTask("taskName", "deprecatedTaskSA"))), + map[string]string{ + "unknown": "deprecatedSA", + "taskName": "deprecatedTaskSA", + }, + }, + { + "mixed default SA", + tb.PipelineRun("defaultSA", "defaultSA", + tb.PipelineRunSpec("defaultSA", + tb.PipelineRunDeprecatedServiceAccountName("defaultSA", "deprecatedSA"), + tb.PipelineRunServiceAccountNameTask("task1", "task1SA"), + tb.PipelineRunServiceAccountNameTask("task2", "task2SA"), + tb.PipelineRunDeprecatedServiceAccountTask("deprecatedTask3", "deprecatedTask3SA"), + tb.PipelineRunDeprecatedServiceAccountTask("task2", "deprecated"))), + map[string]string{ + "unknown": "defaultSA", + "task1": "task1SA", + "task2": "task2SA", + "deprecatedTask3": "deprecatedTask3SA", + }, + }, + } { + 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) + } + } + } +} diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_validation_test.go index 9f3dca2aaf8..027f838bb73 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_validation_test.go @@ -60,7 +60,7 @@ func TestPipelineRun_Invalidate(t *testing.T) { Name: "pipelinelineName", }, Spec: v1alpha1.PipelineRunSpec{ - ServiceAccount: "foo", + ServiceAccountName: "foo", }, }, want: apis.ErrMissingField("spec.pipelineRef.name, spec.pipelineSpec"), diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go b/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go index f02d394a134..c9384b1d231 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go @@ -48,7 +48,7 @@ func (trs *TaskRunSpec) SetDefaults(ctx context.Context) { } defaultSA := cfg.Defaults.DefaultServiceAccount - if trs.ServiceAccount == "" && defaultSA != "" { - trs.ServiceAccount = defaultSA + if trs.ServiceAccountName == "" && defaultSA != "" { + trs.ServiceAccountName = defaultSA } } diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go index 25a4b59d834..b54753c2db0 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go @@ -155,9 +155,9 @@ func TestTaskRunDefaulting(t *testing.T) { }, want: &v1alpha1.TaskRun{ Spec: v1alpha1.TaskRunSpec{ - TaskRef: &v1alpha1.TaskRef{Name: "foo", Kind: v1alpha1.NamespacedTaskKind}, - Timeout: &metav1.Duration{Duration: 5 * time.Minute}, - ServiceAccount: "tekton", + TaskRef: &v1alpha1.TaskRef{Name: "foo", Kind: v1alpha1.NamespacedTaskKind}, + Timeout: &metav1.Duration{Duration: 5 * time.Minute}, + ServiceAccountName: "tekton", }, }, wc: func(ctx context.Context) context.Context { diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go index 0c5e542c874..74f46da130d 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_types.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_types.go @@ -42,7 +42,11 @@ type TaskRunSpec struct { // +optional Results *Results `json:"results,omitempty"` // +optional - ServiceAccount string `json:"serviceAccount,omitempty"` + ServiceAccountName string `json:"serviceAccountName"` + // DeprecatedServiceAccount is a depreciated alias for ServiceAccountName. + // Deprecated: Use serviceAccountName instead. + // +optional + DeprecatedServiceAccount string `json:"serviceAccount,omitempty"` // no more than one of the TaskRef and TaskSpec may be specified. // +optional TaskRef *TaskRef `json:"taskRef,omitempty"` @@ -290,3 +294,11 @@ func (tr *TaskRun) GetRunKey() string { // The address of the pointer is a threadsafe unique identifier for the taskrun return fmt.Sprintf("%s/%p", "TaskRun", tr) } + +func (tr *TaskRun) GetServiceAccountName() string { + name := tr.Spec.ServiceAccountName + if name == "" { + name = tr.Spec.DeprecatedServiceAccount + } + return name +} diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_types_test.go index fa535144743..0ff772a7800 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_types_test.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_types_test.go @@ -152,3 +152,30 @@ func TestTaskRunHasStarted(t *testing.T) { }) } } + +func TestTaskRunGetServiceAccountName(t *testing.T) { + for _, tt := range []struct { + name string + tr *v1alpha1.TaskRun + expectedSA string + }{{ + "service account", + tb.TaskRun("name", "ns", tb.TaskRunSpec(tb.TaskRunServiceAccountName("defaultSA"))), + "defaultSA", + }, + { + "deprecated SA", + tb.TaskRun("name", "ns", tb.TaskRunSpec(tb.TaskRunDeprecatedServiceAccount("", "deprecatedSA"))), + "deprecatedSA", + }, + { + "both SA", + tb.TaskRun("name", "ns", tb.TaskRunSpec(tb.TaskRunDeprecatedServiceAccount("defaultSA", "deprecatedSA"))), + "defaultSA", + }, + } { + if e, a := tt.expectedSA, tt.tr.GetServiceAccountName(); e != a { + t.Errorf("%s: wrong service account name: got: %q want: %q", tt.name, a, e) + } + } +} diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 3da48c9eda2..c8e8360badf 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -410,6 +410,22 @@ func (in *DAG) DeepCopy() *DAG { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DeprecatedPipelineRunSpecServiceAccount) DeepCopyInto(out *DeprecatedPipelineRunSpecServiceAccount) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DeprecatedPipelineRunSpecServiceAccount. +func (in *DeprecatedPipelineRunSpecServiceAccount) DeepCopy() *DeprecatedPipelineRunSpecServiceAccount { + if in == nil { + return nil + } + out := new(DeprecatedPipelineRunSpecServiceAccount) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GCSResource) DeepCopyInto(out *GCSResource) { *out = *in @@ -1002,9 +1018,14 @@ func (in *PipelineRunSpec) DeepCopyInto(out *PipelineRunSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.ServiceAccounts != nil { - in, out := &in.ServiceAccounts, &out.ServiceAccounts - *out = make([]PipelineRunSpecServiceAccount, len(*in)) + if in.DeprecatedServiceAccounts != nil { + in, out := &in.DeprecatedServiceAccounts, &out.DeprecatedServiceAccounts + *out = make([]DeprecatedPipelineRunSpecServiceAccount, len(*in)) + copy(*out, *in) + } + if in.ServiceAccountNames != nil { + in, out := &in.ServiceAccountNames, &out.ServiceAccountNames + *out = make([]PipelineRunSpecServiceAccountName, len(*in)) copy(*out, *in) } if in.Results != nil { @@ -1032,17 +1053,17 @@ func (in *PipelineRunSpec) DeepCopy() *PipelineRunSpec { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PipelineRunSpecServiceAccount) DeepCopyInto(out *PipelineRunSpecServiceAccount) { +func (in *PipelineRunSpecServiceAccountName) DeepCopyInto(out *PipelineRunSpecServiceAccountName) { *out = *in return } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineRunSpecServiceAccount. -func (in *PipelineRunSpecServiceAccount) DeepCopy() *PipelineRunSpecServiceAccount { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineRunSpecServiceAccountName. +func (in *PipelineRunSpecServiceAccountName) DeepCopy() *PipelineRunSpecServiceAccountName { if in == nil { return nil } - out := new(PipelineRunSpecServiceAccount) + out := new(PipelineRunSpecServiceAccountName) in.DeepCopyInto(out) return out } diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index ebdda7153bf..57979f9c877 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -524,9 +524,9 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr * Inputs: v1alpha1.TaskRunInputs{ Params: rprt.PipelineTask.Params, }, - ServiceAccount: getServiceAccount(pr, rprt.PipelineTask.Name), - Timeout: getTaskRunTimeout(pr), - PodTemplate: pr.Spec.PodTemplate, + ServiceAccountName: pr.GetServiceAccountName(rprt.PipelineTask.Name), + Timeout: getTaskRunTimeout(pr), + PodTemplate: pr.Spec.PodTemplate, }} resources.WrapSteps(&tr.Spec, rprt.PipelineTask, rprt.ResolvedTaskResources.Inputs, rprt.ResolvedTaskResources.Outputs, storageBasePath) @@ -596,17 +596,6 @@ func getTaskRunTimeout(pr *v1alpha1.PipelineRun) *metav1.Duration { return taskRunTimeout } -func getServiceAccount(pr *v1alpha1.PipelineRun, pipelineTaskName string) string { - // If service account is configured for a given PipelineTask, override PipelineRun's serviceAccount - serviceAccount := pr.Spec.ServiceAccount - for _, sa := range pr.Spec.ServiceAccounts { - if sa.TaskName == pipelineTaskName { - serviceAccount = sa.ServiceAccount - } - } - return serviceAccount -} - func (c *Reconciler) updateStatus(pr *v1alpha1.PipelineRun) (*v1alpha1.PipelineRun, error) { newPr, err := c.pipelineRunLister.PipelineRuns(pr.Namespace).Get(pr.Name) if err != nil { @@ -656,8 +645,8 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin Annotations: getTaskrunAnnotations(pr), // Propagate annotations from PipelineRun to TaskRun. }, Spec: v1alpha1.TaskRunSpec{ - TaskSpec: taskSpec, - ServiceAccount: getServiceAccount(pr, rprt.PipelineTask.Name), + TaskSpec: taskSpec, + ServiceAccountName: pr.GetServiceAccountName(rprt.PipelineTask.Name), Inputs: v1alpha1.TaskRunInputs{ Params: rcc.PipelineTaskCondition.Params, Resources: rcc.ToTaskResourceBindings(), diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 2981ea40fb7..b8abb1d9033 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -87,7 +87,7 @@ func TestReconcile(t *testing.T) { prs := []*v1alpha1.PipelineRun{ tb.PipelineRun("test-pipeline-run-success", "foo", tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), tb.PipelineRunResourceBinding("git-repo", tb.PipelineResourceBindingRef("some-repo")), tb.PipelineRunResourceBinding("best-image", tb.PipelineResourceBindingResourceSpec( &v1alpha1.PipelineResourceSpec{ @@ -225,7 +225,7 @@ func TestReconcile(t *testing.T) { tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineTaskLabelKey, "unit-test-1"), tb.TaskRunSpec( tb.TaskRunTaskRef("unit-test-task"), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), tb.TaskRunInputs( tb.TaskRunInputsParam("foo", "somethingfun"), tb.TaskRunInputsParam("bar", "somethingmorefun"), @@ -452,7 +452,7 @@ func TestUpdateTaskRunsState(t *testing.T) { )) taskrun := tb.TaskRun("test-pipeline-run-success-unit-test-1", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef("unit-test-task"), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), ), tb.TaskRunStatus( tb.StatusCondition(apis.Condition{Type: apis.ConditionSucceeded}), tb.StepState(tb.StateTerminated(0)), @@ -626,7 +626,7 @@ func TestUpdateTaskRunStateWithConditionChecks(t *testing.T) { func TestReconcileOnCompletedPipelineRun(t *testing.T) { taskRunName := "test-pipeline-run-completed-hello-world" prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-completed", "foo", - tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccount("test-sa")), + tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccountName("test-sa")), tb.PipelineRunStatus(tb.PipelineRunStatusCondition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, @@ -717,7 +717,7 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) { func TestReconcileOnCancelledPipelineRun(t *testing.T) { prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-cancelled", "foo", - tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccountName("test-sa"), tb.PipelineRunCancelled, ), )} @@ -731,7 +731,7 @@ func TestReconcileOnCancelledPipelineRun(t *testing.T) { tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineLabelKey, "test-pipeline-run-cancelled"), tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineRunLabelKey, "test-pipeline"), tb.TaskRunSpec(tb.TaskRunTaskRef("hello-world"), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), ), ), } @@ -775,7 +775,7 @@ func TestReconcileWithTimeout(t *testing.T) { ))} prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-timeout", "foo", tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), tb.PipelineRunTimeout(12*time.Hour), ), tb.PipelineRunStatus( @@ -896,7 +896,7 @@ func TestReconcilePropagateLabels(t *testing.T) { tb.TaskRunLabel("PipelineRunLabel", "PipelineRunValue"), tb.TaskRunSpec( tb.TaskRunTaskRef("hello-world"), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), ), ), }, { @@ -911,7 +911,7 @@ func TestReconcilePropagateLabels(t *testing.T) { tb.TaskRunLabel("PipelineRunLabel", "PipelineRunValue"), tb.TaskRunSpec( tb.TaskRunTaskRef("hello-world"), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), ), ), }, @@ -926,7 +926,7 @@ func TestReconcilePropagateLabels(t *testing.T) { tb.PipelineRunLabel("PipelineRunLabel", "PipelineRunValue"), tb.PipelineRunLabel("tekton.dev/pipeline", "WillNotBeUsed"), tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), ), )} ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")} @@ -975,8 +975,8 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) { ))} prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-different-service-accs", "foo", tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa-0"), - tb.PipelineRunServiceAccountTask("hello-world-1", "test-sa-1"), + tb.PipelineRunServiceAccountName("test-sa-0"), + tb.PipelineRunServiceAccountNameTask("hello-world-1", "test-sa-1"), ), )} ts := []*v1alpha1.Task{ @@ -1015,7 +1015,7 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) { ), tb.TaskRunSpec( tb.TaskRunTaskRef("hello-world-task"), - tb.TaskRunServiceAccount("test-sa-0"), + tb.TaskRunServiceAccountName("test-sa-0"), ), tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), @@ -1028,7 +1028,7 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) { ), tb.TaskRunSpec( tb.TaskRunTaskRef("hello-world-task"), - tb.TaskRunServiceAccount("test-sa-1"), + tb.TaskRunServiceAccountName("test-sa-1"), ), tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), @@ -1049,6 +1049,138 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) { } +func TestReconcileWithDeprecatedServiceAccounts(t *testing.T) { + names.TestingSeed() + + ps := []*v1alpha1.Pipeline{ + tb.Pipeline("test-sa-0", "foo", tb.PipelineSpec(tb.PipelineTask("deprecated-sa-0", "hello-world-task"))), + tb.Pipeline("test-sa-1", "foo", tb.PipelineSpec(tb.PipelineTask("sa-1", "hello-world-task"))), + tb.Pipeline("test-sa-2", "foo", tb.PipelineSpec(tb.PipelineTask("task-deprecated-sa-2", "hello-world-task"))), + tb.Pipeline("test-sa-3", "foo", tb.PipelineSpec(tb.PipelineTask("task-sa-3", "hello-world-task"))), + } + prs := []*v1alpha1.PipelineRun{ + tb.PipelineRun("test-pipeline-run-deprecated-sa-0", "foo", + tb.PipelineRunSpec("test-sa-0", + tb.PipelineRunDeprecatedServiceAccountName("", "sa-0"), + ), + ), + tb.PipelineRun("test-pipeline-run-sa-1", "foo", + tb.PipelineRunSpec("test-sa-1", + tb.PipelineRunDeprecatedServiceAccountName("sa-1", "sa-0"), + ), + ), + tb.PipelineRun("test-pipeline-run-task-deprecated-sa-2", "foo", + tb.PipelineRunSpec("test-sa-2", + tb.PipelineRunServiceAccountName("wrong-sa"), + tb.PipelineRunDeprecatedServiceAccountTask("task-deprecated-sa-2", "sa-2"), + ), + ), + tb.PipelineRun("test-pipeline-run-task-sa-3", "foo", + tb.PipelineRunSpec("test-sa-3", + tb.PipelineRunServiceAccountName("wrong-sa"), + tb.PipelineRunServiceAccountNameTask("task-sa-3", "sa-3"), + tb.PipelineRunDeprecatedServiceAccountTask("task-sa-3", "wrong-deprecated-sa"), + ), + ), + } + + ts := []*v1alpha1.Task{ + tb.Task("hello-world-task", "foo"), + } + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + + testAssets, cancel := getPipelineRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + + for _, pr := range prs { + err := c.Reconciler.Reconcile(context.Background(), fmt.Sprintf("%s/%s", pr.GetNamespace(), pr.GetName())) + if err != nil { + t.Errorf("Did not expect to see error when reconciling completed PipelineRun but saw %s", err) + } + + // Check that the PipelineRun was reconciled correctly + _, err = clients.Pipeline.Tekton().PipelineRuns(pr.GetNamespace()).Get(pr.GetName(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err) + } + } + + expectedTaskRuns := []*v1alpha1.TaskRun{ + tb.TaskRun("test-pipeline-run-deprecated-sa-0-deprecated-sa-0-9l9zj", "foo", + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-deprecated-sa-0", + tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world-task"), + tb.TaskRunServiceAccountName("sa-0"), + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-sa-0"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-deprecated-sa-0"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "deprecated-sa-0"), + ), + tb.TaskRun("test-pipeline-run-sa-1-sa-1-mz4c7", "foo", + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-sa-1", + tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world-task"), + tb.TaskRunServiceAccountName("sa-1"), + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-sa-1"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-sa-1"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "sa-1"), + ), + tb.TaskRun("test-pipeline-run-task-deprecated-sa-2-task-deprecated-sa-mssqb", "foo", + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-task-deprecated-sa-2", + tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world-task"), + tb.TaskRunServiceAccountName("sa-2"), + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-sa-2"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-task-deprecated-sa-2"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "task-deprecated-sa-2"), + ), + tb.TaskRun("test-pipeline-run-task-sa-3-task-sa-3-78c5n", "foo", + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-task-sa-3", + tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world-task"), + tb.TaskRunServiceAccountName("sa-3"), + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-sa-3"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-task-sa-3"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "task-sa-3"), + ), + } + + for _, taskRun := range expectedTaskRuns { + // Check that the expected TaskRun was created + actual, err := clients.Pipeline.Tekton().TaskRuns(taskRun.GetNamespace()).Get(taskRun.GetName(), metav1.GetOptions{}) + if err != nil { + t.Fatalf("Expected a TaskRun to be created, but it wasn't: %s", err) + } + if d := cmp.Diff(actual, taskRun); d != "" { + t.Errorf("expected to see TaskRun %v created. Diff %s", taskRun, d) + } + + } + +} + func TestReconcileWithTimeoutAndRetry(t *testing.T) { tcs := []struct { @@ -1076,7 +1208,7 @@ func TestReconcileWithTimeoutAndRetry(t *testing.T) { ))} prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-retry-run-with-timeout", "foo", tb.PipelineRunSpec("test-pipeline-retry", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), tb.PipelineRunTimeout(12*time.Hour), ), tb.PipelineRunStatus( @@ -1156,7 +1288,7 @@ func TestReconcilePropagateAnnotations(t *testing.T) { prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-annotations", "foo", tb.PipelineRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), ), )} ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")} @@ -1199,7 +1331,7 @@ func TestReconcilePropagateAnnotations(t *testing.T) { tb.TaskRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), tb.TaskRunSpec( tb.TaskRunTaskRef("hello-world"), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), ), ) @@ -1274,7 +1406,7 @@ func TestReconcileWithConditionChecks(t *testing.T) { prs := []*v1alpha1.PipelineRun{tb.PipelineRun(prName, "foo", tb.PipelineRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), ), )} ts := []*v1alpha1.Task{tb.Task("hello-world", "foo")} @@ -1344,7 +1476,7 @@ func TestReconcileWithFailingConditionChecks(t *testing.T) { prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-conditions", "foo", tb.PipelineRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), ), tb.PipelineRunStatus(tb.PipelineRunStatusCondition(apis.Condition{ Type: apis.ConditionSucceeded, @@ -1427,7 +1559,7 @@ func TestReconcileWithFailingConditionChecks(t *testing.T) { tb.TaskRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), tb.TaskRunSpec( tb.TaskRunTaskRef("hello-world"), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), ), ) @@ -1452,7 +1584,7 @@ func makeExpectedTr(condName, ccName string) *v1alpha1.TaskRun { tb.Step("condition-check-"+condName, "foo", tb.StepArgs("bar")), tb.TaskInputs(), ), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), ), ) } diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 5f3303d6a91..e75c51dfb5e 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -39,7 +39,7 @@ import ( func TestRecorderOptions(t *testing.T) { prs := []*v1alpha1.PipelineRun{tb.PipelineRun("test-pipeline-run-completed", "foo", - tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccount("test-sa")), + tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccountName("test-sa")), tb.PipelineRunStatus(tb.PipelineRunStatusCondition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, diff --git a/pkg/reconciler/taskrun/entrypoint/entrypoint.go b/pkg/reconciler/taskrun/entrypoint/entrypoint.go index e6235770c1c..7a3ae8a7c5d 100644 --- a/pkg/reconciler/taskrun/entrypoint/entrypoint.go +++ b/pkg/reconciler/taskrun/entrypoint/entrypoint.go @@ -233,7 +233,7 @@ func getRemoteImage(image string, kubeclient kubernetes.Interface, taskRun *v1al kc, err := k8schain.New(kubeclient, k8schain.Options{ Namespace: taskRun.Namespace, - ServiceAccountName: taskRun.Spec.ServiceAccount, + ServiceAccountName: taskRun.GetServiceAccountName(), }) if err != nil { return nil, xerrors.Errorf("Failed to create k8schain: %w", err) diff --git a/pkg/reconciler/taskrun/entrypoint/entrypoint_test.go b/pkg/reconciler/taskrun/entrypoint/entrypoint_test.go index 15ded68c0f9..d9b3450bdc2 100644 --- a/pkg/reconciler/taskrun/entrypoint/entrypoint_test.go +++ b/pkg/reconciler/taskrun/entrypoint/entrypoint_test.go @@ -266,7 +266,7 @@ func TestGetRemoteEntrypoint(t *testing.T) { Name: "taskRun", }, Spec: v1alpha1.TaskRunSpec{ - ServiceAccount: "default", + ServiceAccountName: "default", TaskSpec: &v1alpha1.TaskSpec{ Steps: []v1alpha1.Step{{Container: corev1.Container{ Image: "ubuntu", @@ -314,7 +314,7 @@ func TestGetRemoteEntrypointStale(t *testing.T) { Name: "taskRun", }, Spec: v1alpha1.TaskRunSpec{ - ServiceAccount: "default", + ServiceAccountName: "default", }, } c := fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{ @@ -391,38 +391,50 @@ func TestGetRemoteEntrypointWithNonDefaultSA(t *testing.T) { image := path.Join(strings.TrimPrefix(server.URL, "http://"), expectedRepo) finalDigest := image + "@" + digetsSha - entrypointCache, err := NewCache() - if err != nil { - t.Fatalf("couldn't create new entrypoint cache: %v", err) - } - taskRun := &v1alpha1.TaskRun{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "foo", - Name: "taskRun", - }, - Spec: v1alpha1.TaskRunSpec{ - ServiceAccount: "some-other-sa", - TaskSpec: &v1alpha1.TaskSpec{ - Steps: []v1alpha1.Step{{Container: corev1.Container{ - Image: "ubuntu", - Command: []string{"echo"}, - Args: []string{"hello"}, - }}}, - }, - }, - } c := fakekubeclientset.NewSimpleClientset(&corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: "some-other-sa", Namespace: "foo", }, }) - ep, err := GetRemoteEntrypoint(entrypointCache, finalDigest, c, taskRun) - if err != nil { - t.Errorf("couldn't get entrypoint remote: %v", err) - } - if !reflect.DeepEqual(ep, expectedEntrypoint) { - t.Errorf("entrypoints do not match: %s should be %s", ep[0], expectedEntrypoint) + + for _, tt := range []func(taskRun *v1alpha1.TaskRun) *v1alpha1.TaskRun{ + func(tr *v1alpha1.TaskRun) *v1alpha1.TaskRun { return tr }, + func(tr *v1alpha1.TaskRun) *v1alpha1.TaskRun { + tr.Spec.ServiceAccountName = "" + tr.Spec.DeprecatedServiceAccount = "some-other-sa" + return tr + }, + } { + taskRun := &v1alpha1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "taskRun", + }, + Spec: v1alpha1.TaskRunSpec{ + ServiceAccountName: "some-other-sa", + TaskSpec: &v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Image: "ubuntu", + Command: []string{"echo"}, + Args: []string{"hello"}, + }}}, + }, + }, + } + + entrypointCache, err := NewCache() + if err != nil { + t.Fatalf("couldn't create new entrypoint cache: %v", err) + } + + ep, err := GetRemoteEntrypoint(entrypointCache, finalDigest, c, tt(taskRun)) + if err != nil { + t.Errorf("couldn't get entrypoint remote: %v", err) + } + if !reflect.DeepEqual(ep, expectedEntrypoint) { + t.Errorf("entrypoints do not match: %s should be %s", ep[0], expectedEntrypoint) + } } } diff --git a/pkg/reconciler/taskrun/resources/pod.go b/pkg/reconciler/taskrun/resources/pod.go index eea57c256ed..91437b36751 100644 --- a/pkg/reconciler/taskrun/resources/pod.go +++ b/pkg/reconciler/taskrun/resources/pod.go @@ -244,7 +244,7 @@ func TryGetPod(taskRunStatus v1alpha1.TaskRunStatus, gp GetPod) (*corev1.Pod, er // MakePod converts TaskRun and TaskSpec objects to a Pod which implements the taskrun specified // by the supplied CRD. func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha1.TaskSpec, kubeclient kubernetes.Interface) (*corev1.Pod, error) { - cred, secrets, err := makeCredentialInitializer(images.CredsImage, taskRun.Spec.ServiceAccount, taskRun.Namespace, kubeclient) + cred, secrets, err := makeCredentialInitializer(images.CredsImage, taskRun.GetServiceAccountName(), taskRun.Namespace, kubeclient) if err != nil { return nil, err } @@ -349,7 +349,7 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha RestartPolicy: corev1.RestartPolicyNever, InitContainers: mergedInitContainers, Containers: mergedPodContainers, - ServiceAccountName: taskRun.Spec.ServiceAccount, + ServiceAccountName: taskRun.GetServiceAccountName(), Volumes: volumes, NodeSelector: taskRun.Spec.PodTemplate.NodeSelector, Tolerations: taskRun.Spec.PodTemplate.Tolerations, diff --git a/pkg/reconciler/taskrun/resources/pod_test.go b/pkg/reconciler/taskrun/resources/pod_test.go index 11049f6a6d0..e2a8b067f93 100644 --- a/pkg/reconciler/taskrun/resources/pod_test.go +++ b/pkg/reconciler/taskrun/resources/pod_test.go @@ -176,7 +176,51 @@ func TestMakePod(t *testing.T) { }}}, }, trs: v1alpha1.TaskRunSpec{ - ServiceAccount: "service-account", + ServiceAccountName: "service-account", + }, + want: &corev1.PodSpec{ + ServiceAccountName: "service-account", + RestartPolicy: corev1.RestartPolicyNever, + InitContainers: []corev1.Container{{ + Name: containerPrefix + credsInit + "-mz4c7", + Image: credsImage, + Command: []string{"/ko-app/creds-init"}, + Args: []string{ + "-basic-docker=multi-creds=https://docker.io", + "-basic-docker=multi-creds=https://us.gcr.io", + "-basic-git=multi-creds=github.com", + "-basic-git=multi-creds=gitlab.com", + }, + Env: implicitEnvVars, + VolumeMounts: implicitVolumeMountsWithSecrets, + WorkingDir: workspaceDir, + }}, + Containers: []corev1.Container{{ + Name: "step-name", + Image: "image", + Env: implicitEnvVars, + VolumeMounts: implicitVolumeMounts, + WorkingDir: workspaceDir, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("0"), + corev1.ResourceMemory: resource.MustParse("0"), + corev1.ResourceEphemeralStorage: resource.MustParse("0"), + }, + }, + }}, + Volumes: implicitVolumesWithSecrets, + }, + }, { + desc: "with-deprecated-service-account", + ts: v1alpha1.TaskSpec{ + Steps: []v1alpha1.Step{{Container: corev1.Container{ + Name: "name", + Image: "image", + }}}, + }, + trs: v1alpha1.TaskRunSpec{ + DeprecatedServiceAccount: "service-account", }, want: &corev1.PodSpec{ ServiceAccountName: "service-account", diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index b477af3d5ee..2e744184869 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -288,7 +288,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { )) taskRunWithSaSuccess := tb.TaskRun("test-taskrun-with-sa-run-success", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(saTask.Name, tb.TaskRefAPIVersion("a1")), - tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunServiceAccountName("test-sa"), )) taskruns := []*v1alpha1.TaskRun{taskRunSuccess, taskRunWithSaSuccess} d := test.Data{ @@ -381,7 +381,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { defer cancel() c := testAssets.Controller clients := testAssets.Clients - saName := tc.taskRun.Spec.ServiceAccount + saName := tc.taskRun.Spec.ServiceAccountName if saName == "" { saName = defaultSAName } @@ -447,7 +447,10 @@ func TestReconcile(t *testing.T) { tb.TaskRunTaskRef(simpleTask.Name, tb.TaskRefAPIVersion("a1")), )) taskRunWithSaSuccess := tb.TaskRun("test-taskrun-with-sa-run-success", "foo", tb.TaskRunSpec( - tb.TaskRunTaskRef(saTask.Name, tb.TaskRefAPIVersion("a1")), tb.TaskRunServiceAccount("test-sa"), + tb.TaskRunTaskRef(saTask.Name, tb.TaskRefAPIVersion("a1")), tb.TaskRunServiceAccountName("test-sa"), + )) + taskRunWithDeprecatedSaSuccess := tb.TaskRun("test-taskrun-with-deprecated-sa-run-success", "foo", tb.TaskRunSpec( + tb.TaskRunTaskRef(saTask.Name, tb.TaskRefAPIVersion("a1")), tb.TaskRunDeprecatedServiceAccount("", "test-deprecated-sa"), )) taskRunTaskEnv := tb.TaskRun("test-taskrun-task-env", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(taskEnvTask.Name, tb.TaskRefAPIVersion("a1")), @@ -579,7 +582,7 @@ func TestReconcile(t *testing.T) { ) taskruns := []*v1alpha1.TaskRun{ - taskRunSuccess, taskRunWithSaSuccess, + taskRunSuccess, taskRunWithSaSuccess, taskRunWithDeprecatedSaSuccess, taskRunSubstitution, taskRunInputOutput, taskRunWithTaskSpec, taskRunWithClusterTask, taskRunWithResourceSpecAndTaskSpec, taskRunWithLabels, taskRunWithAnnotations, taskRunWithResourceRequests, taskRunTaskEnv, taskRunWithPod, @@ -660,6 +663,39 @@ func TestReconcile(t *testing.T) { ), ), ), + }, { + name: "deprecated-serviceaccount", + taskRun: taskRunWithDeprecatedSaSuccess, + wantPod: tb.Pod("test-taskrun-with-deprecated-sa-run-success-pod-d406f0", "foo", + tb.PodAnnotation("tekton.dev/ready", ""), + tb.PodLabel(taskNameLabelKey, "test-with-sa"), + tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-deprecated-sa-run-success"), + tb.PodLabel(resources.ManagedByLabelKey, resources.ManagedByLabelValue), + tb.PodOwnerReference("TaskRun", "test-taskrun-with-deprecated-sa-run-success", + tb.OwnerReferenceAPIVersion(currentApiVersion)), + tb.PodSpec( + tb.PodServiceAccountName("test-deprecated-sa"), + tb.PodVolumes(toolsVolume, downward, workspaceVolume, homeVolume), + tb.PodRestartPolicy(corev1.RestartPolicyNever), + getCredentialsInitContainer("9l9zj"), + getPlaceToolsInitContainer(), + tb.PodContainer("step-sa-step", "foo", + tb.Command(entrypointLocation), + tb.Args("-wait_file", "/builder/downward/ready", "-post_file", "/builder/tools/0", "-wait_file_content", "-entrypoint", "/mycmd", "--"), + tb.WorkingDir(workspaceDir), + tb.EnvVar("HOME", "/builder/home"), + tb.VolumeMount("tools", "/builder/tools"), + tb.VolumeMount("downward", "/builder/downward"), + tb.VolumeMount("workspace", workspaceDir), + tb.VolumeMount("home", "/builder/home"), + tb.Resources(tb.Requests( + tb.CPU("0"), + tb.Memory("0"), + tb.EphemeralStorage("0"), + )), + ), + ), + ), }, { name: "params", taskRun: taskRunSubstitution, @@ -1218,7 +1254,7 @@ func TestReconcile(t *testing.T) { defer cancel() c := testAssets.Controller clients := testAssets.Clients - saName := tc.taskRun.Spec.ServiceAccount + saName := tc.taskRun.GetServiceAccountName() if saName == "" { saName = "default" } @@ -1578,7 +1614,7 @@ func TestReconcilePodUpdateStatus(t *testing.T) { func TestCreateRedirectedTaskSpec(t *testing.T) { tr := tb.TaskRun("tr", "tr", tb.TaskRunSpec( - tb.TaskRunServiceAccount("sa"), + tb.TaskRunServiceAccountName("sa"), )) task := tb.Task("tr-ts", "tr", tb.TaskSpec( tb.Step("foo1", "bar1", tb.StepCommand("abcd"), tb.StepArgs("efgh")), @@ -1964,7 +2000,7 @@ func TestReconcileCloudEvents(t *testing.T) { c := testAssets.Controller clients := testAssets.Clients - saName := tc.taskRun.Spec.ServiceAccount + saName := tc.taskRun.Spec.ServiceAccountName if saName == "" { saName = "default" } diff --git a/pkg/reconciler/timeout_handler_test.go b/pkg/reconciler/timeout_handler_test.go index 70a2a2c767e..673eedcb259 100644 --- a/pkg/reconciler/timeout_handler_test.go +++ b/pkg/reconciler/timeout_handler_test.go @@ -163,7 +163,7 @@ func TestPipelinRunCheckTimeouts(t *testing.T) { )) prTimeout := tb.PipelineRun("test-pipeline-run-with-timeout", testNs, tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunServiceAccountName("test-sa"), tb.PipelineRunTimeout(1*time.Second), ), tb.PipelineRunStatus( @@ -201,7 +201,7 @@ func TestPipelinRunCheckTimeouts(t *testing.T) { ), ) prCancelled := tb.PipelineRun("test-pipeline-cancel", testNs, - tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccount("test-sa"), + tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccountName("test-sa"), tb.PipelineRunCancelled, tb.PipelineRunTimeout(config.DefaultTimeoutMinutes*time.Minute), ), diff --git a/test/builder/examples_test.go b/test/builder/examples_test.go index 4d5d55d0549..a43ead56470 100644 --- a/test/builder/examples_test.go +++ b/test/builder/examples_test.go @@ -120,7 +120,7 @@ func ExamplePipeline() { func ExamplePipelineRun() { pipelineRun := tb.PipelineRun("pear", "namespace", - tb.PipelineRunSpec("tomatoes", tb.PipelineRunServiceAccount("inexistent")), + tb.PipelineRunSpec("tomatoes", tb.PipelineRunServiceAccountName("inexistent")), ) expectedPipelineRun := &v1alpha1.PipelineRun{ // […] diff --git a/test/builder/pipeline.go b/test/builder/pipeline.go index d8e43af6bcd..c805f31003f 100644 --- a/test/builder/pipeline.go +++ b/test/builder/pipeline.go @@ -342,18 +342,36 @@ func PipelineResourceBindingResourceSpec(spec *v1alpha1.PipelineResourceSpec) Pi } // PipelineRunServiceAccount sets the service account to the PipelineRunSpec. -func PipelineRunServiceAccount(sa string) PipelineRunSpecOp { +func PipelineRunServiceAccountName(sa string) PipelineRunSpecOp { return func(prs *v1alpha1.PipelineRunSpec) { - prs.ServiceAccount = sa + prs.ServiceAccountName = sa + } +} + +// PipelineRunServiceAccount sets the service account to the PipelineRunSpec. +func PipelineRunDeprecatedServiceAccountName(sa, deprecatedSA string) PipelineRunSpecOp { + return func(prs *v1alpha1.PipelineRunSpec) { + prs.ServiceAccountName = sa + prs.DeprecatedServiceAccount = deprecatedSA + } +} + +// PipelineRunServiceAccountTask configures the service account for given Task in PipelineRun. +func PipelineRunServiceAccountNameTask(taskName, sa string) PipelineRunSpecOp { + return func(prs *v1alpha1.PipelineRunSpec) { + prs.ServiceAccountNames = append(prs.ServiceAccountNames, v1alpha1.PipelineRunSpecServiceAccountName{ + TaskName: taskName, + ServiceAccountName: sa, + }) } } // PipelineRunServiceAccountTask configures the service account for given Task in PipelineRun. -func PipelineRunServiceAccountTask(taskName, sa string) PipelineRunSpecOp { +func PipelineRunDeprecatedServiceAccountTask(taskName, sa string) PipelineRunSpecOp { return func(prs *v1alpha1.PipelineRunSpec) { - prs.ServiceAccounts = append(prs.ServiceAccounts, v1alpha1.PipelineRunSpecServiceAccount{ - TaskName: taskName, - ServiceAccount: sa, + prs.DeprecatedServiceAccounts = append(prs.DeprecatedServiceAccounts, v1alpha1.DeprecatedPipelineRunSpecServiceAccount{ + TaskName: taskName, + DeprecatedServiceAccount: sa, }) } } diff --git a/test/builder/pipeline_test.go b/test/builder/pipeline_test.go index ea18ea73204..b8242b0e87d 100644 --- a/test/builder/pipeline_test.go +++ b/test/builder/pipeline_test.go @@ -129,12 +129,12 @@ func TestPipelineRun(t *testing.T) { completedTime := startTime.Add(5 * time.Minute) pipelineRun := tb.PipelineRun("pear", "foo", tb.PipelineRunSpec( - "tomatoes", tb.PipelineRunServiceAccount("sa"), + "tomatoes", tb.PipelineRunServiceAccountName("sa"), tb.PipelineRunParam("first-param-string", "first-value"), tb.PipelineRunParam("second-param-array", "some", "array"), tb.PipelineRunTimeout(1*time.Hour), tb.PipelineRunResourceBinding("some-resource", tb.PipelineResourceBindingRef("my-special-resource")), - tb.PipelineRunServiceAccountTask("foo", "sa-2"), + tb.PipelineRunServiceAccountNameTask("foo", "sa-2"), ), tb.PipelineRunStatus(tb.PipelineRunStatusCondition( apis.Condition{Type: apis.ConditionSucceeded}), tb.PipelineRunStartTime(startTime), @@ -152,9 +152,9 @@ func TestPipelineRun(t *testing.T) { }, }, Spec: v1alpha1.PipelineRunSpec{ - PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, - ServiceAccount: "sa", - ServiceAccounts: []v1alpha1.PipelineRunSpecServiceAccount{{TaskName: "foo", ServiceAccount: "sa-2"}}, + PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, + ServiceAccountName: "sa", + ServiceAccountNames: []v1alpha1.PipelineRunSpecServiceAccountName{{TaskName: "foo", ServiceAccountName: "sa-2"}}, Params: []v1alpha1.Param{{ Name: "first-param-string", Value: *tb.ArrayOrString("first-value"), @@ -191,7 +191,7 @@ func TestPipelineRunWithResourceSpec(t *testing.T) { completedTime := startTime.Add(5 * time.Minute) pipelineRun := tb.PipelineRun("pear", "foo", tb.PipelineRunSpec( - "tomatoes", tb.PipelineRunServiceAccount("sa"), + "tomatoes", tb.PipelineRunServiceAccountName("sa"), tb.PipelineRunParam("first-param-string", "first-value"), tb.PipelineRunParam("second-param-array", "some", "array"), tb.PipelineRunTimeout(1*time.Hour), @@ -202,7 +202,7 @@ func TestPipelineRunWithResourceSpec(t *testing.T) { Name: "url", Value: "git", }}})), - tb.PipelineRunServiceAccountTask("foo", "sa-2"), + tb.PipelineRunServiceAccountNameTask("foo", "sa-2"), ), tb.PipelineRunStatus(tb.PipelineRunStatusCondition( apis.Condition{Type: apis.ConditionSucceeded}), tb.PipelineRunStartTime(startTime), @@ -220,9 +220,9 @@ func TestPipelineRunWithResourceSpec(t *testing.T) { }, }, Spec: v1alpha1.PipelineRunSpec{ - PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, - ServiceAccount: "sa", - ServiceAccounts: []v1alpha1.PipelineRunSpecServiceAccount{{TaskName: "foo", ServiceAccount: "sa-2"}}, + PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, + ServiceAccountName: "sa", + ServiceAccountNames: []v1alpha1.PipelineRunSpecServiceAccountName{{TaskName: "foo", ServiceAccountName: "sa-2"}}, Params: []v1alpha1.Param{{ Name: "first-param-string", Value: *tb.ArrayOrString("first-value"), diff --git a/test/builder/task.go b/test/builder/task.go index daca7cbff9f..780690f7052 100644 --- a/test/builder/task.go +++ b/test/builder/task.go @@ -508,9 +508,17 @@ func TaskRunTaskSpec(ops ...TaskSpecOp) TaskRunSpecOp { } // TaskRunServiceAccount sets the serviceAccount to the TaskRunSpec. -func TaskRunServiceAccount(sa string) TaskRunSpecOp { +func TaskRunServiceAccountName(sa string) TaskRunSpecOp { return func(trs *v1alpha1.TaskRunSpec) { - trs.ServiceAccount = sa + trs.ServiceAccountName = sa + } +} + +// TaskRunServiceAccount sets the serviceAccount to the TaskRunSpec. +func TaskRunDeprecatedServiceAccount(sa, deprecatedSA string) TaskRunSpecOp { + return func(trs *v1alpha1.TaskRunSpec) { + trs.ServiceAccountName = sa + trs.DeprecatedServiceAccount = deprecatedSA } } diff --git a/test/builder/task_test.go b/test/builder/task_test.go index 0f7836ce9c4..077d7c8e01f 100644 --- a/test/builder/task_test.go +++ b/test/builder/task_test.go @@ -249,7 +249,7 @@ func TestTaskRunWithTaskSpec(t *testing.T) { tb.TaskRunTaskSpec( tb.Step("step", "image", tb.StepCommand("/mycmd")), ), - tb.TaskRunServiceAccount("sa"), + tb.TaskRunServiceAccountName("sa"), tb.TaskRunTimeout(2*time.Minute), tb.TaskRunSpecStatus(v1alpha1.TaskRunSpecStatusCancelled), )) @@ -266,9 +266,9 @@ func TestTaskRunWithTaskSpec(t *testing.T) { Command: []string{"/mycmd"}, }}}, }, - ServiceAccount: "sa", - Status: v1alpha1.TaskRunSpecStatusCancelled, - Timeout: &metav1.Duration{Duration: 2 * time.Minute}, + ServiceAccountName: "sa", + Status: v1alpha1.TaskRunSpecStatusCancelled, + Timeout: &metav1.Duration{Duration: 2 * time.Minute}, }, } if d := cmp.Diff(expectedTaskRun, taskRun); d != "" { diff --git a/test/entrypoint_test.go b/test/entrypoint_test.go index bea98197790..dcda1573ece 100644 --- a/test/entrypoint_test.go +++ b/test/entrypoint_test.go @@ -50,7 +50,7 @@ func TestEntrypointRunningStepsInOrder(t *testing.T) { t.Fatalf("Failed to create Task: %s", err) } taskRun := tb.TaskRun(epTaskRunName, namespace, tb.TaskRunSpec( - tb.TaskRunTaskRef(epTaskName), tb.TaskRunServiceAccount("default"), + tb.TaskRunTaskRef(epTaskName), tb.TaskRunServiceAccountName("default"), )) if _, err := c.TaskRunClient.Create(taskRun); err != nil { t.Fatalf("Failed to create TaskRun: %s", err) diff --git a/test/pipelinerun_test.go b/test/pipelinerun_test.go index 1e0209fdae1..166c8edccb7 100644 --- a/test/pipelinerun_test.go +++ b/test/pipelinerun_test.go @@ -374,7 +374,7 @@ func getHelloWorldPipelineRun(suffix int, namespace string) *v1alpha1.PipelineRu tb.PipelineRunSpec(getName(pipelineName, suffix), tb.PipelineRunParam("path", "docker://gcr.io/build-crd-testing/secret-sauce"), tb.PipelineRunParam("dest", "dir:///tmp/"), - tb.PipelineRunServiceAccount(fmt.Sprintf("%s%d", saName, suffix)), + tb.PipelineRunServiceAccountName(fmt.Sprintf("%s%d", saName, suffix)), ), ) } diff --git a/test/status_test.go b/test/status_test.go index d42e32bece6..6eaf1f37bd2 100644 --- a/test/status_test.go +++ b/test/status_test.go @@ -43,7 +43,7 @@ func TestTaskRunPipelineRunStatus(t *testing.T) { t.Fatalf("Failed to create Task: %s", err) } taskRun := tb.TaskRun("apple", namespace, tb.TaskRunSpec( - tb.TaskRunTaskRef("banana"), tb.TaskRunServiceAccount("inexistent"), + tb.TaskRunTaskRef("banana"), tb.TaskRunServiceAccountName("inexistent"), )) if _, err := c.TaskRunClient.Create(taskRun); err != nil { t.Fatalf("Failed to create TaskRun: %s", err) @@ -58,7 +58,7 @@ func TestTaskRunPipelineRunStatus(t *testing.T) { tb.PipelineSpec(tb.PipelineTask("foo", "banana")), ) pipelineRun := tb.PipelineRun("pear", namespace, tb.PipelineRunSpec( - "tomatoes", tb.PipelineRunServiceAccount("inexistent"), + "tomatoes", tb.PipelineRunServiceAccountName("inexistent"), )) if _, err := c.PipelineClient.Create(pipeline); err != nil { t.Fatalf("Failed to create Pipeline `%s`: %s", "tomatoes", err)