From 9a1faabea92753c78b6b1857478fff0b23a89011 Mon Sep 17 00:00:00 2001
From: Antonin Stefanutti <antonin@stefanutti.fr>
Date: Fri, 29 Jan 2021 18:05:03 +0100
Subject: [PATCH] chore(build): Sort builder steps by phase in Build spec

---
 pkg/trait/builder.go      | 29 +++++++++++++++++++----------
 pkg/trait/quarkus.go      |  5 ++---
 pkg/trait/quarkus_test.go | 13 ++++++-------
 3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/pkg/trait/builder.go b/pkg/trait/builder.go
index 3fa45d784e..1c1c4df7b1 100644
--- a/pkg/trait/builder.go
+++ b/pkg/trait/builder.go
@@ -20,10 +20,10 @@ package trait
 import (
 	"fmt"
 	"path"
+	"sort"
 	"strconv"
 	"strings"
 
-	"github.com/apache/camel-k/pkg/builder/spectrum"
 	"github.com/pkg/errors"
 
 	corev1 "k8s.io/api/core/v1"
@@ -34,6 +34,7 @@ import (
 	"github.com/apache/camel-k/pkg/builder"
 	"github.com/apache/camel-k/pkg/builder/kaniko"
 	"github.com/apache/camel-k/pkg/builder/s2i"
+	"github.com/apache/camel-k/pkg/builder/spectrum"
 	"github.com/apache/camel-k/pkg/util/defaults"
 )
 
@@ -179,25 +180,33 @@ func (t *builderTrait) builderTask(e *Environment) *v1.BuilderTask {
 		BaseImage:    e.Platform.Status.Build.BaseImage,
 		Runtime:      e.CamelCatalog.Runtime,
 		Dependencies: e.IntegrationKit.Spec.Dependencies,
-		//TODO: sort steps for easier read
-		Steps:      builder.StepIDsFor(builder.DefaultSteps...),
-		Properties: e.Platform.Status.Build.Properties,
-		Timeout:    e.Platform.Status.Build.GetTimeout(),
-		Maven:      e.Platform.Status.Build.Maven,
+		Properties:   e.Platform.Status.Build.Properties,
+		Timeout:      e.Platform.Status.Build.GetTimeout(),
+		Maven:        e.Platform.Status.Build.Maven,
 	}
 
+	steps := make([]builder.Step, 0)
+	steps = append(steps, builder.DefaultSteps...)
+
 	switch e.Platform.Status.Build.PublishStrategy {
 	case v1.IntegrationPlatformBuildPublishStrategyBuildah, v1.IntegrationPlatformBuildPublishStrategyKaniko:
 		task.BuildDir = path.Join(builderDir, e.IntegrationKit.Name)
 
 	case v1.IntegrationPlatformBuildPublishStrategyS2I:
-		task.Steps = append(task.Steps, builder.StepIDsFor(s2i.S2iSteps...)...)
+		steps = append(steps, s2i.S2iSteps...)
 	case v1.IntegrationPlatformBuildPublishStrategySpectrum:
-		task.Steps = append(task.Steps, builder.StepIDsFor(spectrum.SpectrumSteps...)...)
+		steps = append(steps, spectrum.SpectrumSteps...)
 	}
 
 	quarkus := e.Catalog.GetTrait("quarkus").(*quarkusTrait)
-	quarkus.addBuildSteps(task)
+	quarkus.addBuildSteps(&steps)
+
+	// sort steps by phase
+	sort.SliceStable(steps, func(i, j int) bool {
+		return steps[i].Phase() < steps[j].Phase()
+	})
+
+	task.Steps = builder.StepIDsFor(steps...)
 
 	return task
 }
@@ -241,7 +250,7 @@ func (t *builderTrait) buildahTask(e *Environment) (*v1.ImageTask, error) {
 		}
 		mountRegistryConfigMap(e.Platform.Status.Build.Registry.CA, config, &volumes, &volumeMounts)
 		// This is easier to use the --cert-dir option, otherwise Buildah defaults to looking up certificates
-		//into a directory named after the registry address
+		// into a directory named after the registry address
 		bud = append(bud[:2], append([]string{"--cert-dir=/etc/containers/certs.d"}, bud[2:]...)...)
 		push = append(push[:2], append([]string{"--cert-dir=/etc/containers/certs.d"}, push[2:]...)...)
 	}
diff --git a/pkg/trait/quarkus.go b/pkg/trait/quarkus.go
index ea2b3d5128..4227221f59 100644
--- a/pkg/trait/quarkus.go
+++ b/pkg/trait/quarkus.go
@@ -18,7 +18,6 @@ limitations under the License.
 package trait
 
 import (
-	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
 	"github.com/apache/camel-k/pkg/builder"
 	"github.com/apache/camel-k/pkg/builder/runtime"
 )
@@ -62,6 +61,6 @@ func (t *quarkusTrait) InfluencesKit() bool {
 	return true
 }
 
-func (t *quarkusTrait) addBuildSteps(task *v1.BuilderTask) {
-	task.Steps = append(task.Steps, builder.StepIDsFor(runtime.QuarkusSteps...)...)
+func (t *quarkusTrait) addBuildSteps(steps *[]builder.Step) {
+	*steps = append(*steps, runtime.QuarkusSteps...)
 }
diff --git a/pkg/trait/quarkus_test.go b/pkg/trait/quarkus_test.go
index 9ee8ae9abd..9061f574fb 100644
--- a/pkg/trait/quarkus_test.go
+++ b/pkg/trait/quarkus_test.go
@@ -20,11 +20,11 @@ package trait
 import (
 	"testing"
 
-	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+	"github.com/stretchr/testify/assert"
 
+	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
 	"github.com/apache/camel-k/pkg/builder"
 	"github.com/apache/camel-k/pkg/util/camel"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestConfigureQuarkusTraitShouldSucceed(t *testing.T) {
@@ -57,13 +57,12 @@ func TestApplyQuarkusTraitDoesNothing(t *testing.T) {
 func TestQuarkusTraitAddBuildStepsShouldSucceed(t *testing.T) {
 	quarkusTrait, _ := createNominalQuarkusTest()
 
-	task := &v1.BuilderTask{
-		Steps: builder.StepIDsFor(builder.DefaultSteps...),
-	}
+	steps := make([]builder.Step, 0)
+	steps = append(steps, builder.DefaultSteps...)
 
-	quarkusTrait.addBuildSteps(task)
+	quarkusTrait.addBuildSteps(&steps)
 
-	assert.Len(t, task.Steps, 9)
+	assert.Len(t, steps, 9)
 }
 
 func createNominalQuarkusTest() (*quarkusTrait, *Environment) {