From 1155c160c478a739ebade81ad9c9f161fe26d9aa Mon Sep 17 00:00:00 2001 From: joey Date: Mon, 11 Sep 2023 16:13:23 +0800 Subject: [PATCH] 1. add container type substitution expresions to pipeline task result reference 2. propagate results to embedded task spec Part of work on issue #7086 Signed-off-by: chengjoey --- docs/pipelineruns.md | 87 ++++++++ ...ropagating_results_implicit_resultref.yaml | 34 +++ pkg/apis/pipeline/v1/container_types.go | 58 +++++ pkg/apis/pipeline/v1/pipeline_validation.go | 15 ++ pkg/apis/pipeline/v1/resultref.go | 2 + pkg/apis/pipeline/v1/resultref_test.go | 36 +++ pkg/reconciler/pipelinerun/pipelinerun.go | 3 + pkg/reconciler/pipelinerun/resources/apply.go | 21 ++ .../pipelinerun/resources/apply_test.go | 205 ++++++++++++++++++ 9 files changed, 461 insertions(+) create mode 100644 examples/v1/pipelineruns/propagating_results_implicit_resultref.yaml diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 70a6af40278..82377f91905 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -744,6 +744,93 @@ spec: then `test-task` will execute using the `sa-1` account while `build-task` will execute with `sa-for-build`. +#### Propagated Results + +When using an embedded spec, results from the parent `PipelineRun` will be +propagated to any inlined specs without needing to be explicitly defined. This +allows authors to simplify specs by automatically propagating top-level +results down to other inlined resources. +**Result substutions will only be made for `name`, `commands`, `args`, `env` and `script` fields of `steps`, `sidecars`.** + +```yaml +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + name: uid-pipeline-run +spec: + pipelineSpec: + tasks: + - name: add-uid + taskSpec: + results: + - name: uid + type: string + steps: + - name: add-uid + image: busybox + command: ["/bin/sh", "-c"] + args: + - echo "1001" | tee $(results.uid.path) + - name: show-uid + # params: + # - name: uid + # value: $(tasks.add-uid.results.uid) + taskSpec: + steps: + - name: show-uid + image: busybox + command: ["/bin/sh", "-c"] + args: + - echo + # - $(params.uid) + - $(tasks.add-uid.results.uid) +``` + +On executing the pipeline run, the results will be interpolated during resolution. + +```yaml +Name: uid-pipeline-run-show-uid +API Version: tekton.dev/v1 +Kind: TaskRun +Metadata: + ... +Spec: + Task Spec: + Steps: + Args: + echo + 1001 + Command: + /bin/sh + -c + Compute Resources: + Image: busybox + Name: show-uid +Status: + Completion Time: 2023-09-11T07:34:28Z + Conditions: + Last Transition Time: 2023-09-11T07:34:28Z + Message: All Steps have completed executing + Reason: Succeeded + Status: True + Type: Succeeded + Pod Name: uid-pipeline-run-show-uid-pod + Steps: + Container: step-show-uid + Name: show-uid + Task Spec: + Steps: + Args: + echo + 1001 + Command: + /bin/sh + -c + Compute Resources: + Image: busybox + Name: show-uid +``` + ### Specifying a `Pod` template You can specify a [`Pod` template](podtemplates.md) configuration that will serve as the configuration starting diff --git a/examples/v1/pipelineruns/propagating_results_implicit_resultref.yaml b/examples/v1/pipelineruns/propagating_results_implicit_resultref.yaml new file mode 100644 index 00000000000..35c086393c6 --- /dev/null +++ b/examples/v1/pipelineruns/propagating_results_implicit_resultref.yaml @@ -0,0 +1,34 @@ +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: uid-task +spec: + results: + - name: uid + type: string + steps: + - name: uid + image: busybox + command: ["/bin/sh", "-c"] + args: + - echo "1001" | tee $(results.uid.path) +--- +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + name: uid-pipeline-run +spec: + pipelineSpec: + tasks: + - name: add-uid + taskRef: + name: uid-task + - name: show-uid + taskSpec: + steps: + - name: show-uid + image: busybox + command: ["/bin/sh", "-c"] + args: + - echo + - $(tasks.add-uid.results.uid) \ No newline at end of file diff --git a/pkg/apis/pipeline/v1/container_types.go b/pkg/apis/pipeline/v1/container_types.go index 79c9922f462..88cb1e0d668 100644 --- a/pkg/apis/pipeline/v1/container_types.go +++ b/pkg/apis/pipeline/v1/container_types.go @@ -188,6 +188,35 @@ func (s *Step) SetContainerFields(c corev1.Container) { s.SecurityContext = c.SecurityContext } +// GetVarSubstitutionExpressions walks all the places a substitution reference can be used +func (s *Step) GetVarSubstitutionExpressions() ([]string, bool) { + var allExpressions []string + allExpressions = append(allExpressions, validateString(s.Name)...) + allExpressions = append(allExpressions, validateString(s.Image)...) + allExpressions = append(allExpressions, validateString(string(s.ImagePullPolicy))...) + allExpressions = append(allExpressions, validateString(s.Script)...) + for _, cmd := range s.Command { + allExpressions = append(allExpressions, validateString(cmd)...) + } + for _, arg := range s.Args { + allExpressions = append(allExpressions, validateString(arg)...) + } + for _, env := range s.Env { + allExpressions = append(allExpressions, validateString(env.Value)...) + if env.ValueFrom != nil { + if env.ValueFrom.SecretKeyRef != nil { + allExpressions = append(allExpressions, validateString(env.ValueFrom.SecretKeyRef.Key)...) + allExpressions = append(allExpressions, validateString(env.ValueFrom.SecretKeyRef.LocalObjectReference.Name)...) + } + if env.ValueFrom.ConfigMapKeyRef != nil { + allExpressions = append(allExpressions, validateString(env.ValueFrom.ConfigMapKeyRef.Key)...) + allExpressions = append(allExpressions, validateString(env.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name)...) + } + } + } + return allExpressions, len(allExpressions) != 0 +} + // StepTemplate is a template for a Step type StepTemplate struct { // Image reference name. @@ -541,3 +570,32 @@ func (s *Sidecar) SetContainerFields(c corev1.Container) { s.StdinOnce = c.StdinOnce s.TTY = c.TTY } + +// GetVarSubstitutionExpressions walks all the places a substitution reference can be used +func (s *Sidecar) GetVarSubstitutionExpressions() ([]string, bool) { + var allExpressions []string + allExpressions = append(allExpressions, validateString(s.Name)...) + allExpressions = append(allExpressions, validateString(s.Image)...) + allExpressions = append(allExpressions, validateString(string(s.ImagePullPolicy))...) + allExpressions = append(allExpressions, validateString(s.Script)...) + for _, cmd := range s.Command { + allExpressions = append(allExpressions, validateString(cmd)...) + } + for _, arg := range s.Args { + allExpressions = append(allExpressions, validateString(arg)...) + } + for _, env := range s.Env { + allExpressions = append(allExpressions, validateString(env.Value)...) + if env.ValueFrom != nil { + if env.ValueFrom.SecretKeyRef != nil { + allExpressions = append(allExpressions, validateString(env.ValueFrom.SecretKeyRef.Key)...) + allExpressions = append(allExpressions, validateString(env.ValueFrom.SecretKeyRef.LocalObjectReference.Name)...) + } + if env.ValueFrom.ConfigMapKeyRef != nil { + allExpressions = append(allExpressions, validateString(env.ValueFrom.ConfigMapKeyRef.Key)...) + allExpressions = append(allExpressions, validateString(env.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name)...) + } + } + } + return allExpressions, len(allExpressions) != 0 +} diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 97ddd8801b1..8568d1a9cd8 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -497,6 +497,21 @@ func (pt *PipelineTask) extractAllParams() Params { return allParams } +func (pt *PipelineTask) GetVarSubstitutionExpressions() ([]string, bool) { + var allExpressions []string + if pt.TaskSpec != nil { + for _, step := range pt.TaskSpec.Steps { + stepExpressions, _ := step.GetVarSubstitutionExpressions() + allExpressions = append(allExpressions, stepExpressions...) + } + for _, sidecar := range pt.TaskSpec.Sidecars { + sidecarExpressions, _ := sidecar.GetVarSubstitutionExpressions() + allExpressions = append(allExpressions, sidecarExpressions...) + } + } + return allExpressions, len(allExpressions) != 0 +} + func containsExecutionStatusRef(p string) bool { if strings.HasPrefix(p, "tasks.") && strings.HasSuffix(p, ".status") { return true diff --git a/pkg/apis/pipeline/v1/resultref.go b/pkg/apis/pipeline/v1/resultref.go index 5a7610e0325..b9e59b1e2e5 100644 --- a/pkg/apis/pipeline/v1/resultref.go +++ b/pkg/apis/pipeline/v1/resultref.go @@ -215,5 +215,7 @@ func PipelineTaskResultRefs(pt *PipelineTask) []*ResultRef { expressions, _ := whenExpression.GetVarSubstitutionExpressions() refs = append(refs, NewResultRefs(expressions)...) } + taskSubExpressions, _ := pt.GetVarSubstitutionExpressions() + refs = append(refs, NewResultRefs(taskSubExpressions)...) return refs } diff --git a/pkg/apis/pipeline/v1/resultref_test.go b/pkg/apis/pipeline/v1/resultref_test.go index 5db4029405f..dbf1d37ad4a 100644 --- a/pkg/apis/pipeline/v1/resultref_test.go +++ b/pkg/apis/pipeline/v1/resultref_test.go @@ -23,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/selection" ) @@ -650,6 +651,20 @@ func TestPipelineTaskResultRefs(t *testing.T) { }, { Value: *v1.NewStructuredValues("$(tasks.pt7.results.r7)", "$(tasks.pt8.results.r8)"), }}}, + TaskSpec: &v1.EmbeddedTask{ + TaskSpec: v1.TaskSpec{ + Steps: []v1.Step{ + { + Name: "$(tasks.pt8.results.r8)", + Image: "$(tasks.pt9.results.r9)", + ImagePullPolicy: corev1.PullPolicy("$(tasks.pt10.results.r10)"), + Command: []string{"$(tasks.pt11.results.r11)"}, + Args: []string{"$(tasks.pt12.results.r12)", "$(tasks.pt13.results.r13)"}, + Script: "$(tasks.pt14.results.r14)", + }, + }, + }, + }, } refs := v1.PipelineTaskResultRefs(&pt) expectedRefs := []*v1.ResultRef{{ @@ -679,6 +694,27 @@ func TestPipelineTaskResultRefs(t *testing.T) { }, { PipelineTask: "pt9", Result: "r9", + }, { + PipelineTask: "pt8", + Result: "r8", + }, { + PipelineTask: "pt9", + Result: "r9", + }, { + PipelineTask: "pt10", + Result: "r10", + }, { + PipelineTask: "pt11", + Result: "r11", + }, { + PipelineTask: "pt12", + Result: "r12", + }, { + PipelineTask: "pt13", + Result: "r13", + }, { + PipelineTask: "pt14", + Result: "r14", }} if d := cmp.Diff(refs, expectedRefs, cmpopts.SortSlices(lessResultRef)); d != "" { t.Errorf("%v", d) diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index e5f10129ef0..e370e993ba2 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -768,6 +768,9 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1.Pipeline continue } + // propagate previous task results + resources.PropagateResults(rpt, pipelineRunFacts.State) + // Validate parameter types in matrix after apply substitutions from Task Results if rpt.PipelineTask.IsMatrixed() { if err := resources.ValidateParameterTypesInMatrix(pipelineRunFacts.State); err != nil { diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 63171581f2c..f3a026fdf55 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -315,6 +315,27 @@ func propagateParams(t v1.PipelineTask, stringReplacements map[string]string, ar return t } +// PropagateResults propagate the result of the completed task to the unfinished task that is not explicitly specify in the params +func PropagateResults(rpt *ResolvedPipelineTask, runStates PipelineRunState) { + if rpt.ResolvedTask == nil || rpt.ResolvedTask.TaskSpec == nil { + return + } + stringReplacements := map[string]string{} + arrayReplacements := map[string][]string{} + for taskName, taskResults := range runStates.GetTaskRunsResults() { + for _, res := range taskResults { + switch res.Type { + case v1.ResultsTypeString: + stringReplacements[fmt.Sprintf("tasks.%s.results.%s", taskName, res.Name)] = res.Value.StringVal + case v1.ResultsTypeArray: + arrayReplacements[fmt.Sprintf("tasks.%s.results.%s", taskName, res.Name)] = res.Value.ArrayVal + default: + } + } + } + rpt.ResolvedTask.TaskSpec = resources.ApplyReplacements(rpt.ResolvedTask.TaskSpec, stringReplacements, arrayReplacements) +} + // ApplyTaskResultsToPipelineResults applies the results of completed TasksRuns and Runs to a Pipeline's // list of PipelineResults, returning the computed set of PipelineRunResults. References to // non-existent TaskResults or failed TaskRuns or Runs result in a PipelineResult being considered invalid diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index d03998064aa..7c20078ccff 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -26,9 +26,13 @@ import ( v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resources "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" + taskresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/selection" + "knative.dev/pkg/apis" + duckv1 "knative.dev/pkg/apis/duck/v1" ) func TestApplyParameters(t *testing.T) { @@ -4103,3 +4107,204 @@ func TestApplyTaskRunContext(t *testing.T) { t.Fatalf("ApplyTaskRunContext() %s", diff.PrintWantGot(d)) } } + +func TestPropagateResults(t *testing.T) { + for _, tt := range []struct { + name string + resolvedTask *resources.ResolvedPipelineTask + runStates resources.PipelineRunState + expectedResolvedTask *resources.ResolvedPipelineTask + }{ + { + name: "propagate string result", + resolvedTask: &resources.ResolvedPipelineTask{ + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{ + { + Name: "$(tasks.pt1.results.r1)", + Command: []string{"$(tasks.pt1.results.r2)"}, + Args: []string{"$(tasks.pt2.results.r1)"}, + }, + }, + }, + }, + }, + runStates: resources.PipelineRunState{ + { + PipelineTask: &v1.PipelineTask{ + Name: "pt1", + }, + TaskRuns: []*v1.TaskRun{ + { + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{ + { + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }, + }, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{ + { + Name: "r1", + Type: v1.ResultsTypeString, + Value: v1.ResultValue{ + StringVal: "step1", + }, + }, + { + Name: "r2", + Type: v1.ResultsTypeString, + Value: v1.ResultValue{ + StringVal: "echo", + }, + }, + }, + }, + }, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "pt2", + }, + TaskRuns: []*v1.TaskRun{ + { + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{ + { + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }, + }, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{ + { + Name: "r1", + Type: v1.ResultsTypeString, + Value: v1.ResultValue{ + StringVal: "arg1", + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedResolvedTask: &resources.ResolvedPipelineTask{ + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{ + { + Name: "step1", + Command: []string{"echo"}, + Args: []string{"arg1"}, + }, + }, + }, + }, + }, + }, { + name: "propagate array result", + resolvedTask: &resources.ResolvedPipelineTask{ + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{ + { + Command: []string{"$(tasks.pt1.results.r1[*])"}, + Args: []string{"$(tasks.pt2.results.r1[*])"}, + }, + }, + }, + }, + }, + runStates: resources.PipelineRunState{ + { + PipelineTask: &v1.PipelineTask{ + Name: "pt1", + }, + TaskRuns: []*v1.TaskRun{ + { + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{ + { + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }, + }, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{ + { + Name: "r1", + Type: v1.ResultsTypeArray, + Value: v1.ResultValue{ + ArrayVal: []string{"bash", "-c"}, + }, + }, + }, + }, + }, + }, + }, + }, { + PipelineTask: &v1.PipelineTask{ + Name: "pt2", + }, + TaskRuns: []*v1.TaskRun{ + { + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{ + { + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }, + }, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{ + { + Name: "r1", + Type: v1.ResultsTypeArray, + Value: v1.ResultValue{ + ArrayVal: []string{"echo", "arg1"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedResolvedTask: &resources.ResolvedPipelineTask{ + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{ + { + Command: []string{"bash", "-c"}, + Args: []string{"echo", "arg1"}, + }, + }, + }, + }, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + resources.PropagateResults(tt.resolvedTask, tt.runStates) + if d := cmp.Diff(tt.expectedResolvedTask, tt.resolvedTask); d != "" { + t.Fatalf("PropagateResults() %s", diff.PrintWantGot(d)) + } + }) + } +}