MM-10730 Added support for empty environment variables to viper (#8973)

Gopkg.lock

@@ -473,10 +473,10 @@
   version = "v1.0.1"
 
 [[projects]]
-  branch = "env-settings"
+  branch = "mattermost"
   name = "github.com/mattermost/viper"
   packages = ["."]
-  revision = "4f5003aa93559718c866d86fbc795439079484f5"
+  revision = "1b00ce64485c7a2449f239de3cbc9040a9c46aa0"
   source = "https://github.com/mattermost/viper"
 
 [[projects]]

Gopkg.toml

@@ -48,7 +48,7 @@
 [[constraint]]
   name = "github.com/mattermost/viper"
   source = "https://github.com/mattermost/viper"
-  branch = "env-settings"
+  branch = "mattermost"
 
 # Keep back because of breaking API changes
 [[constraint]]

utils/config_test.go

@@ -335,6 +335,9 @@ func TestConfigFromEnviroVars(t *testing.T) {
 		"TeamSettings": {
 			"SiteName": "Mattermost",
 			"CustomBrandText": ""
+		},
+		"SupportSettings": {
+			"TermsOfServiceLink": "https://about.mattermost.com/default-terms/"
 		}
 	}`
 
@@ -447,6 +450,28 @@ func TestConfigFromEnviroVars(t *testing.T) {
 			}
 		}
 	})
+
+	t.Run("empty string setting", func(t *testing.T) {
+		os.Setenv("MM_SUPPORTSETTINGS_TERMSOFSERVICELINK", "")
+		defer os.Unsetenv("MM_SUPPORTSETTINGS_TERMSOFSERVICELINK")
+
+		cfg, envCfg, err := ReadConfig(strings.NewReader(config), true)
+		require.Nil(t, err)
+
+		if *cfg.SupportSettings.TermsOfServiceLink != "" {
+			t.Fatal("Couldn't read empty TermsOfServiceLink from environment var")
+		}
+
+		if supportSettings, ok := envCfg["SupportSettings"]; !ok {
+			t.Fatal("SupportSettings is missing from envConfig")
+		} else if supportSettingsAsMap, ok := supportSettings.(map[string]interface{}); !ok {
+			t.Fatal("SupportSettings is not a map in envConfig")
+		} else {
+			if termsOfServiceLinkInEnv, ok := supportSettingsAsMap["TermsOfServiceLink"].(bool); !ok || !termsOfServiceLinkInEnv {
+				t.Fatal("TermsOfServiceLink should be in envConfig")
+			}
+		}
+	})
 }
 
 func TestValidateLocales(t *testing.T) {

vendor/github.com/mattermost/viper/flags_test.go

@@ -0,0 +1,65 @@
+package viper
+
+import (
+	"testing"
+
+	"github.com/spf13/pflag"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestBindFlagValueSet(t *testing.T) {
+	flagSet := pflag.NewFlagSet("test", pflag.ContinueOnError)
+
+	var testValues = map[string]*string{
+		"host":     nil,
+		"port":     nil,
+		"endpoint": nil,
+	}
+
+	var mutatedTestValues = map[string]string{
+		"host":     "localhost",
+		"port":     "6060",
+		"endpoint": "/public",
+	}
+
+	for name := range testValues {
+		testValues[name] = flagSet.String(name, "", "test")
+	}
+
+	flagValueSet := pflagValueSet{flagSet}
+
+	err := BindFlagValues(flagValueSet)
+	if err != nil {
+		t.Fatalf("error binding flag set, %v", err)
+	}
+
+	flagSet.VisitAll(func(flag *pflag.Flag) {
+		flag.Value.Set(mutatedTestValues[flag.Name])
+		flag.Changed = true
+	})
+
+	for name, expected := range mutatedTestValues {
+		assert.Equal(t, Get(name), expected)
+	}
+}
+
+func TestBindFlagValue(t *testing.T) {
+	var testString = "testing"
+	var testValue = newStringValue(testString, &testString)
+
+	flag := &pflag.Flag{
+		Name:    "testflag",
+		Value:   testValue,
+		Changed: false,
+	}
+
+	flagValue := pflagValue{flag}
+	BindFlagValue("testvalue", flagValue)
+
+	assert.Equal(t, testString, Get("testvalue"))
+
+	flag.Value.Set("testing_mutate")
+	flag.Changed = true //hack for pflag usage
+
+	assert.Equal(t, "testing_mutate", Get("testvalue"))
+}

vendor/github.com/mattermost/viper/overrides_test.go

@@ -0,0 +1,173 @@
+package viper
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/spf13/cast"
+	"github.com/stretchr/testify/assert"
+)
+
+type layer int
+
+const (
+	defaultLayer layer = iota + 1
+	overrideLayer
+)
+
+func TestNestedOverrides(t *testing.T) {
+	assert := assert.New(t)
+	var v *Viper
+
+	// Case 0: value overridden by a value
+	overrideDefault(assert, "tom", 10, "tom", 20) // "tom" is first given 10 as default value, then overridden by 20
+	override(assert, "tom", 10, "tom", 20)        // "tom" is first given value 10, then overridden by 20
+	overrideDefault(assert, "tom.age", 10, "tom.age", 20)
+	override(assert, "tom.age", 10, "tom.age", 20)
+	overrideDefault(assert, "sawyer.tom.age", 10, "sawyer.tom.age", 20)
+	override(assert, "sawyer.tom.age", 10, "sawyer.tom.age", 20)
+
+	// Case 1: key:value overridden by a value
+	v = overrideDefault(assert, "tom.age", 10, "tom", "boy") // "tom.age" is first given 10 as default value, then "tom" is overridden by "boy"
+	assert.Nil(v.Get("tom.age"))                             // "tom.age" should not exist anymore
+	v = override(assert, "tom.age", 10, "tom", "boy")
+	assert.Nil(v.Get("tom.age"))
+
+	// Case 2: value overridden by a key:value
+	overrideDefault(assert, "tom", "boy", "tom.age", 10) // "tom" is first given "boy" as default value, then "tom" is overridden by map{"age":10}
+	override(assert, "tom.age", 10, "tom", "boy")
+
+	// Case 3: key:value overridden by a key:value
+	v = overrideDefault(assert, "tom.size", 4, "tom.age", 10)
+	assert.Equal(4, v.Get("tom.size")) // value should still be reachable
+	v = override(assert, "tom.size", 4, "tom.age", 10)
+	assert.Equal(4, v.Get("tom.size"))
+	deepCheckValue(assert, v, overrideLayer, []string{"tom", "size"}, 4)
+
+	// Case 4: key:value overridden by a map
+	v = overrideDefault(assert, "tom.size", 4, "tom", map[string]interface{}{"age": 10}) // "tom.size" is first given "4" as default value, then "tom" is overridden by map{"age":10}
+	assert.Equal(4, v.Get("tom.size"))                                                   // "tom.size" should still be reachable
+	assert.Equal(10, v.Get("tom.age"))                                                   // new value should be there
+	deepCheckValue(assert, v, overrideLayer, []string{"tom", "age"}, 10)                 // new value should be there
+	v = override(assert, "tom.size", 4, "tom", map[string]interface{}{"age": 10})
+	assert.Nil(v.Get("tom.size"))
+	assert.Equal(10, v.Get("tom.age"))
+	deepCheckValue(assert, v, overrideLayer, []string{"tom", "age"}, 10)
+
+	// Case 5: array overridden by a value
+	overrideDefault(assert, "tom", []int{10, 20}, "tom", 30)
+	override(assert, "tom", []int{10, 20}, "tom", 30)
+	overrideDefault(assert, "tom.age", []int{10, 20}, "tom.age", 30)
+	override(assert, "tom.age", []int{10, 20}, "tom.age", 30)
+
+	// Case 6: array overridden by an array
+	overrideDefault(assert, "tom", []int{10, 20}, "tom", []int{30, 40})
+	override(assert, "tom", []int{10, 20}, "tom", []int{30, 40})
+	overrideDefault(assert, "tom.age", []int{10, 20}, "tom.age", []int{30, 40})
+	v = override(assert, "tom.age", []int{10, 20}, "tom.age", []int{30, 40})
+	// explicit array merge:
+	s, ok := v.Get("tom.age").([]int)
+	if assert.True(ok, "tom[\"age\"] is not a slice") {
+		v.Set("tom.age", append(s, []int{50, 60}...))
+		assert.Equal([]int{30, 40, 50, 60}, v.Get("tom.age"))
+		deepCheckValue(assert, v, overrideLayer, []string{"tom", "age"}, []int{30, 40, 50, 60})
+	}
+}
+
+func overrideDefault(assert *assert.Assertions, firstPath string, firstValue interface{}, secondPath string, secondValue interface{}) *Viper {
+	return overrideFromLayer(defaultLayer, assert, firstPath, firstValue, secondPath, secondValue)
+}
+func override(assert *assert.Assertions, firstPath string, firstValue interface{}, secondPath string, secondValue interface{}) *Viper {
+	return overrideFromLayer(overrideLayer, assert, firstPath, firstValue, secondPath, secondValue)
+}
+
+// overrideFromLayer performs the sequential override and low-level checks.
+//
+// First assignment is made on layer l for path firstPath with value firstValue,
+// the second one on the override layer (i.e., with the Set() function)
+// for path secondPath with value secondValue.
+//
+// firstPath and secondPath can include an arbitrary number of dots to indicate
+// a nested element.
+//
+// After each assignment, the value is checked, retrieved both by its full path
+// and by its key sequence (successive maps).
+func overrideFromLayer(l layer, assert *assert.Assertions, firstPath string, firstValue interface{}, secondPath string, secondValue interface{}) *Viper {
+	v := New()
+	firstKeys := strings.Split(firstPath, v.keyDelim)
+	if assert == nil ||
+		len(firstKeys) == 0 || len(firstKeys[0]) == 0 {
+		return v
+	}
+
+	// Set and check first value
+	switch l {
+	case defaultLayer:
+		v.SetDefault(firstPath, firstValue)
+	case overrideLayer:
+		v.Set(firstPath, firstValue)
+	default:
+		return v
+	}
+	assert.Equal(firstValue, v.Get(firstPath))
+	deepCheckValue(assert, v, l, firstKeys, firstValue)
+
+	// Override and check new value
+	secondKeys := strings.Split(secondPath, v.keyDelim)
+	if len(secondKeys) == 0 || len(secondKeys[0]) == 0 {
+		return v
+	}
+	v.Set(secondPath, secondValue)
+	assert.Equal(secondValue, v.Get(secondPath))
+	deepCheckValue(assert, v, overrideLayer, secondKeys, secondValue)
+
+	return v
+}
+
+// deepCheckValue checks that all given keys correspond to a valid path in the
+// configuration map of the given layer, and that the final value equals the one given
+func deepCheckValue(assert *assert.Assertions, v *Viper, l layer, keys []string, value interface{}) {
+	if assert == nil || v == nil ||
+		len(keys) == 0 || len(keys[0]) == 0 {
+		return
+	}
+
+	// init
+	var val interface{}
+	var ms string
+	switch l {
+	case defaultLayer:
+		val = v.defaults
+		ms = "v.defaults"
+	case overrideLayer:
+		val = v.override
+		ms = "v.override"
+	}
+
+	// loop through map
+	var m map[string]interface{}
+	err := false
+	for _, k := range keys {
+		if val == nil {
+			assert.Fail(fmt.Sprintf("%s is not a map[string]interface{}", ms))
+			return
+		}
+
+		// deep scan of the map to get the final value
+		switch val.(type) {
+		case map[interface{}]interface{}:
+			m = cast.ToStringMap(val)
+		case map[string]interface{}:
+			m = val.(map[string]interface{})
+		default:
+			assert.Fail(fmt.Sprintf("%s is not a map[string]interface{}", ms))
+			return
+		}
+		ms = ms + "[\"" + k + "\"]"
+		val = m[k]
+	}
+	if !err {
+		assert.Equal(value, val)
+	}
+}

vendor/github.com/mattermost/viper/remote/remote.go

@@ -0,0 +1,105 @@
+// Copyright © 2015 Steve Francia <spf@spf13.com>.
+//
+// Use of this source code is governed by an MIT-style
+// license that can be found in the LICENSE file.
+
+// Package remote integrates the remote features of Viper.
+package remote
+
+import (
+	"bytes"
+	"io"
+	"os"
+
+	"github.com/spf13/viper"
+	crypt "github.com/xordataexchange/crypt/config"
+)
+
+type remoteConfigProvider struct{}
+
+func (rc remoteConfigProvider) Get(rp viper.RemoteProvider) (io.Reader, error) {
+	cm, err := getConfigManager(rp)
+	if err != nil {
+		return nil, err
+	}
+	b, err := cm.Get(rp.Path())
+	if err != nil {
+		return nil, err
+	}
+	return bytes.NewReader(b), nil
+}
+
+func (rc remoteConfigProvider) Watch(rp viper.RemoteProvider) (io.Reader, error) {
+	cm, err := getConfigManager(rp)
+	if err != nil {
+		return nil, err
+	}
+	resp, err := cm.Get(rp.Path())
+	if err != nil {
+		return nil, err
+	}
+
+	return bytes.NewReader(resp), nil
+}
+
+func (rc remoteConfigProvider) WatchChannel(rp viper.RemoteProvider) (<-chan *viper.RemoteResponse, chan bool) {
+	cm, err := getConfigManager(rp)
+	if err != nil {
+		return nil, nil
+	}
+	quit := make(chan bool)
+	quitwc := make(chan bool)
+	viperResponsCh := make(chan *viper.RemoteResponse)
+	cryptoResponseCh := cm.Watch(rp.Path(), quit)
+	// need this function to convert the Channel response form crypt.Response to viper.Response
+	go func(cr <-chan *crypt.Response, vr chan<- *viper.RemoteResponse, quitwc <-chan bool, quit chan<- bool) {
+		for {
+			select {
+			case <-quitwc:
+				quit <- true
+				return
+			case resp := <-cr:
+				vr <- &viper.RemoteResponse{
+					Error: resp.Error,
+					Value: resp.Value,
+				}
+
+			}
+
+		}
+	}(cryptoResponseCh, viperResponsCh, quitwc, quit)
+
+	return viperResponsCh, quitwc
+}
+
+func getConfigManager(rp viper.RemoteProvider) (crypt.ConfigManager, error) {
+	var cm crypt.ConfigManager
+	var err error
+
+	if rp.SecretKeyring() != "" {
+		kr, err := os.Open(rp.SecretKeyring())
+		defer kr.Close()
+		if err != nil {
+			return nil, err
+		}
+		if rp.Provider() == "etcd" {
+			cm, err = crypt.NewEtcdConfigManager([]string{rp.Endpoint()}, kr)
+		} else {
+			cm, err = crypt.NewConsulConfigManager([]string{rp.Endpoint()}, kr)
+		}
+	} else {
+		if rp.Provider() == "etcd" {
+			cm, err = crypt.NewStandardEtcdConfigManager([]string{rp.Endpoint()})
+		} else {
+			cm, err = crypt.NewStandardConsulConfigManager([]string{rp.Endpoint()})
+		}
+	}
+	if err != nil {
+		return nil, err
+	}
+	return cm, nil
+}
+
+func init() {
+	viper.RemoteConfig = &remoteConfigProvider{}
+}

vendor/github.com/mattermost/viper/util_test.go

@@ -0,0 +1,54 @@
+// Copyright © 2016 Steve Francia <spf@spf13.com>.
+//
+// Use of this source code is governed by an MIT-style
+// license that can be found in the LICENSE file.
+
+// Viper is a application configuration system.
+// It believes that applications can be configured a variety of ways
+// via flags, ENVIRONMENT variables, configuration files retrieved
+// from the file system, or a remote key/value store.
+
+package viper
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestCopyAndInsensitiviseMap(t *testing.T) {
+	var (
+		given = map[string]interface{}{
+			"Foo": 32,
+			"Bar": map[interface{}]interface {
+			}{
+				"ABc": "A",
+				"cDE": "B"},
+		}
+		expected = map[string]interface{}{
+			"foo": 32,
+			"bar": map[string]interface {
+			}{
+				"abc": "A",
+				"cde": "B"},
+		}
+	)
+
+	got := copyAndInsensitiviseMap(given)
+
+	if !reflect.DeepEqual(got, expected) {
+		t.Fatalf("Got %q\nexpected\n%q", got, expected)
+	}
+
+	if _, ok := given["foo"]; ok {
+		t.Fatal("Input map changed")
+	}
+
+	if _, ok := given["bar"]; ok {
+		t.Fatal("Input map changed")
+	}
+
+	m := given["Bar"].(map[interface{}]interface{})
+	if _, ok := m["ABc"]; !ok {
+		t.Fatal("Input map changed")
+	}
+}

vendor/github.com/mattermost/viper/viper.go

@@ -334,14 +334,14 @@ func (v *Viper) mergeWithEnvPrefix(in string) string {
 // rewriting keys many things, Ex: Get('someKey') -> some_key
 // (camel case to snake case for JSON keys perhaps)
 
-// getEnv is a wrapper around os.Getenv which replaces characters in the original
+// getEnv is a wrapper around os.LookupEnv which replaces characters in the original
 // key. This allows env vars which have different keys than the config object
 // keys.
-func (v *Viper) getEnv(key string) string {
+func (v *Viper) getEnv(key string) (string, bool) {
 	if v.envKeyReplacer != nil {
 		key = v.envKeyReplacer.Replace(key)
 	}
-	return os.Getenv(key)
+	return os.LookupEnv(key)
 }
 
 // ConfigFileUsed returns the file used to populate the config registry.
@@ -568,10 +568,10 @@ func (v *Viper) isPathShadowedInFlatMap(path []string, mi interface{}) string {
 //       "foo.bar.baz" in a lower-priority map
 func (v *Viper) isPathShadowedInAutoEnv(path []string) string {
 	var parentKey string
-	var val string
+	var ok bool
 	for i := 1; i < len(path); i++ {
 		parentKey = strings.Join(path[0:i], v.keyDelim)
-		if val = v.getEnv(v.mergeWithEnvPrefix(parentKey)); val != "" {
+		if _, ok = v.getEnv(v.mergeWithEnvPrefix(parentKey)); ok {
 			return parentKey
 		}
 	}
@@ -934,7 +934,7 @@ func (v *Viper) find(lcaseKey string) interface{} {
 	if v.automaticEnvApplied {
 		// even if it hasn't been registered, if automaticEnv is used,
 		// check any Get request
-		if val = v.getEnv(v.mergeWithEnvPrefix(lcaseKey)); val != "" {
+		if val, ok := v.getEnv(v.mergeWithEnvPrefix(lcaseKey)); ok {
 			return val
 		}
 		if nested && v.isPathShadowedInAutoEnv(path) != "" {
@@ -943,7 +943,7 @@ func (v *Viper) find(lcaseKey string) interface{} {
 	}
 	envkey, exists := v.env[lcaseKey]
 	if exists {
-		if val = v.getEnv(envkey); val != "" {
+		if val, ok := v.getEnv(envkey); ok {
 			return val
 		}
 	}
@@ -1682,8 +1682,8 @@ func (v *Viper) EnvSettings() map[string]interface{} {
 	m := map[string]interface{}{}
 	// start from the list of keys, and construct the map one value at a time
 	for _, k := range v.AllKeys() {
-		value := v.getEnv(v.mergeWithEnvPrefix(k))
+		_, ok := v.getEnv(v.mergeWithEnvPrefix(k))
-		if value == "" {
+		if !ok {
 			continue
 		}
 		path := strings.Split(k, v.keyDelim)