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

Nested keys don't work with AutomaticEnv #160

Closed
kofalt opened this issue Feb 8, 2016 · 11 comments
Closed

Nested keys don't work with AutomaticEnv #160

kofalt opened this issue Feb 8, 2016 · 11 comments

Comments

@kofalt
Copy link

kofalt commented Feb 8, 2016

Periods are not appropriate for environment variables, so for a nested key a.b with an environment prefix example, you might run your program like this:

EXAMPLE_A_B=3 ./program

But this example code:

package main

import (
    . "fmt"
    "github.com/spf13/viper"
)

func main() {
    v := viper.New()

    v.SetEnvPrefix("example")
    v.AutomaticEnv()

    Println(v.Get("a.b"))
    Println(v.Get("a_b"))
}

Will print:

<nil>
3

This is inconsistent, as nested keys in viper are accessed with a . separator, not an underscore.
Tangential relation to #71.

@kofalt kofalt changed the title Nested keys don't work with Nested keys don't work with AutomaticEnv Feb 8, 2016
@jacobstr
Copy link
Collaborator

I believe this works in my confer fork - dots are substituted with underscores. It's a behavior I realized I needed to clarify during testing. I don't particularly encourage the usage of my fork because I've drifted away from go-land, BUT it would be good to confirm your use case. It might rekindle my interest in reconciling my changes!

I struggled to merge confer back a while ago because there were some rather sweeping changes along with behavioral differences that may have broken things for existing users of viper. I've seen a few issues mentioned recently that I incidentally addressed in confer.

@four2five
Copy link

We just ran into this today. Between this issue and #71 , it's getting pretty hard to work with nested keys.

@four2five
Copy link

I've put together a simple change for viper that simply converts "." characters into "_" characters when retrieving an environment variable. What do people think about that as a solution to this issue? Are there edge cases that I'm missing?

I included a test case to validate the behavior of setting an environment A_B and then calling Get("a.b")

#163

@awdrius
Copy link

awdrius commented Feb 27, 2016

I'm just familiarizing with viper as I want to replace my own env/config utility. I found out that SetEnvKeyReplacer(strings.NewReplacer(".", "_")) works well for the scenario you describe.

In config file I have something like { "cassandra": { "hosts": [ "host1", "host2", "host3" ], "database": "dbname" } }

I can retrieve it using GetStringSlice("cassandra.hosts")

If env prefix is set using
SetEnvPrefix("STS")
then
STS_CASSANDRA_HOSTS="host3 host4" app
and
GetStringSlice("cassandra.hosts")
gets that exact value ([host3 host4]).

Furthermore I can use github.com/spf13/pflag library and define a flag like this:
fs := flag.NewFlagSet("default", flag.ContinueOnError)
fs.String("cassandra.hosts", "", "Cassandra hosts or IPs")
fs.Parse(os.Args)
and then link it to viper using
cfg.BindPFlags(fs)
and then I can set it with
app --cassandra.hosts="host4 host5"

Again GetStringSlice("cassandra.hosts") works well and retrieves these values over config file ones.

It's sad that documentation is non existent and one has to dig into code to find little gems like this.

@four2five
Copy link

@awdrius Interesting. Have you tried incorporating your approach with calls to AutomaticEnv() or Unmarshal() ? The issues seem to crop up in there, when Viper should map environment variables to nested structures but fails to do so.

Your first example works because configuration files are parsed correctly, in terms of nested keys.

The second example, where you parse STS_CASSANDRA_HOSTS from the environment, is interesting. I would guess that it's parsing STS_CASSANDRA_HOSTS into CASSANDRA_HOSTS (a single string), replacing the underscore with a dot and then that is being found when you call GetStringSlice() but it's being found as a single string, not as a nested struct. Not sure how to prove that off-hand, I'm going off of some other experiments that I've done.

I have no idea on the third example, I have not tried working with command-line flags yet.

@awdrius
Copy link

awdrius commented Feb 29, 2016

@four2five Actually these were all a single example. And yes, AutomaticEnv() is part of it but I have not tried it with Unmarshal(). Here is a cleaned version of my test (settings.go):

package main

import (
    "fmt"
    "os"
    "strings"

    flag "github.com/spf13/pflag"
    "github.com/spf13/viper"
)

func main() {
    fs := flag.NewFlagSet("default", flag.ContinueOnError)
    fs.String("cassandra.hosts", "", "Cassandra hosts or IPs")
    fs.Parse(os.Args)

    cfg := viper.New()
    cfg.SetConfigName("config")
    cfg.AutomaticEnv()
    cfg.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
    cfg.SetEnvPrefix("STS")
    cfg.AddConfigPath("./")
    cfg.AddConfigPath("$HOME/sh/config/")
    err := cfg.BindPFlags(fs)
    if err != nil {
        fmt.Println(err)
    }
    err = cfg.ReadInConfig()
    if err != nil {
        fmt.Println(err)
    }

    fmt.Println(cfg.ConfigFileUsed())
    fmt.Println(cfg.GetStringSlice("cassandra.hosts"))
    //cfg.Debug()
    //fs.PrintDefaults()
}

