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 20, 2023 · 98f857f · 98f857f
1 parent c52f937
commit 98f857f
Show file tree

Hide file tree

Showing 12 changed files with 1,951 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,36 @@ 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)...)
+	allExpressions = append(allExpressions, validateString(s.WorkingDir)...)
+	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 +571,33 @@ 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)...)
+	allExpressions = append(allExpressions, validateString(s.WorkingDir)...)
+	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/container_types_test.go b/pkg/apis/pipeline/v1/container_types_test.go
@@ -0,0 +1,122 @@
+/*
+ *
+ * Copyright 2019 The Tekton Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * /
+ */
+
+package v1
+
+import (
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestStepGetVarSubstitutionExpressions(t *testing.T) {
+	s := Step{
+		Name:            "$(tasks.task1.results.stepName)",
+		Image:           "$(tasks.task1.results.imageResult)",
+		ImagePullPolicy: corev1.PullPolicy("$(tasks.task1.results.imagePullPolicy)"),
+		Script:          "substitution within string $(tasks.task1.results.scriptResult)",
+		WorkingDir:      "$(tasks.task1.results.workingDir)",
+		Command: []string{
+			"$(tasks.task2.results.command[*])",
+		},
+		Args: []string{
+			"$(tasks.task2.results.args[*])",
+		},
+		Env: []corev1.EnvVar{
+			{
+				Name:  "env1",
+				Value: "$(tasks.task2.results.env1)",
+				ValueFrom: &corev1.EnvVarSource{
+					SecretKeyRef: &corev1.SecretKeySelector{
+						Key: "$(tasks.task2.results.secretKeyRef)",
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: "$(tasks.task2.results.secretNameRef)",
+						},
+					},
+					ConfigMapKeyRef: &corev1.ConfigMapKeySelector{
+						Key: "$(tasks.task2.results.configMapKeyRef)",
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: "$(tasks.task2.results.configMapNameRef)",
+						},
+					},
+				},
+			},
+		},
+	}
+	subRefExpressions := s.GetVarSubstitutionExpressions()
+	wantRefExpressions := []string{
+		"tasks.task1.results.stepName",
+		"tasks.task1.results.imageResult",
+		"tasks.task1.results.imagePullPolicy",
+		"tasks.task1.results.scriptResult",
+		"tasks.task1.results.workingDir",
+		"tasks.task2.results.command[*]",
+		"tasks.task2.results.args[*]",
+		"tasks.task2.results.env1",
+		"tasks.task2.results.secretKeyRef",
+		"tasks.task2.results.secretNameRef",
+		"tasks.task2.results.configMapKeyRef",
+		"tasks.task2.results.configMapNameRef",
+	}
+	if d := cmp.Diff(wantRefExpressions, subRefExpressions); d != "" {
+		t.Fatalf("Unexpected result (-want, +got): %s", d)
+	}
+}
+
+func TestSidecarGetVarSubstitutionExpressions(t *testing.T) {
+	s := Sidecar{
+		Name:            "$(tasks.task1.results.sidecarName)",
+		Image:           "$(tasks.task1.results.sidecarImage)",
+		ImagePullPolicy: corev1.PullPolicy("$(tasks.task1.results.sidecarImagePullPolicy)"),
+		Env: []corev1.EnvVar{
+			{
+				Name:  "env1",
+				Value: "$(tasks.task2.results.sidecarEnv1)",
+				ValueFrom: &corev1.EnvVarSource{
+					SecretKeyRef: &corev1.SecretKeySelector{
+						Key: "$(tasks.task2.results.sidecarSecretKeyRef)",
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: "$(tasks.task2.results.sidecarSecretNameRef)",
+						},
+					},
+					ConfigMapKeyRef: &corev1.ConfigMapKeySelector{
+						Key: "$(tasks.task2.results.sidecarConfigMapKeyRef)",
+						LocalObjectReference: corev1.LocalObjectReference{
+							Name: "$(tasks.task2.results.sidecarConfigMapNameRef)",
+						},
+					},
+				},
+			},
+		},
+	}
+	subRefExpressions := s.GetVarSubstitutionExpressions()
+	wantRefExpressions := []string{
+		"tasks.task1.results.sidecarName",
+		"tasks.task1.results.sidecarImage",
+		"tasks.task1.results.sidecarImagePullPolicy",
+		"tasks.task2.results.sidecarEnv1",
+		"tasks.task2.results.sidecarSecretKeyRef",
+		"tasks.task2.results.sidecarSecretNameRef",
+		"tasks.task2.results.sidecarConfigMapKeyRef",
+		"tasks.task2.results.sidecarConfigMapNameRef",
+	}
+	if d := cmp.Diff(wantRefExpressions, subRefExpressions); d != "" {
+		t.Fatalf("Unexpected result (-want, +got): %s", d)
+	}
+}
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
 }