config:set reports success even when writing the env file fails
When dokku saves an app's config it writes the env file and then ignores whether the write actually worked. If the write fails, the command still prints Setting config vars, still fires the update trigger, still restarts the app, and still exits 0. Nothing tells you the config was not saved.
This happens in three places in plugins/config/config.go — SetMany (line 58), UnsetMany (line 94) and UnsetAll (line 121). All three call env.Write() as a bare statement and throw the returned error away. The common.SetPermissions call immediately after gets the same treatment.
env.Write()
common.SetPermissions(common.SetPermissionInput{
Filename: env.Filename(),
Mode: os.FileMode(0600),
})
triggerUpdate(appName, "set", keys)Env.Write() really can fail. Apart from the usual disk and permission problems, it returns an error immediately when the Env has no file behind it:
func (e *Env) Write() error {
if e.filename == "" {
return errors.New("this Env was created unbound to a file")
}
return godotenv.Write(e.Map(), e.filename)
}LoadMergedAppEnv blanks out filename, so anything that ends up writing a merged env takes that branch.
How I came across it: I was tracking down why a plugin's call to config.SetMany looked like it had worked but had no effect. In that case the write itself did succeed — it was going to the pre-0.38 DOKKU_ROOT/<app>/ENV path, which was a bug on my side, not here. So this issue was not the cause. What stood out while tracing it is that a genuinely failed write would have produced exactly the same output: the "Setting config vars" line, the variable echoed back, and a zero exit code.
The fix should be to return the error from env.Write() rather than discarding it, so callers and the CLI can fail loudly. SetPermissions is probably worth handling too.
Source: dokku/dokku