Allow ConfigParseError to unwrap (#1433)

* Allow ConfigParseError to unwrap

* wip

* Avoid pointer type

.github/workflows/checks.yaml

@@ -11,7 +11,7 @@ jobs:
 
     steps:
       - name: Check minimum labels
-        uses: mheap/github-action-required-labels@v3
+        uses: mheap/github-action-required-labels@v4
         with:
           mode: minimum
           count: 1

.github/workflows/ci.yaml

@@ -21,9 +21,9 @@ jobs:
 
     steps:
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v4
         with:
-          go-version: 1.19
+          go-version: '1.20'
 
       - name: Checkout code
         uses: actions/checkout@v3
@@ -43,13 +43,13 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-latest, macos-latest, windows-latest]
-        go: ['1.17', '1.18', '1.19']
+        go: ['1.17', '1.18', '1.19', '1.20']
     env:
       GOFLAGS: -mod=readonly
 
     steps:
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v4
         with:
           go-version: ${{ matrix.go }}
 
@@ -72,9 +72,9 @@ jobs:
 
     steps:
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v4
         with:
-          go-version: 1.19
+          go-version: '1.20'
 
       - name: Checkout code
         uses: actions/checkout@v3
@@ -82,4 +82,4 @@ jobs:
       - name: Lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.50.1
+          version: v1.52.2

.golangci.yaml

@@ -16,7 +16,6 @@ linters:
     disable-all: true
     enable:
         - bodyclose
-        - deadcode
         - dogsled
         - dupl
         - durationcheck
@@ -43,14 +42,12 @@ linters:
         - rowserrcheck
         - sqlclosecheck
         - staticcheck
-        - structcheck
         - stylecheck
         - tparallel
         - typecheck
         - unconvert
         - unparam
         - unused
-        - varcheck
         - wastedassign
         - whitespace
 
@@ -83,6 +80,11 @@ linters:
         # - goheader
         # - gomodguard
 
+        # deprecated
+        # - deadcode
+        # - structcheck
+        # - varcheck
+
         # don't enable:
         # - asciicheck
         # - funlen

Makefile

@@ -15,8 +15,8 @@ TEST_FORMAT = short-verbose
 endif
 
 # Dependency versions
-GOTESTSUM_VERSION = 1.8.0
-GOLANGCI_VERSION = 1.50.1
+GOTESTSUM_VERSION = 1.9.0
+GOLANGCI_VERSION = 1.52.2
 
 # Add the ability to override some variables
 # Use with care

README.md

