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 <[email protected]>
tektoncd · Oct 7, 2023 · 07e1121 · 07e1121
1 parent 5dd04bf
commit 07e1121
Show file tree

Hide file tree

Showing 9 changed files with 559 additions and 0 deletions.
diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md
@@ -744,6 +744,92 @@ 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` substitutions 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 `PipelineRun`, the `Results` will be interpolated during resolution.
+
+```yaml
+name:         uid-pipeline-run-show-uid
+apiVersion:  tekton.dev/v1
+kind:         TaskRun
+metadata:
+  ...
+spec:
+  taskSpec:
+    steps:
+      args:
+        echo
+        1001
+      command:
+        - /bin/sh
+        - -c
+      image:  busybox
+      name:   show-uid
+status:
+  completionTime:  2023-09-11T07:34:28Z
+  conditions:
+    lastTransitionTime:  2023-09-11T07:34:28Z
+    message:               All Steps have completed executing
+    reason:                Succeeded
+    status:                True
+    type:                  Succeeded
+  podName:                uid-pipeline-run-show-uid-pod
+  steps:
+    container:  step-show-uid
+    name:       show-uid
+  taskSpec:
+    steps:
+      args:
+        echo
+        1001
+      command:
+        /bin/sh
+        -c
+      computeResources:
+      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
@@ -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)
diff --git 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 {
+	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
+}
+
 // 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 {
+	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
+}
diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go
@@ -497,6 +497,22 @@ func (pt *PipelineTask) extractAllParams() Params {
 	return allParams
 }
 
+// GetVarSubstitutionExpressions extract all values between the parameters "$(" and ")" of steps and sidecars
+func (pt *PipelineTask) GetVarSubstitutionExpressions() []string {
+	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
+}
+
 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
@@ -172,6 +172,8 @@ func ParseResultName(resultName string) (string, string) {
 // in a PipelineTask and returns a list of any references that are found.
 func PipelineTaskResultRefs(pt *PipelineTask) []*ResultRef {
 	refs := []*ResultRef{}
+	// TODO move the whenExpression.GetVarSubstitutionExpressions() and GetVarSubstitutionExpressionsForParam(p) as well
+	// separate cleanup, reference https://github.com/tektoncd/pipeline/pull/7121
 	for _, p := range pt.extractAllParams() {
 		expressions, _ := p.GetVarSubstitutionExpressions()
 		refs = append(refs, NewResultRefs(expressions)...)
@@ -180,5 +182,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
@@ -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
@@ -769,6 +769,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
@@ -315,6 +315,30 @@ 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
+			case v1.ResultsTypeObject:
+				for k, v := range res.Value.ObjectVal {
+					stringReplacements[fmt.Sprintf("tasks.%s.results.%s.%s", taskName, res.Name, k)] = v
+				}
+			}
+		}
+	}
+	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