From 5bf1ef67d0b470a86ff785c69d18990ccf75a796 Mon Sep 17 00:00:00 2001 From: cezkuj Date: Tue, 21 May 2019 20:41:00 +0000 Subject: [PATCH] Adding possibility to configure multiple service accounts for different tasks The pipeline specification allows multiple different service accounts for tasks so that different task can use the right service account. E.g. build task may need different privileges than deploy task Fixes - #777 --- docs/pipelineruns.md | 30 +++++++ .../pipeline/v1alpha1/pipelinerun_types.go | 8 ++ .../v1alpha1/zz_generated.deepcopy.go | 21 +++++ .../v1alpha1/pipelinerun/pipelinerun.go | 10 ++- .../v1alpha1/pipelinerun/pipelinerun_test.go | 85 +++++++++++++++++++ test/builder/pipeline.go | 10 +++ test/builder/pipeline_test.go | 6 +- 7 files changed, 167 insertions(+), 3 deletions(-) diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 99e6a63faed..a2de5dfc82f 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -39,6 +39,8 @@ following fields: - [`serviceAccount`](#service-account) - Specifies a `ServiceAccount` resource object that enables your build to run with the defined authentication information. + - [`serviceAccounts`](#service-accounts) - Specifies a list of `ServiceAccount` + and `PipelineTask` pairs that enable you to overwrite `ServiceAccount` for concrete `PipelineTask`. - `timeout` - Specifies timeout after which the `PipelineRun` will fail. - [`nodeSelector`] - A selector which must be true for the pod to fit on a node. The selector which must match a node's labels for the pod to be @@ -98,6 +100,34 @@ of the `TaskRun` resource object. For examples and more information about specifying service accounts, see the [`ServiceAccount`](./auth.md) reference topic. +### Service Accounts + +Specifies the list of `ServiceAccount` and `PipelineTask` pairs. Specified +`PipelineTask` will be run with configured `ServiceAccount`, +overwriting [`serviceAccount`](#service-account) configuration, for example: + +```yaml +spec: + serviceAccount: sa-1 + serviceAccounts: + - taskName: build-task + serviceAccount: sa-for-build +``` +If used with this `Pipeline`, `test-task` will use the `ServiceAccount` `sa-1`, while `build-task` will use `sa-for-build`. + +```yaml +kind: Pipeline +spec: + tasks: + - name: build-task + taskRef: + name: build-push + tasks: + - name: test-task + taskRef: + name: test +``` + ## Cancelling a PipelineRun In order to cancel a running pipeline (`PipelineRun`), you need to update its diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index 41c046b7c79..2773fad9919 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go @@ -49,6 +49,8 @@ type PipelineRunSpec struct { // +optional ServiceAccount string `json:"serviceAccount"` // +optional + ServiceAccounts []PipelineRunSpecServiceAccount `json:"serviceAccounts,omitempty"` + // +optional Results *Results `json:"results,omitempty"` // Used for cancelling a pipelinerun (and maybe more later on) // +optional @@ -147,6 +149,12 @@ 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"` +} + // SetCondition sets the condition, unsetting previous conditions with the same // type as necessary. func (pr *PipelineRunStatus) SetCondition(newCond *apis.Condition) { diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index 92966758d9c..4ab1c54c5db 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -700,6 +700,11 @@ func (in *PipelineRunSpec) DeepCopyInto(out *PipelineRunSpec) { *out = make([]Param, len(*in)) copy(*out, *in) } + if in.ServiceAccounts != nil { + in, out := &in.ServiceAccounts, &out.ServiceAccounts + *out = make([]PipelineRunSpecServiceAccount, len(*in)) + copy(*out, *in) + } if in.Results != nil { in, out := &in.Results, &out.Results if *in == nil { @@ -754,6 +759,22 @@ func (in *PipelineRunSpec) DeepCopy() *PipelineRunSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PipelineRunSpecServiceAccount) DeepCopyInto(out *PipelineRunSpecServiceAccount) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineRunSpecServiceAccount. +func (in *PipelineRunSpecServiceAccount) DeepCopy() *PipelineRunSpecServiceAccount { + if in == nil { + return nil + } + out := new(PipelineRunSpecServiceAccount) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineRunStatus) DeepCopyInto(out *PipelineRunStatus) { *out = *in diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 05045bc8766..3787a304440 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -457,6 +457,14 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re taskRunTimeout = nil } + // If service account is configured for a given PipelineTask, override PipelineRun's seviceAccount + serviceAccount := pr.Spec.ServiceAccount + for _, sa := range pr.Spec.ServiceAccounts { + if sa.TaskName == rprt.PipelineTask.Name { + serviceAccount = sa.ServiceAccount + } + } + // Propagate labels from PipelineRun to TaskRun. labels := make(map[string]string, len(pr.ObjectMeta.Labels)+1) for key, val := range pr.ObjectMeta.Labels { @@ -500,7 +508,7 @@ func (c *Reconciler) createTaskRun(logger *zap.SugaredLogger, rprt *resources.Re Inputs: v1alpha1.TaskRunInputs{ Params: rprt.PipelineTask.Params, }, - ServiceAccount: pr.Spec.ServiceAccount, + ServiceAccount: serviceAccount, Timeout: taskRunTimeout, NodeSelector: pr.Spec.NodeSelector, Tolerations: pr.Spec.Tolerations, diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index 8de3dff2fe0..c5de39d7788 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go @@ -826,6 +826,91 @@ func TestReconcilePropagateLabels(t *testing.T) { } } +func TestReconcileWithDifferentServiceAccounts(t *testing.T) { + names.TestingSeed() + + ps := []*v1alpha1.Pipeline{tb.Pipeline("test-pipeline", "foo", tb.PipelineSpec( + tb.PipelineTask("hello-world-0", "hello-world-task"), + tb.PipelineTask("hello-world-1", "hello-world-task"), + ))} + 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"), + ), + )} + ts := []*v1alpha1.Task{ + tb.Task("hello-world-task", "foo"), + } + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + + // create fake recorder for testing + fr := record.NewFakeRecorder(2) + + testAssets := getPipelineRunController(t, d, fr) + c := testAssets.Controller + clients := testAssets.Clients + + err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-different-service-accs") + + 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("foo").Get("test-pipeline-run-different-service-accs", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err) + } + taskRunNames := []string{"test-pipeline-run-different-service-accs-hello-world-0-9l9zj", "test-pipeline-run-different-service-accs-hello-world-1-mz4c7"} + + expectedTaskRuns := []*v1alpha1.TaskRun{ + tb.TaskRun(taskRunNames[0], "foo", + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-different-service-accs", + tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world-task"), + tb.TaskRunServiceAccount("test-sa-0"), + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "hello-world-0"), + ), + tb.TaskRun(taskRunNames[1], "foo", + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run-different-service-accs", + tb.OwnerReferenceAPIVersion("tekton.dev/v1alpha1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world-task"), + tb.TaskRunServiceAccount("test-sa-1"), + ), + tb.TaskRunLabel("tekton.dev/pipeline", "test-pipeline"), + tb.TaskRunLabel("tekton.dev/pipelineRun", "test-pipeline-run-different-service-accs"), + tb.TaskRunLabel("tekton.dev/pipelineTask", "hello-world-1"), + ), + } + for i := range ps[0].Spec.Tasks { + // Check that the expected TaskRun was created + actual, err := clients.Pipeline.Tekton().TaskRuns("foo").Get(taskRunNames[i], metav1.GetOptions{}) + if err != nil { + t.Fatalf("Expected a TaskRun to be created, but it wasn't: %s", err) + } + if d := cmp.Diff(actual, expectedTaskRuns[i]); d != "" { + t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRuns[i], d) + } + + } + +} + func TestReconcileWithTimeoutAndRetry(t *testing.T) { tcs := []struct { diff --git a/test/builder/pipeline.go b/test/builder/pipeline.go index 8ac4c9ecef3..8f6615d5be4 100644 --- a/test/builder/pipeline.go +++ b/test/builder/pipeline.go @@ -310,6 +310,16 @@ func PipelineRunServiceAccount(sa string) PipelineRunSpecOp { } } +// PipelineRunServiceAccountTask configures the service account for given Task in PipelineRun. +func PipelineRunServiceAccountTask(taskName, sa string) PipelineRunSpecOp { + return func(prs *v1alpha1.PipelineRunSpec) { + prs.ServiceAccounts = append(prs.ServiceAccounts, v1alpha1.PipelineRunSpecServiceAccount{ + TaskName: taskName, + ServiceAccount: sa, + }) + } +} + // PipelineRunParam add a param, with specified name and value, to the PipelineRunSpec. func PipelineRunParam(name, value string) PipelineRunSpecOp { return func(prs *v1alpha1.PipelineRunSpec) { diff --git a/test/builder/pipeline_test.go b/test/builder/pipeline_test.go index 3f554682ef6..ec1a7d25308 100644 --- a/test/builder/pipeline_test.go +++ b/test/builder/pipeline_test.go @@ -103,6 +103,7 @@ func TestPipelineRun(t *testing.T) { tb.PipelineRunParam("first-param", "first-value"), tb.PipelineRunTimeout(&metav1.Duration{Duration: 1 * time.Hour}), tb.PipelineRunResourceBinding("some-resource", tb.PipelineResourceBindingRef("my-special-resource")), + tb.PipelineRunServiceAccountTask("foo", "sa-2"), ), tb.PipelineRunStatus(tb.PipelineRunStatusCondition( apis.Condition{Type: apis.ConditionSucceeded}), tb.PipelineRunStartTime(startTime), @@ -117,8 +118,9 @@ func TestPipelineRun(t *testing.T) { }, }, Spec: v1alpha1.PipelineRunSpec{ - PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, - ServiceAccount: "sa", + PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, + ServiceAccount: "sa", + ServiceAccounts: []v1alpha1.PipelineRunSpecServiceAccount{{TaskName: "foo", ServiceAccount: "sa-2"}}, Params: []v1alpha1.Param{{ Name: "first-param", Value: "first-value",