From 2f6547a8a02811029def935afe87334d8b89ca83 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Wed, 4 May 2022 22:36:45 +0000 Subject: [PATCH] [TEP-0076] Add array support for emitting results This commit provides support for emitting array results via changing TaskRunResult value from string to ArrayorString and add ResultValue for PipelineResourceResult. Previous to this commit we can only emit string type result. --- docs/tasks.md | 52 +++-- .../taskruns/alpha/emit-array-results.yaml | 39 ++++ .../pipeline/v1beta1/openapi_generated.go | 7 +- pkg/apis/pipeline/v1beta1/param_types.go | 44 +++- pkg/apis/pipeline/v1beta1/param_types_test.go | 47 ++++ pkg/apis/pipeline/v1beta1/result_types.go | 6 +- .../pipeline/v1beta1/result_validation.go | 15 +- pkg/apis/pipeline/v1beta1/swagger.json | 4 +- .../pipeline/v1beta1/task_validation_test.go | 28 ++- .../pipeline/v1beta1/zz_generated.deepcopy.go | 5 +- pkg/entrypoint/entrypointer.go | 6 +- pkg/entrypoint/entrypointer_test.go | 86 +++++++ pkg/pod/status.go | 28 ++- pkg/pod/status_test.go | 211 +++++++++++++++++- pkg/reconciler/pipelinerun/resources/apply.go | 4 +- .../pipelinerun/resources/apply_test.go | 18 +- .../resources/pipelinerunresolution_test.go | 4 +- .../resources/pipelinerunstate_test.go | 12 +- .../resources/resultrefresolution.go | 12 +- .../resources/resultrefresolution_test.go | 10 +- pkg/reconciler/taskrun/taskrun.go | 3 +- pkg/termination/parse_test.go | 9 + test/ignore_step_error_test.go | 2 +- 23 files changed, 562 insertions(+), 90 deletions(-) create mode 100644 examples/v1beta1/taskruns/alpha/emit-array-results.yaml diff --git a/docs/tasks.md b/docs/tasks.md index 9e7c84c127c..9d8c6600b09 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -330,7 +330,7 @@ steps: echo "I am supposed to sleep for 60 seconds!" sleep 60 timeout: 5s -``` +``` #### Specifying `onError` for a `step` @@ -450,7 +450,7 @@ Parameter names: For example, `foo.Is-Bar_` is a valid parameter name, but `barIsBa$` or `0banana` are not. -> NOTE: +> NOTE: > 1. Parameter names are **case insensitive**. For example, `APPLE` and `apple` will be treated as equal. If they appear in the same TaskSpec's params, it will be rejected as invalid. > 2. If a parameter name contains dots (.), it must be referenced by using the [bracket notation](#substituting-parameters-and-resources) with either single or double quotes i.e. `$(params['foo.bar'])`, `$(params["foo.bar"])`. See the following example for more information. @@ -684,6 +684,30 @@ or [at the `Pipeline` level](./pipelines.md#configuring-execution-results-at-the **Note:** The maximum size of a `Task's` results is limited by the container termination message feature of Kubernetes, as results are passed back to the controller via this mechanism. At present, the limit is ["4096 bytes"](https://github.com/kubernetes/kubernetes/blob/96e13de777a9eb57f87889072b68ac40467209ac/pkg/kubelet/container/runtime.go#L632). + +**Note:** The result type currently support `string` and `array` (`array` is alpha gated feature), you can write `array` results via JSON escaped format. In the example below, the task specifies one files in the `results` field and write `array` to the file. And `array` is currently supported in Task level not in Pipeline level. + +``` +kind: Task +apiVersion: tekton.dev/v1beta1 +metadata: + name: write-array + annotations: + description: | + A simple task that writes array +spec: + results: + - name: array-results + type: array + description: The array results + steps: + - name: write-array + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "[\"hello\",\"world\"]" | tee $(results.array-results.path) +``` + Results are written to the termination message encoded as JSON objects and Tekton uses those objects to pass additional information to the controller. As such, `Task` results are best suited for holding small amounts of data, such as commit SHAs, branch names, ephemeral namespaces, and so on. @@ -1181,10 +1205,10 @@ log into the `Pod` and add a `Step` that pauses the `Task` at the desired stage. ### Running Step Containers as a Non Root User -All steps that do not require to be run as a root user should make use of TaskRun features to -designate the container for a step runs as a user without root permissions. As a best practice, -running containers as non root should be built into the container image to avoid any possibility -of the container being run as root. However, as a further measure of enforcing this practice, +All steps that do not require to be run as a root user should make use of TaskRun features to +designate the container for a step runs as a user without root permissions. As a best practice, +running containers as non root should be built into the container image to avoid any possibility +of the container being run as root. However, as a further measure of enforcing this practice, steps can make use of a `securityContext` to specify how the container should run. An example of running Task steps as a non root user is shown below: @@ -1228,17 +1252,17 @@ spec: runAsUser: 1001 ``` -In the example above, the step `show-user-2000` specifies via a `securityContext` that the container -for the step should run as user 2000. A `securityContext` must still be specified via a TaskRun `podTemplate` -for this TaskRun to run in a Kubernetes environment that enforces running containers as non root as a requirement. +In the example above, the step `show-user-2000` specifies via a `securityContext` that the container +for the step should run as user 2000. A `securityContext` must still be specified via a TaskRun `podTemplate` +for this TaskRun to run in a Kubernetes environment that enforces running containers as non root as a requirement. -The `runAsNonRoot` property specified via the `podTemplate` above validates that steps part of this TaskRun are -running as non root users and will fail to start any step container that attempts to run as root. Only specifying -`runAsNonRoot: true` will not actually run containers as non root as the property simply validates that steps are not +The `runAsNonRoot` property specified via the `podTemplate` above validates that steps part of this TaskRun are +running as non root users and will fail to start any step container that attempts to run as root. Only specifying +`runAsNonRoot: true` will not actually run containers as non root as the property simply validates that steps are not running as root. It is the `runAsUser` property that is actually used to set the non root user ID for the container. -If a step defines its own `securityContext`, it will be applied for the step container over the `securityContext` -specified at the pod level via the TaskRun `podTemplate`. +If a step defines its own `securityContext`, it will be applied for the step container over the `securityContext` +specified at the pod level via the TaskRun `podTemplate`. More information about Pod and Container Security Contexts can be found via the [Kubernetes website](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-pod). diff --git a/examples/v1beta1/taskruns/alpha/emit-array-results.yaml b/examples/v1beta1/taskruns/alpha/emit-array-results.yaml new file mode 100644 index 00000000000..60ef713445e --- /dev/null +++ b/examples/v1beta1/taskruns/alpha/emit-array-results.yaml @@ -0,0 +1,39 @@ +kind: Task +apiVersion: tekton.dev/v1beta1 +metadata: + name: write-array + annotations: + description: | + A simple task that writes array +spec: + results: + - name: array-results + type: array + description: The array results + steps: + - name: write-array + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "[\"hello\",\"world\"]" | tee $(results.array-results.path) + - name: check-results-array + image: ubuntu + script: | + #!/bin/bash + VALUE=$(cat $(results.array-results.path)) + EXPECTED=[\"hello\",\"world\"] + diff=$(diff <(printf "%s\n" "${VALUE[@]}") <(printf "%s\n" "${EXPECTED[@]}")) + if [[ -z "$diff" ]]; then + echo "TRUE" + else + echo "FALSE" + fi +--- +kind: TaskRun +apiVersion: tekton.dev/v1beta1 +metadata: + name: write-array-tr +spec: + taskRef: + name: write-array + kind: task diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index bc7495422b8..77a5ce8f05f 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -4670,15 +4670,16 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunResult(ref common.ReferenceCallback "value": { SchemaProps: spec.SchemaProps{ Description: "Value the given value of the result", - Default: "", - Type: []string{"string"}, - Format: "", + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString"), }, }, }, Required: []string{"name", "value"}, }, }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString"}, } } diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 4f2c594bc3f..068278264fd 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -141,17 +141,43 @@ type ArrayOrString struct { // UnmarshalJSON implements the json.Unmarshaller interface. func (arrayOrString *ArrayOrString) UnmarshalJSON(value []byte) error { - switch value[0] { - case '[': - arrayOrString.Type = ParamTypeArray - return json.Unmarshal(value, &arrayOrString.ArrayVal) - case '{': - arrayOrString.Type = ParamTypeObject - return json.Unmarshal(value, &arrayOrString.ObjectVal) - default: + // ArrayOrString is used for Results Value as well, the results can be any kind of + // data so we need to check if it is empty. + if len(value) == 0 { arrayOrString.Type = ParamTypeString - return json.Unmarshal(value, &arrayOrString.StringVal) + return nil + } + if value[0] == '[' { + // We're trying to Unmarshal to []string, but for cases like []int or other types + // of nested array which we don't support yet, we should continue and Unmarshal + // it to String. If the Type being set doesn't match what it actually should be, + // it will be captured by validation in reconciler. + // if failed to unmarshal to array, we will convert the value to string and marshal it to string + var a []string + if err := json.Unmarshal(value, &a); err == nil { + arrayOrString.Type = ParamTypeArray + arrayOrString.ArrayVal = a + return nil + } + } + if value[0] == '{' { + // if failed to unmarshal to map, we will convert the value to string and marshal it to string + var m map[string]string + if err := json.Unmarshal(value, &m); err == nil { + arrayOrString.Type = ParamTypeObject + arrayOrString.ObjectVal = m + return nil + } + } + + // By default we unmarshal to string + arrayOrString.Type = ParamTypeString + if err := json.Unmarshal(value, &arrayOrString.StringVal); err == nil { + return nil } + arrayOrString.StringVal = string(value) + + return nil } // MarshalJSON implements the json.Marshaller interface. diff --git a/pkg/apis/pipeline/v1beta1/param_types_test.go b/pkg/apis/pipeline/v1beta1/param_types_test.go index 1ec6610cb0f..b9dedac0100 100644 --- a/pkg/apis/pipeline/v1beta1/param_types_test.go +++ b/pkg/apis/pipeline/v1beta1/param_types_test.go @@ -227,6 +227,10 @@ func TestArrayOrString_UnmarshalJSON(t *testing.T) { input map[string]interface{} result v1beta1.ArrayOrString }{ + { + input: map[string]interface{}{"val": 123}, + result: *v1beta1.NewArrayOrString("123"), + }, { input: map[string]interface{}{"val": "123"}, result: *v1beta1.NewArrayOrString("123"), @@ -282,6 +286,49 @@ func TestArrayOrString_UnmarshalJSON(t *testing.T) { } } +func TestArrayOrString_UnmarshalJSON_Directly(t *testing.T) { + cases := []struct { + desc string + input string + expected v1beta1.ArrayOrString + }{ + {desc: "empty value", input: ``, expected: *v1beta1.NewArrayOrString("")}, + {desc: "int value", input: `1`, expected: *v1beta1.NewArrayOrString("1")}, + {desc: "int array", input: `[1,2,3]`, expected: *v1beta1.NewArrayOrString("[1,2,3]")}, + {desc: "nested array", input: `[1,\"2\",3]`, expected: *v1beta1.NewArrayOrString(`[1,\"2\",3]`)}, + {desc: "string value", input: `hello`, expected: *v1beta1.NewArrayOrString("hello")}, + {desc: "array value", input: `["hello","world"]`, expected: *v1beta1.NewArrayOrString("hello", "world")}, + {desc: "object value", input: `{"hello":"world"}`, expected: *v1beta1.NewObject(map[string]string{"hello": "world"})}, + } + + for _, c := range cases { + aos := v1beta1.ArrayOrString{} + if err := aos.UnmarshalJSON([]byte(c.input)); err != nil { + t.Errorf("Failed to unmarshal input '%v': %v", c.input, err) + } + if !reflect.DeepEqual(aos, c.expected) { + t.Errorf("Failed to unmarshal input '%v': expected %+v, got %+v", c.input, c.expected, aos) + } + } +} + +func TestArrayOrString_UnmarshalJSON_Error(t *testing.T) { + cases := []struct { + desc string + input string + }{ + {desc: "empty value", input: "{\"val\": }"}, + {desc: "wrong beginning value", input: "{\"val\": @}"}, + } + + for _, c := range cases { + var result ArrayOrStringHolder + if err := json.Unmarshal([]byte(c.input), &result); err == nil { + t.Errorf("Should return err but got nil '%v'", c.input) + } + } +} + func TestArrayOrString_MarshalJSON(t *testing.T) { cases := []struct { input v1beta1.ArrayOrString diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index 2848d45949f..a4fd77b26da 100644 --- a/pkg/apis/pipeline/v1beta1/result_types.go +++ b/pkg/apis/pipeline/v1beta1/result_types.go @@ -39,7 +39,7 @@ type TaskRunResult struct { Type ResultsType `json:"type,omitempty"` // Value the given value of the result - Value string `json:"value"` + Value ArrayOrString `json:"value"` } // ResultsType indicates the type of a result; @@ -54,7 +54,9 @@ type ResultsType string // Valid ResultsType: const ( ResultsTypeString ResultsType = "string" + ResultsTypeArray ResultsType = "array" + ResultsTypeObject ResultsType = "object" ) // AllResultsTypes can be used for ResultsTypes validation. -var AllResultsTypes = []ResultsType{ResultsTypeString} +var AllResultsTypes = []ResultsType{ResultsTypeString, ResultsTypeArray, ResultsTypeObject} diff --git a/pkg/apis/pipeline/v1beta1/result_validation.go b/pkg/apis/pipeline/v1beta1/result_validation.go index 4c64b780d16..f8b31857280 100644 --- a/pkg/apis/pipeline/v1beta1/result_validation.go +++ b/pkg/apis/pipeline/v1beta1/result_validation.go @@ -17,22 +17,21 @@ import ( "context" "fmt" + "github.com/tektoncd/pipeline/pkg/apis/config" "knative.dev/pkg/apis" ) // Validate implements apis.Validatable -func (tr TaskResult) Validate(_ context.Context) *apis.FieldError { +func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) { if !resultNameFormatRegex.MatchString(tr.Name) { return apis.ErrInvalidKeyName(tr.Name, "name", fmt.Sprintf("Name must consist of alphanumeric characters, '-', '_', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my-name', or 'my_name', regex used for validation is '%s')", ResultNameFormat)) } - // Validate the result type - validType := false - for _, allowedType := range AllResultsTypes { - if tr.Type == allowedType { - validType = true - } + // Array and Object is alpha feature + if tr.Type == ResultsTypeArray || tr.Type == ResultsTypeObject { + return errs.Also(ValidateEnabledAPIFields(ctx, "results type", config.AlphaAPIFields)) } - if !validType { + + if tr.Type != ResultsTypeString { return apis.ErrInvalidValue(tr.Type, "type", fmt.Sprintf("type must be string")) } diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 9cd98f072b9..8f38a7472f1 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -2605,8 +2605,8 @@ }, "value": { "description": "Value the given value of the result", - "type": "string", - "default": "" + "default": {}, + "$ref": "#/definitions/v1beta1.ArrayOrString" } } }, diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 31f34452865..51165e915f5 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -305,7 +305,7 @@ func TestTaskSpecValidate(t *testing.T) { }}, }, }, { - name: "valid result type", + name: "valid result type string", fields: fields{ Steps: []v1beta1.Step{{ Image: "my-image", @@ -317,6 +317,32 @@ func TestTaskSpecValidate(t *testing.T) { Description: "my great result", }}, }, + }, { + name: "valid result type array", + fields: fields{ + Steps: []v1beta1.Step{{ + Image: "my-image", + Args: []string{"arg"}, + }}, + Results: []v1beta1.TaskResult{{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeArray, + Description: "my great result", + }}, + }, + }, { + name: "valid result type object", + fields: fields{ + Steps: []v1beta1.Step{{ + Image: "my-image", + Args: []string{"arg"}, + }}, + Results: []v1beta1.TaskResult{{ + Name: "MY-RESULT", + Type: v1beta1.ResultsTypeObject, + Description: "my great result", + }}, + }, }, { name: "valid task name context", fields: fields{ diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 70b62ad2027..fc1f9a4ccf1 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1971,6 +1971,7 @@ func (in *TaskRunResources) DeepCopy() *TaskRunResources { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TaskRunResult) DeepCopyInto(out *TaskRunResult) { *out = *in + in.Value.DeepCopyInto(&out.Value) return } @@ -2135,7 +2136,9 @@ func (in *TaskRunStatusFields) DeepCopyInto(out *TaskRunStatusFields) { if in.TaskRunResults != nil { in, out := &in.TaskRunResults, &out.TaskRunResults *out = make([]TaskRunResult, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Sidecars != nil { in, out := &in.Sidecars, &out.Sidecars diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index d4c3877fe65..92a13219a21 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -184,7 +184,7 @@ func (e Entrypointer) Go() error { // strings.Split(..) with an empty string returns an array that contains one element, an empty string. // This creates an error when trying to open the result folder as a file. if len(e.Results) >= 1 && e.Results[0] != "" { - if err := e.readResultsFromDisk(); err != nil { + if err := e.readResultsFromDisk(pipeline.DefaultResultPath); err != nil { logger.Fatalf("Error while handling results: %s", err) } } @@ -192,13 +192,13 @@ func (e Entrypointer) Go() error { return err } -func (e Entrypointer) readResultsFromDisk() error { +func (e Entrypointer) readResultsFromDisk(resultDir string) error { output := []v1beta1.PipelineResourceResult{} for _, resultFile := range e.Results { if resultFile == "" { continue } - fileContents, err := ioutil.ReadFile(filepath.Join(pipeline.DefaultResultPath, resultFile)) + fileContents, err := ioutil.ReadFile(filepath.Join(resultDir, resultFile)) if os.IsNotExist(err) { continue } else if err != nil { diff --git a/pkg/entrypoint/entrypointer_test.go b/pkg/entrypoint/entrypointer_test.go index ec872f56e85..2cc06095d22 100644 --- a/pkg/entrypoint/entrypointer_test.go +++ b/pkg/entrypoint/entrypointer_test.go @@ -31,7 +31,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/termination" "github.com/tektoncd/pipeline/test/diff" + "knative.dev/pkg/logging" ) func TestEntrypointerFailures(t *testing.T) { @@ -249,6 +252,89 @@ func TestEntrypointer(t *testing.T) { } } +func TestReadResultsFromDisk(t *testing.T) { + for _, c := range []struct { + desc string + results []string + resultContent []v1beta1.ArrayOrString + want []v1beta1.PipelineResourceResult + }{{ + desc: "read string result file", + results: []string{"results"}, + resultContent: []v1beta1.ArrayOrString{*v1beta1.NewArrayOrString("hello world")}, + want: []v1beta1.PipelineResourceResult{ + {Value: `"hello world"`, + ResultType: 1}}, + }, { + desc: "read array result file", + results: []string{"results"}, + resultContent: []v1beta1.ArrayOrString{*v1beta1.NewArrayOrString("hello", "world")}, + want: []v1beta1.PipelineResourceResult{ + {Value: `["hello","world"]`, + ResultType: 1}}, + }, { + desc: "read string and array result files", + results: []string{"resultsArray", "resultsString"}, + resultContent: []v1beta1.ArrayOrString{*v1beta1.NewArrayOrString("hello", "world"), *v1beta1.NewArrayOrString("hello world")}, + want: []v1beta1.PipelineResourceResult{ + {Value: `["hello","world"]`, + ResultType: 1}, + {Value: `"hello world"`, + ResultType: 1}, + }, + }, + } { + t.Run(c.desc, func(t *testing.T) { + terminationPath := "termination" + if terminationFile, err := ioutil.TempFile("", "termination"); err != nil { + t.Fatalf("unexpected error creating temporary termination file: %v", err) + } else { + terminationPath = terminationFile.Name() + defer os.Remove(terminationFile.Name()) + } + resultsFilePath := []string{} + for i, r := range c.results { + if resultsFile, err := ioutil.TempFile("", r); err != nil { + t.Fatalf("unexpected error creating temporary termination file: %v", err) + } else { + resultName := resultsFile.Name() + c.want[i].Key = resultName + resultsFilePath = append(resultsFilePath, resultName) + d, err := json.Marshal(c.resultContent[i]) + if err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(resultName, d, 0777); err != nil { + t.Fatal(err) + } + defer os.Remove(resultName) + } + } + + e := Entrypointer{ + Results: resultsFilePath, + TerminationPath: terminationPath, + } + if err := e.readResultsFromDisk(""); err != nil { + t.Fatal(err) + } + msg, err := ioutil.ReadFile(terminationPath) + if err != nil { + t.Fatal(err) + } + logger, _ := logging.NewLogger("", "status") + got, _ := termination.ParseMessage(logger, string(msg)) + for _, g := range got { + aos := v1beta1.ArrayOrString{} + aos.UnmarshalJSON([]byte(g.Value)) + } + if d := cmp.Diff(got, c.want); d != "" { + t.Fatalf("Diff(-want,+got): %v", d) + } + }) + } +} + func TestEntrypointer_ReadBreakpointExitCodeFromDisk(t *testing.T) { expectedExitCode := 1 // setup test diff --git a/pkg/pod/status.go b/pkg/pod/status.go index 082201797a0..68800ff09bf 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -17,6 +17,7 @@ limitations under the License. package pod import ( + "context" "encoding/json" "fmt" "strconv" @@ -24,6 +25,7 @@ import ( "time" "github.com/hashicorp/go-multierror" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/termination" "go.uber.org/zap" @@ -97,7 +99,7 @@ func SidecarsReady(podStatus corev1.PodStatus) bool { } // MakeTaskRunStatus returns a TaskRunStatus based on the Pod's status. -func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev1.Pod) (v1beta1.TaskRunStatus, error) { +func MakeTaskRunStatus(ctx context.Context, logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev1.Pod) (v1beta1.TaskRunStatus, error) { trs := &tr.Status if trs.GetCondition(apis.ConditionSucceeded) == nil || trs.GetCondition(apis.ConditionSucceeded).Status == corev1.ConditionUnknown { // If the taskRunStatus doesn't exist yet, it's because we just started running @@ -129,7 +131,7 @@ func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev } var merr *multierror.Error - if err := setTaskRunStatusBasedOnStepStatus(logger, stepStatuses, &tr); err != nil { + if err := setTaskRunStatusBasedOnStepStatus(ctx, logger, stepStatuses, &tr); err != nil { merr = multierror.Append(merr, err) } @@ -140,7 +142,7 @@ func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev return *trs, merr.ErrorOrNil() } -func setTaskRunStatusBasedOnStepStatus(logger *zap.SugaredLogger, stepStatuses []corev1.ContainerStatus, tr *v1beta1.TaskRun) *multierror.Error { +func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredLogger, stepStatuses []corev1.ContainerStatus, tr *v1beta1.TaskRun) *multierror.Error { trs := &tr.Status var merr *multierror.Error @@ -163,7 +165,7 @@ func setTaskRunStatusBasedOnStepStatus(logger *zap.SugaredLogger, stepStatuses [ logger.Errorf("error extracting the exit code of step %q in taskrun %q: %v", s.Name, tr.Name, err) merr = multierror.Append(merr, err) } - taskResults, pipelineResourceResults, filteredResults := filterResultsAndResources(results) + taskResults, pipelineResourceResults, filteredResults := filterResultsAndResources(ctx, results) if tr.IsSuccessful() { trs.TaskRunResults = append(trs.TaskRunResults, taskResults...) trs.ResourcesResult = append(trs.ResourcesResult, pipelineResourceResults...) @@ -217,16 +219,30 @@ func createMessageFromResults(results []v1beta1.PipelineResourceResult) (string, return string(bytes), nil } -func filterResultsAndResources(results []v1beta1.PipelineResourceResult) ([]v1beta1.TaskRunResult, []v1beta1.PipelineResourceResult, []v1beta1.PipelineResourceResult) { +func filterResultsAndResources(ctx context.Context, results []v1beta1.PipelineResourceResult) ([]v1beta1.TaskRunResult, []v1beta1.PipelineResourceResult, []v1beta1.PipelineResourceResult) { var taskResults []v1beta1.TaskRunResult var pipelineResourceResults []v1beta1.PipelineResourceResult var filteredResults []v1beta1.PipelineResourceResult for _, r := range results { switch r.ResultType { case v1beta1.TaskRunResultType: + cfg := config.FromContextOrDefaults(ctx) + aos := v1beta1.ArrayOrString{} + if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { + // Note that for unsupported types such as []int will be Unmarshalled to String + err := aos.UnmarshalJSON([]byte(r.Value)) + if err != nil { + continue + } + } else { + aos.StringVal = r.Value + aos.Type = v1beta1.ParamTypeString + } + taskRunResult := v1beta1.TaskRunResult{ Name: r.Key, - Value: r.Value, + Type: v1beta1.ResultsType(aos.Type), + Value: aos, } taskResults = append(taskResults, taskRunResult) filteredResults = append(filteredResults, r) diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go index 6cbdb0f85a2..40cf91ceb41 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status_test.go @@ -17,12 +17,14 @@ limitations under the License. package pod import ( + "context" "strings" "testing" "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" @@ -78,9 +80,9 @@ func TestSetTaskRunStatusBasedOnStepStatus(t *testing.T) { }, }, } - + ctx := context.Background() logger, _ := logging.NewLogger("", "status") - merr := setTaskRunStatusBasedOnStepStatus(logger, c.ContainerStatuses, &tr) + merr := setTaskRunStatusBasedOnStepStatus(ctx, logger, c.ContainerStatuses, &tr) if merr != nil { t.Errorf("setTaskRunStatusBasedOnStepStatus: %s", merr) } @@ -629,7 +631,8 @@ func TestMakeTaskRunStatus(t *testing.T) { }}, TaskRunResults: []v1beta1.TaskRunResult{{ Name: "resultName", - Value: "resultValue", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString("resultValue"), }}, // We don't actually care about the time, just that it's not nil CompletionTime: &metav1.Time{Time: time.Now()}, @@ -667,7 +670,8 @@ func TestMakeTaskRunStatus(t *testing.T) { }}, TaskRunResults: []v1beta1.TaskRunResult{{ Name: "resultName", - Value: "resultValue", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString("resultValue"), }}, // We don't actually care about the time, just that it's not nil CompletionTime: &metav1.Time{Time: time.Now()}, @@ -714,10 +718,12 @@ func TestMakeTaskRunStatus(t *testing.T) { Sidecars: []v1beta1.SidecarState{}, TaskRunResults: []v1beta1.TaskRunResult{{ Name: "resultNameOne", - Value: "resultValueThree", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString("resultValueThree"), }, { Name: "resultNameTwo", - Value: "resultValueTwo", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString("resultValueTwo"), }}, // We don't actually care about the time, just that it's not nil CompletionTime: &metav1.Time{Time: time.Now()}, @@ -806,7 +812,8 @@ func TestMakeTaskRunStatus(t *testing.T) { }}, TaskRunResults: []v1beta1.TaskRunResult{{ Name: "resultNameThree", - Value: "", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString(""), }}, // We don't actually care about the time, just that it's not nil CompletionTime: &metav1.Time{Time: time.Now()}, @@ -844,7 +851,8 @@ func TestMakeTaskRunStatus(t *testing.T) { }}, TaskRunResults: []v1beta1.TaskRunResult{{ Name: "resultNameThree", - Value: "", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString(""), }}, // We don't actually care about the time, just that it's not nil CompletionTime: &metav1.Time{Time: time.Now()}, @@ -1014,9 +1022,189 @@ func TestMakeTaskRunStatus(t *testing.T) { }, }, } + ctx := context.Background() + logger, _ := logging.NewLogger("", "status") + got, err := MakeTaskRunStatus(ctx, logger, tr, &c.pod) + if err != nil { + t.Errorf("MakeTaskRunResult: %s", err) + } + // Common traits, set for test case brevity. + c.want.PodName = "pod" + c.want.StartTime = &metav1.Time{Time: startTime} + + ensureTimeNotNil := cmp.Comparer(func(x, y *metav1.Time) bool { + if x == nil { + return y == nil + } + return y != nil + }) + if d := cmp.Diff(c.want, got, ignoreVolatileTime, ensureTimeNotNil); d != "" { + t.Errorf("Diff %s", diff.PrintWantGot(d)) + } + if tr.Status.StartTime.Time != c.want.StartTime.Time { + t.Errorf("Expected TaskRun startTime to be unchanged but was %s", tr.Status.StartTime) + } + }) + } +} + +func TestMakeTaskRunStatusAlpha(t *testing.T) { + ctx := context.Background() + cfg := config.FromContextOrDefaults(ctx) + cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields + ctx = config.ToContext(ctx, cfg) + + for _, c := range []struct { + desc string + podStatus corev1.PodStatus + pod corev1.Pod + want v1beta1.TaskRunStatus + }{{ + desc: "test empty result", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-bar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"", "type":1}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: statusSuccess(), + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}},{"key":"resultName","value":"","type":1}]`, + }}, + Name: "bar", + ContainerName: "step-bar", + }}, + Sidecars: []v1beta1.SidecarState{}, + ResourcesResult: []v1beta1.PipelineResourceResult{{ + Key: "digest", + Value: "sha256:1234", + ResourceRef: &v1beta1.PipelineResourceRef{Name: "source-image"}, + }}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultName", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString(""), + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "test string result", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-bar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"hello", "type":1}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: statusSuccess(), + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}},{"key":"resultName","value":"hello","type":1}]`, + }}, + Name: "bar", + ContainerName: "step-bar", + }}, + Sidecars: []v1beta1.SidecarState{}, + ResourcesResult: []v1beta1.PipelineResourceResult{{ + Key: "digest", + Value: "sha256:1234", + ResourceRef: &v1beta1.PipelineResourceRef{Name: "source-image"}, + }}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultName", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString("hello"), + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }, { + desc: "test array result", + podStatus: corev1.PodStatus{ + Phase: corev1.PodSucceeded, + ContainerStatuses: []corev1.ContainerStatus{{ + Name: "step-bar", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"resultName","value":"[\"hello\",\"world\"]", "type":1}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`, + }, + }, + }}, + }, + want: v1beta1.TaskRunStatus{ + Status: statusSuccess(), + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + Steps: []v1beta1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}},{"key":"resultName","value":"[\"hello\",\"world\"]","type":1}]`, + }}, + Name: "bar", + ContainerName: "step-bar", + }}, + Sidecars: []v1beta1.SidecarState{}, + ResourcesResult: []v1beta1.PipelineResourceResult{{ + Key: "digest", + Value: "sha256:1234", + ResourceRef: &v1beta1.PipelineResourceRef{Name: "source-image"}, + }}, + TaskRunResults: []v1beta1.TaskRunResult{{ + Name: "resultName", + Type: v1beta1.ResultsTypeArray, + Value: *v1beta1.NewArrayOrString("hello", "world"), + }}, + // We don't actually care about the time, just that it's not nil + CompletionTime: &metav1.Time{Time: time.Now()}, + }, + }, + }} { + t.Run(c.desc, func(t *testing.T) { + now := metav1.Now() + if cmp.Diff(c.pod, corev1.Pod{}) == "" { + c.pod = corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod", + Namespace: "foo", + CreationTimestamp: now, + }, + Status: c.podStatus, + } + } + + startTime := time.Date(2010, 1, 1, 1, 1, 1, 1, time.UTC) + tr := v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "task-run", + Namespace: "foo", + }, + Status: v1beta1.TaskRunStatus{ + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: &metav1.Time{Time: startTime}, + }, + }, + } logger, _ := logging.NewLogger("", "status") - got, err := MakeTaskRunStatus(logger, tr, &c.pod) + got, err := MakeTaskRunStatus(ctx, logger, tr, &c.pod) if err != nil { t.Errorf("MakeTaskRunResult: %s", err) } @@ -1039,6 +1227,7 @@ func TestMakeTaskRunStatus(t *testing.T) { } }) } + } func TestMakeRunStatusJSONError(t *testing.T) { @@ -1134,9 +1323,9 @@ func TestMakeRunStatusJSONError(t *testing.T) { Namespace: "foo", }, } - + ctx := context.Background() logger, _ := logging.NewLogger("", "status") - gotTr, err := MakeTaskRunStatus(logger, tr, pod) + gotTr, err := MakeTaskRunStatus(ctx, logger, tr, pod) if err == nil { t.Error("Expected error, got nil") } diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 4187b915a84..089b5fc7bf8 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -182,6 +182,8 @@ func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]st // and omitted from the returned slice. A nil slice is returned if no results are passed in or all // results are invalid. func ApplyTaskResultsToPipelineResults( + // TODO(#4723): Change PipelineResult Value to support array, so we can update + // PipelineResults, right now we only update the StringVal from TaskResults to PipelineResults results []v1beta1.PipelineResult, taskRunResults map[string][]v1beta1.TaskRunResult, customTaskResults map[string][]v1alpha1.RunResult) []v1beta1.PipelineRunResult { @@ -231,7 +233,7 @@ func ApplyTaskResultsToPipelineResults( func taskResultValue(taskName string, resultName string, taskResults map[string][]v1beta1.TaskRunResult) *string { for _, trResult := range taskResults[taskName] { if trResult.Name == resultName { - return &trResult.Value + return &trResult.Value.StringVal } } return nil diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index 8dd15896080..495f39b9483 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -791,7 +791,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": {{ Name: "foo", - Value: "bar", + Value: *v1beta1.NewArrayOrString("bar"), }}, }, expected: nil, @@ -804,7 +804,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": {{ Name: "foo", - Value: "bar", + Value: *v1beta1.NewArrayOrString("bar"), }}, }, expected: nil, @@ -827,7 +827,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { taskResults: map[string][]v1beta1.TaskRunResult{ "definitely-not-pt1": {{ Name: "foo", - Value: "bar", + Value: *v1beta1.NewArrayOrString("bar"), }}, }, expected: nil, @@ -840,7 +840,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": {{ Name: "definitely-not-foo", - Value: "bar", + Value: *v1beta1.NewArrayOrString("bar"), }}, }, expected: nil, @@ -864,7 +864,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { taskResults: map[string][]v1beta1.TaskRunResult{ "pt2": {{ Name: "bar", - Value: "rae", + Value: *v1beta1.NewArrayOrString("rae"), }}, }, expected: []v1beta1.PipelineRunResult{{ @@ -884,15 +884,15 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { "pt1": { { Name: "foo", - Value: "do", + Value: *v1beta1.NewArrayOrString("do"), }, { Name: "bar", - Value: "mi", + Value: *v1beta1.NewArrayOrString("mi"), }, }, "pt2": {{ Name: "baz", - Value: "rae", + Value: *v1beta1.NewArrayOrString("rae"), }}, }, expected: []v1beta1.PipelineRunResult{{ @@ -969,7 +969,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { taskResults: map[string][]v1beta1.TaskRunResult{ "normaltask": {{ Name: "baz", - Value: "rae", + Value: *v1beta1.NewArrayOrString("rae"), }}, }, expected: []v1beta1.PipelineRunResult{{ diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index d2551a214f8..0c38ba4af19 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -3250,7 +3250,7 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "commit", - Value: "SHA2", + Value: *v1beta1.NewArrayOrString("SHA2"), }}, }, }, @@ -3383,7 +3383,7 @@ func TestResolvedPipelineRunTask_IsFinalTask(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "commit", - Value: "SHA2", + Value: *v1beta1.NewArrayOrString("SHA2"), }}, }, }, diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index 74c05999566..ddb480143ca 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -2426,10 +2426,10 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "foo", - Value: "oof", + Value: *v1beta1.NewArrayOrString("oof"), }, { Name: "bar", - Value: "rab", + Value: *v1beta1.NewArrayOrString("rab"), }}, }, }, @@ -2462,7 +2462,7 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "fail-foo", - Value: "fail-oof", + Value: *v1beta1.NewArrayOrString("fail-oof"), }}, }, }, @@ -2481,7 +2481,7 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "unknown-foo", - Value: "unknown-oof", + Value: *v1beta1.NewArrayOrString("unknown-oof"), }}, }, }, @@ -2578,10 +2578,10 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { expectedTaskResults := map[string][]v1beta1.TaskRunResult{ "successful-task-with-results-1": {{ Name: "foo", - Value: "oof", + Value: *v1beta1.NewArrayOrString("oof"), }, { Name: "bar", - Value: "rab", + Value: *v1beta1.NewArrayOrString("rab"), }}, "successful-task-without-results-1": nil, } diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index 641355415b6..bd82529e488 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -138,11 +138,13 @@ func resolveResultRef(pipelineState PipelineRunState, resultRef *v1beta1.ResultR return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not successful", referencedPipelineTask.PipelineTask.Name) } - var runName, taskRunName, resultValue string + var runName, runValue, taskRunName string + var resultValue v1beta1.ArrayOrString var err error if referencedPipelineTask.IsCustomTask() { runName = referencedPipelineTask.Run.Name - resultValue, err = findRunResultForParam(referencedPipelineTask.Run, resultRef) + runValue, err = findRunResultForParam(referencedPipelineTask.Run, resultRef) + resultValue = *v1beta1.NewArrayOrString(runValue) if err != nil { return nil, resultRef.PipelineTask, err } @@ -155,7 +157,7 @@ func resolveResultRef(pipelineState PipelineRunState, resultRef *v1beta1.ResultR } return &ResolvedResultRef{ - Value: *v1beta1.NewArrayOrString(resultValue), + Value: resultValue, FromTaskRun: taskRunName, FromRun: runName, ResultReference: *resultRef, @@ -172,14 +174,14 @@ func findRunResultForParam(run *v1alpha1.Run, reference *v1beta1.ResultRef) (str return "", fmt.Errorf("Could not find result with name %s for task %s", reference.Result, reference.PipelineTask) } -func findTaskResultForParam(taskRun *v1beta1.TaskRun, reference *v1beta1.ResultRef) (string, error) { +func findTaskResultForParam(taskRun *v1beta1.TaskRun, reference *v1beta1.ResultRef) (v1beta1.ArrayOrString, error) { results := taskRun.Status.TaskRunStatusFields.TaskRunResults for _, result := range results { if result.Name == reference.Result { return result.Value, nil } } - return "", fmt.Errorf("Could not find result with name %s for task %s", reference.Result, reference.PipelineTask) + return v1beta1.ArrayOrString{}, fmt.Errorf("Could not find result with name %s for task %s", reference.Result, reference.PipelineTask) } func (rs ResolvedResultRefs) getStringReplacements() map[string]string { diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go index 810ab509cde..327585cfd7f 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go @@ -43,7 +43,7 @@ var pipelineRunState = PipelineRunState{{ TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "aResult", - Value: "aResultValue", + Value: *v1beta1.NewArrayOrString("aResultValue"), }}, }, }, @@ -159,7 +159,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "aResult", - Value: "aResultValue", + Value: *v1beta1.NewArrayOrString("aResultValue"), }}, }, }, @@ -195,7 +195,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "aResult", - Value: "aResultValue", + Value: *v1beta1.NewArrayOrString("aResultValue"), }}, }, }, @@ -215,7 +215,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "bResult", - Value: "bResultValue", + Value: *v1beta1.NewArrayOrString("bResultValue"), }}, }, }, @@ -258,7 +258,7 @@ func TestTaskParamResolver_ResolveResultRefs(t *testing.T) { TaskRunStatusFields: v1beta1.TaskRunStatusFields{ TaskRunResults: []v1beta1.TaskRunResult{{ Name: "aResult", - Value: "aResultValue", + Value: *v1beta1.NewArrayOrString("aResultValue"), }}, }, }, diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 119959f8ec5..ac1efc68adc 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -455,10 +455,11 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, rtr *re } // Convert the Pod's status to the equivalent TaskRun Status. - tr.Status, err = podconvert.MakeTaskRunStatus(logger, *tr, pod) + tr.Status, err = podconvert.MakeTaskRunStatus(ctx, logger, *tr, pod) if err != nil { return err } + // TODO(#4723): Validate the taskrun results against taskresults for object val logger.Infof("Successfully reconciled taskrun %s/%s with status: %#v", tr.Name, tr.Namespace, tr.Status.GetCondition(apis.ConditionSucceeded)) return nil diff --git a/pkg/termination/parse_test.go b/pkg/termination/parse_test.go index dae81ac510c..e7aa7d8f9d2 100644 --- a/pkg/termination/parse_test.go +++ b/pkg/termination/parse_test.go @@ -39,6 +39,15 @@ func TestParseMessage(t *testing.T) { Key: "foo", Value: "bar", }}, + }, { + desc: "invalid key in message", + msg: `[{"invalid": "digest","value":"hereisthedigest"},{"key":"foo","value":"bar"}]`, + want: []v1beta1.PipelineResourceResult{{ + Value: "hereisthedigest", + }, { + Key: "foo", + Value: "bar", + }}, }, { desc: "empty message", msg: "", diff --git a/test/ignore_step_error_test.go b/test/ignore_step_error_test.go index 9229643284b..bc77fdcc8f0 100644 --- a/test/ignore_step_error_test.go +++ b/test/ignore_step_error_test.go @@ -100,7 +100,7 @@ spec: } for _, r := range taskrunItem.Status.TaskRunResults { - if r.Name == "result1" && r.Value != "123" { + if r.Name == "result1" && r.Value.StringVal != "123" { t.Fatalf("task1 should have initialized a result \"result1\" to \"123\"") } }