Skip to content

Commit

Permalink
Capture stdout and stderr even if cmd.Run fails
Browse files Browse the repository at this point in the history
In its current form, if `cmd.Run` fails, then the function immediately
exits with no reason given for the termination. This makes debugging
scripts hard to understand as no indication is given for the failure,
other than a failure occured.

By trapping the error returned from `cmd.Run`, we first process the
buffers for both `stdout` and `stderr` and set these to the `dxr`.

Additionally we capture a truncated `stderr` from the command error
which can help point to where the command failed, setting this as the
Fatal error message returned as an event.

Signed-off-by: Martin Proffitt <mproffitt@choclab.net>
  • Loading branch information
mproffitt committed Jan 11, 2025
1 parent a905f7e commit 7c9454c
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 15 deletions.
29 changes: 18 additions & 11 deletions fn.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package main
import (
"bytes"
"context"
"fmt"
"os/exec"
"strings"

"github.com/crossplane-contrib/function-shell/input/v1alpha1"
Expand Down Expand Up @@ -84,7 +86,7 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequ
shellCmd = in.ShellCommandField
}

var shellEnvVars = make(map[string]string)
shellEnvVars := make(map[string]string)
for _, envVar := range in.ShellEnvVars {
if envVar.ValueRef != "" {
envValue, err := fromValueRef(req, envVar.ValueRef)
Expand All @@ -107,7 +109,7 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequ
}

var exportCmds string
//exportCmds = "export PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin;"
// exportCmds = "export PATH=/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin;"
for k, v := range shellEnvVars {
exportCmds = exportCmds + "export " + k + "=\"" + v + "\";"
}
Expand All @@ -118,25 +120,30 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1beta1.RunFunctionRequ
cmd := shell.Commandf(exportCmds + shellCmd)
cmd.Stdout = &stdout
cmd.Stderr = &stderr
err = cmd.Run()
if err != nil {
response.Fatal(rsp, errors.Wrapf(err, "shellCmd %s for %s failed", shellCmd, oxr.Resource.GetKind()))
return rsp, nil
}

cmderr := cmd.Run()
out := strings.TrimSpace(stdout.String())
err = dxr.Resource.SetValue(stdoutField, out)
if err != nil {
response.Fatal(rsp, errors.Wrapf(err, "cannot set field %s to %s for %s", stdoutField, out, oxr.Resource.GetKind()))
return rsp, nil
}
err = dxr.Resource.SetValue(stderrField, strings.TrimSpace(stderr.String()))

serr := strings.TrimSpace(stderr.String())
err = dxr.Resource.SetValue(stderrField, serr)
if err != nil {
response.Fatal(rsp, errors.Wrapf(err, "cannot set field %s to %s for %s", stderrField, out, oxr.Resource.GetKind()))
return rsp, nil
response.Fatal(rsp, errors.Wrapf(err, "cannot set field %s to %s for %s", stderrField, serr, oxr.Resource.GetKind()))
}
if err := response.SetDesiredCompositeResource(rsp, dxr); err != nil {
response.Fatal(rsp, errors.Wrapf(err, "cannot set desired composite resources from %T", req))
return rsp, nil
}

if cmderr != nil {
if exiterr, ok := cmderr.(*exec.ExitError); ok {
msg := fmt.Sprintf("shellCmd %q for %q failed with %s", shellCmd, oxr.Resource.GetKind(), exiterr.Stderr)
response.Fatal(rsp, errors.Wrap(cmderr, msg))
return rsp, nil
}
}

return rsp, nil
Expand Down
65 changes: 61 additions & 4 deletions fn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
)

func TestRunFunction(t *testing.T) {

type args struct {
ctx context.Context
req *fnv1beta1.RunFunctionRequest
Expand Down Expand Up @@ -119,6 +118,48 @@ func TestRunFunction(t *testing.T) {
},
},
},
"ResponseIsErrorIfInvalidShellCommand": {
reason: "The function should write to the specified stderr when the shell command is invalid",
args: args{
req: &fnv1beta1.RunFunctionRequest{
Meta: &fnv1beta1.RequestMeta{Tag: "hello"},
Input: resource.MustStructJSON(`{
"apiVersion": "template.fn.crossplane.io/v1alpha1",
"kind": "Parameters",
"shellCommand": "set -euo pìpefail",
"stdoutField": "status.atFunction.shell.stdout",
"stderrField": "status.atFunction.shell.stderr"
}`),
},
},
want: want{
rsp: &fnv1beta1.RunFunctionResponse{
Meta: &fnv1beta1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
Desired: &fnv1beta1.State{
Composite: &fnv1beta1.Resource{
Resource: resource.MustStructJSON(`{
"apiVersion": "",
"kind": "",
"status": {
"atFunction": {
"shell": {
"stdout": "",
"stderr": "/bin/sh: 1: set: Illegal option -o pìpefail"
}
}
}
}`),
},
},
Results: []*fnv1beta1.Result{
{
Severity: fnv1beta1.Severity_SEVERITY_FATAL,
Message: "shellCmd \"set -euo pìpefail\" for \"\" failed with : exit status 2",
},
},
},
},
},
"ResponseIsErrorWhenShellCommandNotFound": {
reason: "The Function should write to the specified stderr field when the shellCommand is not found",
args: args{
Expand All @@ -128,18 +169,34 @@ func TestRunFunction(t *testing.T) {
"apiVersion": "template.fn.crossplane.io/v1alpha1",
"kind": "Parameters",
"shellCommand": "unkown-shell-command",
"stdoutField": "spec.atFunction.shell.stdout",
"stderrField": "spec.atFunction.shell.stderr"
"stdoutField": "status.atFunction.shell.stdout",
"stderrField": "status.atFunction.shell.stderr"
}`),
},
},
want: want{
rsp: &fnv1beta1.RunFunctionResponse{
Meta: &fnv1beta1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
Desired: &fnv1beta1.State{
Composite: &fnv1beta1.Resource{
Resource: resource.MustStructJSON(`{
"apiVersion": "",
"kind": "",
"status": {
"atFunction": {
"shell": {
"stdout": "",
"stderr": "/bin/sh: 1: unkown-shell-command: not found"
}
}
}
}`),
},
},
Results: []*fnv1beta1.Result{
{
Severity: fnv1beta1.Severity_SEVERITY_FATAL,
Message: "shellCmd unkown-shell-command for failed: exit status 127",
Message: "shellCmd \"unkown-shell-command\" for \"\" failed with : exit status 127",
},
},
},
Expand Down

0 comments on commit 7c9454c

Please sign in to comment.