viper.GetStringMapString(key) doesn't work with AutomaticEnv

[amir20]

I currently use Viper to configure a Docker service. It doesn't make sense for people to change the entrypoint. So I rely on ENV vars heavily.

Following up with #608, using AutomaticEnv() with GetStringMapString() still doesn't work.

Looking through the source code, Viper branches based on the source. I debugged and found the function below gets an exception because it can't parse foo=bar style values with JSON. It also unexpectedly swallows and exception here :/

func ToStringMapString(i interface{}) map[string]string {
	v, _ := ToStringMapStringE(i)
	return v
}

As a user, I expect viper.Get***() to work regardless of the source.

I believe the chang would actually have to be in https://github.com/spf13/cast/blob/master/cast.go#L108-L110.

[amir20]

Correction, the fix would need to be at https://github.com/spf13/cast/blob/1ffadf551085444af981432dd0f6d1160c11ec64/caste.go#L855-L880

[sagikazarmark]

I'm not sure this needs fixing. In environment variables Viper already accepts JSON strings which is more flexible than the key-value pairs supported by pflags.

[amir20]

Hmm. That seems confusing. I prioritize for users' ease of use. It would be confusing for some flags via --filter to take key=value while using ENV requiring JSON. That seems counterintuitive.

Also, to provide some context, my project is an open source tool paired with Docker. Docker uses key=value for --filter so I wanted to mirror the same behavior. For a user, it would be much easier to provide DOZZLE_FILTER=name=dozzle in docker-compose than {"DOZZLE_FILTER": "name=dozzle"}.

For anybody who is interested, I handle this myself.

if value, ok := os.LookupEnv("DOZZLE_FILTER"); ok {
	log.Infof("Parsing %s", value)
	urlValues, err := url.ParseQuery(strings.ReplaceAll(value, ",", "&"))
	if err != nil {
		log.Fatal(err)
	}
	filters = map[string]string{}
	for k, v := range urlValues {
		filters[k] = v[0]
	}
}

I can add a PR for the above code. But maybe it doesn't make sense to have JSON and key value format.

[sagikazarmark]

Well, whether it's confusing or not, this is the current behavior. Breaking it is not an option at the moment, but I agree it would be nice to have a key-value pair like format for environment variables.

This is exactly why I don't really like the current getter-based fetching, because it doesn't give you enough flexibility. You would assume that an env var should behave the same way as this type of flag, others need the flexibility of JSON. (Keep in mind that flags are registered for each type whereas env vars are just key-value pairs, so flags can support both styles at the same time, env vars can't.)

I usually suggest everyone to create a config struct with customized types, for example:

type config struct{
    filter filterType
}

type filterType map[string]string

Then for this filter type you can create your own mapstructure decoding hook, which can decode both strings (env vars) and maps (flags).

After that, you can unmarshal your config onto that struct:

var c config

viper.Unmarshal(&c)

// or even
viper.UnmarshalKey("filter", &c.filter)

Don't think of it as a workaround, for me this is the de facto solution for these (and basically all) use cases.

[amir20]

Interesting, I hadn't pursued creating my own config struct. I started with only a few flags which then ended up needing more. I'll look at it again.

[sagikazarmark]

IMHO using a config struct is always superior to everything else.

Here is an example: https://github.com/sagikazarmark/modern-go-application/blob/master/cmd/modern-go-application/config.go

[amir20]

Thanks! I was about to ping you for an example :)

[sagikazarmark]

You can also find a couple decoding hook examples here:

https://github.com/banzaicloud/pipeline/blob/f30951333348d54ad9055cafd876e2aa3c4fa3de/pkg/hook/hook.go#L24

[digitebs]

Correction, the fix would need to be at https://github.com/spf13/cast/blob/1ffadf551085444af981432dd0f6d1160c11ec64/caste.go#L855-L880

why no merge?