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

viper.Unmarshal() ignoring UnmarshalYAML interface #338

Open
TimJones opened this issue Apr 21, 2017 · 4 comments
Open

viper.Unmarshal() ignoring UnmarshalYAML interface #338

TimJones opened this issue Apr 21, 2017 · 4 comments

Comments

@TimJones
Copy link

TimJones commented Apr 21, 2017

I am trying to implement a custom unmarshaler for a YAML config to allow a key to support multiple types (in my personal case, a string and sequence). I am trying to configure that if the YAML value is a string type, I can manipulate it into a sequence (e.g. splitting on spaces, quotes, etc).

package main

import (
	"bytes"
	"fmt"

	"github.com/kr/pretty"
	"github.com/spf13/viper"
)

type CommandType []string

type Config struct {
	Project string
	Alias   map[string]struct {
		Command     CommandType
		Environment string
	}
}

func (cmd *CommandType) UnmarshalYAML(unmarshal func(interface{}) error) error {
	if err := unmarshal(&cmd); err != nil {
		return err
	}
	// Simply overwrite unmarshalled data PoC
	*cmd = []string{"overwritten", "values"}
	return nil
}

var data = []byte(`---
project: proof-of-concept
alias:
  poc:
    command: this can be split
    environment: poc
  other:
    command:
      - this
      - can
      - be
      - left
    environment: poc`)

func main() {
	viper.SetConfigType("yaml")
	viper.ReadConfig(bytes.NewBuffer(data))

	var cfg Config
	if err := viper.Unmarshal(&cfg); err != nil {
		panic(fmt.Errorf("Fatal error loading config: %s \n", err))
	}
	fmt.Printf("%# v\n", pretty.Formatter(cfg))
}

With the above code, I expect an output of:

main.Config{
    Project: "proof-of-concept",
    Alias:   {
        "poc": {
            Command:     {"overwritten", "values"},
            Environment: "poc",
        },
        "other": {
            Command:     {"overwritten", "values"},
            Environment: "poc",
        },
    },
}

but I get the unchanged original data instead:

main.Config{
    Project: "proof-of-concept",
    Alias:   {
        "poc": {
            Command:     {"this can be split"},
            Environment: "poc",
        },
        "other": {
            Command:     {"this", "can", "be", "left"},
            Environment: "poc",
        },
    },
}

I'm not 100% sure I have the interface right, but the API doc for gopkg.in/yaml.v2 doesn't go into detail.

@augier
Copy link

augier commented Oct 20, 2017

Just had the same problem and my interface looked the same as yours. @TimJones did you find a way to do this?

@TimJones
Copy link
Author

TimJones commented Mar 1, 2018

@augier Sorry this went so long without a reply. Unfortunately I never found a way around this, I simply had to force one format over the other. If you had any better luck I'd be interested in hearing about it!

@tubocurarin
Copy link

tubocurarin commented Apr 18, 2018

Took me a while to find the reason...

If you call ReadInConfig() or ReadConfig() on a viper-object, it first parses everything into a map[string]interface{}. However, your structure you want to marshal into is not in play yet, so the used yaml package will blindly parse everything into the map (as your struct types aren't known there), so all UnmarshalYAML (and even UnmarshalJSON) functions will be completely ignored.
If you now unmarshal into your struct, viper uses mapstructure to now map the map[string]interface{} to your structs, but it's not YAML any more.

Ideally mapstructure would provide similar unmarshaling interfaces so this can be handled easily, like

func (m *MyType) UnmarshalMap(value interface{}) error {
    // your code ...
}

Imo this is not a bug in viper itself, that's kind of "by design".

(Filed an issue on mapstructure as well to implement such a marshaling interface: mitchellh/mapstructure#115)

@TimJones
Copy link
Author

Great work @tubocurarin !! This was quite the gotcha for me!!

l0nax added a commit to fabmation-gmbh/oima that referenced this issue Aug 13, 2019
This is related of how Viper processes the Config File.

Read more at: spf13/viper#338
Ullaakut added a commit to Ullaakut/GoneyPot that referenced this issue Jul 5, 2020
Turns out Viper does not support YAML unmarshalling directly but goes through a map structure as an intermediate: spf13/viper#338
ajm188 added a commit to ajm188/vitess that referenced this issue Nov 6, 2021
[This issue][1] does a good job explaining the why behind this
admittedly-hacky code. What I would have preferred to do is add
`UnmarshalJSON`, `UnmarshalTOML`, (and so on) methods to both
`FileConfig` and `Config`, but switching to viper means we don't invoke
those custom unmarshalers _at all_, because viper is going from
$file_bytes=>`map[string]interface{}`=(via mapstructure)>$your_type.

So, we define a temporary struct (so far, so good) that will play nicely
with mapstructure's unmarshaling. We then take those string maps and
hand them off to the individual Config structs to run through the
kv-`parseOne` loop as before.

Since we're using viper to load from files now (and viper does not
invoke our custom unmarshal functions ...), we no longer need them, so I
deleted all of that code as well.

[1]: spf13/viper#338 (comment)

Signed-off-by: Andrew Mason <amason@slack-corp.com>
robinbraemer added a commit to minekube/gate that referenced this issue Jul 12, 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

3 participants