From 1a97932cdf93356af67158c3fd3479e5ad383403 Mon Sep 17 00:00:00 2001 From: Austin Zhao Date: Mon, 2 May 2022 19:51:10 -0400 Subject: [PATCH] TEP-0106: Support Specifying Metadata per Task in Runtime While currently Tekton supports specifying metadata for a Task in a Pipeline during "authoring time", specific requirements from a running context lead to this work to offer a flexibility to add metadata during "runtime". The chosen approach is to add a Metadata field under PipelineTaskRunSpec as for its proper runtime context and easier for a user to follow and utilize, rather than a whole new field. Further consideration and tradeoff can be found in the TEP-0106. --- docs/pipelineruns.md | 52 +++++--- .../pipeline/v1beta1/openapi_generated.go | 8 +- .../pipeline/v1beta1/pipelinerun_types.go | 4 + pkg/apis/pipeline/v1beta1/swagger.json | 4 + .../pipeline/v1beta1/zz_generated.deepcopy.go | 1 + pkg/reconciler/pipelinerun/pipelinerun.go | 46 +++---- .../pipelinerun/pipelinerun_test.go | 123 ++++++++++++++++++ 7 files changed, 197 insertions(+), 41 deletions(-) diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 2848e2c93d3..995100789ef 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -9,28 +9,28 @@ weight: 500 - [PipelineRuns](#pipelineruns) - [Overview](#overview) - - [Configuring a PipelineRun](#configuring-a-pipelinerun) - - [Specifying the target Pipeline](#specifying-the-target-pipeline) + - [Configuring a `PipelineRun`](#configuring-a-pipelinerun) + - [Specifying the target `Pipeline`](#specifying-the-target-pipeline) - [Tekton Bundles](#tekton-bundles) - [Remote Pipelines](#remote-pipelines) - - [Specifying Resources](#specifying-resources) - - [Specifying Parameters](#specifying-parameters) + - [Specifying `Resources`](#specifying-resources) + - [Specifying `Parameters`](#specifying-parameters) - [Implicit Parameters](#implicit-parameters) - - [Specifying custom ServiceAccount credentials](#specifying-custom-serviceaccount-credentials) - - [Mapping ServiceAccount credentials to Tasks](#mapping-serviceaccount-credentials-to-tasks) - - [Specifying a Pod template](#specifying-a-pod-template) + - [Specifying custom `ServiceAccount` credentials](#specifying-custom-serviceaccount-credentials) + - [Mapping `ServiceAccount` credentials to `Tasks`](#mapping-serviceaccount-credentials-to-tasks) + - [Specifying a `Pod` template](#specifying-a-pod-template) - [Specifying taskRunSpecs](#specifying-taskrunspecs) - - [Specifying Workspaces](#specifying-workspaces) - - [Specifying LimitRange values](#specifying-limitrange-values) + - [Specifying `Workspaces`](#specifying-workspaces) + - [Specifying `LimitRange` values](#specifying-limitrange-values) - [Configuring a failure timeout](#configuring-a-failure-timeout) - - [PipelineRun status](#pipelinerun-status) - - [The status field](#the-status-field) - - [Configuring usage of TaskRun and Run embedded statuses](#configuring-usage-of-taskrun-and-run-embedded-statuses) + - [`PipelineRun` status](#pipelinerun-status) + - [The `status` field](#the-status-field) + - [Configuring usage of `TaskRun` and `Run` embedded statuses](#configuring-usage-of-taskrun-and-run-embedded-statuses) - [Monitoring execution status](#monitoring-execution-status) - - [Cancelling a PipelineRun](#cancelling-a-pipelinerun) - - [Gracefully cancelling a PipelineRun](#gracefully-cancelling-a-pipelinerun) - - [Gracefully stopping a PipelineRun](#gracefully-stopping-a-pipelinerun) - - [Pending PipelineRuns](#pending-pipelineruns) + - [Cancelling a `PipelineRun`](#cancelling-a-pipelinerun) + - [Gracefully cancelling a `PipelineRun`](#gracefully-cancelling-a-pipelinerun) + - [Gracefully stopping a `PipelineRun`](#gracefully-stopping-a-pipelinerun) + - [Pending `PipelineRuns`](#pending-pipelineruns) @@ -70,7 +70,7 @@ A `PipelineRun` definition supports the following fields: 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`. - - [`taskRunSpecs`](#specifying-taskrunspecs) - Specifies a list of `PipelineRunTaskSpec` which allows for setting `ServiceAccountName` and [`Pod` template](./podtemplates.md) for each task. This overrides the `Pod` template set for the entire `Pipeline`. + - [`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. - [`timeouts`](#configuring-a-failure-timeout) - Specifies the timeout before the `PipelineRun` fails. `timeouts` allows more granular timeout configuration, at the pipeline, tasks, and finally levels - [`podTemplate`](#specifying-a-pod-template) - Specifies a [`Pod` template](./podtemplates.md) to use as the basis for the configuration of the `Pod` that executes each `Task`. @@ -479,6 +479,24 @@ If used with this `Pipeline`, `build-task` will use the task specific `PodTempl `PipelineTaskRunSpec` may also contain `StepOverrides` and `SidecarOverrides`; see [Overriding `Task` `Steps` and `Sidecars`](./taskruns.md#overriding-task-steps-and-sidecars) for more information. +An optional `Metadata` field can be added for each task to have the flexibility to add annotations and labels required in a specific running context. + +An example for rendering needed secrets with Vault: + +```yaml +spec: + pipelineRef: + name: pipeline-name + taskRunSpecs: + - pipelineTaskName: task-name + metadata: + annotations: + vault.hashicorp.com/agent-inject-secret-foo: "/path/to/foo" + vault.hashicorp.com/role: role-name +``` + +The specified metadata will have a higher precedence to keep its value than the metadata added in `PipelineRun.metadata.*` and `Pipeline.spec.tasks.taskSpec.metadata.*` if they hold a same key value. + ### Specifying `Workspaces` If your `Pipeline` specifies one or more `Workspaces`, you must map those `Workspaces` to diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 7b6a2b6db6b..224eaba5bab 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -2850,11 +2850,17 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineTaskRunSpec(ref common.ReferenceCa }, }, }, + "metadata": { + SchemaProps: spec.SchemaProps{ + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineTaskMetadata"), + }, + }, }, }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod.Template", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunSidecarOverride", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunStepOverride"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod.Template", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineTaskMetadata", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunSidecarOverride", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskRunStepOverride"}, } } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index 55531db0b79..954a88dee99 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -601,6 +601,9 @@ type PipelineTaskRunSpec struct { StepOverrides []TaskRunStepOverride `json:"stepOverrides,omitempty"` // +listType=atomic SidecarOverrides []TaskRunSidecarOverride `json:"sidecarOverrides,omitempty"` + + // +optional + Metadata PipelineTaskMetadata `json:"metadata,omitempty"` } // GetTaskRunSpec returns the task specific spec for a given @@ -621,6 +624,7 @@ func (pr *PipelineRun) GetTaskRunSpec(pipelineTaskName string) PipelineTaskRunSp } s.StepOverrides = task.StepOverrides s.SidecarOverrides = task.SidecarOverrides + s.Metadata = task.Metadata } } return s diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 2101389df62..bbe398c0552 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -1593,6 +1593,10 @@ "description": "PipelineTaskRunSpec can be used to configure specific specs for a concrete Task", "type": "object", "properties": { + "metadata": { + "default": {}, + "$ref": "#/definitions/v1beta1.PipelineTaskMetadata" + }, "pipelineTaskName": { "type": "string" }, diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 50e3cdbc3a7..0df1fca569f 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1212,6 +1212,7 @@ func (in *PipelineTaskRunSpec) DeepCopyInto(out *PipelineTaskRunSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + in.Metadata.DeepCopyInto(&out.Metadata) return } diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index e8947b9b0a7..63aba3382f4 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -988,43 +988,43 @@ func getTaskrunLabels(pr *v1beta1.PipelineRun, pipelineTaskName string, includeP } func combineTaskRunAndTaskSpecLabels(pr *v1beta1.PipelineRun, pipelineTask *v1beta1.PipelineTask) map[string]string { - var tsLabels map[string]string - trLabels := getTaskrunLabels(pr, pipelineTask.Name, true) + labels := make(map[string]string) + + taskRunSpec := pr.GetTaskRunSpec(pipelineTask.Name) + addMetadataByPrecedence(labels, taskRunSpec.Metadata.Labels) + + addMetadataByPrecedence(labels, getTaskrunLabels(pr, pipelineTask.Name, true)) if pipelineTask.TaskSpec != nil { - tsLabels = pipelineTask.TaskSpecMetadata().Labels + addMetadataByPrecedence(labels, pipelineTask.TaskSpecMetadata().Labels) } - // labels from TaskRun takes higher precedence over the ones specified in Pipeline through TaskSpec - // initialize labels with TaskRun labels - labels := trLabels - for key, value := range tsLabels { - // add labels from TaskSpec if the label does not exist - if _, ok := labels[key]; !ok { - labels[key] = value - } - } return labels } func combineTaskRunAndTaskSpecAnnotations(pr *v1beta1.PipelineRun, pipelineTask *v1beta1.PipelineTask) map[string]string { - var tsAnnotations map[string]string - trAnnotations := getTaskrunAnnotations(pr) + annotations := make(map[string]string) + + taskRunSpec := pr.GetTaskRunSpec(pipelineTask.Name) + addMetadataByPrecedence(annotations, taskRunSpec.Metadata.Annotations) + + addMetadataByPrecedence(annotations, getTaskrunAnnotations(pr)) if pipelineTask.TaskSpec != nil { - tsAnnotations = pipelineTask.TaskSpecMetadata().Annotations + addMetadataByPrecedence(annotations, pipelineTask.TaskSpecMetadata().Annotations) } - // annotations from TaskRun takes higher precedence over the ones specified in Pipeline through TaskSpec - // initialize annotations with TaskRun annotations - annotations := trAnnotations - for key, value := range tsAnnotations { - // add annotations from TaskSpec if the annotation does not exist - if _, ok := annotations[key]; !ok { - annotations[key] = value + return annotations +} + +// Metadata Precedence Order: PipelineTaskRunSpec > PipelineRun > PipelineTaskSpec +func addMetadataByPrecedence(metadata map[string]string, addedMetadata map[string]string) { + for key, value := range addedMetadata { + // add new annotations if the key not exists in current ones + if _, ok := metadata[key]; !ok { + metadata[key] = value } } - return annotations } // getFinallyTaskRunTimeout returns the timeout to set when creating the ResolvedPipelineRunTask, which is a finally Task. diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 91a52a1d9d1..37a221c7241 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -7395,3 +7395,126 @@ func checkPipelineRunConditionStatusAndReason(t *testing.T, reconciledRun *v1bet t.Errorf("Expected reason %s but was %s", conditionReason, condition.Reason) } } + +func TestPropagatePipelineTaskRunSpecMetadata(t *testing.T) { + names.TestingSeed() + prName := "test-pipeline-run" + ps := []*v1beta1.Pipeline{simpleHelloWorldPipeline} + prs := []*v1beta1.PipelineRun{parse.MustParsePipelineRun(t, ` +metadata: + name: test-pipeline-run + namespace: foo +spec: + pipelineRef: + name: test-pipeline + taskRunSpecs: + - pipelineTaskName: hello-world-1 + metadata: + labels: + PipelineTaskRunSpecLabel: PipelineTaskRunSpecValue + annotations: + PipelineTaskRunSpecAnnotation: PipelineTaskRunSpecValue + taskServiceAccountName: custom-sa +`)} + ts := []*v1beta1.Task{simpleHelloWorldTask} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + _, clients := prt.reconcileRun("foo", prName, []string{}, false) + + actual := getTaskRunCreations(t, clients.Pipeline.Actions(), 2)[0] + expectedTaskRunObjectMeta := taskRunObjectMeta("test-pipeline-run-hello-world-1", "foo", "test-pipeline-run", "test-pipeline", "hello-world-1", false) + expectedTaskRunObjectMeta.Labels["PipelineTaskRunSpecLabel"] = "PipelineTaskRunSpecValue" + expectedTaskRunObjectMeta.Annotations["PipelineTaskRunSpecAnnotation"] = "PipelineTaskRunSpecValue" + expectedTaskRun := mustParseTaskRunWithObjectMeta(t, expectedTaskRunObjectMeta, ` +spec: + resources: {} + serviceAccountName: custom-sa + taskRef: + name: hello-world + timeout: 1h0m0s +`) + + if d := cmp.Diff(actual, expectedTaskRun, ignoreTypeMeta); d != "" { + t.Errorf("expected to see propagated metadata from PipelineTaskRunSpec in TaskRun %v created. Diff %s", expectedTaskRun, diff.PrintWantGot(d)) + } +} + +func TestMetadataPrecedence(t *testing.T) { + names.TestingSeed() + prName := "test-pipeline-run" + ps := []*v1beta1.Pipeline{parse.MustParsePipeline(t, ` +metadata: + name: test-pipeline + namespace: foo +spec: + tasks: + - name: hello-world-1 + taskSpec: + steps: + - name: foo-step + image: foo-image + metadata: + labels: + TestPrecedenceLabel: PipelineTaskSpecValue + annotations: + TestPrecedenceAnnotation: PipelineTaskSpecValue +`)} + prs := []*v1beta1.PipelineRun{parse.MustParsePipelineRun(t, ` +metadata: + name: test-pipeline-run + namespace: foo + metadata: + labels: + TestPrecedenceLabel: PipelineRunValue + annotations: + TestPrecedenceAnnotation: PipelineRunValue +spec: + pipelineRef: + name: test-pipeline + taskRunSpecs: + - pipelineTaskName: hello-world-1 + metadata: + labels: + TestPrecedenceLabel: PipelineTaskRunSpecValue + annotations: + TestPrecedenceAnnotation: PipelineTaskRunSpecValue + taskServiceAccountName: custom-sa +`)} + ts := []*v1beta1.Task{simpleHelloWorldTask} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + prt := newPipelineRunTest(d, t) + defer prt.Cancel() + + _, clients := prt.reconcileRun("foo", prName, []string{}, false) + + actual := getTaskRunCreations(t, clients.Pipeline.Actions(), 2)[0] + expectedTaskRunObjectMeta := taskRunObjectMeta("test-pipeline-run-hello-world-1", "foo", "test-pipeline-run", "test-pipeline", "hello-world-1", false) + expectedTaskRunObjectMeta.Labels["TestPrecedenceLabel"] = "PipelineTaskRunSpecValue" + expectedTaskRunObjectMeta.Annotations["TestPrecedenceAnnotation"] = "PipelineTaskRunSpecValue" + expectedTaskRun := mustParseTaskRunWithObjectMeta(t, expectedTaskRunObjectMeta, ` +spec: + resources: {} + serviceAccountName: custom-sa + taskSpec: + steps: + - name: foo-step + image: foo-image + timeout: 1h0m0s +`) + + if d := cmp.Diff(actual, expectedTaskRun, ignoreTypeMeta); d != "" { + t.Errorf("expected to see propagated metadata by the precedence from PipelineTaskRunSpec in TaskRun %v created. Diff %s", expectedTaskRun, diff.PrintWantGot(d)) + } +}