From 9b5a6617a6095978d41932f53a7851c6ea955eac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Budai?= <ondrej@budai.cz>
Date: Thu, 25 Mar 2021 08:13:19 +0100
Subject: [PATCH] osbuild1: make the conversion from v2 result stable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A result from manifest v2 contains logs from pipelines. The individual
pipelines are stored as an object, thus they have no order. Well, at least
in Go, because it doesn't guarantee one when parsing maps, see:

https://github.com/golang/go/issues/27179

Unfortunately, this makes the Result.fromV2 method return unpredictable
results because the pipeline results are processed in basically a random
order.

This caused the TestUnmarshalV2Failure test (result_test.go:124) to randomly
fail because it expects the ordering to be stable. I decided to fix this by
ordering the pipeline results by their name. When this fix is applied, the
output from Result.fromV2 is well-defined.

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
---
 internal/osbuild1/result.go | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/internal/osbuild1/result.go b/internal/osbuild1/result.go
index 05f7923d52..3a17b3c063 100644
--- a/internal/osbuild1/result.go
+++ b/internal/osbuild1/result.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"sort"
 
 	"github.com/osbuild/osbuild-composer/internal/osbuild2"
 )
@@ -197,13 +198,42 @@ func (cr *Result) fromV2(crv2 osbuild2.Result) {
 	cr.Build = new(buildResult)
 	cr.Assembler = new(rawAssemblerResult)
 
+	// crv2.Log contains a map of pipelines. Unfortunately, Go doesn't
+	// preserve the order of keys in a map. See:
+	// https://github.com/golang/go/issues/27179
+	//
+	// I think it makes sense for this function to always return
+	// a well-defined output, therefore we need to invent an ordering
+	// for pipeline results. Otherwise, the ordering is basically random.
+	//
+	// The following lines convert the map of pipeline results to an array
+	// of pipeline results. In the last step, the array is sorted by
+	// the pipeline name. This isn't ideal but at least it's predictable.
+	//
+	// See: https://github.com/osbuild/osbuild/issues/619
+	type pipelineResult struct {
+		pipelineName string
+		stageResults []osbuild2.StageResult
+	}
+
+	var pipelineResults []pipelineResult
+
+	for pname, stageResults := range crv2.Log {
+		pipelineResults = append(pipelineResults, pipelineResult{pipelineName: pname, stageResults: stageResults})
+	}
+
+	// Sort the pipelineResult array by the pipeline name to ensure a stable order.
+	sort.Slice(pipelineResults, func(i, j int) bool {
+		return pipelineResults[i].pipelineName < pipelineResults[j].pipelineName
+	})
+
 	// convert all stages logs from all pipelines into v1 StageResult objects
-	for pname, stages := range crv2.Log {
-		for idx, stage := range stages {
+	for _, pr := range pipelineResults {
+		for idx, stage := range pr.stageResults {
 			stageResult := StageResult{
 				// Create uniquely identifiable name for the stage:
 				// <pipeline name>:<stage index>-<stage type>
-				Name:    fmt.Sprintf("%s:%d-%s", pname, idx, stage.Type),
+				Name:    fmt.Sprintf("%s:%d-%s", pr.pipelineName, idx, stage.Type),
 				Success: stage.Success,
 				Output:  stage.Output,
 			}