Configuration file is like this:

{
    "cassandra": {
        "hosts": [ "cassandra-1", "cassandra-2", "cassandra-3" ],
        "database": "database"
    }
}

These are test run results:

  1. go run settings.go = [cassandra-1 cassandra-2 cassandra-3]
  2. STS_CASSANDRA_HOSTS="cassandra-1 cassandra-3" go run settings.go = [cassandra-1 cassandra-3]
  3. go run settings.go --cassandra.hosts="cassandra-2 cassandra-3" = [cassandra-2 cassandra-3]
  4. STS_CASSANDRA_HOSTS="cassandra-1 cassandra-3" go run settings.go --cassandra.hosts="cassandra-2 cassandra-6" [cassandra-2 cassandra-6]

@doublerebel
Copy link

The functionality in #163 duplicates SetEnvKeyReplacer, but we do need the test for nested env keys. We also need some better docs for SetEnvKeyReplacer as mentioned by @awdrius.

@jyellick
Copy link

I too am being bitten by this issue, please see a failing test below:

package viper_test

import (
        "bytes"
        "github.com/spf13/viper"
        "os"
        "strings"
        "testing"
)

func TestNestedEnv(t *testing.T) {
        data := "---\nnested:\n    field: foo"
        expected := "bar"
        os.Setenv("NESTED_FIELD", expected)
        viper.AutomaticEnv()
        viper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
        viper.SetConfigType("yaml")
        viper.ReadConfig(bytes.NewReader([]byte(data)))

        if v1 := viper.GetString("nested.field"); v1 != expected {
                // Passes
                t.Errorf("GetString got %s, expected %s", v1, expected)
        }

        m := viper.AllSettings()
        if v2 := m["nested"].(map[interface{}]interface{})["field"]; v2 != expected {
                t.Errorf("AllSettings got %s, expected %s", v2, expected)
        }
}

For now, I am working around this problem by doing a second pass over the returned values from AllSettings, it's extremely hacky, but it works.

...
result := AllSettingsHack("", v, v.AllSettings())
...

func AllSettingsHack(base string, v *viper.Viper, nodeKeys map[string]interface{}) map[string]interface{} {
        result := make(map[string]interface{})
        for key := range nodeKeys {
                fqKey := base + key
                val := v.Get(fqKey)
                if m, ok := val.(map[interface{}]interface{}); ok {
                        tmp := make(map[string]interface{})
                        for ik, iv := range m {
                                cik, ok := ik.(string)
                                if !ok {
                                        panic("Non string key-entry")
                                }
                                tmp[cik] = iv
                        }
                        result[key] = getKeysRecursively(fqKey+".", v, tmp)
                } else {
                        result[key] = val
                }
        }
        return result
}

awfm9 pushed a commit that referenced this issue Oct 8, 2016
Fixes #71, #93, #158, #168, #209, #141, #160, #162, #190

* Fixed: indentation in comment
* Fixed: Get() returns nil when nested element not found
* Fixed: insensitiviseMaps() made recursive so that nested keys are lowercased
* Fixed: order of expected<=>actual in assert.Equal() statements
* Fixed: find() looks into "overrides" first
* Fixed: TestBindPFlags() to use a new Viper instance
* Fixed: removed extra aliases from display in Debug()
* Added: test for checking precedence of dot-containing keys.
* Fixed: Set() and SetDefault() insert nested values
* Added: tests for overriding nested values
* Changed: AllKeys() includes all keys / AllSettings() includes overridden nested values
* Added: test for shadowed nested key
* Fixed: properties parsing generates nested maps
* Fixed: Get() and IsSet() work correctly on nested values
* Changed: modifier README.md to reflect changes
@awfm9
Copy link

awfm9 commented Oct 8, 2016

Fixed by #195

@navossoc
Copy link

navossoc commented Nov 2, 2019

This works only for environment variables?

I mean, the .env files seems to have the same problem.

EXAMPLE_A_B=3

viper/viper.go

Lines 1423 to 1430 in 72b022e

case "dotenv", "env":
env, err := gotenv.StrictParse(buf)
if err != nil {
return ConfigParseError{err}
}
for k, v := range env {
c[k] = v
}

The same principle shouldn't be applied here?

I'm new using viper, so am I missing something here?

Thanks

@d4nyll
Copy link

d4nyll commented Jul 13, 2021

I came across this issue just now. For me, the root cause was strings.NewReplacer seems to only work with single-character strings. I.e. strings.NewReplacer(".", "_") works, but strings.NewReplacer(".", "__") doesn't. When I used single underscores as delimiters, it worked again.

seniordeveloper1008 pushed a commit to seniordeveloper1008/GO-microservce-architecture that referenced this issue Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants