Remove Empty Results from RunResult Slice Without Changing Order

[zou2699]

The current implementation for filtering invalid RunResult objects in the r slice removes items by replacing them with the last element and truncating the slice. This approach changes the order of the slice, which may not be desirable in scenarios where order is significant.

pkg/termination/parse.go

func ParseMessage(logger *zap.SugaredLogger, msg string) ([]result.RunResult, error) {
// ...
	for i, rr := range r {
		if rr == (result.RunResult{}) {
			// Erase incorrect result
			r[i] = r[len(r)-1]     // change the order of the slice
			r = r[:len(r)-1]
			logger.Errorf("termination message contains non taskrun or pipelineresource result keys")
		}
	}
// ...
}

{
		msg: `[
		{"key":"foo","value":"first"},
		{},
		{"key":"foo","value":"middle"},
		{"key":"foo","value":"last"}]`,
		want: []result.RunResult{{
			Key:   "foo",
			Value: "last",   // currently, get `middle`,  expecte `last`
		}}
}

Expected Behavior

Implement a method to remove empty results while maintaining the order of elements in the slice.

Actual Behavior

change the order of the slice

Additional Info

• Kubernetes version:

  Output of kubectl version:

Client Version: v1.29.1
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.30.5

• Tekton Pipeline version:

  Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

Client version: 0.39.0
Pipeline version: v0.66.0
Triggers version: v0.30.1
Dashboard version: v0.53.0

[afrittoli]

Thank you @zou2699 for reporting this issue. Since you've done a deep analysis into the code already, would you be interested in contributing a fix as well?

[zou2699]

Thank you @zou2699 for reporting this issue. Since you've done a deep analysis into the code already, would you be interested in contributing a fix as well?

I have already submitted a PR. #8484