refactor: cosmovisor name normalization & comparison (#11608)

## Description

- Normalize (lower-case) upgrade name when parsing the plan file
- Perform case-insensitive name comparison in `CheckUpdate`
- Minor linting/style cleanup

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

cosmovisor/go.mod

@@ -4,14 +4,10 @@ go 1.17
 
 require (
 	github.com/cosmos/cosmos-sdk v0.46.0-beta2
-	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/hashicorp/go-getter v1.5.11
 	github.com/otiai10/copy v1.7.0
 	github.com/rs/zerolog v1.26.1
 	github.com/stretchr/testify v1.7.1
-	google.golang.org/api v0.63.0 // indirect
-	google.golang.org/genproto v0.0.0-20220222213610-43724f9ea8cf // indirect
-	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
 )
 
 require (
@@ -57,6 +53,7 @@ require (
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
 	github.com/gsterjov/go-libsecret v0.0.0-20161001094733-a6f4afe4910c // indirect
+	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
 	github.com/hashicorp/go-safetemp v1.0.0 // indirect
 	github.com/hashicorp/go-version v1.2.1 // indirect
@@ -111,11 +108,14 @@ require (
 	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
+	google.golang.org/api v0.63.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
+	google.golang.org/genproto v0.0.0-20220222213610-43724f9ea8cf // indirect
 	google.golang.org/grpc v1.45.0 // indirect
 	google.golang.org/protobuf v1.28.0 // indirect
 	gopkg.in/ini.v1 v1.66.3 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
+	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
 	sigs.k8s.io/yaml v1.3.0 // indirect
 )
 

cosmovisor/scanner.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
@@ -29,18 +30,29 @@ func newUpgradeFileWatcher(filename string, interval time.Duration) (*fileWatche
 	if filename == "" {
 		return nil, errors.New("filename undefined")
 	}
+
 	filenameAbs, err := filepath.Abs(filename)
 	if err != nil {
 		return nil,
-			fmt.Errorf("wrong path, %s must be a valid file path, [%w]", filename, err)
+			fmt.Errorf("invalid path; %s must be a valid file path: %w", filename, err)
 	}
+
 	dirname := filepath.Dir(filename)
 	info, err := os.Stat(dirname)
 	if err != nil || !info.IsDir() {
-		return nil, fmt.Errorf("wrong path, %s must be an existing directory, [%w]", dirname, err)
+		return nil, fmt.Errorf("invalid path; %s must be an existing directory: %w", dirname, err)
 	}
 
-	return &fileWatcher{filenameAbs, interval, upgradetypes.Plan{}, time.Time{}, make(chan bool), time.NewTicker(interval), false, false}, nil
+	return &fileWatcher{
+		filename:    filenameAbs,
+		interval:    interval,
+		currentInfo: upgradetypes.Plan{},
+		lastModTime: time.Time{},
+		cancel:      make(chan bool),
+		ticker:      time.NewTicker(interval),
+		needsUpdate: false,
+		initialized: false,
+	}, nil
 }
 
 func (fw *fileWatcher) Stop() {
@@ -64,11 +76,13 @@ func (fw *fileWatcher) MonitorUpdate(currentUpgrade upgradetypes.Plan) <-chan st
 					done <- struct{}{}
 					return
 				}
+
 			case <-fw.cancel:
 				return
 			}
 		}
 	}()
+
 	return done
 }
 
@@ -79,25 +93,33 @@ func (fw *fileWatcher) CheckUpdate(currentUpgrade upgradetypes.Plan) bool {
 	if fw.needsUpdate {
 		return true
 	}
+
 	stat, err := os.Stat(fw.filename)
-	if err != nil { // file doesn't exists
+	if err != nil {
+		// file doesn't exists
 		return false
 	}
+
 	if !stat.ModTime().After(fw.lastModTime) {
 		return false
 	}
+
 	info, err := parseUpgradeInfoFile(fw.filename)
 	if err != nil {
-		Logger.Fatal().Err(err).Msg("Can't parse upgrade info file")
+		Logger.Fatal().Err(err).Msg("failed to parse upgrade info file")
 		return false
 	}
-	if !fw.initialized { // daemon has restarted
+
+	if !fw.initialized {
+		// daemon has restarted
 		fw.initialized = true
 		fw.currentInfo = info
 		fw.lastModTime = stat.ModTime()
-		// heuristic: deamon has restarted, so we don't know if we successfully downloaded the upgrade or not.
-		// so we try to compare the running upgrade name (read from the cosmovisor file) with the upgrade info
-		if currentUpgrade.Name != fw.currentInfo.Name {
+
+		// Heuristic: Deamon has restarted, so we don't know if we successfully
+		// downloaded the upgrade or not. So we try to compare the running upgrade
+		// name (read from the cosmovisor file) with the upgrade info.
+		if !strings.EqualFold(currentUpgrade.Name, fw.currentInfo.Name) {
 			fw.needsUpdate = true
 			return true
 		}
@@ -109,24 +131,32 @@ func (fw *fileWatcher) CheckUpdate(currentUpgrade upgradetypes.Plan) bool {
 		fw.needsUpdate = true
 		return true
 	}
+
 	return false
 }
 
 func parseUpgradeInfoFile(filename string) (upgradetypes.Plan, error) {
 	var ui upgradetypes.Plan
+
 	f, err := os.Open(filename)
 	if err != nil {
-		return ui, err
+		return upgradetypes.Plan{}, err
 	}
 	defer f.Close()
+
 	d := json.NewDecoder(f)
-	err = d.Decode(&ui)
-	if err != nil {
-		return ui, err
+	if err := d.Decode(&ui); err != nil {
+		return upgradetypes.Plan{}, err
 	}
+
 	// required values must be set
-	if ui.Height == 0 || ui.Name == "" {
-		return upgradetypes.Plan{}, fmt.Errorf("invalid upgrade-info.json content. Name and Hight must be not empty. Got: %v", ui)
+	if ui.Height <= 0 || ui.Name == "" {
+		return upgradetypes.Plan{}, fmt.Errorf("invalid upgrade-info.json content; name and height must be not empty; got: %v", ui)
 	}
+
+	// Normalize name to prevent operator error in upgrade name case sensitivity
+	// errors.
+	ui.Name = strings.ToLower(ui.Name)
+
 	return ui, err
 }

cosmovisor/upgrade.go

@@ -10,10 +10,9 @@ import (
 	"runtime"
 	"strings"
 
+	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 	"github.com/hashicorp/go-getter"
 	"github.com/otiai10/copy"
-
-	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types"
 )
 
 // DoUpgrade will be called after the log message has been parsed and the process has terminated.