Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ resourceGroups:
steps:
- name: deploy
action: Shell
command: ["make", "deploy"]
command: make deploy
env:
- name: ARO_HCP_IMAGE_ACR
configRef: svcAcrName
Expand Down
2 changes: 1 addition & 1 deletion dev-infrastructure/mgmt-pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dev-infrastructure/svc-pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion frontend/pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ resourceGroups:
steps:
- name: deploy
action: Shell
command: ["make", "deploy"]
command: make deploy
dryRun:
envVars:
- name: HELM_DRY_RUN
Expand Down
8 changes: 1 addition & 7 deletions maestro/server/pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tooling/templatize/internal/end2end/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")

Expand Down
2 changes: 1 addition & 1 deletion tooling/templatize/pkg/pipeline/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
14 changes: 7 additions & 7 deletions tooling/templatize/pkg/pipeline/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down Expand Up @@ -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
},
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
},
Expand Down
21 changes: 11 additions & 10 deletions tooling/templatize/pkg/pipeline/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
}

Expand Down
101 changes: 68 additions & 33 deletions tooling/templatize/pkg/pipeline/shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package pipeline

import (
"context"
"fmt"
"strings"
"testing"

"gotest.tools/v3/assert"
Expand All @@ -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{
{
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
4 changes: 2 additions & 2 deletions tooling/templatize/pkg/pipeline/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions tooling/templatize/testdata/pipeline.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading