Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unmarshal non-bound environment variables #761

Closed
sagikazarmark opened this issue Sep 12, 2019 · 19 comments · Fixed by #1429
Closed

Unmarshal non-bound environment variables #761

sagikazarmark opened this issue Sep 12, 2019 · 19 comments · Fixed by #1429

Comments

@sagikazarmark
Copy link
Collaborator

sagikazarmark commented Sep 12, 2019

This is going to be an umbrella issue for other open issues for this problem.

These are:

Problem

Currently Viper cannot unmarshal automatically loaded environment variables.

Here is a failing test case reproducing the issue:

package viper

import (
	"os"
	"testing"
)

func TestUnmarshal_AutomaticEnv(t *testing.T) {
	os.Setenv("MYKEY", "myvalue")

	type Config struct {
		MyKey string `mapstructure:"mykey"`
	}

	var c Config

	AutomaticEnv()

	if err := Unmarshal(&c); err != nil {
		t.Fatal(err)
	}

	if c.MyKey != "myvalue" {
		t.Error("failed to unmarshal automatically loaded environment variable")
	}

	os.Clearenv()
}

Root cause

Automatic environment variable loading works as follows:

  1. User requests a value from Viper: viper.GetString("mykey")
  2. Viper runs the env key replacer on the key
  3. Viper compares the key (case insensitively) with the list of environment variables
  4. Returns the value if one is found

Unmarshal, however, doesn't work by requesting a key, quite the opposite: it tries to unmarshal existing keys onto the given structure. Since automatically loaded environment variables are not in the list of existing keys (unlike defaults and bind env vars), these variables won't be unmarshaled.

Workaround

  1. Use BindEnv to manually bind environment variables
  2. Use SetDefault to set a default value (even an empty one), so Viper can find the key and try to find a matching env var

Possible solutions

The linked issues list a number of workarounds to fix this issue.

Personally, I'm in favor of the following:

  1. Unmarshal the given struct onto a map[string]interface{}. That will give use a nested structure of all the keys otherwise mapstructure expects.
  2. Flatten the structure, so we get a list of expected keys
  3. Check that every key is defined. If not, check if there is a matching environment variable.
  4. Merge the available settings with the matching, automatically loaded environment variables.
  5. Use mapstructure as usual

Although I think that this is currently the best available solution, it doesn't mean we can't do better, so please do suggest if you have an idea.

@ktong
Copy link

ktong commented May 5, 2020

Below is a wapper to unmarshal config with environment variables.

func Unmarshal(v View, key string, rawVal interface{}) error {
	cfg := v.(*viper.Viper)
	bindEnvs(cfg, key, rawVal)

	return cfg.UnmarshalKey(key, rawVal)
}

// Workaround because viper does not treat env vars the same as other config.
// See https://github.com/spf13/viper/issues/761.
func bindEnvs(cfg *viper.Viper, key string, rawVal interface{}) {
	for _, k := range allKeys(key, rawVal) {
		val := cfg.Get(k)
		cfg.Set(k, val)
	}
}

func allKeys(key string, rawVal interface{}) []string {
	b, err := yaml.Marshal(
		map[string]interface{}{
			key: rawVal,
		},
	)
	if err != nil {
		return nil
	}

	v := viper.New()
	v.SetConfigType("yaml")
	if err := v.ReadConfig(bytes.NewReader(b)); err != nil {
		return nil
	}

	return v.AllKeys()
}

@celian-garcia
Copy link

celian-garcia commented May 9, 2020

Hi guys, do you have a plan to support it ? I struggled with this as well and I used a solution inspired from one given by @krak3n but supporting pointers and ,squash feature of mapstructure.

Would be cool if mapstructure was exposing the feature of giving a tree of names based on the mapstructure tags. Then it would be easy to browse the tree and call the viper.BindEnv function with all the leaves.

For the moment, here is the solution I mentioned:

