mod: use upstream go-yaml

[tormath1]

github.com/coreos/yaml is not even maintained, let's switch to the upstream.

This drops:

• conversion from hyphen to underscore (e.g write-files-> write_files)
• usage of wrong values for a given key (e.g reboot-strategy: [1, 2, 3])

Related to: flatcar/Flatcar#1271

[krnowak]

The normalization is the icky part. I'm not sure if this won't cause some nasty bugs.

[krnowak]

Suggested change

	// Normalize transform the "-" in "_" for the keys
	// in the YAML configuration.
	// e.g reboot-strategy -> reboot_strategy
	// Normalize transforms dashes into underlines for the keys
	// in the YAML configuration.
	// e.g reboot-strategy -> reboot_strategy

[krnowak]

Honestly, this is a rather risky function. Will it mess some inline bash script like:

- some_inline_stuff: |
  echo 'foo-bar: stuff'

https://play.golang.com/p/u6ogWtIHI9Q

This kind of transform is probably done safe only by actually parsing YAML.

[gabriel-samfira]

I tend to agree here. If we mutate the source yaml, we're asking for trouble. It would be nice if the yaml package supported multiple tags for the same field, but sadly it does not.

[tormath1]

I agree with all of you. I tried to provide a "best effort" approach to continue maintaining the current behavior but honestly I'd be in favor of dropping this "compatibility" (e.g reboot-strategy == reboot_strategy). I understand it was quite convenient back in the days, but today it's a nonsense to me: with a clear spec, such a behavior would not be needed. I never seen Kubernetes YAML manifest or other well known YAML configuration doing such a thing.

This best effort is just to provide backward compatibility with folks still using coreos-cloudinit with "bad" keys.

[krnowak]

Personally, I'd drop this backward compatibility then, because continuing to support invalid keys adds too much to maintenance overhead. Otherwise we risk mangling unrelated stuff in config file. If you decide to drop it then we also will need to announce it. I think a changelog entry when bumping the commit ID in scripts repo would be sufficient.

[tormath1]

Thanks. Then I dropped the compatibility.

[krnowak]

You probably could avoid this duplication by putting the common code into an inline function (excuse my rusty go, haven't typed that in a long time):

handleKV := func(k,v interface{}) {
	cn := node{name: fmt.Sprintf("%s", k)}
	c, ok := findKey(cn.name, c)
	if ok {
		cn.line = c.lineNumber
	}
	toNode(cv, c, &cn)
	n.children = append(n.children, cn)
}
cpv := v
v, ok := cpv.(map[interface{}]interface{})
if ok {
	for k, cv := range v {
		handleKV(k, cv)
	}
} else {
	sv := cpv.(map[string]interface{})
	for k, cv := range sv {
		handleKV(k, cv)
	}
}

Probably could be made even shorter by using Go's generics, but I have no experience with those.

[krnowak]

Hm, this normalizes the yaml config now, whereas the old code had a no-op transform at this point. Not sure if it matters, though.