From 749e55c738bf702b5f1265f85018c450a7b4e3b4 Mon Sep 17 00:00:00 2001 From: Francis Lavoie Date: Mon, 8 Nov 2021 13:35:46 -0500 Subject: 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 | 15 +++++++++++++++ cmd/packagesfuncs.go | 35 ++++++++++++++++------------------- cmd/removebinary.go | 30 ++++++++++++++++++++++++++++++ cmd/removebinary_windows.go | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 19 deletions(-) create mode 100644 cmd/removebinary.go create mode 100644 cmd/removebinary_windows.go (limited to 'cmd') diff --git a/cmd/commands.go b/cmd/commands.go index f562430..1e2c40d 100644 --- a/cmd/commands.go +++ b/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 + }(), }) } diff --git a/cmd/packagesfuncs.go b/cmd/packagesfuncs.go index c4f41ea..ca15ea3 100644 --- a/cmd/packagesfuncs.go +++ b/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 { - return caddy.ExitCodeFailedStartup, fmt.Errorf("download succeeded, but unable to clean up backup binary: %v", err) + 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) { diff --git a/cmd/removebinary.go b/cmd/removebinary.go new file mode 100644 index 0000000..adef6b1 --- /dev/null +++ b/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) +} diff --git a/cmd/removebinary_windows.go b/cmd/removebinary_windows.go new file mode 100644 index 0000000..3d7ade5 --- /dev/null +++ b/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) +} -- cgit v1.2.3