// bindenv: workaround to make the unmarshal work with environment variables
// Inspired from solution found here : https://github.com/spf13/viper/issues/188#issuecomment-399884438
func (b *viperConfigBuilder) bindenvs(iface interface{}, parts ...string) {
	ifv := reflect.ValueOf(iface)
	if ifv.Kind() == reflect.Ptr {
		ifv = ifv.Elem()
	}
	for i := 0; i < ifv.NumField(); i++ {
		v := ifv.Field(i)
		t := ifv.Type().Field(i)
		tv, ok := t.Tag.Lookup("mapstructure")
		if !ok {
			continue
		}
		if tv == ",squash" {
			b.bindenvs(v.Interface(), parts...)
			continue
		}
		switch v.Kind() {
		case reflect.Struct:
			b.bindenvs(v.Interface(), append(parts, tv)...)
		default:
			b.v.BindEnv(strings.Join(append(parts, tv), "."))
		}
	}
}

clafoutis42 pushed a commit to Gustavobelfort/42-jitsi that referenced this issue May 14, 2020
This will be revertable when spf13/viper#761 is resolved.
@iamolegga
Copy link

Hi all, in case someone need ready-to-use-package for this feature, I've just created one based on @celian-garcia and @krak3n examples, thank you guys!

@celian-garcia
Copy link

Thanks @iamolegga, I will use it :)

@segevfiner
Copy link

If mapstructure either returned metadata about fields not unmarshalled into in the target struct (Rather then the source), or otherwise had a callback to handle such fields, it could have been an easy way for viper to fix this. Otherwise using reflection in a similar way to what mapstructure does to handle the struct seems like the most viable approach, though it will have to correctly handle the various mapstructure tags. You can find various snippets of code doing just that around the issue tracker and the internet. Even then, it still won't work with dynamic parts of the struct, e.g. map[string]interface{} and so on, without further cleverness from viper rather then it just doing something akin to BindEnv.

Serializing the struct won't work correctly with omitempty.

@ricoleabricot
Copy link

ricoleabricot commented Aug 18, 2020

I've also use a custom method for binding env vars without config file nor flags like so:

import (
	"github.com/fatih/structs"
	"github.com/jeremywohl/flatten"
)

func SetupConfigFromEnv() error {
	// Setup automatic env binding
	viper.AutomaticEnv()
	viper.SetEnvPrefix("")
	viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))

	// Transform config struct to map
	confMap := structs.Map(Config{})

	// Flatten nested conf map
	flat, err := flatten.Flatten(confMap, "", flatten.DotStyle)
	if err != nil {
		return errors.Wrap(err, "Unable to flatten config")
	}

	// Bind each conf fields to environment vars
	for key, _ := range flat {
		err := viper.BindEnv(key)
		if err != nil {
			return errors.Wrapf(err, "Unable to bind env var: %s", key)
		}
	}

	return nil
}

It minimises "home-made" custom code but it has to use 2 other librairies to work.

@Blquinn
Copy link

Blquinn commented Aug 20, 2020

@sagikazarmark Any news on this? Is it just a matter of a pull request?

@nikunjy
Copy link

nikunjy commented Oct 6, 2020

Any update here ? Having to use BindEnv doesn't make sense. I have to write a constant in the mapstructure and specify the same string in BindEnv

@sagikazarmark
Copy link
Collaborator Author

Here is a rough solution:

type Config struct {
	MyKey string `mapstructure:"mykey"`
}

// extract the keys form a struct and flatten them using a dot separator
keys := viper.ExtractKeysFromStruct(Config{}, ".")

// define keys in Viper
// this would essentially let Viper know about a key without setting a default or any value for it
// since these keys would not have a value initially, IsSet would return false, AllKeys would not return them
// AutomaticEnv detection could use these keys though
viper.Define(keys...)

// or
// BindEnvs would be the same as BindEnv for multiple keys
// though this would render AutomaticEnv less useful when used with Unmarshal
viper.BindEnvs(keys...)

