caddycmd: Add --keep-backup to upgrade commands (#4387)

* caddycmd: Add `--skip-cleanup` to upgrade commands

This is a partial fix for https://github.com/caddyserver/caddy/issues/4057, making it possible to retain the old build of Caddy, in case something went wrong.

* caddycmd: Fix duplicate error message

The error message "download succeeded, but unable to execute" was repeated, because it was both in the `listModules`/`showVersion` functions and in the calling `upgradeBuild` function. Oversight when this was refactored.

* caddycmd: Implement fix for performing cleanup on Windows

Without this, the cleanup operation would fail with an error message like this:

upgrade: download succeeded, but unable to clean up backup binary: remove C:\caddy\caddy.exe.tmp: Access is denied.

* caddycmd: Rename to `--keep-backup`, simplify build constraints

cmd/commands.go

@@ -296,6 +296,11 @@ is always printed to stdout.`,
 		Long: `
 Downloads an updated Caddy binary with the same modules/plugins at the
 latest versions. EXPERIMENTAL: May be changed or removed.`,
+		Flags: func() *flag.FlagSet {
+			fs := flag.NewFlagSet("upgrade", flag.ExitOnError)
+			fs.Bool("keep-backup", false, "Keep the backed up binary, instead of deleting it")
+			return fs
+		}(),
 	})
 
 	RegisterCommand(Command{
@@ -308,6 +313,11 @@ Downloads an updated Caddy binary with the specified packages (module/plugin)
 added. Retains existing packages. Returns an error if the any of packages are 
 already included. EXPERIMENTAL: May be changed or removed.
 `,
+		Flags: func() *flag.FlagSet {
+			fs := flag.NewFlagSet("add-package", flag.ExitOnError)
+			fs.Bool("keep-backup", false, "Keep the backed up binary, instead of deleting it")
+			return fs
+		}(),
 	})
 
 	RegisterCommand(Command{
@@ -320,6 +330,11 @@ Downloads an updated Caddy binaries without the specified packages (module/plugi
 Returns an error if any of the packages are not included. 
 EXPERIMENTAL: May be changed or removed.
 `,
+		Flags: func() *flag.FlagSet {
+			fs := flag.NewFlagSet("remove-package", flag.ExitOnError)
+			fs.Bool("keep-backup", false, "Keep the backed up binary, instead of deleting it")
+			return fs
+		}(),
 	})
 
 }

cmd/packagesfuncs.go

@@ -31,7 +31,7 @@ import (
 	"go.uber.org/zap"
 )
 
-func cmdUpgrade(_ Flags) (int, error) {
+func cmdUpgrade(fl Flags) (int, error) {
 	_, nonstandard, _, err := getModules()
 	if err != nil {
 		return caddy.ExitCodeFailedStartup, fmt.Errorf("unable to enumerate installed plugins: %v", err)
@@ -41,7 +41,7 @@ func cmdUpgrade(_ Flags) (int, error) {
 		return caddy.ExitCodeFailedStartup, err
 	}
 
-	return upgradeBuild(pluginPkgs)
+	return upgradeBuild(pluginPkgs, fl)
 }
 
 func cmdAddPackage(fl Flags) (int, error) {
@@ -64,7 +64,7 @@ func cmdAddPackage(fl Flags) (int, error) {
 		pluginPkgs[arg] = struct{}{}
 	}
 
-	return upgradeBuild(pluginPkgs)
+	return upgradeBuild(pluginPkgs, fl)
 }
 
 func cmdRemovePackage(fl Flags) (int, error) {
@@ -88,10 +88,10 @@ func cmdRemovePackage(fl Flags) (int, error) {
 		delete(pluginPkgs, arg)
 	}
 
-	return upgradeBuild(pluginPkgs)
+	return upgradeBuild(pluginPkgs, fl)
 }
 
-func upgradeBuild(pluginPkgs map[string]struct{}) (int, error) {
+func upgradeBuild(pluginPkgs map[string]struct{}, fl Flags) (int, error) {
 	l := caddy.Log()
 
 	thisExecPath, err := os.Executable()
@@ -152,18 +152,23 @@ func upgradeBuild(pluginPkgs map[string]struct{}) (int, error) {
 	// use the new binary to print out version and module info
 	fmt.Print("\nModule versions:\n\n")
 	if err = listModules(thisExecPath); err != nil {
-		return caddy.ExitCodeFailedStartup, fmt.Errorf("download succeeded, but unable to execute: %v", err)
+		return caddy.ExitCodeFailedStartup, fmt.Errorf("download succeeded, but unable to execute 'caddy list-modules': %v", err)
 	}
 	fmt.Println("\nVersion:")
 	if err = showVersion(thisExecPath); err != nil {
-		return caddy.ExitCodeFailedStartup, fmt.Errorf("download succeeded, but unable to execute: %v", err)
+		return caddy.ExitCodeFailedStartup, fmt.Errorf("download succeeded, but unable to execute 'caddy version': %v", err)
 	}
 	fmt.Println()
 
 	// clean up the backup file
-	if err = os.Remove(backupExecPath); err != nil {
+	if !fl.Bool("keep-backup") {
+		if err = removeCaddyBinary(backupExecPath); err != nil {
 			return caddy.ExitCodeFailedStartup, fmt.Errorf("download succeeded, but unable to clean up backup binary: %v", err)
 		}
+	} else {
+		l.Info("skipped cleaning up the backup file", zap.String("backup_path", backupExecPath))
+	}
+
 	l.Info("upgrade successful; please restart any running Caddy instances", zap.String("executable", thisExecPath))
 
 	return caddy.ExitCodeSuccess, nil
@@ -223,22 +228,14 @@ func listModules(path string) error {
 	cmd := exec.Command(path, "list-modules", "--versions", "--skip-standard")
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
-	err := cmd.Run()
-	if err != nil {
-		return fmt.Errorf("download succeeded, but unable to execute: %v", err)
-	}
-	return nil
+	return cmd.Run()
 }
 
 func showVersion(path string) error {
 	cmd := exec.Command(path, "version")
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
-	err := cmd.Run()
-	if err != nil {
-		return fmt.Errorf("download succeeded, but unable to execute: %v", err)
-	}
-	return nil
+	return cmd.Run()
 }
 
 func downloadBuild(qs url.Values) (*http.Response, error) {

cmd/removebinary.go

@@ -0,0 +1,30 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build !windows
+// +build !windows
+
+package caddycmd
+
+import (
+	"os"
+)
+
+// removeCaddyBinary removes the Caddy binary at the given path.
+//
+// On any non-Windows OS, this simply calls os.Remove, since they should
+// probably not exhibit any issue with processes deleting themselves.
+func removeCaddyBinary(path string) error {
+	return os.Remove(path)
+}

cmd/removebinary_windows.go

@@ -0,0 +1,36 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package caddycmd
+
+import (
+	"os"
+	"path/filepath"
+	"syscall"
+)
+
+// removeCaddyBinary removes the Caddy binary at the given path.
+//
+// On Windows, this uses a syscall to indirectly remove the file,
+// because otherwise we get an "Access is denied." error when trying
+// to delete the binary while Caddy is still running and performing
+// the upgrade. "cmd.exe /C" executes a command specified by the
+// following arguments, i.e. "del" which will run as a separate process,
+// which avoids the "Access is denied." error.
+func removeCaddyBinary(path string) error {
+	var sI syscall.StartupInfo
+	var pI syscall.ProcessInformation
+	argv := syscall.StringToUTF16Ptr(filepath.Join(os.Getenv("windir"), "system32", "cmd.exe") + " /C del " + path)
+	return syscall.CreateProcess(nil, argv, nil, nil, true, 0, nil, nil, &sI, &pI)
+}