Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding possibility to configure multiple service accounts for different tasks #902

Merged
merged 1 commit into from
Jun 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions docs/pipelineruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
```
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this interface looks great! 👍

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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks for adding this!

```

## Cancelling a PipelineRun

In order to cancel a running pipeline (`PipelineRun`), you need to update its
Expand Down
8 changes: 8 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
21 changes: 21 additions & 0 deletions pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
85 changes: 85 additions & 0 deletions pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions test/builder/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
cezkuj marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down
6 changes: 4 additions & 2 deletions test/builder/pipeline_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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",
Expand Down