It would use mapstructure under the hood, so that it can stay close to the unmarshal behavior.

Cases when it could fall short:

type Config struct {
	MyKey string `mapstructure:"mykey"`

	// The default value here is nil, so fields of the config would probably not show up by default?
	SubConfig *SubConfig
}

type SubConfig struct {
	MyKey string `mapstructure:"mykey"`
}

WDYT?

@segevfiner
Copy link

If you use mapstructure as it is now to implement ExtractKeysFromStruct it would miss omitempty fields. (Telling users not to use them is weird, error prone and they might want to use them, e.g. When writing the config to a file).

Personally I believe it's best if Unmarshal just handled this internally so it just works with no side effects to viper like calling BindEnvs from it.

@sagikazarmark
Copy link
Collaborator Author

@segevfiner

If you use mapstructure as it is now to implement ExtractKeysFromStruct it would miss omitempty fields. (Telling users not to use them is weird, error prone and they might want to use them, e.g. When writing the config to a file).

That's entirely correct, we would have to workaround that issue somehow. See the issue above I opened in the mapstructure repo.

Personally I believe it's best if Unmarshal just handled this internally so it just works with no side effects to viper like calling BindEnvs from it.

I think it would be very hard to do, if possible at all. The way it works currently, is Unmarshal gets a list of values from the Viper instance and tries to unmarshal them onto the struct. At this point environment variables are already evaluated.

I don't think that side effect is actually a problem, because it's exactly the same thing when you set a default for example: you define a key in one of the config sources and that key in this case would be exactly the same for all sources (if you were to call Set manually for example, to override a default or an env var for that matter, you would use this exact same key).

What BindEnvs would do is essentially telling Viper to expect these keys if it doesn't expect them already. It feels like a better approach to me, because it fits nicely into how config sources are evaluated.

@GiancarlosIO
Copy link

👀

carolynvs added a commit to getporter/viper that referenced this issue May 12, 2021
In cases where there isn't a configuration file, none of the fields on
the struct are bound to environment variables unless BindEnv was
explicitly called. AutomaticEnv essentially doesn't work with the config
file per spf13#761.

If you need to support this scenario, call ReadConfigFrom on the
destination struct before calling Unmarshal.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit to getporter/viper that referenced this issue May 12, 2021
In cases where there isn't a configuration file, none of the fields on
the struct are bound to environment variables unless BindEnv was
explicitly called. AutomaticEnv essentially doesn't work with the config
file per spf13#761.

If you need to support this scenario, call ReadConfigFrom on the
destination struct before calling Unmarshal.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit to getporter/viper that referenced this issue May 12, 2021
In cases where there isn't a configuration file, none of the fields on
the struct are bound to environment variables unless BindEnv was
explicitly called. AutomaticEnv essentially doesn't work with the config
file per spf13#761.

If you need to support this scenario, call ReadConfigFrom on the
destination struct before calling Unmarshal.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit to getporter/viper that referenced this issue May 14, 2021
In cases where there isn't a configuration file, none of the fields on
the struct are bound to environment variables unless BindEnv was
explicitly called. AutomaticEnv essentially doesn't work with the config
file per spf13#761.

If you need to support this scenario, call SetDefaultsFrom on the
destination struct before calling Unmarshal.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@renta
Copy link

renta commented Jun 11, 2021

There is a way to automate BindEnv envoking with retrieving mapstructure keys from Config struct:

package viper

import (
	"os"
	"testing"
	
	"github.com/mitchellh/mapstructure"
)

