Viper: ^1.6: Unsupported Config Type "" when config file not found and falling back to extension-less filename

[andig]

1.6.0 has introduced a breaking change that will lead to failure if not config file is found. I used to do this:

// If a config file is found, read it in
if err := viper.ReadInConfig(); err == nil {
    // using config file
    cfgFile = viper.ConfigFileUsed()
} else if _, ok := err.(viper.ConfigFileNotFoundError); !ok {
    // parsing failed - exit
    fmt.Println(err)
    os.Exit(1)
} else {
    // not using config file
    cfgFile = ""
}

Starting with 1.6.0, viper.ReadInConfig will return

Unsupported Config Type ""

if the config file does not exist. This is highly confusing as it should really be ConfigFileNotFoundError instead of UnsupportedConfigError. Please advise.

[sagikazarmark]

@andig unfortunately I cannot reproduce the issue with your code sample.

I tried to simplify it a little bit:

package main

import (
    "fmt"

    "github.com/spf13/viper"
)

func main() {
    viper.AddConfigPath(".")
    viper.SetConfigName("config")

    // If a config file is found, read it in
    if err := viper.ReadInConfig(); err != nil {
        fmt.Println(err)
    }
}

But I'm unable to reproduce it with 1.6.0 and 1.6.1.

There was indeed a breaking change introduced in 1.6.0, which the above code sample causes to return Unsupported Config Type: when there is a directory with the same name (eg. config is the config name and there is a directory with that name). But that was fixed in 1.6.1.

Please try to provide a breaking test, or a simple code sample that reproduces the issue. It might have environmental components (eg. existing file/directory, envorinment variable, etc), so please take those into account as well (eg. what config file are you trying to read? is there a directory with the same name?, etc).

[sagikazarmark]

@andig any chance you can provide an example reproducing this bug?

[andig]

Will do but need some time for debugging to find the faulty case.

[andig]

Closing this one. I have the sneaking suspicion that I had a cobra PersistentFlags().StringVarP in somewhere for an unrelated option pointing to the config file setting. Please forgive me :/

[sagikazarmark]

No worries :)

[andig]

I've finally found what happens and why I didn't see it every time:

Problem ist das in searchInPath, if config file doesn't exist, it will fall back to filename without extension. That file exists- during dev and for some user reports- it is the binary. When that is read the config type is not known:

func (v *Viper) searchInPath(in string) (filename string) {
    jww.DEBUG.Println("Searching for config in ", in)
    for _, ext := range SupportedExts {
        jww.DEBUG.Println("Checking for", filepath.Join(in, v.configName+"."+ext))
        if b, _ := exists(v.fs, filepath.Join(in, v.configName+"."+ext)); b {
            jww.DEBUG.Println("Found: ", filepath.Join(in, v.configName+"."+ext))
            return filepath.Join(in, v.configName+"."+ext)
        }
    }

    if b, _ := exists(v.fs, filepath.Join(in, v.configName)); b {
        return filepath.Join(in, v.configName)
    }

    return ""
}

I haven't checked why the fallback problem didn't exist in 1.5 (or maybe it did), but it is a troublesome. What do you think?

The culprit is https://github.com/spf13/viper/pull/722, /cc @pedromss

[pedromss]

Hey @andig thanks for the heads up. I'm curious: why is there a file with the same name as the config file but with no extension? Why not just name it something different or give it a meaningful extension?

You also say the file exists during dev, this suggests it doesn't always exist, if so, wouldn't it be better to be in a build directory that would be clean every now and then?

Having a config file without an extension is something that happens quite alot.

I don't agree that it should be ConfigFileNotFound as the file acctually exists, it just can't be parsed as you said, because its a binary file

[andig]

Thanks @pedromss for joining. Please let me start by reiterating the issue I perceive:

With 1.6, untyped config files will be accepted by default. Unprepared applications, i.e. those that don't set a config type, will experience this as breaking change if such a file- for whatever reasons- exists in any of the search paths.

I'm curious: why is there a file with the same name as the config file but with no extension? Why not just name it something different or give it a meaningful extension?

My app is specifically using yaml config (with extension). It comes bundled as .dist.yaml. The extension-less variant is just a compiled binary in my case but might be anything else.

You also say the file exists during dev, this suggests it doesn't always exist, if so, wouldn't it be better to be in a build directory that would be clean every now and then?

Sure. I still cannot control if such a file- that I'd really like to ignore- might not exist anywhere in the search path, especially if that includes user home dir or similar.

Having a config file without an extension is something that happens quite a lot.

I accept that. I do not want this to impact my application though, especially when I have SupportedExts to control what type of config I'd like to support.

I don't agree that it should be ConfigFileNotFound as the file acctually exists, it just can't be parsed as you said, because its a binary file

This is not about parsing. It is rather about treating arbitrary files as config which- as with etcd- needs special preparation on the application side like setting config type.

So... maybe one way to do this could be using SupportedExts to allow "" as supported extension if so desired and leave off by default?

[sagikazarmark]

It is rather about treating arbitrary files

Well, it's not arbitrary, you gave it a name to look for. But I understand how it might be a problem.

Since setting a config type for extensionless files is required anyway, do you think @pedromss that we could restrict the search to only check extensionless files if there is a config type set?

It's kind of a compromise solution: if you set a config type @andig Viper will find and try to parse those files.

[andig]

do you think @pedromss that we could restrict the search to only check extensionless files if there is a config type set?

Sounds really reasonable and especially consistent with 1.5 behaviour to me. If this is the way forward it would be great to cut a point release asap.

[pedromss]

@sagikazarmark sounds ok to me. Should I get on it?

[sagikazarmark]

Fixed in v1.6.2

Thanks @andig and @pedromss ! 🎉