templates: use yq's indentation style

[AkihiroSuda]

lima/pkg/yqutil/yqutil_test.go

Lines 30 to 55 in 2992029

	func TestEvaluateExpressionComplex(t *testing.T) {
	expression := `.mounts += {"location": "foo", "mountPoint": "bar"}`
	content := `
	# Expose host directories to the guest, the mount point might be accessible from all UIDs in the guest
	# 🟢 Builtin default: null (Mount nothing)
	# 🔵 This file: Mount the home as read-only, /tmp/lima as writable
	mounts:
	- location: "~"
	# Configure the mountPoint inside the guest.
	# 🟢 Builtin default: value of location
	mountPoint: null
	`
	// Note: yq will use canonical yaml, with indented sequences
	// Note: yq will not explicitly quote strings, when not needed
	expected := `
	# Expose host directories to the guest, the mount point might be accessible from all UIDs in the guest
	# 🟢 Builtin default: null (Mount nothing)
	# 🔵 This file: Mount the home as read-only, /tmp/lima as writable
	mounts:
	- location: "~"
	# Configure the mountPoint inside the guest.
	# 🟢 Builtin default: value of location
	mountPoint: null
	- location: foo
	mountPoint: bar
	`

Lots of users are expected to use --set (yq) soon, so I'd prefer to adopt yq's indentation style.

Revert:

• Don't indent sequences in YAML files #512

[afbjorklund]

I think the "style" of yq is mostly related to bugs in the go-yaml library, maybe they can be addressed ?

https://github.com/goccy/go-yaml

Or possibly the issue is with the gopkg.in/yaml.v3, it is hard to tell because it uses both (and then some)

• v3: (Encoder) setting indentation for sequences go-yaml/yaml#661

[afbjorklund]

The idiosyncrasies here is that this document:

# comment
foo: "x"
bar: y
baz: null

map:
- foo: 1
- bar: 2
- baz: 3

Gets reformatted, when piped through yq, as:

# comment
foo: "x"
bar: y
baz: null
map:
  - foo: 1
  - bar: 2
  - baz: 3

Even though the first document is valid YAML.

Basically, missing lines and extra indentation.

---

  2:1       warning  missing document start "---"  (document-start)
  7:1       error    wrong indentation: expected 2 but found 0  (indentation)

rules:
  document-start: disable
  indentation:
    indent-sequences: false

[jandubois]

I think the "style" of yq is mostly related to bugs in the go-yaml library, maybe they can be addressed ?

Yes, please!

Or possibly the issue is with the gopkg.in/yaml.v3, it is hard to tell because it uses both (and then some)

I thought it was because of go-yaml v3. The indentation change is a regression from go-yaml v2, but the library is essentially unmaintained:

$ git log --pretty=format:"%h %ad %s" --date=short -5
f6f7691 2022-05-27 The Billion Dollar Mistake
00bbc09 2022-05-21 github: drop broken semgrep workflow
8f96da9 2022-05-21 Explicitly check the parser for errors on peek
539c8e7 2022-05-12 Fix decoding of merge tags.
496545a 2021-01-07 Improve inline comments on map keys (#656).

$ gh issue list

Showing 30 of 239 open issues in go-yaml/yaml
[...]

$ gh pr list

Showing 30 of 100 open pull requests in go-yaml/yaml
[...]

There are many issues and several PRs about this regression. kubernetes-sigs/yaml has decided to create a fork with the fix:

kubernetes-sigs/yaml#72

But this is rather slow-moving:

kubernetes-sigs/yaml#76

I was thinking about trying to use one of the existing forks for now, e.g. from

go-yaml/yaml#924

But it may require a fork of the fork, to make the v2 behaviour the default, as we don't want to modify the yq code itself.

Note that according to the yq documentation it should be using the compact list form with -I2: https://mikefarah.gitbook.io/yq/usage/output-format#indent

But that is also broken due to the go-yaml regression:

mikefarah/yq#825

[AkihiroSuda]

I think the "style" of yq is mostly related to bugs in the go-yaml library, maybe they can be addressed ?

The removal of empty lines might be a bug, but I assume the indentation of bullet point lists isn't likely to change in a future release of yq.

[jandubois]

The removal of empty lines might be a bug,

This is a separate problem from fixing list indentation. Strangely I couldn't find any open issues about this behaviour, but I didn't try very hard.

the indentation of bullet point lists isn't likely to change in a future release of yq.

Yeah, but we don't really care about yq itself, but about how our --set option will modify lima.yaml. So if we can get limactl to link with a modified version of go-yaml that fixes this regression, then we should still be good, even if yq releases continue to use the broken¹ version.

Footnotes

1. I know it is not really broken, it is just badly formatted. ↩

[afbjorklund]

• Can't update YAML keeping newlines goccy/go-yaml#285

EDIT: same issue, different library

[afbjorklund]

I don't really like copying external libraries to internal, like kustomize did here. Better to use go mod replace.

• compact seq indent option go-yaml/yaml#753

• https://patch-diff.githubusercontent.com/raw/go-yaml/yaml/pull/753.patch

I made a proof-of-concept, where I patched yaml v3 and then added a bool to the yq prefs for lima to use:

• afbjorklund/yq@cdbfb91

• afbjorklund@32c5970

Unfortunately that means having to carry patch branches, and rebase them with any new yaml/yq releases.

replace (
	github.com/mikefarah/yq/v4 => github.com/afbjorklund/yq/v4 v4.33.3-seq
	gopkg.in/yaml.v3 => github.com/afbjorklund/yaml/v3 v3.0.1-kyaml
)

Should go under lima-vm organization, if this forking path is taken (instead of changing lima indents).

[AkihiroSuda]

if this forking path is taken (instead of changing lima indents).

replace() is often unmaintainable, so I'm wondering if we can just (temporarily?) accept yq's current implementation.

[afbjorklund]

That is possible, can also wait for kyaml to appear...
One of the patches (4/4) was actually totally unrelated, and only "worked" by rigging the test - so I think I will revert it. Too bad, I was hoping for it to fix the newlines in comments, for round-trip...
If the library fixed both things, it would be much better

[afbjorklund]

The newlines are treated as a separator (between comments), so they are not actually included in the comment (nor elsewhere)

That makes them missing in Node, and "disappear" when doing a round-trip yaml. It needs a similar boolean, to keep newlines in comments.

[jandubois]

I'm wondering if we can just (temporarily?) accept yq's current implementation.

Why is this necessary? Users who use --set don't even see how lima.yaml is formatted, so if the indentation gets mangled at that point it is unfortunate, but won't affect most of them.

So we would preemptively mangle the formatting, just to keep the diff small for users that likely don't care or even notice?

[AkihiroSuda]

Why

Not just for --set; I want to use yq (or something else similar) for automating bumping up the templates.
#1347

[jandubois]

I want to use yq (or something else similar) for automating bumping up the templates.

Thanks! I totally missed that, and I understand why this is necessary.

But it means we don't need to modify the yq code used by limactl; we could just have a fixed version of yq in a fork, and fetch the binary from its Github release page as part of the updater script.

And the fixed fork probably wouldn't have to be kept in sync with upstream yq changes either. Once it works, we can just keep it as-is for internal build purposes.