Provider error messages use four different prefix conventions

[retr0h]

A provider's errors surface in job results, the audit log and the CLI. They read as four different systems.

What is there

sysctl        "sysctl create: ..."               <domain> <verb>
netplan       "netplan apply: ..."               <domain> <verb>
ntp           "ntp: ..."                         <domain>
service       "service: ..."                     <domain>
certificate   "create certificate: ..."          <verb> <domain>
cron          "create cron entry: ..."           <verb> <domain>
file          "failed to execute template: ..."  failed to <verb>
user          "chpasswd failed: ..."             <command> failed

Reproduce:

for d in node/sysctl node/ntp node/user node/service node/certificate \
         scheduled/cron file network/netplan; do
  printf '%-20s ' "$d"
  grep -rho 'fmt.Errorf("[a-z][a-z ]*:' internal/provider/$d/*.go \
    | sed 's/fmt.Errorf("//' | sort -u | head -3 | tr '\n' ' '
  echo
done

Why it matters beyond tidiness

file's failed to ... prefix is the one worth changing on its own merits. These errors are almost always wrapped by a caller, so the rendered string becomes "deploy cron entry: failed to execute template: ...". Go's own guidance is that error strings do not announce failure, because the context does.

user's chpasswd failed: names the command rather than the operation, which leaks the implementation into a message a CLI user reads.

Not in scope

The sentinel errors are already consistent and should stay as they are:

ErrUnsupported  = errors.New("operation not supported on this OS family")
ErrNotFound     = errors.New("not found")
ErrNotManaged   = errors.New("not managed by osapi")
ErrNotInstalled = errors.New("not installed")

So is wrapping with %w, which every provider does on the paths that carry a cause. #536 fixed the one place that did not.

Suggested convention

<domain> <verb>: <what went wrong>, which is what sysctl and netplan already use and which sorts and greps well:

sysctl create: key must not be empty
cron delete: not managed by osapi
file deploy: execute template: ...

Picking one and applying it is the whole change. It should be settled in components/osapi/providers.md first so new providers inherit it rather than copying whichever neighbour they read.

[retr0h]

Done. Convention in osapi-io/specs#245, applied in #573, #574, #575, #576 and #577.

Every provider now reads <domain> <verb>: what went wrong:

node/sysctl          sysctl create:      sysctl delete:      sysctl get:
node/ntp             ntp create:         ntp delete:         ntp get:
node/user            group create:       group delete:       user create:
node/service         service create:     service delete:     service disable:
node/certificate     certificate create: certificate delete: certificate list:
schedule/cron        schedule create:    schedule delete:    schedule list:
file                 file deploy:        file status:        file template:

chpasswd: and netplan apply: stay as they are. They are inner frames wrapped by an operation that already names itself, which is the rule the work arrived at:

Only the outermost frame names the operation. Everything beneath it is detail.

That rule is worth more than the rename. Breaking it produced four separate bugs along the way, each caught by tests rather than review:

user set-password: user set-password: chpasswd: chpasswd failed
user: user: password hash must not contain a colon or line break
sysctl create: sysctl: value must not contain line breaks
group: invalid group name "x": must match ...

The last of those was validateAccountName taking a kind argument purely to name the resource, while all twelve callers wrapped the result with that same word. #577 dropped the parameter.

Two other things this turned up, both worth knowing for the next sweep of this kind:

• A sed over test files rewrote a mock's command argument, turning the systemctl flag "daemon-reload" into an error message. The mock then expected a flag that does not exist.
• sysctl was missed by every earlier pass because it was already mostly correct, and only an audit of all providers afterwards found its two stragglers. The issue's original list was not the whole list.