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

yaml: deprecate non-strict YAML #1045

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Sep 8, 2022

Having duplicated keys is now prohibited, but having unsupported keys is still allowed (with deprecation warning)

Fix #1044

$ cat correct.yaml 
images:
- location: "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-amd64.img"

networks:
- lima: shared
- lima: bridged

$ limactl validate correct.yaml 
INFO[0000] "correct.yaml": OK 
$ cat duplicated-key.yaml 
images:
- location: "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-amd64.img"

networks:
- lima: shared
  lima: bridged

$ limactl validate duplicated-key.yaml 
FATA[0000] failed to load YAML file "duplicated-key.yaml": failed to unmarshal YAML (main file "/tmp/foo/duplicated-key.yaml"): [6:3] duplicate key "lima"
       4 | networks:
       5 | - lima: shared
    >  6 |   lima: bridged
             ^ 
$ cat unknown-key.yaml 
images:
- location: "https://cloud-images.ubuntu.com/releases/22.04/release/ubuntu-22.04-server-cloudimg-amd64.img"

networks:
- lima: shared
lima: bridged

$ limactl validate unknown-key.yaml 
WARN[0000] Non-strict YAML is deprecated and will be unsupported in a future version of Lima  comment="main file \"/tmp/foo/unknown-key.yaml\"" error="[6:1] unknown field \"lima\"\n       4 | networks:\n       5 | - lima: shared\n    >  6 | lima: bridged\n           ^"
INFO[0000] "unknown-key.yaml": OK

Having duplicated keys is now prohibited, but having unsupported keys is still allowed
(with deprecation warning)

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda AkihiroSuda added this to the v0.11.4 milestone Sep 8, 2022
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'm fine with merging it.

I have 2 observations though, running it against a RD config that has already been updated with socketVMNet support for testing:

$ limactl validate ~/Library/Application\ Support/rancher-desktop/lima/0/lima.yaml
WARN[0000] Non-strict YAML is deprecated and will be unsupported in a future version of Lima  comment="main file \"/Users/jan/Library/Application Support/rancher-desktop/lima/0/lima.yaml\"" error="[150:1] unknown field \"k3s\"\n      147 |     lima-rancher-desktop: lima-0\n      148 |     host.rancher-desktop.internal: host.lima.internal\n      149 |     host.docker.internal: host.lima.internal\n    > 150 | k3s:\n            ^\n      151 |   version: 1.24.4"
FATA[0000] failed to load YAML file "/Users/jan/Library/Application Support/rancher-desktop/lima/0/lima.yaml": cannot parse "/Users/jan/.lima/_config/networks.yaml": [15:3] unknown field "socketVMNet"
      12 | paths:
      13 | # socketVMNet requires Lima >= 0.12 .
      14 | # socketVMNet has precedence over vdeVMNet.
    > 15 |   socketVMNet: /opt/socket_vmnet/bin/socket_vmnet
             ^
      16 | # vdeSwitch and vdeVMNet are DEPRECATED.
      17 |   vdeSwitch: /opt/vde/bin/vde_switch
      18 |   vdeVMNet: /opt/vde/bin/vde_vmnet
      19 |

The WARN output looks a lot less readable than the FATA output, due to the way the errors are reported. I think this is fine though, given that we will make "Strict" the default soon, so the warning will go away again. But if there is an easy fix for this, please apply it!

And the error about networks.yaml should not happen with the release version because by then the socketVMNet support will have been merged as well.

So despite these observations I'm fine with merging.

@AkihiroSuda AkihiroSuda merged commit 1d2c7e5 into lima-vm:master Sep 8, 2022
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

Successfully merging this pull request may close these issues.

YAML: duplicated keys should raise an error (with human-readable message)
2 participants