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

Validate generated user-data yaml with jsonschema #2267

Closed
wants to merge 6 commits into from

Conversation

afbjorklund
Copy link
Contributor

@afbjorklund afbjorklund commented Apr 4, 2024

Currently there are 3 validation errors being reported:

jsonschema: '/ca_certs/remove_defaults' does not validate with https://raw.githubusercontent.com/canonical/cloud-init/main/cloudinit/config/schemas/schema-cloud-config-v1.json#/allOf/8/$ref/properties/ca_certs/$ref/properties/remove_defaults/type: expected boolean, but got string

jsonschema: '/ca_certs/trusted' does not validate with https://raw.githubusercontent.com/canonical/cloud-init/main/cloudinit/config/schemas/schema-cloud-config-v1.json#/allOf/8/$ref/properties/ca_certs/$ref/properties/trusted/type: expected array, but got null

jsonschema: '/users/0' does not validate with https://raw.githubusercontent.com/canonical/cloud-init/main/cloudinit/config/schemas/schema-cloud-config-v1.json#/allOf/49/$ref/properties/users/items/oneOf/0/type: expected string, but got object

And 1 deprecation warning, not yet being shown here.

        "uid": {
          "description": "The user's ID. Default value [system default]",
          "oneOf": [
            {
              "type": "integer"
            },
            {
              "type": "string",
              "changed": true,
              "changed_description": "The use of ``string`` type is deprecated. Use an ``integer`` instead.",
              "changed_version": "22.3"
            }
          ]
        }

https://github.com/canonical/cloud-init/blob/main/cloudinit/config/schemas/schema-cloud-config-v1.json


Issue:

The first two are fixed by

But "ssh-authorized-keys" is a problem.

        "ssh_authorized_keys": {
          "description": "List of SSH keys to add to user's authkeys file. Can not be combined with ``ssh_redirect_user``",
          "type": "array",
          "items": {
            "type": "string"
          },
          "minItems": 1
        },

https://git.launchpad.net/cloud-init/commit/?id=b27f713a

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Apr 5, 2024

As an example there is a warning in place for ca-certs, something that affected lima earlier (326bbaa)

    "cc_ca_certs": {
      "type": "object",
      "properties": {
        "ca_certs": {
          "$ref": "#/$defs/ca_certs.properties"
        },
        "ca-certs": {
          "allOf": [
            {
              "$ref": "#/$defs/ca_certs.properties"
            },
            {
              "deprecated": true,
              "deprecated_version": "22.3",
              "deprecated_description": "Use ``ca_certs`` instead."
            }
          ]
        }
      }
    },

But no such deprecation for ssh-authorized-keys.

@afbjorklund
Copy link
Contributor Author

afbjorklund commented Apr 7, 2024

The cidata tests are supposed to be failing, until the schema is valid (i.e. ssh_authorized_keys)

But it would be a good idea to get the lima-init fix merged first, and check all other cloud-init

@jandubois
Copy link
Member

The cidata tests are supposed to be failing, until the schema is valid (i.e. ssh_authorized_keys)

I don't think validation should fail for using deprecated expressions. We cannot assume that all supported distros will have updated to a new cloud-init immediately, so Lima needs to continue to use the deprecated form until the new variant is supported everywhere.

This is kind of the point of deprecation instead of outright removal.

pkg/cidata/schema.go Outdated Show resolved Hide resolved
@afbjorklund
Copy link
Contributor Author

afbjorklund commented Apr 9, 2024

I don't think validation should fail for using deprecated expressions.

It is not deprecated in the jsonschema, it is outright missing (and failing)

That is probably a bug in the schema, since implementation supports it*

For instance ca-certs is deprecated, then again deprecation is "annotation"

* for decades, while schema is years

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>
@afbjorklund
Copy link
Contributor Author

afbjorklund commented Apr 9, 2024

I added cloud-init jsonschema "latest" documentation version 24.1.3

Then I added my cloud-init patch as well: canonical/cloud-init#5162

--- a/cloudinit/config/schemas/schema-cloud-config-v1.json
+++ b/cloudinit/config/schemas/schema-cloud-config-v1.json
@@ -361,6 +361,22 @@
           },
           "minItems": 1
         },
+        "ssh-authorized-keys": {
+          "allOf": [
+            {
+              "type": "array",
+              "items": {
+                "type": "string"
+              },
+              "minItems": 1
+            },
+            {
+              "deprecated": true,
+              "deprecated_version": "18.3",
+              "deprecated_description": "Use ``ssh_authorized_keys`` instead."
+            }
+          ]
+        },
         "ssh_import_id": {
           "description": "List of ssh ids to import for user. Can not be combined with ``ssh_redirect_user``. See the man page[1] for more details. [1] https://manpages.ubuntu.com/manpages/noble/en/man1
/ssh-import-id.1.html",
           "type": "array",

Note: the latest version on github is 24.1.4

Note: the jsonschema on main is different

index ff61dcaa6..a71cec996 100644
--- a/cloudinit/config/schemas/schema-cloud-config-v1.json
+++ b/cloudinit/config/schemas/schema-cloud-config-v1.json
@@ -835,6 +835,13 @@
                 "vault_password_file": {
                   "type": "string"
                 },
+                "verify_commit": {
+                  "type": "boolean",
+                  "default": false
+                },
+                "inventory": {
+                  "type": "string"
+                },
                 "module_name": {
                   "type": "string"
                 },

canonical/cloud-init@b1bfa59

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
@afbjorklund
Copy link
Contributor Author

The patch for deprecating ssh-authorized-keys has now been accepted upstream, and is on main

Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
Signed-off-by: Anders F Björklund <anders.f.bjorklund@gmail.com>
@afbjorklund
Copy link
Contributor Author

afbjorklund commented May 5, 2024

Now when cloud-config.yaml is available in the instance directory, we can use limactl validate with it too.

I think eventually it will be left to the user, to run their favorite schema validation tool with their favorite URL:

check-jsonschema --schemafile https://raw.githubusercontent.com/canonical/cloud-init/main/cloudinit/config/schemas/schema-cloud-config-v1.json ~/.lima/default/cloud-config.yaml

@afbjorklund
Copy link
Contributor Author

It is possible to use the go version through cli, so we don't need to included it with lima...

go install github.com/santhosh-tekuri/jsonschema/cmd/jv@latest

And then it (jv) can be invoked with the schema file, similar to check-jsonschema above:

jv https://raw.githubusercontent.com/canonical/cloud-init/main/cloudinit/config/schemas/schema-cloud-config-v1.json ~/.lima/default/cloud-config.yaml

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.

None yet

3 participants