Capture stdout and stderr even if cmd.Run fails

Dockerfile

@@ -38,7 +38,7 @@ RUN --mount=target=. \
     GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -o /function .
 
 # Produce the Function image.
-FROM python:3.13-bookworm as image
+FROM python:3.13-bookworm AS image
 RUN apt-get update && apt-get install -y coreutils curl jq unzip zsh less
 RUN groupadd -g 65532 nonroot
 RUN useradd -u 65532 -g 65532 -d /home/nonroot --system --shell /usr/sbin/nologin nonroot

fn.go

@@ -3,6 +3,8 @@ package main
 import (
 	"bytes"
 	"context"
+	"fmt"
+	"os/exec"
 	"strings"
 	"time"
 
@@ -95,7 +97,7 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1.RunFunctionRequest)
 		shellCmd = in.ShellCommandField
 	}
 
-	var shellEnvVars = make(map[string]string)
+	shellEnvVars := make(map[string]string)
 	for _, envVar := range in.ShellEnvVars {
 		switch t := envVar.GetType(); t {
 		case v1alpha1.ShellEnvVarTypeValue:
@@ -129,7 +131,7 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1.RunFunctionRequest)
 	}
 
 	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 + "\";"
 	}
@@ -140,25 +142,32 @@ func (f *Function) RunFunction(_ context.Context, req *fnv1.RunFunctionRequest)
 	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
-	}
-	out := strings.TrimSpace(stdout.String())
-	err = dxr.Resource.SetValue(stdoutField, out)
+
+	cmderr := cmd.Run()
+	sout := strings.TrimSpace(stdout.String())
+	serr := strings.TrimSpace(stderr.String())
+
+	log.Debug(shellCmd, "stdout", sout, "stderr", serr)
+
+	err = dxr.Resource.SetValue(stdoutField, sout)
 	if err != nil {
-		response.Fatal(rsp, errors.Wrapf(err, "cannot set field %s to %s for %s", stdoutField, out, oxr.Resource.GetKind()))
+		response.Fatal(rsp, errors.Wrapf(err, "cannot set field %s to %s for %s", stdoutField, sout, oxr.Resource.GetKind()))
 		return rsp, nil
 	}
-	err = dxr.Resource.SetValue(stderrField, 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

fn_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"regexp"
 	"testing"
 	"time"
 
@@ -17,10 +18,10 @@ import (
 )
 
 func TestRunFunction(t *testing.T) {
-
 	type args struct {
-		ctx context.Context
-		req *fnv1.RunFunctionRequest
+		ctx      context.Context
+		req      *fnv1.RunFunctionRequest
+		useRegex bool // regex match on message due to differing error messages between shells
 	}
 	type want struct {
 		rsp *fnv1.RunFunctionResponse
@@ -122,6 +123,50 @@ func TestRunFunction(t *testing.T) {
 				},
 			},
 		},
+		"ResponseIsErrorIfInvalidShellCommand": {
+			reason: "The function should write to the specified stderr when the shell command is invalid",
+			args: args{
+				req: &fnv1.RunFunctionRequest{
+					Meta: &fnv1.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"
+                    }`),
+				},
+				useRegex: true,
+			},
+			want: want{
+				rsp: &fnv1.RunFunctionResponse{
+					Meta: &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
+					Desired: &fnv1.State{
+						Composite: &fnv1.Resource{
+							Resource: resource.MustStructJSON(`{
+								"apiVersion": "",
+								"kind": "",
+								"status": {
+									"atFunction": {
+										"shell": {
+                                            "stdout": "",
+											"stderr": "/bin/sh: .*set: .*pìpefail"
+										}
+									}
+								}
+							}`),
+						},
+					},
+					Results: []*fnv1.Result{
+						{
+							Severity: fnv1.Severity_SEVERITY_FATAL,
+							Message:  "shellCmd \"set -euo pìpefail\" for \"\" failed with : exit status 2",
+							Target:   fnv1.Target_TARGET_COMPOSITE.Enum(),
+						},
+					},
+				},
+			},
+		},
 		"ResponseIsErrorWhenShellCommandNotFound": {
 			reason: "The Function should write to the specified stderr field when the shellCommand is not found",
 			args: args{
@@ -131,18 +176,78 @@ func TestRunFunction(t *testing.T) {
 						"apiVersion": "template.fn.crossplane.io/v1alpha1",
 						"kind": "Parameters",
 						"shellCommand": "unknown-shell-command",
-						"stdoutField": "spec.atFunction.shell.stdout",
-						"stderrField": "spec.atFunction.shell.stderr"
+						"stdoutField": "status.atFunction.shell.stdout",
+						"stderrField": "status.atFunction.shell.stderr"
 					}`),
 				},
+				useRegex: true,
 			},
 			want: want{
 				rsp: &fnv1.RunFunctionResponse{
 					Meta: &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
+					Desired: &fnv1.State{
+						Composite: &fnv1.Resource{
+							Resource: resource.MustStructJSON(`{
+								"apiVersion": "",
+								"kind": "",
+								"status": {
+									"atFunction": {
+										"shell": {
+                                            "stdout": "",
+											"stderr": "/bin/sh: .*unknown-shell-command.*"
+										}
+									}
+								}
+							}`),
+						},
+					},
 					Results: []*fnv1.Result{
 						{
 							Severity: fnv1.Severity_SEVERITY_FATAL,
-							Message:  "shellCmd unknown-shell-command for  failed: exit status 127",
+							Message:  "shellCmd unknown-shell-command for failed: exit status 127",
+							Target:   fnv1.Target_TARGET_COMPOSITE.Enum(),
+						},
+					},
+				},
+			},
+		},
+		"ResponseIsFailingCommandWithOutput": {
+			reason: "The Function should capture both stdout and stderr when a command fails but produces output",
+			args: args{
+				req: &fnv1.RunFunctionRequest{
+					Meta: &fnv1.RequestMeta{Tag: "hello"},
+					Input: resource.MustStructJSON(`{
+						"apiVersion": "template.fn.crossplane.io/v1alpha1",
+						"kind": "Parameters",
+						"shellCommand": "echo 'success output'; echo 'error output' >&2; exit 1",
+						"stdoutField": "status.atFunction.shell.stdout",
+						"stderrField": "status.atFunction.shell.stderr"
+					}`),
+				},
+			},
+			want: want{
+				rsp: &fnv1.RunFunctionResponse{
+					Meta: &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
+					Desired: &fnv1.State{
+						Composite: &fnv1.Resource{
+							Resource: resource.MustStructJSON(`{
+								"apiVersion": "",
+								"kind": "",
+								"status": {
+									"atFunction": {
+										"shell": {
+											"stdout": "success output",
+											"stderr": "error output"
+										}
+									}
+								}
+							}`),
+						},
+					},
+					Results: []*fnv1.Result{
+						{
+							Severity: fnv1.Severity_SEVERITY_FATAL,
+							Message:  "shellCmd \"echo 'success output'; echo 'error output' >&2; exit 1\" for \"\" failed with error output",
 							Target:   fnv1.Target_TARGET_COMPOSITE.Enum(),
 						},
 					},
@@ -416,7 +521,21 @@ func TestRunFunction(t *testing.T) {
 			f := &Function{log: logging.NewNopLogger()}
 			rsp, err := f.RunFunction(tc.args.ctx, tc.args.req)
 
-			if diff := cmp.Diff(tc.want.rsp, rsp, protocmp.Transform()); diff != "" {
+			var cmpOpts []cmp.Option
+			cmpOpts = append(cmpOpts, protocmp.Transform(), protocmp.IgnoreFields(&fnv1.Result{}, "message"))
+
+			if tc.args.useRegex {
+				cmpOpts = append(cmpOpts, cmp.Comparer(func(expected, actual string) bool {
+					// If expected looks like a regex pattern, use regex matching
+					if regexp.MustCompile(`^.*\.\*.*$`).MatchString(expected) {
+						matched, _ := regexp.MatchString(expected, actual)
+						return matched
+					}
+					return expected == actual
+				}))
+			}
+
+			if diff := cmp.Diff(tc.want.rsp, rsp, cmpOpts...); diff != "" {
 				t.Errorf("%s\nf.RunFunction(...): -want rsp, +got rsp:\n%s", tc.reason, diff)
 			}