Sort mixin output in order declared

pkg/build/dockerfile-generator.go

@@ -182,7 +182,7 @@ func (g *DockerfileGenerator) buildMixinsSection(ctx context.Context) ([]string,
 
 	lines := make([]string, 0)
 	for _, result := range results {
-		l := strings.Split(result, "\n")
+		l := strings.Split(result.Stdout, "\n")
 		lines = append(lines, l...)
 	}
 	return lines, nil

pkg/build/dockerfile-generator_test.go

@@ -29,8 +29,8 @@ func TestPorter_buildDockerfile(t *testing.T) {
 	m, err := manifest.LoadManifestFrom(context.Background(), c.Config, config.Name)
 	require.NoError(t, err, "could not load manifest")
 
-	// ignore mixins in the unit tests
-	m.Mixins = []manifest.MixinDeclaration{}
+	// Add another mixin to ensure we are consistently sorting the results
+	m.Mixins = append(m.Mixins, manifest.MixinDeclaration{Name: "testmixin"})
 
 	mp := mixin.NewTestMixinProvider()
 	g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
@@ -65,8 +65,6 @@ COPY mybin /cnab/app/
 `
 		c.TestContext.AddTestFileContents([]byte(customFrom), "Dockerfile.template")
 
-		// ignore mixins in the unit tests
-		m.Mixins = []manifest.MixinDeclaration{}
 		mp := mixin.NewTestMixinProvider()
 		g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
 		gotlines, err := g.buildDockerfile(context.Background())
@@ -98,8 +96,6 @@ COPY mybin /cnab/app/
 `
 		c.TestContext.AddTestFileContents([]byte(customFrom), "Dockerfile.template")
 
-		// ignore mixins in the unit tests
-		m.Mixins = []manifest.MixinDeclaration{}
 		mp := mixin.NewTestMixinProvider()
 		g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
 		gotlines, err := g.buildDockerfile(context.Background())
@@ -128,8 +124,8 @@ func TestPorter_generateDockerfile(t *testing.T) {
 	m, err := manifest.LoadManifestFrom(ctx, c.Config, config.Name)
 	require.NoError(t, err, "could not load manifest")
 
-	// ignore mixins in the unit tests
-	m.Mixins = []manifest.MixinDeclaration{}
+	// Add another mixin to ensure we are consistently sorting the results
+	m.Mixins = append(m.Mixins, manifest.MixinDeclaration{Name: "testmixin"})
 
 	mp := mixin.NewTestMixinProvider()
 	g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
@@ -224,8 +220,6 @@ func TestPorter_buildMixinsSection_mixinErr(t *testing.T) {
 	m, err := manifest.LoadManifestFrom(ctx, c.Config, config.Name)
 	require.NoError(t, err, "could not load manifest")
 
-	m.Mixins = []manifest.MixinDeclaration{{Name: "exec"}}
-
 	mp := mixin.NewTestMixinProvider()
 	mp.ReturnBuildError = true
 	g := NewDockerfileGenerator(c.Config, m, tmpl, mp)

pkg/build/testdata/buildkit.Dockerfile

@@ -11,6 +11,8 @@ RUN rm -f /etc/apt/apt.conf.d/docker-clean; echo 'Binary::apt::APT::Keep-Downloa
 RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt \
     apt-get update && apt-get install -y ca-certificates
 
+# exec mixin has no buildtime dependencies
+# testmixin mixin has no buildtime dependencies
 
 COPY --link . ${BUNDLE_DIR}
 RUN rm ${BUNDLE_DIR}/porter.yaml

pkg/build/testdata/custom-dockerfile-expected-output.Dockerfile

@@ -8,6 +8,7 @@ ARG BUNDLE_GID=0
 RUN useradd ${BUNDLE_USER} -m -u ${BUNDLE_UID} -g ${BUNDLE_GID} -o
 COPY mybin /cnab/app/
 
+# exec mixin has no buildtime dependencies
 RUN rm ${BUNDLE_DIR}/porter.yaml
 RUN rm -fr ${BUNDLE_DIR}/.cnab
 COPY --link .cnab /cnab

pkg/build/testdata/missing-args-expected-output.Dockerfile

@@ -7,6 +7,7 @@ ARG BUNDLE_UID=65532
 ARG BUNDLE_USER=nonroot
 ARG BUNDLE_GID=0
 RUN useradd ${BUNDLE_USER} -m -u ${BUNDLE_UID} -g ${BUNDLE_GID} -o
+# exec mixin has no buildtime dependencies
 RUN rm ${BUNDLE_DIR}/porter.yaml
 RUN rm -fr ${BUNDLE_DIR}/.cnab
 COPY --link .cnab /cnab

pkg/build/testdata/missing-mixins-token-expected-output.Dockerfile

@@ -8,7 +8,6 @@ ARG BUNDLE_USER=nonroot
 ARG BUNDLE_GID=0
 RUN useradd ${BUNDLE_USER} -m -u ${BUNDLE_UID} -g ${BUNDLE_GID} -o
 # exec mixin has no buildtime dependencies
-
 RUN rm ${BUNDLE_DIR}/porter.yaml
 RUN rm -fr ${BUNDLE_DIR}/.cnab
 COPY --link .cnab /cnab

pkg/linter/linter.go

@@ -167,11 +167,19 @@ func (l *Linter) Lint(ctx context.Context, m *manifest.Manifest) (Results, error
 	}
 
 	var results Results
-	for mixin, response := range responses {
+	for _, response := range responses {
+		if response.Error != nil {
+			// Ignore mixins that do not support the lint command
+			if strings.Contains(response.Stdout, "unknown command") {
+				continue
+			}
+			return nil, span.Error(fmt.Errorf("lint command failed for mixin %s: %s", response.Name, response.Stdout))
+		}
+
 		var r Results
-		err = json.Unmarshal([]byte(response), &r)
+		err = json.Unmarshal([]byte(response.Stdout), &r)
 		if err != nil {
-			return nil, span.Error(fmt.Errorf("unable to parse lint response from mixin %q: %w", mixin, err))
+			return nil, span.Error(fmt.Errorf("unable to parse lint response from mixin %s: %w", response.Name, err))
 		}
 
 		results = append(results, r...)

pkg/manifest/manifest_test.go

@@ -429,9 +429,10 @@ func TestMixinDeclaration_UnmarshalYAML(t *testing.T) {
 	m, err := ReadManifest(cxt.Context, config.Name)
 
 	require.NoError(t, err)
-	assert.Len(t, m.Mixins, 2, "expected 2 mixins")
+	assert.Len(t, m.Mixins, 3, "expected 3 mixins")
 	assert.Equal(t, "exec", m.Mixins[0].Name)
 	assert.Equal(t, "az", m.Mixins[1].Name)
+	assert.Equal(t, "terraform", m.Mixins[2].Name)
 	assert.Equal(t, map[string]interface{}{"extensions": []interface{}{"iot"}}, m.Mixins[1].Config)
 }
 
@@ -471,6 +472,7 @@ func TestMixinDeclaration_MarshalYAML(t *testing.T) {
 		[]MixinDeclaration{
 			{Name: "exec"},
 			{Name: "az", Config: map[string]interface{}{"extensions": []interface{}{"iot"}}},
+			{Name: "terraform"},
 		},
 	}
 

pkg/manifest/testdata/mixin-with-config.yaml

@@ -3,3 +3,4 @@ mixins:
   - az:
       extensions:
         - iot
+  - terraform

pkg/mixin/helpers.go

@@ -53,19 +53,19 @@ func NewTestMixinProvider() *TestMixinProvider {
 	}
 
 	provider.RunAssertions = []func(pkgContext *portercontext.Context, name string, commandOpts pkgmgmt.CommandOptions) error{
-		provider.PrintExecOutput,
+		provider.PrintMixinOutput,
 	}
 
 	return &provider
 }
 
-func (p *TestMixinProvider) PrintExecOutput(pkgContext *portercontext.Context, name string, commandOpts pkgmgmt.CommandOptions) error {
+func (p *TestMixinProvider) PrintMixinOutput(pkgContext *portercontext.Context, name string, commandOpts pkgmgmt.CommandOptions) error {
 	switch commandOpts.Command {
 	case "build":
 		if p.ReturnBuildError {
 			return errors.New("encountered build error")
 		}
-		fmt.Fprintln(pkgContext.Out, "# exec mixin has no buildtime dependencies")
+		fmt.Fprintf(pkgContext.Out, "# %s mixin has no buildtime dependencies", name)
 	case "lint":
 		b, _ := json.Marshal(p.LintResults)
 		fmt.Fprintln(pkgContext.Out, string(b))

pkg/mixin/query/query.go

@@ -46,39 +46,48 @@ type MixinInputGenerator interface {
 	BuildInput(mixinName string) ([]byte, error)
 }
 
+type MixinBuildOutput struct {
+	// Name of the mixin.
+	Name string
+
+	// Stdout is the contents of stdout from calling the mixin.
+	Stdout string
+
+	// Error returned when the mixin was called.
+	Error error
+}
+
 // Execute the specified command using an input generator.
 // For example, the ManifestGenerator will iterate over the mixins in a manifest and send
 // them their config and the steps associated with their mixin.
-func (q *MixinQuery) Execute(ctx context.Context, cmd string, inputGenerator MixinInputGenerator) (map[string]string, error) {
+// The mixins are queried in parallel but the results are sorted in the order that the mixins were defined in the manifest.
+func (q *MixinQuery) Execute(ctx context.Context, cmd string, inputGenerator MixinInputGenerator) ([]MixinBuildOutput, error) {
 	ctx, span := tracing.StartSpan(ctx)
 	defer span.EndSpan()
 
 	mixinNames := inputGenerator.ListMixins()
-	results := make(map[string]string, len(mixinNames))
-	type queryResponse struct {
-		mixinName string
-		output    string
-		runErr    error
-	}
-
-	var responses = make(chan queryResponse, len(mixinNames))
+	results := make([]MixinBuildOutput, len(mixinNames))
 	gerr := errgroup.Group{}
 
-	for _, mn := range mixinNames {
-		mixinName := mn // Force mixinName to be in the go routine's closure below
+	for i, mn := range mixinNames {
+		// Force variables to be in the go routine's closure below
+		i := i
+		mn := mn
+		results[i].Name = mn
+
 		gerr.Go(func() error {
 			// Copy the existing context and tweak to pipe the output differently
 			mixinStdout := &bytes.Buffer{}
 			mixinContext := *q.Context
-			mixinContext.Out = mixinStdout // mixin stdout -> mixin response
+			mixinContext.Out = mixinStdout // mixin stdout -> mixin result
 
 			if q.LogMixinErrors {
 				mixinContext.Err = q.Context.Out // mixin stderr -> porter logs
 			} else {
 				mixinContext.Err = io.Discard
 			}
 
-			inputB, err := inputGenerator.BuildInput(mixinName)
+			inputB, err := inputGenerator.BuildInput(mn)
 			if err != nil {
 				return err
 			}
@@ -87,34 +96,29 @@ func (q *MixinQuery) Execute(ctx context.Context, cmd string, inputGenerator Mix
 				Command: cmd,
 				Input:   string(inputB),
 			}
-			runErr := q.Mixins.Run(ctx, &mixinContext, mixinName, cmd)
+			runErr := q.Mixins.Run(ctx, &mixinContext, mn, cmd)
 
-			// Pack the error from running the command in the response so we can
+			// Pack the error from running the command in the result so we can
 			// decide if we care about it, if we returned it normally, the
 			// waitgroup will short circuit immediately on the first error
-			responses <- queryResponse{
-				mixinName: mixinName,
-				output:    mixinStdout.String(),
-				runErr:    runErr,
-			}
+			results[i].Stdout = mixinStdout.String()
+			results[i].Error = runErr
+
 			return nil
 		})
 	}
 
 	err := gerr.Wait()
-	close(responses)
 	if err != nil {
 		return nil, err
 	}
 
 	// Collect responses and errors
 	var runErr error
-	for response := range responses {
-		if response.runErr == nil {
-			results[response.mixinName] = response.output
-		} else {
+	for _, result := range results {
+		if result.Error != nil {
 			runErr = multierror.Append(runErr,
-				fmt.Errorf("error encountered from mixin %q: %w", response.mixinName, response.runErr))
+				fmt.Errorf("error encountered from mixin %q: %w", result.Name, result.Error))
 		}
 	}
 

pkg/pkgmgmt/client/helpers.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"path"
+	"sync"
 	"testing"
 
 	"get.porter.sh/porter/pkg/pkgmgmt"
@@ -20,20 +21,23 @@ type TestPackageManager struct {
 	RunAssertions []func(pkgContext *portercontext.Context, name string, commandOpts pkgmgmt.CommandOptions) error
 
 	// called keeps track of which mixins/plugins were called
-	called map[string]int
+	called sync.Map
+	lock   sync.Mutex
 }
 
 // GetCalled tracks how many times each package was called
-func (p *TestPackageManager) GetCalled() map[string]int {
-	return p.called
+func (p *TestPackageManager) GetCalled(mixin string) int {
+	calls, _ := p.called.Load(mixin)
+	return calls.(int)
 }
 
 func (p *TestPackageManager) recordCalled(name string) {
-	if p.called == nil {
-		p.called = make(map[string]int, 1)
-	}
+	p.lock.Lock()
+	defer p.lock.Unlock()
 
-	p.called[name]++
+	hits, _ := p.called.LoadOrStore(name, 0)
+	hits = hits.(int) + 1
+	p.called.Store(name, hits)
 }
 
 func (p *TestPackageManager) List() ([]string, error) {

pkg/porter/build_test.go

@@ -34,5 +34,5 @@ func TestPorter_GetUsedMixins(t *testing.T) {
 	results, err := p.getUsedMixins(p.RootContext, m)
 	require.NoError(t, err, "getUsedMixins failed")
 	assert.Len(t, results, 1)
-	assert.Equal(t, map[string]int{"exec": 1}, testMixins.GetCalled(), "expected the exec mixin to be called once")
+	assert.Equal(t, 1, testMixins.GetCalled("exec"), "expected the exec mixin to be called once")
 }