func TestUnmarshal_AutomaticEnv(t *testing.T) {
	os.Setenv("MYKEY", "myvalue")

	type Config struct {
		MyKey string `mapstructure:"mykey"`
	}

	var c Config

	AutomaticEnv()

        // ADD START
        envKeysMap := &map[string]interface{}{}
        if err := mapstructure.Decode(c, &envKeysMap); err != nil {
                t.Fatal(err)
        }
        for k := range *envKeysMap {
                if bindErr := viper.BindEnv(k); bindErr != nil {
                        t.Fatal(err)
                }
        }
        // ADD END

	if err = Unmarshal(&c); err != nil {
		t.Fatal(err)
	}

	if c.MyKey != "myvalue" {
		t.Error("failed to unmarshal automatically loaded environment variable")
	}

	os.Clearenv()
}

@tunhuit
Copy link

tunhuit commented Jun 18, 2021

Unmarshal*() work only with config file.

I have just tested with Env and PFlags, Unmarshal*() bypass overrided values from Env and PFlags.

I have to use viper.Set() to workaround this serious issue.

nielskrijger added a commit to nielskrijger/goboot that referenced this issue Sep 11, 2021
…ults are defined within yaml files

For more information see: spf13/viper#761
skibish added a commit to skibish/ddns that referenced this issue Jun 5, 2022
Added default value to `DDNS_TOKEN` so that Viper would be able
to find the key and matching env.

More details can be found in spf13/viper#761.

Closes #31
gadgethm pushed a commit to gadgethm/ddns that referenced this issue Jun 5, 2022
Added default value to `DDNS_TOKEN` so that Viper would be able
to find the key and matching env.

More details can be found in spf13/viper#761.

Closes skibish#31
@ashutoshsinghparmar
Copy link

For us this worked...

`viper.AutomaticEnv()
viper.SetConfigFile(configFile)
viper.SetConfigType("yaml")

if err := viper.ReadInConfig(); err != nil {
	log.Fatalf("Error reading config file, %s", err)
}

for _, k := range viper.AllKeys() {
	v := viper.GetString(k)
	viper.Set(k, os.ExpandEnv(v))
}

if err := viper.Unmarshal(&appConfig); err != nil {
	log.Fatalf("Unable to decode into struct, %v", err)
}

return nil`

@justmumu
Copy link

justmumu commented Jun 3, 2023

Hey friends, i think i found a solution generic enough

// To get all keys even has zero value, nil pointer
func getAllKeys(iface interface{}, parts ...string) []string {
	var keys []string

	ifv := reflect.ValueOf(iface)
	if ifv.Kind() == reflect.Ptr {
		ifv = ifv.Elem()
	}

	for i := 0; i < ifv.NumField(); i++ {
		v := ifv.Field(i)
		t := ifv.Type().Field(i)
		tv, ok := t.Tag.Lookup("mapstructure")
		if !ok {
			continue
		}

		switch v.Kind() {
		case reflect.Struct:
			keys = append(keys, getAllKeys(v.Interface(), append(parts, tv)...)...)
		case reflect.Ptr:
			if v.IsNil() && v.CanSet() {
				v.Set(reflect.New(v.Type().Elem()))
			}
			if v.Elem().Kind() == reflect.Struct {
				keys = append(keys, getAllKeys(v.Interface(), append(parts, tv)...)...)
			}
			keys = append(keys, strings.Join(append(parts, tv), "."))
		default:
			keys = append(keys, strings.Join(append(parts, tv), "."))
		}
	}

	return keys
}

func ParseConfig() *Config {
	.....

	envPrefix := "APP_NAME"
	vv := viper.New()
	vv.AutomaticEnv()
	vv.SetEnvPrefix(envPrefix)
	vv.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
	vv.AllowEmptyEnv(false)

	for _, key := range getAllKeys(&Config{}) {
		envKey := strings.ReplaceAll(strings.ToUpper(fmt.Sprintf("%s_%s",envPrefix, key)), ".", "_")
		vv.BindEnv(key, envKey)
                val, ok := os.LookupEnv(envKey)
                if ok {
			vv.Set(key, val)
                }
	}
	
	config := &Config{}
	if err := vv.Unmarshal(config); err != nil {
		return nil, err
	}
	
	// Now all env variables overrides config and its populated
}