@@ -27,6 +27,9 @@ Many Go projects are built using Viper including:
 * [doctl](https://github.com/digitalocean/doctl)
 * [Clairctl](https://github.com/jgsqware/clairctl)
 * [Mercure](https://mercure.rocks)
+* [Meshery](https://github.com/meshery/meshery)
+* [Bearer](https://github.com/bearer/bearer)
+* [Coder](https://github.com/coder/coder)
 
 
 ## Install

go.mod

@@ -7,24 +7,24 @@ require (
 	github.com/hashicorp/hcl v1.0.0
 	github.com/magiconair/properties v1.8.7
 	github.com/mitchellh/mapstructure v1.5.0
-	github.com/pelletier/go-toml/v2 v2.0.6
-	github.com/sagikazarmark/crypt v0.9.0
-	github.com/spf13/afero v1.9.3
-	github.com/spf13/cast v1.5.0
+	github.com/pelletier/go-toml/v2 v2.0.8
+	github.com/sagikazarmark/crypt v0.10.0
+	github.com/spf13/afero v1.9.5
+	github.com/spf13/cast v1.5.1
 	github.com/spf13/jwalterweatherman v1.1.0
 	github.com/spf13/pflag v1.0.5
-	github.com/stretchr/testify v1.8.1
+	github.com/stretchr/testify v1.8.3
 	github.com/subosito/gotenv v1.4.2
 	gopkg.in/ini.v1 v1.67.0
 	gopkg.in/yaml.v3 v3.0.1
 )
 
 require (
-	cloud.google.com/go v0.105.0 // indirect
-	cloud.google.com/go/compute v1.14.0 // indirect
+	cloud.google.com/go v0.110.0 // indirect
+	cloud.google.com/go/compute v1.19.0 // indirect
 	cloud.google.com/go/compute/metadata v0.2.3 // indirect
 	cloud.google.com/go/firestore v1.9.0 // indirect
-	cloud.google.com/go/longrunning v0.3.0 // indirect
+	cloud.google.com/go/longrunning v0.4.1 // indirect
 	github.com/armon/go-metrics v0.4.0 // indirect
 	github.com/coreos/go-semver v0.3.0 // indirect
 	github.com/coreos/go-systemd/v22 v22.3.2 // indirect
@@ -32,11 +32,12 @@ require (
 	github.com/fatih/color v1.13.0 // indirect
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
-	github.com/golang/protobuf v1.5.2 // indirect
+	github.com/golang/protobuf v1.5.3 // indirect
 	github.com/google/go-cmp v0.5.9 // indirect
-	github.com/googleapis/enterprise-certificate-proxy v0.2.1 // indirect
-	github.com/googleapis/gax-go/v2 v2.7.0 // indirect
-	github.com/hashicorp/consul/api v1.18.0 // indirect
+	github.com/google/s2a-go v0.1.3 // indirect
+	github.com/googleapis/enterprise-certificate-proxy v0.2.3 // indirect
+	github.com/googleapis/gax-go/v2 v2.8.0 // indirect
+	github.com/hashicorp/consul/api v1.20.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
 	github.com/hashicorp/go-hclog v1.2.0 // indirect
 	github.com/hashicorp/go-immutable-radix v1.3.1 // indirect
@@ -51,25 +52,25 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
-	go.etcd.io/etcd/api/v3 v3.5.6 // indirect
-	go.etcd.io/etcd/client/pkg/v3 v3.5.6 // indirect
-	go.etcd.io/etcd/client/v2 v2.305.6 // indirect
-	go.etcd.io/etcd/client/v3 v3.5.6 // indirect
+	go.etcd.io/etcd/api/v3 v3.5.9 // indirect
+	go.etcd.io/etcd/client/pkg/v3 v3.5.9 // indirect
+	go.etcd.io/etcd/client/v2 v2.305.7 // indirect
+	go.etcd.io/etcd/client/v3 v3.5.9 // indirect
 	go.opencensus.io v0.24.0 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
 	go.uber.org/multierr v1.8.0 // indirect
 	go.uber.org/zap v1.21.0 // indirect
-	golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e // indirect
-	golang.org/x/net v0.4.0 // indirect
-	golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783 // indirect
+	golang.org/x/crypto v0.9.0 // indirect
+	golang.org/x/net v0.10.0 // indirect
+	golang.org/x/oauth2 v0.7.0 // indirect
 	golang.org/x/sync v0.1.0 // indirect
-	golang.org/x/sys v0.3.0 // indirect
-	golang.org/x/text v0.5.0 // indirect
+	golang.org/x/sys v0.8.0 // indirect
+	golang.org/x/text v0.9.0 // indirect
 	golang.org/x/time v0.1.0 // indirect
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
-	google.golang.org/api v0.107.0 // indirect
+	google.golang.org/api v0.122.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
-	google.golang.org/genproto v0.0.0-20221227171554-f9683d7f8bef // indirect
-	google.golang.org/grpc v1.52.0 // indirect
-	google.golang.org/protobuf v1.28.1 // indirect
+	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
+	google.golang.org/grpc v1.55.0 // indirect
+	google.golang.org/protobuf v1.30.0 // indirect
 )

util.go

@@ -31,6 +31,11 @@ func (pe ConfigParseError) Error() string {
 	return fmt.Sprintf("While parsing config: %s", pe.err.Error())
 }
 
+// Unwrap returns the wrapped error.
+func (pe ConfigParseError) Unwrap() error {
+	return pe.err
+}
+
 // toCaseInsensitiveValue checks if the value is a  map;
 // if so, create a copy and lower-case the keys recursively.
 func toCaseInsensitiveValue(value interface{}) interface{} {

viper.go

@@ -25,7 +25,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -206,6 +205,7 @@ type Viper struct {
 	envKeyReplacer      StringReplacer
 	allowEmptyEnv       bool
 
+	parents        []string
 	config         map[string]interface{}
 	override       map[string]interface{}
 	defaults       map[string]interface{}
@@ -232,6 +232,7 @@ func New() *Viper {
 	v.configPermissions = os.FileMode(0o644)
 	v.fs = afero.NewOsFs()
 	v.config = make(map[string]interface{})
+	v.parents = []string{}
 	v.override = make(map[string]interface{})
 	v.defaults = make(map[string]interface{})
 	v.kvstore = make(map[string]interface{})
@@ -439,13 +440,14 @@ func (v *Viper) WatchConfig() {
 	go func() {
 		watcher, err := newWatcher()
 		if err != nil {
-			log.Fatal(err)
+			v.logger.Error(fmt.Sprintf("failed to create watcher: %s", err))
+			os.Exit(1)
 		}
 		defer watcher.Close()
 		// we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way
 		filename, err := v.getConfigFile()
 		if err != nil {
-			log.Printf("error: %v\n", err)
+			v.logger.Error(fmt.Sprintf("get config file: %s", err))
 			initWG.Done()
 			return
 		}
@@ -474,7 +476,7 @@ func (v *Viper) WatchConfig() {
 						realConfigFile = currentConfigFile
 						err := v.ReadInConfig()
 						if err != nil {
-							log.Printf("error reading config file: %v\n", err)
+							v.logger.Error(fmt.Sprintf("read config file: %s", err))
 						}
 						if v.onConfigChange != nil {
 							v.onConfigChange(event)
@@ -486,7 +488,7 @@ func (v *Viper) WatchConfig() {
 
 				case err, ok := <-watcher.Errors:
 					if ok { // 'Errors' channel is not closed
-						log.Printf("watcher error: %v\n", err)
+						v.logger.Error(fmt.Sprintf("watcher error: %s", err))
 					}
 					eventsWG.Done()
 					return
@@ -928,6 +930,8 @@ func (v *Viper) Get(key string) interface{} {
 			return cast.ToStringSlice(val)
 		case []int:
 			return cast.ToIntSlice(val)
+		case []time.Duration:
+			return cast.ToDurationSlice(val)
 		}
 	}
 
@@ -946,6 +950,10 @@ func (v *Viper) Sub(key string) *Viper {
 	}
 
 	if reflect.TypeOf(data).Kind() == reflect.Map {
+		subv.parents = append(v.parents, strings.ToLower(key))
+		subv.automaticEnvApplied = v.automaticEnvApplied
+		subv.envPrefix = v.envPrefix
+		subv.envKeyReplacer = v.envKeyReplacer
 		subv.config = cast.ToStringMap(data)
 		return subv
 	}
@@ -1099,7 +1107,7 @@ func (v *Viper) Unmarshal(rawVal interface{}, opts ...DecoderConfigOption) error
 	return decode(v.AllSettings(), defaultDecoderConfig(rawVal, opts...))
 }
 
-// defaultDecoderConfig returns default mapsstructure.DecoderConfig with suppot
+// defaultDecoderConfig returns default mapstructure.DecoderConfig with support
 // of time.Duration values & string slices
 func defaultDecoderConfig(output interface{}, opts ...DecoderConfigOption) *mapstructure.DecoderConfig {
 	c := &mapstructure.DecoderConfig{
@@ -1274,8 +1282,15 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} {
 			s = strings.TrimSuffix(s, "]")
 			res, _ := readAsCSV(s)
 			return cast.ToIntSlice(res)
+		case "durationSlice":
+			s := strings.TrimPrefix(flag.ValueString(), "[")
+			s = strings.TrimSuffix(s, "]")
+			slice := strings.Split(s, ",")
+			return cast.ToDurationSlice(slice)
 		case "stringToString":
 			return stringToStringConv(flag.ValueString())
+		case "stringToInt":
+			return stringToIntConv(flag.ValueString())
 		default:
 			return flag.ValueString()
 		}
@@ -1286,9 +1301,10 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} {
 
 	// Env override next
 	if v.automaticEnvApplied {
+		envKey := strings.Join(append(v.parents, lcaseKey), ".")
 		// even if it hasn't been registered, if automaticEnv is used,
 		// check any Get request
-		if val, ok := v.getEnv(v.mergeWithEnvPrefix(lcaseKey)); ok {
+		if val, ok := v.getEnv(v.mergeWithEnvPrefix(envKey)); ok {
 			return val
 		}
 		if nested && v.isPathShadowedInAutoEnv(path) != "" {
@@ -1355,6 +1371,13 @@ func (v *Viper) find(lcaseKey string, flagDefault bool) interface{} {
 				return cast.ToIntSlice(res)
 			case "stringToString":
 				return stringToStringConv(flag.ValueString())
+			case "stringToInt":
+				return stringToIntConv(flag.ValueString())
+			case "durationSlice":
+				s := strings.TrimPrefix(flag.ValueString(), "[")
+				s = strings.TrimSuffix(s, "]")
+				slice := strings.Split(s, ",")
+				return cast.ToDurationSlice(slice)
 			default:
 				return flag.ValueString()
 			}
@@ -1398,6 +1421,30 @@ func stringToStringConv(val string) interface{} {
 	return out
 }
 
+// mostly copied from pflag's implementation of this operation here https://github.com/spf13/pflag/blob/d5e0c0615acee7028e1e2740a11102313be88de1/string_to_int.go#L68
+// alterations are: errors are swallowed, map[string]interface{} is returned in order to enable cast.ToStringMap
+func stringToIntConv(val string) interface{} {
+	val = strings.Trim(val, "[]")
+	// An empty string would cause an empty map
+	if len(val) == 0 {
+		return map[string]interface{}{}
+	}
+	ss := strings.Split(val, ",")
+	out := make(map[string]interface{}, len(ss))
+	for _, pair := range ss {
+		kv := strings.SplitN(pair, "=", 2)
+		if len(kv) != 2 {
+			return nil
+		}
+		var err error
+		out[kv[0]], err = strconv.Atoi(kv[1])
+		if err != nil {
+			return nil
+		}
+	}
+	return out
+}
+
 // IsSet checks to see if the key has been set in any of the data locations.
 // IsSet is case-insensitive for a key.
 func IsSet(key string) bool { return v.IsSet(key) }

viper_test.go

@@ -8,8 +8,9 @@ package viper
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"io"
-	"io/ioutil" //nolint:staticcheck
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"path"
@@ -733,6 +734,24 @@ func TestEnvKeyReplacer(t *testing.T) {
 	assert.Equal(t, "30s", v.Get("refresh-interval"))
 }
 
+func TestEnvSubConfig(t *testing.T) {
+	initYAML()
+
+	v.AutomaticEnv()
+
+	v.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
+
+	testutil.Setenv(t, "CLOTHING_PANTS_SIZE", "small")
+	subv := v.Sub("clothing").Sub("pants")
+	assert.Equal(t, "small", subv.Get("size"))
+
+	// again with EnvPrefix
+	v.SetEnvPrefix("foo") // will be uppercased automatically
+	subWithPrefix := v.Sub("clothing").Sub("pants")
+	testutil.Setenv(t, "FOO_CLOTHING_PANTS_SIZE", "large")
+	assert.Equal(t, "large", subWithPrefix.Get("size"))
+}
+
 func TestAllKeys(t *testing.T) {
 	initConfigs()
 
@@ -874,8 +893,10 @@ func TestAliasesOfAliases(t *testing.T) {
 }
 
 func TestRecursiveAliases(t *testing.T) {
+	Set("baz", "bat")
 	RegisterAlias("Baz", "Roo")
 	RegisterAlias("Roo", "baz")
+	assert.Equal(t, "bat", Get("Baz"))
 }
 
 func TestUnmarshal(t *testing.T) {
@@ -1094,6 +1115,28 @@ func TestBindPFlagsStringArray(t *testing.T) {
 	}
 }
 
+func TestSliceFlagsReturnCorrectType(t *testing.T) {
+	flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	flagSet.IntSlice("int", []int{1, 2}, "")
+	flagSet.StringSlice("str", []string{"3", "4"}, "")
+	flagSet.DurationSlice("duration", []time.Duration{5 * time.Second}, "")
+
+	v := New()
+	v.BindPFlags(flagSet)
+
+	all := v.AllSettings()
+
+	if _, ok := all["int"].([]int); !ok {
+		t.Errorf("unexpected type %T expected []int", all["int"])
+	}
+	if _, ok := all["str"].([]string); !ok {
+		t.Errorf("unexpected type %T expected []string", all["str"])
+	}
+	if _, ok := all["duration"].([]time.Duration); !ok {
+		t.Errorf("unexpected type %T expected []time.Duration", all["duration"])
+	}
+}
+
 //nolint:dupl
 func TestBindPFlagsIntSlice(t *testing.T) {
 	tests := []struct {
@@ -1213,6 +1256,53 @@ func TestBindPFlagStringToString(t *testing.T) {
 	}
 }
 
+func TestBindPFlagStringToInt(t *testing.T) {
+	tests := []struct {
+		Expected map[string]int
+		Value    string
+	}{
+		{map[string]int{"yo": 1, "oh": 21}, "yo=1,oh=21"},
+		{map[string]int{"yo": 100000000, "oh": 0}, "yo=100000000,oh=0"},
+		{map[string]int{}, "yo=2,oh=21.0"},
+		{map[string]int{}, "yo=,oh=20.99"},
+		{map[string]int{}, "yo=,oh="},
+	}
+
+	v := New() // create independent Viper object
+	defaultVal := map[string]int{}
+	v.SetDefault("stringtoint", defaultVal)
+
+	for _, testValue := range tests {
+		flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+		flagSet.StringToInt("stringtoint", testValue.Expected, "test")
+
+		for _, changed := range []bool{true, false} {
+			flagSet.VisitAll(func(f *pflag.Flag) {
+				f.Value.Set(testValue.Value)
+				f.Changed = changed
+			})
+
+			err := v.BindPFlags(flagSet)
+			if err != nil {
+				t.Fatalf("error binding flag set, %v", err)
+			}
+
+			type TestMap struct {
+				StringToInt map[string]int
+			}
+			val := &TestMap{}
+			if err := v.Unmarshal(val); err != nil {
+				t.Fatalf("%+#v cannot unmarshal: %s", testValue.Value, err)
+			}
+			if changed {
+				assert.Equal(t, testValue.Expected, val.StringToInt)
+			} else {
+				assert.Equal(t, defaultVal, val.StringToInt)
+			}
+		}
+	}
+}
+
 func TestBoundCaseSensitivity(t *testing.T) {
 	assert.Equal(t, "brown", Get("eyes"))
 
@@ -1484,6 +1574,18 @@ func TestWrongDirsSearchNotFoundForMerge(t *testing.T) {
 	assert.Equal(t, `default`, v.GetString(`key`))
 }
 
+var yamlInvalid = []byte(`hash: map
+- foo
+- bar
+`)
+
+func TestUnwrapParseErrors(t *testing.T) {
+	SetConfigType("yaml")
+	if !errors.As(ReadConfig(bytes.NewBuffer(yamlInvalid)), &ConfigParseError{}) {
+		t.Fatalf("not a ConfigParseError")
+	}
+}
+
 func TestSub(t *testing.T) {
 	v := New()
 	v.SetConfigType("yaml")
@@ -1500,6 +1602,14 @@ func TestSub(t *testing.T) {
 
 	subv = v.Sub("missing.key")
 	assert.Equal(t, (*Viper)(nil), subv)
+
+	subv = v.Sub("clothing")
+	assert.Equal(t, subv.parents[0], "clothing")
+
+	subv = v.Sub("clothing").Sub("pants")
+	assert.Equal(t, len(subv.parents), 2)
+	assert.Equal(t, subv.parents[0], "clothing")
+	assert.Equal(t, subv.parents[1], "pants")
 }
 
 var hclWriteExpected = []byte(`"foos" = {