feat(cosmovisor): strict boolean argument parsing (#10018)

## Description

Closes: #10017

---

### 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...

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

@@ -35,13 +35,15 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 # Changelog
 
-
 ## [Unreleased]
 
-## Features
+### Features
 
 + [\#8590](https://github.com/cosmos/cosmos-sdk/pull/8590) File watcher for cosmovisor. Instead of parsing logs from stdin and stderr, we watch the `<DAEMON_HOME>/data/upgrade-info.json` file updates using polling mechanism.
 
+### Improvements
+
++ [\#10018](https://github.com/cosmos/cosmos-sdk/pull/10018) Strict boolean argument parsing: cosmovisor will fail if user will not set correctly a boolean variable. Correct values are: "true", "false", "" (not setting) - all case not sensitive.
 
 ## v0.1 2021-08-06
 

cosmovisor/args.go

@@ -9,9 +9,20 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"time"
 )
 
+// environment variable names
+const (
+	envHome           = "DAEMON_HOME"
+	envName           = "DAEMON_NAME"
+	envDownloadBin    = "DAEMON_ALLOW_DOWNLOAD_BINARIES"
+	envRestartUpgrade = "DAEMON_RESTART_AFTER_UPGRADE"
+	envSkipBackup     = "UNSAFE_SKIP_BACKUP"
+	envInterval       = "DAEMON_POLL_INTERVAL"
+)
+
 const (
 	rootName        = "cosmovisor"
 	genesisDir      = "genesis"
@@ -106,19 +117,22 @@ func (cfg *Config) CurrentBin() (string, error) {
 // and then validate it is reasonable
 func GetConfigFromEnv() (*Config, error) {
 	cfg := &Config{
-		Home: os.Getenv("DAEMON_HOME"),
-		Name: os.Getenv("DAEMON_NAME"),
+		Home: os.Getenv(envHome),
+		Name: os.Getenv(envName),
 	}
 
-	if os.Getenv("DAEMON_ALLOW_DOWNLOAD_BINARIES") == "true" {
-		cfg.AllowDownloadBinaries = true
+	var err error
+	if cfg.AllowDownloadBinaries, err = booleanOption(envDownloadBin, false); err != nil {
+		return nil, err
+	}
+	if cfg.RestartAfterUpgrade, err = booleanOption(envRestartUpgrade, false); err != nil {
+		return nil, err
+	}
+	if cfg.UnsafeSkipBackup, err = booleanOption(envSkipBackup, false); err != nil {
+		return nil, err
 	}
 
-	if os.Getenv("DAEMON_RESTART_AFTER_UPGRADE") == "true" {
-		cfg.RestartAfterUpgrade = true
-	}
-
-	interval := os.Getenv("DAEMON_POLL_INTERVAL")
+	interval := os.Getenv(envInterval)
 	if interval != "" {
 		i, err := strconv.ParseUint(interval, 10, 32)
 		if err != nil {
@@ -129,8 +143,6 @@ func GetConfigFromEnv() (*Config, error) {
 		cfg.PollInterval = 300 * time.Millisecond
 	}
 
-	cfg.UnsafeSkipBackup = os.Getenv("UNSAFE_SKIP_BACKUP") == "true"
-
 	if err := cfg.validate(); err != nil {
 		return nil, err
 	}
@@ -142,15 +154,15 @@ func GetConfigFromEnv() (*Config, error) {
 // and that Name is set
 func (cfg *Config) validate() error {
 	if cfg.Name == "" {
-		return errors.New("DAEMON_NAME is not set")
+		return errors.New(envName + " is not set")
 	}
 
 	if cfg.Home == "" {
-		return errors.New("DAEMON_HOME is not set")
+		return errors.New(envHome + " is not set")
 	}
 
 	if !filepath.IsAbs(cfg.Home) {
-		return errors.New("DAEMON_HOME must be an absolute path")
+		return errors.New(envHome + " must be an absolute path")
 	}
 
 	// ensure the root directory exists
@@ -231,3 +243,17 @@ returnError:
 	cfg.currentUpgrade.Name = "_"
 	return cfg.currentUpgrade
 }
+
+// checks and validates env option
+func booleanOption(name string, defaultVal bool) (bool, error) {
+	p := strings.ToLower(os.Getenv(name))
+	switch p {
+	case "":
+		return defaultVal, nil
+	case "false":
+		return false, nil
+	case "true":
+		return true, nil
+	}
+	return false, fmt.Errorf("env variable %q must have a boolean value (\"true\" or \"false\"), got %q", name, p)
+}

cosmovisor/args_test.go

@@ -2,6 +2,7 @@ package cosmovisor
 
 import (
 	"fmt"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -129,3 +130,43 @@ func (s *argsTestSuite) TestEnsureBin() {
 		}
 	}
 }
+
+func (s *argsTestSuite) TestBooleanOption() {
+	require := s.Require()
+	name := "COSMOVISOR_TEST_VAL"
+
+	check := func(def, expected, isErr bool, msg string) {
+		v, err := booleanOption(name, def)
+		if isErr {
+			require.Error(err)
+			return
+		}
+		require.NoError(err)
+		require.Equal(expected, v, msg)
+	}
+
+	os.Setenv(name, "")
+	check(true, true, false, "should correctly set default value")
+	check(false, false, false, "should correctly set default value")
+
+	os.Setenv(name, "wrong")
+	check(true, true, true, "should error on wrong value")
+	os.Setenv(name, "truee")
+	check(true, true, true, "should error on wrong value")
+
+	os.Setenv(name, "false")
+	check(true, false, false, "should handle false value")
+	check(false, false, false, "should handle false value")
+	os.Setenv(name, "faLSe")
+	check(true, false, false, "should handle false value case not sensitive")
+	check(false, false, false, "should handle false value case not sensitive")
+
+	os.Setenv(name, "true")
+	check(true, true, false, "should handle true value")
+	check(false, true, false, "should handle true value")
+
+	os.Setenv(name, "TRUE")
+	check(true, true, false, "should handle true value case not sensitive")
+	check(false, true, false, "should handle true value case not sensitive")
+
+}