@sll552
Copy link

sll552 commented Jun 6, 2023

Another much simpler solution, that works at least for simple structs would be the other way around: Binding all envs that have the prefix.
I am pretty sure there are caveats to this as I haven't tested extensively but for my simple usecase it works and is much easier to understand

Maybe it helps someone, so I will leave it here:

viper.SetEnvPrefix(EnvPrefix)
viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
viper.AutomaticEnv()

for _, e := range os.Environ() {
  split := strings.Split(e, "=")
  k := split[0]
  
  if strings.HasPrefix(k, EnvPrefix) {
    viper.BindEnv(strings.Join(strings.Split(k, "_")[1:], "."))
  }
}

@ashtonian
Copy link

ashtonian commented Jun 26, 2023

I've been wanting to turn this into a PR of some kind but haven't been able to. Its a function that maps env variables to a struct using viper. You can define the env name and default in the struct tag.

const tagName = "envname"

var (
	ErrKindInvalid   = errors.New("Kind is invalid")
	ErrPointerNeeded = errors.New("Expecting a pointer to an object")

	IntType         = reflect.TypeOf(int(0))
	StringType      = reflect.TypeOf("")
	Uint16Type      = reflect.TypeOf(uint16(0))
	Int64Type       = reflect.TypeOf(int64(0))
	Float64Type     = reflect.TypeOf(float64(0))
	BoolType        = reflect.TypeOf(false)
	DurationType    = reflect.TypeOf(time.Duration(0))
	StringSliceType = reflect.TypeOf([]string{})
	ByteType        = reflect.TypeOf(byte(0))
)

// BindToViper takes a pointer to an object and binds the environment named
// variables to viper and the passed in struct object. Default values can be
// passed in as a second value (comma separated)
// Ex
//    type ViperStruct struct {
//      Boolean bool `envname:"BOOLEAN,true"`
//    }
//
// Supported types include
//  - string
//  - []string
//  - int
//  - uint16
//  - bool
//  - int64 / time.Duration (same type)
//  - byte
//	- float64

// TODO: recursive nested object handling
func BindToViper(obj interface{}, prefix string) error {
	var valuePtr reflect.Value
	var value reflect.Value
	objType := reflect.TypeOf(obj)

	if objType.Kind() == reflect.Ptr {
		valuePtr = reflect.ValueOf(obj)
		value = reflect.Indirect(valuePtr)
	} else {
		return ErrPointerNeeded
	}

	for i := 0; i < value.NumField(); i++ {
		// Get the field tag value
		structType := value.Type()
		field := structType.Field(i)
		// fieldType := field.Type

		tag := field.Tag.Get(tagName)
		// Skip if tag is not defined or ignored
		if tag == "" || tag == "-" {
			continue
		}
		tagArgs := strings.Split(tag, ",")
		envName := buildName(prefix, tagArgs[0])
		fieldVal := valuePtr.Elem().Field(i)

		err := setViperDefaults(tagArgs, envName, field.Type)
		if err != nil {
			return err
		}

		err = setViperValue(tagArgs, envName, field.Type, fieldVal)
		if err != nil {
			return err
		}
	}
	return nil
}

func isNil(i interface{}) bool {
	if i == nil {
		return true
	}
	switch reflect.TypeOf(i).Kind() {
	case reflect.Ptr, reflect.Map, reflect.Interface, reflect.Array, reflect.Chan, reflect.Slice, reflect.Func:
		return reflect.ValueOf(i).IsNil()
	}
	return false
}

