-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Loading a YAML file makes all keys lowercase, but YAML keys are case sensitive. #260
Comments
When do you call that method while loading a YAML file? |
If you have:
GetStringMapStringSlice("yaml_key") now gives a map with a |
To conclude: Viper have always tried to be case-insensitive by doing a lower-casing of the keys given. It was however not being doing that deeply for maps. Now that is fixed and we are seeing some breakage in the wild. The main problem seems to be the expectation that you should be able to unmarshal the config map into a struct or similar. Because for plain lookups, like @spf13 should chime in here. |
No, it gives a lower-cased key. |
Yes, I meant it gives the value: I understand your point of view that this is the intended behavior, but I find it surprising for YAML. And I'm not expecting to unmarshall the config map into anything, I'm happy dealing with a map[string][]string. I was just relying on the keys to have the same case I put in the YAML file. My "top" keys are lowercase though, so I didn't realize viper was already returning lowercased versions for anything but maps. |
I understand your situation -- I have a similar breakage in one of my apps. We should probably have handled this in a better way, but I believe it is the intended behavior of Viper from the start. @spf13 should decide. |
The way I think it should work is a bit of a hybrid of case sensitivity. Keys should be treated as case insensitive when getting, setting or Returning keys would ideally be in the case originally defined. The challenge here is that while yaml is case sensitive, not all of the On Fri, Oct 14, 2016 at 7:06 PM Bjørn Erik Pedersen <
|
Which is a very expensive requirement, and in many cases not possible. As I said earlier, this change was pushed earlier but then I did a revert. Viper is getting pretty complex -- it would be nice to have a simple and agreeable understanding of the casing of strings. |
I am not familiar with many configuration use cases and the requirements for case sensitive keys, but if I can chime in, I really think the consistent conversion to lower case keys is the way to go. Feel free to point out some cases that I have no vision on, but I don't really see an upside to complicating the code with some hybrid handling. |
I agree with @awishformore -- I suggest we just ride off the storm. |
Our application uses viper for describing environment variables in a YAML file:
These could have arbitrary names/values since the user defines them. But environment variables are case sensitive, so this means we can no longer represent them naturally as a map. I suppose we will have to switch to something like a list of |
Due to viper lowercasing keys: spf13/viper#260
Hi, to follow up on this issue, I encountered a bug with my configuration, with some Keys in lists and some other unlisted:
Shouldn't all keys be treated the same way ? These two different behaviors look quite error-prone to me. |
It seems to be intentional func insensitiviseMap(m map[string]interface{}) {
for key, val := range m {
switch val.(type) {
case map[interface{}]interface{}:
// nested map: cast and recursively insensitivise
val = cast.ToStringMap(val)
insensitiviseMap(val.(map[string]interface{}))
case map[string]interface{}:
// nested map: recursively insensitivise
insensitiviseMap(val.(map[string]interface{}))
}
lower := strings.ToLower(key)
if key != lower {
// remove old key (not lower-cased)
delete(m, key)
}
// update map
m[lower] = val
}
} |
+1 |
It seems that viper lowercases keys, even if we just get a map[string]string back, which breaks binds and symlinks with case sensitive mount points with uppercase in them. Instead, just go for this ugly, but working, lists-of-lists type. spf13/viper#260 Signed-off-by: Patrick Uiterwijk <patrick@puiterwijk.org>
For what it is worth I recently discovered that the type Position struct {
Row int
Col int
Byt int
SrC string `yaml:",omitempty"`
}
func (p Position) YAML() string {
byt, err := yaml.Marshal(p)
if err != nil {
return ""
}
return string(byt)
}
func ExamplePosition_YAML() {
p := Position{1,2,3,"foo"}
fmt.Print(p.YAML())
// Output:
// Row: 1
// Col: 2
// Byt: 3
// SrC: foo
}
The only way around it is to explicitly tag them the way you want them: type Position struct {
Row int `yaml:"Row"`
Col int `yaml:"Col"`
Byt int `yaml:"Byt"`
SrC string `yaml:"SrC,omitempty"`
} Which gets doubly annoying if you want to type Position struct {
Row int `json:"Row" yaml:"Row"`
Col int `json:"Col" yaml:"Col"`
Byt int `json:"Byt" yaml:"Byt"`
SrC string `json:"SrC,omitempty" yaml:"SrC,omitempty"`
} Of course adding TOML to the mix would get really fun. I really like the idea of being able to represent structures in multiple formats, but this can get tedious fast. Which led me to research what the Kubernetes folks are doing because they have such a massive dependency on YAML and JSON working in what appears to be a case-sensitive way. How Kubernetes Does ItFirst of all, under the hood k8s use https://github.com/ghodss/yaml to get rid of YAML marshaling tags altogether even thought it uses the same Second, they use the explicit name in the struct tags:
Viper Thwarted by go-yaml?I'm betting viper is facing this issue because of the implied names from marshaling structs that are not explicitly named with tags. As long as Anyone feel like writing a Go YAML library that actually respects the same marshaling naming tags like JSON does? I bet a fork of |
+1 for adding a setting to preserve casing. I wanted to document my use case:
Due to this, I'm not going to be able to use viper. |
YAML, TOML, and JSON dictate keys to be case-sensitive. Viper's default behaviour of lowercasing the keys for key insensitivity is incompatible with these standards and has the side effect of making it difficult for use cases such as case sensitive API credentials in configuration. For eg: MyApiKey=MySecret (in TOML). See spf13#131, spf13#260, spf13#293, spf13#371, spf13#373 This commit adds a global function `viper.SetKeyCaseSensitivity()` that enables this behaviour to be turned off, after which, all keys, irrespective of nesting, retain their cases. This respects all configuration operations including getting, setting, and merging.
YAML, TOML, and JSON dictate keys to be case-sensitive. Viper's default behaviour of lowercasing the keys for key insensitivity is incompatible with these standards and has the side effect of making it difficult for use cases such as case sensitive API credentials in configuration. For eg: MyApiKey=MySecret (in TOML). See spf13#131, spf13#260, spf13#293, spf13#371, spf13#373 This commit adds a global function `viper.SetKeyCaseSensitivity()` that enables this behaviour to be turned off, after which, all keys, irrespective of nesting, retain their cases. This respects all configuration operations including getting, setting, and merging.
YAML, TOML, and JSON dictate keys to be case-sensitive. Viper's default behaviour of lowercasing the keys for key insensitivity is incompatible with these standards and has the side effect of making it difficult for use cases such as case sensitive API credentials in configuration. For eg: MyApiKey=MySecret (in TOML). See spf13#131, spf13#260, spf13#293, spf13#371, spf13#373 This commit adds a global function `viper.SetKeysCaseSensitive()` that enables this behaviour to be turned off, after which, all keys, irrespective of nesting, retain their cases. This respects all configuration operations including getting, setting, and merging.
This problem seems to be fixed already, but I don't see new releases since end of May 2019. Does anyone know when next release is scheduled? |
I added it to the 1.5 milestone. New release will happen soon. |
Releasing 1.5.0 |
Can someone give a tl;dr summary of the solution present in 1.5.0? |
I assumed this has been fixed, but looking through the issue I'm not so sure anymore. I'm reopening the issue until I figure out what's going on. |
I confirm that the issue is still present. When using 1.5.0, keys in TOML configuration files (in my case) are still converted to lowercase. |
This issue also happens in json configuration now. Is there anyone working on it? |
Thanks for the pointer @sbourlon. From this comment it looks like Viper is not going to get case sensitivity any time soon, if ever. |
Todo: review and adjust tests
Viper has a feature/bug where all YAML config keys are cast to lowercase (see spf13/viper#260 and spf13/viper#411). Since we use Viper to load our config values, it doesn't seem like there's an easy way to preserve case sensitivity for the access names chosen by users right now. Adding this prompt should help user experience by clarifying that all access names must be lowercase. Change-Id: I47e8344bb0ca7e78458405496f20e78e3c9f9a88
not a workaround in 2020/06/02 master branch. The keys still case-insensitive! |
This is a workaround for case sensitive |
+1 |
It is already 2024, and I still have not seen the v2 version of Viper. The corresponding issue seems to have been put on hold, which has forced me to abandon the use of Viper in some projects. This is because in some cases, I need to ensure that the configuration keys are not converted to lowercase. For instance, I want to place a map in the configuration, but when it is read back, it has changed its appearance.This is because it might be a name. config.yaml: In English, there are also words where the meaning is different when written with different cases, isn’t there? For example, “China” and “china.” |
any update? |
+1 |
Since the recent changes, all keys have become lowercase.
If I'm not mistaken (see this reply from YAML spec author), YAML keys are case sensitive.
For instance, calling GetStringMapStringSlice() while loading a YAML file makes all of the keys of the resulting map lowercase, which is unexpected and was not the previous behavior.
The text was updated successfully, but these errors were encountered: