crossplane-contrib / crossplane-contrib/function-shell

Shell command failures report an empty reason: exiterr.Stderr is always nil

Aperta
#143 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Go
Stelle
15
Fork
12
Merge medio
15g 15h
PR unite (30g)
1

Descrizione

### What happened?

When a `shellCommand` exits non-zero, the composition fails with an error message whose reason is
empty, and the actual stderr never reaches the XR. The only place the real cause is visible is the
function pod's log.

Example: a command that hits a missing binary produces

```
shellCmd "git clone https://..." for "XShell" failed with : exit status 127
```

Note the empty string after `failed with`. `git: not found` appears nowhere in the user's XR.

### Cause

Two separate things combine.

**1. `exiterr.Stderr` is always nil here.** `fn.go:147` assigns `cmd.Stderr` and `fn.go:149` calls
`cmd.Run()`:

```go
cmd.Stderr = &stderr
cmderr := cmd.Run()
```

`exec.ExitError.Stderr` is only populated by `Output()`/`CombinedOutput()`, and only when
`cmd.Stderr` was nil. See https://pkg.go.dev/os/exec#ExitError ("Stderr holds a subset of the
standard error output from the Cmd.Output method if the Cmd.Stderr field was not set"). So at
`fn.go:172`:

```go
msg := fmt.Sprintf("shellCmd %q for %q failed with %s", shellCmd, oxr.Resource.GetKind(), exiterr.Stderr)
```

`exiterr.Stderr` is nil, and `%s` of a nil `[]byte` prints the empty string. Confirmed with a minimal
program: `Run()` with `cmd.Stderr` set yields `ExitError.Stderr == nil`, while `Output()` yields
`"boom\n"`.

The captured stderr is already sitting in `serr` (`fn.go:151`). It just isn't used in the message.

**2. `response.Fatal` discards the desired XR, so `stderrField` never lands.** `fn.go:161` does set
the field:

```go
err = dxr.Resource.SetValue(stderrField, serr)
```

but `response.Fatal` (`function-sdk-go/response/result.go`) appends a fatal result, and Crossplane
returns a `PipelineFatalError` from `composition_functions.go` before the desired resources are
loaded and applied. So `status.atFunction.shell.stderr` is never written for a failing command,
which is exactly the case where a user most wants it.

### How can we reproduce it?

Any `shellCommand` that exits non-zero, e.g. `shellCommand: "exit 1"` or a reference to a binary
that isn't in the image. Observe the composition error on the XR versus the function pod's log.

### Suggested fix

The message fix is one line. Use the stderr that was actually captured:

```go
msg := fmt.Sprintf("shellCmd %q for %q failed with %s", shellCmd, oxr.Resource.GetKind(), serr)
```

The second part is a design question worth a maintainer opinion: should a non-zero exit be `Fatal`
at all? If the intent is for users to read `stderrField` off the XR, a failing command needs
`response.Warning` (or a fatal result emitted only after the desired XR is preserved), otherwise the
field is silently unreachable on the failure path.

### Why it matters now

This is independent of #141, but it's what makes a missing-tool regression in the runtime image very
hard to diagnose: the user sees a fatal composition error with no reason, and has to reach the
function pod's logs to find `git: not found`.

Happy to open a PR for the one-line message fix if that's useful.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Direzione di ricerca

Inizia in fn.go intorno alle righe 147-172 e riproduci il malfunzionamento con shellCommand: "exit 1" o con un binario mancante. Leggi response/result.go e composition_functions.go per capire in che modo il risultato fatale influisce sull’XR desiderato e su stderrField. Il lavoro è completato quando l’errore di Composition include lo stderr acquisito e il comportamento dell’XR nel percorso di errore ha un esito approvato da un maintainer.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
go, shell
Ambito
backend, infrastructure
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Tranquilla
Chiarezza
Abbastanza chiara
Idoneità per principianti
68/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.