// setViperValue will assign the struct field to the value as contained in
// viper. It's the equivalent of calling
//    strct.Field = viper.Get("field")
// but relatively generic
func setViperValue(tagArgs []string, envName string, t reflect.Type, v reflect.Value) error {
	switch t {
	case StringType:
		v.SetString(viper.GetString(envName))
	case StringSliceType:
		sVal := reflect.ValueOf(viper.GetStringSlice(envName))
		v.Set(sVal)
	case IntType:
		sVal := reflect.ValueOf(viper.GetInt(envName))
		v.Set(sVal)
	case Uint16Type:
		sVal := reflect.ValueOf(uint16(viper.GetInt(envName)))
		v.Set(sVal)
	case BoolType:
		sVal := reflect.ValueOf(viper.GetBool(envName))
		v.Set(sVal)
	case Int64Type: // Int64 works for both int64 and derivative types
		sVal := reflect.ValueOf(viper.GetInt64(envName))
		v.Set(sVal)
	case DurationType:
		sVal := reflect.ValueOf(viper.GetDuration(envName))
		v.Set(sVal)
	case ByteType:
		sVal := reflect.ValueOf(byte(viper.GetInt(envName)))
		v.Set(sVal)
	case Float64Type:
		sVal := reflect.ValueOf(viper.GetFloat64(envName))
		v.Set(sVal)
	}

	return nil
}

// setViperDefaults will parse the struct tags and pull out the defaults. It'll
// try to parse the default into the `kind` of the struct field and set that as
// a viper default value
func setViperDefaults(tagArgs []string, envName string, t reflect.Type) error {
	// Don't overwrite any defaults previously set. We don't want the struct's
	// defaults to overwrite any that were set before it because we want to make
	// sure the struct defaults don't take precedence
	if viper.IsSet(envName) || len(tagArgs) < 2 {
		return nil
	}

	switch t {
	case IntType:
		val, err := strconv.Atoi(tagArgs[1])
		if err != nil {
			return err
		}
		viper.SetDefault(envName, val)
	case DurationType:
		durVal, err := time.ParseDuration(tagArgs[1])
		if err != nil {
			// return the original err
			return err
		}
		viper.SetDefault(envName, durVal)
	case Int64Type:
		val, err := strconv.ParseInt(tagArgs[1], 10, 64)
		if err != nil {
			return err
		}
		viper.SetDefault(envName, val)
	case StringType:
		viper.SetDefault(envName, tagArgs[1])
	case Uint16Type:
		val, err := strconv.ParseUint(tagArgs[1], 10, 16)
		if err != nil {
			return err
		}
		viper.SetDefault(envName, val)
	case BoolType:
		val, err := strconv.ParseBool(tagArgs[1])
		if err != nil {
			return err
		}
		viper.SetDefault(envName, val)
	case StringSliceType:
		// viper only supports string slices, so no further processing is needed
		viper.SetDefault(envName, tagArgs[1])
	case ByteType:
		viper.SetDefault(envName, tagArgs[1])
	case Float64Type:
		val, err := strconv.ParseFloat(tagArgs[1], 64)
		if err != nil {
			return err
		}
		viper.SetDefault(envName, val)
	}
	return nil
}

// buildName is part of the Prefix feature. It will prepend the prefix to the
// name or, if no prefix is set, will return the name directly
func buildName(prefix, name string) string {
	if prefix != "" {
		return strings.Join([]string{strings.ToUpper(prefix), strings.ToUpper(name)}, "_")
	} else {
		return name
	}
}

Usage example:

type Config struct {
	JWTKey              string					`envname:"JWT"`
	MaxAge              time.Duration 	 		`envname:"MAX_AGE,5m"`
}

func main(){

	viper.SetConfigType("yaml")
	viper.SetConfigName("config")
	viper.AddConfigPath(".")
	viper.ReadInConfig()
	viper.AutomaticEnv()
	cfg := &Config{}
	BindToViper(&cfg, "") 
}

@jsumners
Copy link

jsumners commented Mar 3, 2024

I know this is closed, but I was surprised to learn this happens and only learned it through some searching (that managed to hit https://renehernandez.io/snippets/bind-environment-variables-to-config-struct-with-viper/). It is really surprising that AutomaticEnv() does not process all environment variables with the defined prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet