pipeline.yaml shell command changes from array to string (#923)

this change follows MSFTs pipeline.yaml format change. the `command` is executed as a shell script in the bash context. Signed-off-by: Gerd Oberlechner <[email protected]>
Azure · Dec 4, 2024 · aa74e90 · aa74e90
1 parent ab04e11
commit aa74e90
Show file tree

Hide file tree

Showing 14 changed files with 102 additions and 76 deletions.
diff --git a/backend/pipeline.yaml b/backend/pipeline.yaml
@@ -7,7 +7,7 @@ resourceGroups:
   steps:
   - name: deploy
     action: Shell
-    command: ["make", "deploy"]
+    command: make deploy
     env:
     - name: ARO_HCP_IMAGE_ACR
       configRef: svcAcrName

diff --git a/dev-infrastructure/mgmt-pipeline.yaml b/dev-infrastructure/mgmt-pipeline.yaml
@@ -22,7 +22,7 @@ resourceGroups:
       output: cs.msi.resourceID
   - name: enable-metrics
     action: Shell
-    command: ["scripts/enable-aks-metrics.sh"]
+    command: scripts/enable-aks-metrics.sh
     env:
     - name: RESOURCEGROUP
       configRef: mgmt.rg

diff --git a/dev-infrastructure/svc-pipeline.yaml b/dev-infrastructure/svc-pipeline.yaml
@@ -11,7 +11,7 @@ resourceGroups:
     parameters: configurations/svc-cluster.tmpl.bicepparam
   - name: enable-metrics
     action: Shell
-    command: ["scripts/enable-aks-metrics.sh"]
+    command: scripts/enable-aks-metrics.sh
     env:
     - name: RESOURCEGROUP
       configRef: svc.rg

diff --git a/frontend/pipeline.yaml b/frontend/pipeline.yaml
@@ -7,7 +7,7 @@ resourceGroups:
   steps:
   - name: deploy
     action: Shell
-    command: ["make", "deploy"]
+    command: make deploy
     dryRun:
       envVars:
         - name: HELM_DRY_RUN

diff --git a/maestro/server/pipeline.yaml b/maestro/server/pipeline.yaml
@@ -5,15 +5,9 @@ resourceGroups:
   subscription: {{ .svc.subscription }}
   aksCluster: {{ .aksName }}
   steps:
-  #- name: bicepTest
-  #  action: ARM
-  #  template: test.bicep
-  #  parameters: test.bicepparam
   - name: deploy
     action: Shell
-    command: ["make", "deploy"]
-    # pwd is inferred from the location of this file
-    # so all path references in the command are relativ to the files location
+    command: make deploy
     env:
     - name: EVENTGRID_NAME
       configRef: maestro.eventGrid.name

diff --git a/tooling/templatize/internal/end2end/e2e_test.go b/tooling/templatize/internal/end2end/e2e_test.go
@@ -38,7 +38,7 @@ func TestE2EMake(t *testing.T) {
 	e2eImpl.AddStep(pipeline.Step{
 		Name:    "test",
 		Action:  "Shell",
-		Command: []string{"make", "test"},
+		Command: "make test",
 		Env: []pipeline.EnvVar{
 			{
 				Name:      "TEST_ENV",
@@ -71,7 +71,7 @@ func TestE2EKubernetes(t *testing.T) {
 	e2eImpl.AddStep(pipeline.Step{
 		Name:    "test",
 		Action:  "Shell",
-		Command: []string{"kubectl", "get", "namespaces"},
+		Command: "kubectl get namespaces",
 	})
 	e2eImpl.SetAKSName("aro-hcp-aks")
 

diff --git a/tooling/templatize/pkg/pipeline/run.go b/tooling/templatize/pkg/pipeline/run.go
@@ -153,7 +153,7 @@ func (s *Step) description() string {
 	var details []string
 	switch s.Action {
 	case "Shell":
-		details = append(details, fmt.Sprintf("Command: %v", strings.Join(s.Command, " ")))
+		details = append(details, fmt.Sprintf("Command: %s", s.Command))
 	case "ARM":
 		details = append(details, fmt.Sprintf("Template: %s", s.Template))
 		details = append(details, fmt.Sprintf("Parameters: %s", s.Parameters))

diff --git a/tooling/templatize/pkg/pipeline/run_test.go b/tooling/templatize/pkg/pipeline/run_test.go
@@ -12,7 +12,7 @@ func TestStepRun(t *testing.T) {
 	s := &Step{
 		Name:    "test",
 		Action:  "Shell",
-		Command: []string{"echo", "hello"},
+		Command: "echo hello",
 		outputFunc: func(output string) {
 			fooundOutput = output
 		},
@@ -90,7 +90,7 @@ func TestResourceGroupRun(t *testing.T) {
 			{
 				Name:    "step",
 				Action:  "Shell",
-				Command: []string{"echo", "hello"},
+				Command: "echo hello",
 				outputFunc: func(output string) {
 					foundOutput = output
 				},
@@ -109,31 +109,31 @@ func TestResourceGroupError(t *testing.T) {
 			{
 				Name:    "step",
 				Action:  "Shell",
-				Command: []string{"echo", "hello"},
+				Command: "echo hello",
 				outputFunc: func(output string) {
 					tmpVals = append(tmpVals, output)
 				},
 			},
 			{
 				Name:    "step",
 				Action:  "Shell",
-				Command: []string{"faaaaafffaa"},
+				Command: "faaaaafffaa",
 				outputFunc: func(output string) {
 					tmpVals = append(tmpVals, output)
 				},
 			},
 			{
 				Name:    "step",
 				Action:  "Shell",
-				Command: []string{"echo", "hallo"},
+				Command: "echo hallo",
 				outputFunc: func(output string) {
 					tmpVals = append(tmpVals, output)
 				},
 			},
 		},
 	}
 	err := rg.run(context.Background(), &PipelineRunOptions{}, &executionTargetImpl{})
-	assert.Error(t, err, "failed to execute shell command:  exec: \"faaaaafffaa\": executable file not found in $PATH")
+	assert.ErrorContains(t, err, "faaaaafffaa: command not found\n exit status 127")
 	// Test processing ends after first error
 	assert.Equal(t, len(tmpVals), 1)
 }
@@ -166,7 +166,7 @@ func TestPipelineRun(t *testing.T) {
 					{
 						Name:    "step",
 						Action:  "Shell",
-						Command: []string{"echo", "hello"},
+						Command: "echo hello",
 						outputFunc: func(output string) {
 							foundOutput = output
 						},

diff --git a/tooling/templatize/pkg/pipeline/shell.go b/tooling/templatize/pkg/pipeline/shell.go
@@ -13,26 +13,27 @@ import (
 )
 
 func (s *Step) createCommand(ctx context.Context, dryRun bool, envVars map[string]string) (*exec.Cmd, bool) {
-	var cmd *exec.Cmd
+	var scriptCommand string = s.Command
 	if dryRun {
-		if s.DryRun.Command == nil && s.DryRun.EnvVars == nil {
+		if s.DryRun.Command == "" && s.DryRun.EnvVars == nil {
 			return nil, true
 		}
+		if s.DryRun.Command != "" {
+			scriptCommand = s.DryRun.Command
+		}
 		for _, e := range s.DryRun.EnvVars {
 			envVars[e.Name] = e.Value
 		}
-		if s.DryRun.Command != nil {
-			cmd = exec.CommandContext(ctx, s.DryRun.Command[0], s.DryRun.Command[1:]...)
-		}
-	}
-	if cmd == nil {
-		// if dry-run is not enabled, use the actual command or also if no dry-run command is defined
-		cmd = exec.CommandContext(ctx, s.Command[0], s.Command[1:]...)
 	}
+	cmd := exec.CommandContext(ctx, "/bin/bash", "-c", buildBashScript(scriptCommand))
 	cmd.Env = append(cmd.Env, utils.MapToEnvVarArray(envVars)...)
 	return cmd, false
 }
 
+func buildBashScript(command string) string {
+	return fmt.Sprintf("set -o errexit -o nounset  -o pipefail\n%s", command)
+}
+
 func (s *Step) runShellStep(ctx context.Context, kubeconfigFile string, options *PipelineRunOptions) error {
 	if s.outputFunc == nil {
 		s.outputFunc = func(output string) {
@@ -54,7 +55,7 @@ func (s *Step) runShellStep(ctx context.Context, kubeconfigFile string, options
 	// execute the command
 	cmd, skipCommand := s.createCommand(ctx, options.DryRun, envVars)
 	if skipCommand {
-		logger.V(5).Info("Skipping step '%s' due to missing dry-run configuiration", s.Name)
+		logger.V(5).Info(fmt.Sprintf("Skipping step '%s' due to missing dry-run configuration", s.Name))
 		return nil
 	}
 

diff --git a/tooling/templatize/pkg/pipeline/shell_test.go b/tooling/templatize/pkg/pipeline/shell_test.go
@@ -2,6 +2,8 @@ package pipeline
 
 import (
 	"context"
+	"fmt"
+	"strings"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -12,39 +14,36 @@ import (
 func TestCreateCommand(t *testing.T) {
 	ctx := context.Background()
 	testCases := []struct {
-		name            string
-		step            *Step
-		dryRun          bool
-		envVars         map[string]string
-		expectedCommand string
-		expectedArgs    []string
-		expectedEnv     []string
-		skipCommand     bool
+		name           string
+		step           *Step
+		dryRun         bool
+		envVars        map[string]string
+		expectedScript string
+		expectedEnv    []string
+		skipCommand    bool
 	}{
 		{
 			name: "basic",
 			step: &Step{
-				Command: []string{"/usr/bin/echo", "hello"},
+				Command: "/usr/bin/echo hello",
 			},
-			expectedCommand: "/usr/bin/echo",
-			expectedArgs:    []string{"hello"},
+			expectedScript: buildBashScript("/usr/bin/echo hello"),
 		},
 		{
 			name: "dry-run",
 			step: &Step{
-				Command: []string{"/usr/bin/echo", "hello"},
+				Command: "/usr/bin/echo hello",
 				DryRun: DryRun{
-					Command: []string{"/usr/bin/echo", "dry-run"},
+					Command: "/usr/bin/echo dry-run",
 				},
 			},
-			dryRun:          true,
-			expectedCommand: "/usr/bin/echo",
-			expectedArgs:    []string{"dry-run"},
+			dryRun:         true,
+			expectedScript: buildBashScript("/usr/bin/echo dry-run"),
 		},
 		{
 			name: "dry-run-env",
 			step: &Step{
-				Command: []string{"/usr/bin/echo"},
+				Command: "/usr/bin/echo",
 				DryRun: DryRun{
 					EnvVars: []EnvVar{
 						{
@@ -54,15 +53,15 @@ func TestCreateCommand(t *testing.T) {
 					},
 				},
 			},
-			dryRun:          true,
-			expectedCommand: "/usr/bin/echo",
-			envVars:         map[string]string{},
-			expectedEnv:     []string{"DRY_RUN=true"},
+			dryRun:         true,
+			expectedScript: buildBashScript("/usr/bin/echo"),
+			envVars:        map[string]string{},
+			expectedEnv:    []string{"DRY_RUN=true"},
 		},
 		{
 			name: "dry-run fail",
 			step: &Step{
-				Command: []string{"/usr/bin/echo"},
+				Command: "/usr/bin/echo",
 			},
 			dryRun:      true,
 			skipCommand: true,
@@ -73,10 +72,7 @@ func TestCreateCommand(t *testing.T) {
 			cmd, skipCommand := tc.step.createCommand(ctx, tc.dryRun, tc.envVars)
 			assert.Equal(t, skipCommand, tc.skipCommand)
 			if !tc.skipCommand {
-				assert.Equal(t, cmd.Path, tc.expectedCommand)
-			}
-			if tc.expectedArgs != nil {
-				assert.DeepEqual(t, cmd.Args[1:], tc.expectedArgs)
+				assert.Equal(t, strings.Join(cmd.Args, " "), fmt.Sprintf("/bin/bash -c %s", tc.expectedScript))
 			}
 			if tc.expectedEnv != nil {
 				assert.DeepEqual(t, cmd.Env, tc.expectedEnv)
@@ -156,13 +152,52 @@ func TestMapStepVariables(t *testing.T) {
 }
 
 func TestRunShellStep(t *testing.T) {
-	expectedOutput := "hello\n"
-	s := &Step{
-		Command: []string{"echo", "hello"},
-		outputFunc: func(output string) {
-			assert.Equal(t, output, expectedOutput)
+	testCases := []struct {
+		name string
+		vars config.Variables
+		step *Step
+		err  string
+	}{
+		{
+			name: "basic",
+			vars: config.Variables{},
+			step: &Step{
+				Command: "echo hello",
+			},
 		},
+		{
+			name: "test nounset",
+			vars: config.Variables{},
+			step: &Step{
+				Command: "echo $DOES_NOT_EXIST",
+			},
+			err: "DOES_NOT_EXIST: unbound variable\n exit status 1",
+		},
+		{
+			name: "test errexit",
+			vars: config.Variables{},
+			step: &Step{
+				Command: "false ; echo hello",
+			},
+			err: "failed to execute shell command:  exit status 1",
+		},
+		{
+			name: "test pipefail",
+			vars: config.Variables{},
+			step: &Step{
+				Command: "false | echo",
+			},
+			err: "failed to execute shell command: \n exit status 1",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.step.runShellStep(context.Background(), "", &PipelineRunOptions{})
+			if tc.err != "" {
+				assert.ErrorContains(t, err, tc.err)
+			} else {
+				assert.NilError(t, err)
+			}
+		})
 	}
-	err := s.runShellStep(context.Background(), "", &PipelineRunOptions{})
-	assert.NilError(t, err)
 }
diff --git a/tooling/templatize/pkg/pipeline/types.go b/tooling/templatize/pkg/pipeline/types.go
@@ -23,7 +23,7 @@ type outPutHandler func(string)
 type Step struct {
 	Name            string   `yaml:"name"`
 	Action          string   `yaml:"action"`
-	Command         []string `yaml:"command,omitempty"`
+	Command         string   `yaml:"command,omitempty"`
 	Env             []EnvVar `yaml:"env,omitempty"`
 	Template        string   `yaml:"template,omitempty"`
 	Parameters      string   `yaml:"parameters,omitempty"`
@@ -36,7 +36,7 @@ type Step struct {
 
 type DryRun struct {
 	EnvVars []EnvVar `yaml:"envVars,omitempty"`
-	Command []string `yaml:"command,omitempty"`
+	Command string   `yaml:"command,omitempty"`
 }
 
 type EnvVar struct {

diff --git a/tooling/templatize/testdata/pipeline.yaml b/tooling/templatize/testdata/pipeline.yaml
@@ -7,13 +7,13 @@ resourceGroups:
   steps:
   - name: deploy
     action: Shell
-    command: ["make", "deploy"]
+    command: make deploy
     env:
     - name: MAESTRO_IMAGE
       configRef: maestro_image
   - name: dry-run
     action: Shell
-    command: ["make", "deploy"]
+    command: make deploy
     dryRun:
       envVars:
       - name: DRY_RUN

diff --git a/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml b/tooling/templatize/testdata/zz_fixture_TestProcessPipelineForEV2pipeline.yaml
@@ -7,17 +7,13 @@ resourceGroups:
       steps:
         - name: deploy
           action: Shell
-          command:
-            - make
-            - deploy
+          command: make deploy
           env:
             - name: MAESTRO_IMAGE
               configRef: maestro_image
         - name: dry-run
           action: Shell
-          command:
-            - make
-            - deploy
+          command: make deploy
           dryRun:
             envVars:
                 - name: DRY_RUN