Skip to content

Commit

Permalink
fix: multiline actions fail faster (#2148)
Browse files Browse the repository at this point in the history
## Description
Adds `-e` to shells so that multiline actions fail faster. Noting that
this only applies to [real
shells](https://github.com/defenseunicorns/zarf/blob/356a4da121ae8cd0ffa720ab17ab859c5a2ac07a/src/pkg/utils/exec/exec.go#L173)
🤣

EDIT: adding capability to Powershell based on comments below

## Related Issue

Fixes defenseunicorns/uds-cli#204

## Type of change

- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

- [X] Test, docs, adr added or updated as needed
- [ ] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed
  • Loading branch information
UncleGedd authored Nov 16, 2023
1 parent 356a4da commit 548e9d2
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 17 deletions.
6 changes: 6 additions & 0 deletions docs/3-create-a-zarf-package/7-component-actions.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ Within each of the `action` lists (`before`, `after`, `onSuccess`, and `onFailur
- `setVariables` - set the standard output of the command to a list of variables that can be used in other actions or components (onDeploy only).
- `shell` - set a preferred shell for the command to run in for a particular operating system (default is `sh` for macOS/Linux and `powershell` for Windows).

:::info

By default, multi-line `cmd` blocks will fail if one of the lines errors out; this is analogous to setting `set -e` in a shell script, as documented in the [GNU bash docs](https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html).

:::

:::note

Any binaries you execute in your `cmd` actions must exist on the machine they are executed on. You can bring binaries with a Zarf Package as `files` with the `executable` key set, or take advantage of the `./zarf ` transformation as described in [action transformations](#action-transformations).
Expand Down
10 changes: 10 additions & 0 deletions examples/component-actions/zarf.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,13 @@ components:
kind: configmap
name: simple-configmap
namespace: zarf

- name: on-deploy-immediate-failure
description: This component will fail on the first error instead of continuing execution
# the default for multi-line commands is set -e
actions:
onDeploy:
after:
- cmd: |
bad_cmd
echo "this text shouldn't be printed"
8 changes: 4 additions & 4 deletions src/internal/cluster/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (c *Cluster) HandleDataInjection(wg *sync.WaitGroup, data types.ZarfDataInj
// Get the OS shell to execute commands in
shell, shellArgs := exec.GetOSShell(types.ZarfComponentActionShell{Windows: "cmd"})

if _, _, err := exec.Cmd(shell, shellArgs, "tar --version"); err != nil {
if _, _, err := exec.Cmd(shell, append(shellArgs, "tar --version")...); err != nil {
message.WarnErr(err, "Unable to execute tar on this system. Please ensure it is installed and on your $PATH.")
return
}
Expand Down Expand Up @@ -96,7 +96,7 @@ iterator:

// Must create the target directory before trying to change to it for untar
mkdirCmd := fmt.Sprintf("%s -- mkdir -p %s", kubectlCmd, data.Target.Path)
if err := exec.CmdWithPrint(shell, shellArgs, mkdirCmd); err != nil {
if err := exec.CmdWithPrint(shell, append(shellArgs, mkdirCmd)...); err != nil {
message.Warnf("Unable to create the data injection target directory %s in pod %s", data.Target.Path, pod.Name)
continue iterator
}
Expand All @@ -109,7 +109,7 @@ iterator:
)

// Do the actual data injection
if err := exec.CmdWithPrint(shell, shellArgs, cpPodCmd); err != nil {
if err := exec.CmdWithPrint(shell, append(shellArgs, cpPodCmd)...); err != nil {
message.Warnf("Error copying data into the pod %#v: %#v\n", pod.Name, err)
continue iterator
}
Expand All @@ -123,7 +123,7 @@ iterator:
untarCmd,
)

if err := exec.CmdWithPrint(shell, shellArgs, cpPodCmd); err != nil {
if err := exec.CmdWithPrint(shell, append(shellArgs, cpPodCmd)...); err != nil {
message.Warnf("Error saving the zarf sync completion file after injection into pod %#v\n", pod.Name)
continue iterator
}
Expand Down
2 changes: 1 addition & 1 deletion src/pkg/packager/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func actionRun(ctx context.Context, cfg types.ZarfComponentActionDefaults, cmd s
execCfg.Stderr = spinner
}

out, errOut, err := exec.CmdWithContext(ctx, execCfg, shell, shellArgs, cmd)
out, errOut, err := exec.CmdWithContext(ctx, execCfg, shell, append(shellArgs, cmd)...)
// Dump final complete output (respect mute to prevent sensitive values from hitting the logs).
if !cfg.Mute {
message.Debug(cmd, out, errOut)
Expand Down
22 changes: 12 additions & 10 deletions src/pkg/utils/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,11 @@ func LaunchURL(url string) error {
}

// GetOSShell returns the shell and shellArgs based on the current OS
func GetOSShell(shellPref types.ZarfComponentActionShell) (string, string) {
func GetOSShell(shellPref types.ZarfComponentActionShell) (string, []string) {
var shell string
var shellArgs string
var shellArgs []string
powershellShellArgs := []string{"-Command", "$ErrorActionPreference = 'Stop';"}
shShellArgs := []string{"-e", "-c"}

switch runtime.GOOS {
case "windows":
Expand All @@ -165,39 +167,39 @@ func GetOSShell(shellPref types.ZarfComponentActionShell) (string, string) {
shell = shellPref.Windows
}

shellArgs = "-Command"
shellArgs = powershellShellArgs
if shell == "cmd" {
// Change shellArgs to /c if cmd is chosen
shellArgs = "/c"
shellArgs = []string{"/c"}
} else if !IsPowershell(shell) {
// Change shellArgs to -c if a real shell is chosen
shellArgs = "-c"
shellArgs = shShellArgs
}
case "darwin":
shell = "sh"
if shellPref.Darwin != "" {
shell = shellPref.Darwin
}

shellArgs = "-c"
shellArgs = shShellArgs
if IsPowershell(shell) {
// Change shellArgs to -Command if pwsh is chosen
shellArgs = "-Command"
shellArgs = powershellShellArgs
}
case "linux":
shell = "sh"
if shellPref.Linux != "" {
shell = shellPref.Linux
}

shellArgs = "-c"
shellArgs = shShellArgs
if IsPowershell(shell) {
// Change shellArgs to -Command if pwsh is chosen
shellArgs = "-Command"
shellArgs = powershellShellArgs
}
default:
shell = "sh"
shellArgs = "-c"
shellArgs = shShellArgs
}

return shell, shellArgs
Expand Down
4 changes: 2 additions & 2 deletions src/pkg/utils/wait.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func ExecuteWait(waitTimeout, waitNamespace, condition, kind, identifier string,
spinner.Updatef(existMsg)
// Check if the resource exists.
zarfKubectlGet := fmt.Sprintf("%s tools kubectl get %s %s %s", zarfCommand, namespaceFlag, kind, identifier)
if stdout, stderr, err := exec.Cmd(shell, shellArgs, zarfKubectlGet); err != nil {
if stdout, stderr, err := exec.Cmd(shell, append(shellArgs, zarfKubectlGet)...); err != nil {
message.Debug(stdout, stderr, err)
continue
}
Expand All @@ -111,7 +111,7 @@ func ExecuteWait(waitTimeout, waitNamespace, condition, kind, identifier string,
zarfCommand, namespaceFlag, kind, identifier, waitType, condition, waitTimeout)

// If there is an error, log it and try again.
if stdout, stderr, err := exec.Cmd(shell, shellArgs, zarfKubectlWait); err != nil {
if stdout, stderr, err := exec.Cmd(shell, append(shellArgs, zarfKubectlWait)...); err != nil {
message.Debug(stdout, stderr, err)
continue
}
Expand Down
7 changes: 7 additions & 0 deletions src/test/e2e/02_component_actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,11 @@ func TestComponentActions(t *testing.T) {
// Remove the templated file at the end of the test
e2e.CleanFiles(deployTemplatedArtifact)
})

t.Run("action on-deploy-immediate-failure", func(t *testing.T) {
t.Parallel()
stdOut, stdErr, err = e2e.Zarf("package", "deploy", path, "--components=on-deploy-immediate-failure", "--confirm")
require.Error(t, err, stdOut, stdErr)
require.Contains(t, stdErr, "Failed to deploy package")
})
}

0 comments on commit 548e9d2

Please sign in to comment.