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

Add command to generate jsonschema for limayaml #2306

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

afbjorklund
Copy link
Contributor

@afbjorklund afbjorklund commented Apr 30, 2024

Closes #2305

https://pypi.org/project/check-jsonschema/

Actually found two bugs, with the current code:

Schema validation errors were encountered.
  examples/default.yaml::$.vmType: None is not of type 'string'
  examples/default.yaml::$.os: None is not of type 'string'
  examples/default.yaml::$.arch: None is not of type 'string'
  examples/default.yaml::$.cpuType.armv7l: None is not of type 'string'
  examples/default.yaml::$.cpuType.aarch64: None is not of type 'string'
  examples/default.yaml::$.cpuType.x86_64: None is not of type 'string'
  examples/default.yaml::$.cpus: None is not of type 'integer'
  examples/default.yaml::$.memory: None is not of type 'string'
  examples/default.yaml::$.disk: None is not of type 'string'
Schema validation errors were encountered.
  examples/docker.yaml::$.probes[0]: Additional properties are not allowed ('hint', 'script' were unexpected)
  examples/docker.yaml::$.probes[0]: 'Mode' is a required property
  examples/docker.yaml::$.probes[0]: 'Description' is a required property
  examples/docker.yaml::$.probes[0]: 'Script' is a required property
  examples/docker.yaml::$.probes[0]: 'Hint' is a required property

But the rest of it is just about adding "nullable".

@afbjorklund

This comment was marked as outdated.

@mac2net
Copy link

mac2net commented Apr 30, 2024

Don't you think this universal rejection of “none” is more an opinionated code base rather than a validation error?

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Apr 30, 2024

Don't you think this universal rejection of “none” is more an opinionated code base rather than a validation error?

It's not a rejection (as such), there is a patch for it (to not have to decorate the go struct) but it has not been merged yet

With the patch, one could (perhaps) mark all string pointers and arrays as accepting null as a valid value (by default).


But it does catch some issues, with string being fed with null - which is valid in JavaScript but not really in Go...

Having to decorate the pointers and arrays I think is not so intrusive, so it seems like a valid workaround to me.

                "vmType": {
                    "type": "string"
                },

examples/default.yaml::$.os: None is not of type 'string'

With "nullable", the following json schema is generated instead:

                "vmType": {
                    "oneOf": [
                        {
                            "type": "string"
                        },
                        {
                            "type": "null"
                        }
                    ]
                },

@afbjorklund
Copy link
Contributor Author

afbjorklund commented May 1, 2024

$ make check-jsonschema 
# The hostagent must be compiled with CGO_ENABLED=1 so that net.LookupIP() in the DNS server
# calls the native resolver library and not the simplistic version in the Go library.
CGO_ENABLED=1 go build -ldflags="-s -w -X github.com/lima-vm/lima/pkg/version.Version=v0.21.0-51-ga177cd8" -tags "" -o _output/bin/limactl ./cmd/limactl
_output/bin/limactl generate-schema >schema-limayaml.json
check-jsonschema --schemafile schema-limayaml.json examples/default.yaml
ok -- validation done

These arrays have to allow null, since they are given that way in the default yaml:

additionalDisks:
# disks should either be a list of disk name strings, for example:
# - "data"
# or a list of disk objects with extra parameters, for example:
# - name: "data"
#   format: true
#   fsType: "ext4"
caCerts:
  ...

  # A list of trusted CA certificate files. The files will be read and passed to cloud-init.
  files:
  # - examples/hello.crt

  # A list of trusted CA certificates. These are directly passed to cloud-init.
  certs:
  # - |
  #   -----BEGIN CERTIFICATE-----
  #   YOUR-ORGS-TRUSTED-CA-CERT-HERE
  #   -----END CERTIFICATE-----
  # - |
  #   -----BEGIN CERTIFICATE-----
  #   YOUR-ORGS-TRUSTED-CA-CERT-HERE
  #   -----END CERTIFICATE-----
networks:
# Lima can manage daemons for networks defined in $LIMA_HOME/_config/networks.yaml
# automatically. The socket_vmnet binary must be installed into
# secure locations only alterable by the "root" user.
# The same applies to vde_switch and vde_vmnet for the deprecated VDE mode.
# - lima: shared
#   # MAC address of the instance; lima will pick one based on the instance name,
#   # so DHCP assigned ip addresses should remain constant over instance restarts.
#   macAddress: ""
#   # Interface name, defaults to "lima0", "lima1", etc.
#   interface: ""
#
# Lima can also connect to "unmanaged" networks addressed by "socket". This
# means that the daemons will not be controlled by Lima, but must be started
# before the instance.  The interface type (host, shared, or bridged) is
# configured in socket_vmnet and not in lima.
# - socket: "/var/run/socket_vmnet"
hostResolver:

  ...
  hosts:
    # guest.name: 127.1.1.1
    # host.name: host.lima.internal

A better alternative would be to comment out the key as well, and leave them undefined?

The other keys are only allowing null values, when they explicitly are given that way:

vmType: null
os: null
arch: null
images:
  - location: https://cloud-images.ubuntu.com/releases/24.04/release-20240423/ubuntu-24.04-server-cloudimg-amd64.img
    arch: x86_64
    digest: sha256:32a9d30d18803da72f5936cf2b7b9efcb4d0bb63c67933f17e3bdfd1751de3f3
  - location: https://cloud-images.ubuntu.com/releases/24.04/release-20240423/ubuntu-24.04-server-cloudimg-arm64.img
    arch: aarch64
    digest: sha256:c841bac00925d3e6892d979798103a867931f255f28fefd9d5e07e3e22d0ef22
  - location: https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-amd64.img
    arch: x86_64
  - location: https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-arm64.img
    arch: aarch64
cpus: null
memory: null
disk: null
mounts:
  - location: "~"
    mountPoint: null
    writable: null
    sshfs:
      cache: null
      followSymlinks: null
      sftpDriver: null
    9p:
      securityModel: null
      protocolVersion: null
      msize: null
      cache: null
  - location: /tmp/lima
    writable: true
mountType: null
mountInotify: null
additionalDisks: null
ssh:
  localPort: 0
  loadDotSSHPubKeys: null
  forwardAgent: null
  forwardX11: null
  forwardX11Trusted: null
caCerts:
  removeDefaults: null
  files: null
  certs: null
upgradePackages: null
containerd:
  system: null
  user: null
cpuType:
  aarch64: null
  armv7l: null
  x86_64: null
rosetta:
  enabled: null
  binfmt: null
timezone: null
firmware:
  legacyBIOS: null
audio:
  device: null
video:
  display: null
  vnc:
    display: null
networks: null
propagateProxyEnv: null
hostResolver:
  enabled: null
  ipv6: null
  hosts: null
guestInstallPrefix: null
plain: null

Currently the only required key is "images", but even that might be optional (for ext).

@afbjorklund
Copy link
Contributor Author

afbjorklund commented May 1, 2024

Here is the empty yaml:

images: []

And here is default yaml:

images:
- location: https://cloud-images.ubuntu.com/releases/24.04/release-20240423/ubuntu-24.04-server-cloudimg-amd64.img
  arch: x86_64
  digest: sha256:32a9d30d18803da72f5936cf2b7b9efcb4d0bb63c67933f17e3bdfd1751de3f3
- location: https://cloud-images.ubuntu.com/releases/24.04/release-20240423/ubuntu-24.04-server-cloudimg-arm64.img
  arch: aarch64
  digest: sha256:c841bac00925d3e6892d979798103a867931f255f28fefd9d5e07e3e22d0ef22
- location: https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-amd64.img
  arch: x86_64
- location: https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-arm64.img
  arch: aarch64
cpuType:
  aarch64: ""
  armv7l: ""
  riscv64: ""
  x86_64: ""
mounts:
- location: ~
- location: /tmp/lima
  writable: true
ssh:
  localPort: 0

Hopefully the images can be made more similar to the containerd archives, and available from the code instead.

Or maybe not the go code, but to keep the nerdctl-full and the distro images in some kind of secondary config ?

---
"ubuntu:24.04":
  images:
# Try to use release-yyyyMMdd image if available. Note that release-yyyyMMdd will be removed after several months.
  - location: "https://cloud-images.ubuntu.com/releases/24.04/release-20240423/ubuntu-24.04-server-cloudimg-amd64.img"
    arch: "x86_64"
    digest: "sha256:32a9d30d18803da72f5936cf2b7b9efcb4d0bb63c67933f17e3bdfd1751de3f3"
  - location: "https://cloud-images.ubuntu.com/releases/24.04/release-20240423/ubuntu-24.04-server-cloudimg-arm64.img"
    arch: "aarch64"
    digest: "sha256:c841bac00925d3e6892d979798103a867931f255f28fefd9d5e07e3e22d0ef22"
  # Fallback to the latest release image.
  # Hint: run `limactl prune` to invalidate the cache
  - location: "https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-amd64.img"
    arch: "x86_64"
  - location: "https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-arm64.img"
    arch: "aarch64"
---
"nerdctl-full:1.7.6":
  images:
  - location: "https://github.com/containerd/nerdctl/releases/download/1.7.6/nerdctl-full-1.7.6-linux-amd64.tar.gz"
    arch: "x86_64"
    digest: "sha256:2c841e097fcfb5a1760bd354b3778cb695b44cd01f9f271c17507dc4a0b25606"
  - location: "https://github.com/containerd/nerdctl/releases/download/1.7.6/nerdctl-full-1.7.6-linux-arm64.tar.gz"
    arch: "aarch64"
    digest: "sha256:77c747f09853ee3d229d77e8de0dd3c85622537d82be57433dc1fca4493bab95"
    # No arm-v7
    # No riscv64

And then expanded by lima.

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Use empty strings for strings and document all the fields

Allow null for pointers and maps with commented out keys

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
The null is only used in the default.yaml as a placeholder

When loaded into a string or array or map, it is "empty".

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Was crashing in regression tests on other platforms

Test also for qemu, in case of new arch or somesuch

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
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.

Generate jsonschema for validating the limayaml
2 participants