From 07e1121004b03689ac420fb64ab684ca095a37cb 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 | 86 +++++ ...ropagating_results_implicit_resultref.yaml | 34 ++ pkg/apis/pipeline/v1/container_types.go | 58 ++++ pkg/apis/pipeline/v1/pipeline_validation.go | 16 + pkg/apis/pipeline/v1/resultref.go | 4 + pkg/apis/pipeline/v1/resultref_test.go | 36 +++ pkg/reconciler/pipelinerun/pipelinerun.go | 3 + pkg/reconciler/pipelinerun/resources/apply.go | 24 ++ .../pipelinerun/resources/apply_test.go | 298 ++++++++++++++++++ 9 files changed, 559 insertions(+) create mode 100644 examples/v1/pipelineruns/propagating_results_implicit_resultref.yaml diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 70a6af40278..a9ebe42f8a1 100644 --- 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 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..908c81e6e1e 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 { + 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 index 71b002d9bcd..15cf1285945 100644 --- 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 index 14b0de17fdb..2de6bb80804 100644 --- 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 index a471418edec..93db4b49255 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 eef81a2e16c..3c960077097 100644 --- 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 index b356947bbbd..75ff4488c0b 100644 --- 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 diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index d03998064aa..9416da2fd02 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,297 @@ 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"}, + }, + }, + }, + }, + }, + }, { + name: "propagate object result", + resolvedTask: &resources.ResolvedPipelineTask{ + ResolvedTask: &taskresources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{ + { + Command: []string{"$(tasks.pt1.results.r1.command1)", "$(tasks.pt1.results.r1.command2)"}, + Args: []string{"$(tasks.pt2.results.r1.arg1)", "$(tasks.pt2.results.r1.arg2)"}, + }, + }, + }, + }, + }, + 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.ResultsTypeObject, + Value: v1.ResultValue{ + ObjectVal: map[string]string{ + "command1": "bash", + "command2": "-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.ResultsTypeObject, + Value: v1.ResultValue{ + ObjectVal: map[string]string{ + "arg1": "echo", + "arg2": "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)) + } + }) + } +}