FAB-14709 Respect env override of vars not in conf

There is a deficiency in the way viper handles environment variable
overrides when unmarshaling to a structure.  If the variable is not
present in the config file (but present in the structure) and is
overridden in the environment, then this override is ignored.  This is
bizzare behavior which we have hacked around by putting all options into
the config file, but this policy is becoming untennable.

This CR adds (yet another) hacky layer in the viperutil package to
include fields from the structure which are not in the config into the
map which is ultimately used to populate the structure values.

The correct long-term approach is to stop using viper and handle this
ourselves.  The actual logic we have stacked on top of viper to handle
these problems is very likely more code than it would take to implement
our own sane config unmarshaling.

Change-Id: I47102c64162f1efcd9e1bd21a563e6aedb88abab
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

common/viperutil/config_util.go

@@ -22,6 +22,7 @@ import (
 	version "github.com/hashicorp/go-version"
 	"github.com/hyperledger/fabric/common/flogging"
 	"github.com/mitchellh/mapstructure"
+	"github.com/pkg/errors"
 	"github.com/spf13/viper"
 )
 
@@ -29,10 +30,31 @@ var logger = flogging.MustGetLogger("viperutil")
 
 type viperGetter func(key string) interface{}
 
-func getKeysRecursively(base string, getKey viperGetter, nodeKeys map[string]interface{}) map[string]interface{} {
+func getKeysRecursively(base string, getKey viperGetter, nodeKeys map[string]interface{}, oType reflect.Type) map[string]interface{} {
+	subTypes := map[string]reflect.Type{}
+
+	if oType != nil && oType.Kind() == reflect.Struct {
+	outer:
+		for i := 0; i < oType.NumField(); i++ {
+			fieldName := oType.Field(i).Name
+			fieldType := oType.Field(i).Type
+
+			for key := range nodeKeys {
+				if strings.EqualFold(fieldName, key) {
+					subTypes[key] = fieldType
+					continue outer
+				}
+			}
+
+			subTypes[fieldName] = fieldType
+			nodeKeys[fieldName] = nil
+		}
+	}
+
 	result := make(map[string]interface{})
 	for key := range nodeKeys {
 		fqKey := base + key
+
 		val := getKey(fqKey)
 		if m, ok := val.(map[interface{}]interface{}); ok {
 			logger.Debugf("Found map[interface{}]interface{} value for %s", fqKey)
@@ -44,10 +66,10 @@ func getKeysRecursively(base string, getKey viperGetter, nodeKeys map[string]int
 				}
 				tmp[cik] = iv
 			}
-			result[key] = getKeysRecursively(fqKey+".", getKey, tmp)
+			result[key] = getKeysRecursively(fqKey+".", getKey, tmp, subTypes[key])
 		} else if m, ok := val.(map[string]interface{}); ok {
 			logger.Debugf("Found map[string]interface{} value for %s", fqKey)
-			result[key] = getKeysRecursively(fqKey+".", getKey, m)
+			result[key] = getKeysRecursively(fqKey+".", getKey, m, subTypes[key])
 		} else if m, ok := unmarshalJSON(val); ok {
 			logger.Debugf("Found real value for %s setting to map[string]string %v", fqKey, m)
 			result[key] = m
@@ -287,10 +309,19 @@ func kafkaVersionDecodeHook() mapstructure.DecodeHookFunc {
 // producing error when extraneous variables are introduced and supporting
 // the time.Duration type
 func EnhancedExactUnmarshal(v *viper.Viper, output interface{}) error {
-	// AllKeys doesn't actually return all keys, it only returns the base ones
+	oType := reflect.TypeOf(output)
+	if oType.Kind() != reflect.Ptr {
+		return errors.Errorf("supplied output argument must be a pointer to a struct but is not pointer")
+	}
+	eType := oType.Elem()
+	if eType.Kind() != reflect.Struct {
+		return errors.Errorf("supplied output argument must be a pointer to a struct, but it is pointer to something else")
+	}
+
 	baseKeys := v.AllSettings()
+
 	getterWithClass := func(key string) interface{} { return v.Get(key) } // hide receiver
-	leafKeys := getKeysRecursively("", getterWithClass, baseKeys)
+	leafKeys := getKeysRecursively("", getterWithClass, baseKeys, eType)
 
 	logger.Debugf("%+v", leafKeys)
 	config := &mapstructure.DecoderConfig{
@@ -318,7 +349,7 @@ func EnhancedExactUnmarshal(v *viper.Viper, output interface{}) error {
 func EnhancedExactUnmarshalKey(baseKey string, output interface{}) error {
 	m := make(map[string]interface{})
 	m[baseKey] = nil
-	leafKeys := getKeysRecursively("", viper.Get, m)
+	leafKeys := getKeysRecursively("", viper.Get, m, nil)
 
 	logger.Debugf("%+v", leafKeys)
 

orderer/common/localconfig/config_test.go

@@ -24,6 +24,27 @@ func TestLoadGoodConfig(t *testing.T) {
 	assert.Nil(t, err, "Load good config returned unexpected error")
 }
 
+func TestMissingConfigValueOverridden(t *testing.T) {
+	t.Run("when the value is missing and not overridden", func(t *testing.T) {
+		cleanup := configtest.SetDevFabricConfigPath(t)
+		defer cleanup()
+		cfg, err := Load()
+		assert.NotNil(t, cfg, "Could not load config")
+		assert.NoError(t, err, "Load good config returned unexpected error")
+		assert.Nil(t, cfg.Kafka.TLS.ClientRootCAs)
+	})
+
+	t.Run("when the value is missing and is overridden", func(t *testing.T) {
+		os.Setenv("ORDERER_KAFKA_TLS_CLIENTROOTCAS", "msp/tlscacerts/tlsroot.pem")
+		cleanup := configtest.SetDevFabricConfigPath(t)
+		defer cleanup()
+		cfg, err := Load()
+		assert.NotNil(t, cfg, "Could not load config")
+		assert.NoError(t, err, "Load good config returned unexpected error")
+		assert.NotNil(t, cfg.Kafka.TLS.ClientRootCAs)
+	})
+}
+
 func TestLoadMissingConfigFile(t *testing.T) {
 	envVar1 := "FABRIC_CFG_PATH"
 	envVal1 := "invalid fabric cfg path"

sampleconfig/orderer.yaml

@@ -326,7 +326,7 @@ Operations:
         ClientAuthRequired: false
 
         # Paths to PEM encoded ca certificates to trust for client authentication
-        RootCAs: []
+        ClientRootCAs: []
 
 ################################################################################
 #
@@ -372,4 +372,4 @@ Consensus:
 
     # SnapDir specifies the location at which snapshots for etcd/raft are
     # stored. Each channel will have its own subdir named after channel ID.
-    SnapDir: /var/hyperledger/production/orderer/etcdraft/snapshot
\ No newline at end of file
+    SnapDir: /var/hyperledger/production/orderer/etcdraft/snapshot