Custom yaml unmarshaling (e.g. defaults)

[palantir]

Custom yaml unmarshaling (e.g. defaults)

[bmoylan]

FYSA @ashrayjain

[nmiyake]

Thought about this further after our in-person discussion. I think the decision here stems on how opinionated we want to be in terms of how we deal with configuration.

As you point out in this issue, right now witchcraft is opinionated in its use of yaml.Unmarshal from go-yaml/yaml (aka gopkg.in/yaml.v2) to perform config unmarshaling. It's probably not unreasonable to allow this function to be configured by allowing the user to specify a custom func([]byte, interface{}) error used in all places where config is unmarshaled (and default the value to yaml.Unmarshal). This would allow the use of other YAML libraries/custom functionality as outlined here. However, because this generically plugs the unmarshal, it would also allow clients to unmarshal config as JSON or whatever else they want as well. Probably not a huge issue, but just worth flagging that allowing configuration of the Unmarshal behavior does technically allow a lot more to be configured. We would probably need to caveat that, in general, witchcraft configuration will be written in a manner that assumes the behavior of yaml.Unmarshal, so if the user provides their own implementation they are potentially responsible for supporting all of its behavior (tags such as ,inline, etc.).

If we decide that the usage of yaml.Unmarshal is one of the opinions of this library (which is also somewhat the case based on the default tags format we use for base config), then I don't think we should allow this to be configured. In this scenario, it's still possible to perform custom operations like setting/loading defaults for config, but it has to be structured as part of the yaml.Unmarshal or as a specialized config byte provider.

Right now I'm leaning slightly towards the first approach, but @bmoylan curious to hear your thoughts/opinion.

[nmiyake]

Concretely, if there's some function like structfields.SetToDefaults (analogous to https://github.com/creasty/defaults), then the approaches described above would be:

WithConfigUnmarshalFunction(func(data []byte, v interface{}) error {
	if err := structfields.SetToDefaults(v); err != nil {
		return err
	}
	return yaml.Unmarshal(data, v)
})

vs. implementing something like:

func (cfg *AppInstallConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
	if err := structfields.SetToDefaults(cfg); err != nil {
		return err
	}
	type RawAppInstallConfig AppInstallConfig
	rawCfg := RawAppInstallConfig(*cfg)
	if err := unmarshal(&rawCfg); err != nil {
		return err
	}
	*cfg = AppInstallConfig(rawCfg)
	return nil
}

For the install and runtime structs.