Capture stdout and stderr even if cmd.Run fails

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.

Author: mproffitt

fn.go

@@ -3,6 +3,8 @@ package main
 import (
 	"bytes"
 	"context"
+	"fmt"
+	"os/exec"
 	"strings"
 
 	"github.com/crossplane-contrib/function-shell/input/v1alpha1"
@@ -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)
@@ -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 + "\";"
 	}
@@ -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

fn_test.go

@@ -16,7 +16,6 @@ import (
 )
 
 func TestRunFunction(t *testing.T) {
-
 	type args struct {
 		ctx context.Context
 		req *fnv1beta1.RunFunctionRequest
@@ -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{
@@ -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",
 						},
 					},
 				},