meta: refactor schema versioning to improve errors (#385)

This greatly improves the quality of the validating error messages. Improvement of error messages comes with a minor downside, as from now on schema validation for meta would not pass on empty files. Users will have to either put real content in meta files to pass schema validation, such as version at least to pass. That was needed for technical reasons as without it the error messages would be considerably less easy to understand. Related: python-jsonschema/jsonschema#1002
ansible · Sep 14, 2022 · 20f13a9 · 20f13a9
1 parent b6b627c
commit 20f13a9
Show file tree

Hide file tree

Showing 15 changed files with 135 additions and 253 deletions.
diff --git a/Taskfile.yml b/Taskfile.yml
@@ -57,7 +57,7 @@ tasks:
       - ".github"
       - ".vscode"
       - data
-      - f
+      - f/**
       - negative_test
       - src/**
       - test/**
@@ -70,10 +70,10 @@ tasks:
       - npm run test
       - task: summary
     sources:
-      - f/
-      - negative_test/
-      - test/
-      - src/
+      - f/**
+      - negative_test/**
+      - test/**
+      - src/**
       - package.json
       - package-lock.json
       - Taskfile.yml

diff --git a/f/ansible-meta.json b/f/ansible-meta.json
@@ -1323,7 +1323,9 @@
           "const": 1
         }
       },
-      "title": "Meta Schema v1 (standalone role)"
+      "required": ["version"],
+      "title": "Meta Schema v1 (standalone role)",
+      "type": "object"
     },
     "v2": {
       "additionalProperties": false,
@@ -1349,7 +1351,8 @@
           "const": 2
         }
       },
-      "title": "Meta Schema v2 (role inside collection)"
+      "title": "Meta Schema v2 (role inside collection)",
+      "type": "object"
     },
     "vCenterPlatformModel": {
       "properties": {
@@ -1392,14 +1395,26 @@
   },
   "$id": "https://raw.githubusercontent.com/ansible/schemas/main/f/ansible-meta.json",
   "$schema": "http://json-schema.org/draft-07/schema",
-  "anyOf": [
-    {
-      "$ref": "#/$defs/v1"
-    },
-    {
-      "$ref": "#/$defs/v2"
-    }
-  ],
+  "else": {
+    "$ref": "#/$defs/v2"
+  },
   "examples": ["meta/main.yml"],
-  "title": "Ansible Meta Schema v1/v2"
+  "if": {
+    "properties": {
+      "version": {
+        "const": 1
+      }
+    }
+  },
+  "properties": {
+    "version": {
+      "enum": [1, 2],
+      "type": "integer"
+    }
+  },
+  "then": {
+    "$ref": "#/$defs/v1"
+  },
+  "title": "Ansible Meta Schema v1/v2",
+  "type": "object"
 }
diff --git a/negative_test/roles/empty_meta/meta/main.yml b/negative_test/roles/empty_meta/meta/main.yml
diff --git a/negative_test/roles/empty_meta/meta/main.yml.md b/negative_test/roles/empty_meta/meta/main.yml.md
@@ -0,0 +1,58 @@
+# ajv errors
+
+```json
+[
+  {
+    "instancePath": "",
+    "keyword": "type",
+    "message": "must be object",
+    "params": {
+      "type": "object"
+    },
+    "schemaPath": "#/type"
+  },
+  {
+    "instancePath": "",
+    "keyword": "if",
+    "message": "must match \"then\" schema",
+    "params": {
+      "failingKeyword": "then"
+    },
+    "schemaPath": "#/if"
+  },
+  {
+    "instancePath": "",
+    "keyword": "type",
+    "message": "must be object",
+    "params": {
+      "type": "object"
+    },
+    "schemaPath": "#/type"
+  }
+]
+```
+
+# check-jsonschema
+
+stdout:
+
+```json
+{
+  "status": "fail",
+  "errors": [
+    {
+      "filename": "negative_test/roles/empty_meta/meta/main.yml",
+      "path": "$",
+      "message": "None is not of type 'object'",
+      "has_sub_errors": false
+    },
+    {
+      "filename": "negative_test/roles/empty_meta/meta/main.yml",
+      "path": "$",
+      "message": "None is not of type 'object'",
+      "has_sub_errors": false
+    }
+  ],
+  "parse_errors": []
+}
+```
diff --git a/negative_test/roles/meta/main.yml b/negative_test/roles/meta/main.yml
@@ -1,3 +1,4 @@
+version: 1
 galaxy_info:
   description: bar
   min_ansible_version: "2.9"

diff --git a/negative_test/roles/meta/main.yml.md b/negative_test/roles/meta/main.yml.md
@@ -12,29 +12,13 @@
     "schemaPath": "#/properties/galaxy_tags/type"
   },
   {
-    "instancePath": "/galaxy_info",
-    "keyword": "additionalProperties",
-    "message": "must NOT have additional properties",
-    "params": {
-      "additionalProperty": "min_ansible_version"
-    },
-    "schemaPath": "#/additionalProperties"
-  },
-  {
-    "instancePath": "/galaxy_info",
-    "keyword": "additionalProperties",
-    "message": "must NOT have additional properties",
+    "instancePath": "",
+    "keyword": "if",
+    "message": "must match \"then\" schema",
     "params": {
-      "additionalProperty": "galaxy_tags"
+      "failingKeyword": "then"
     },
-    "schemaPath": "#/additionalProperties"
-  },
-  {
-    "instancePath": "",
-    "keyword": "anyOf",
-    "message": "must match a schema in anyOf",
-    "params": {},
-    "schemaPath": "#/anyOf"
+    "schemaPath": "#/if"
   }
 ]
 ```
@@ -49,23 +33,9 @@ stdout:
   "errors": [
     {
       "filename": "negative_test/roles/meta/main.yml",
-      "path": "$",
-      "message": "{'galaxy_info': {'description': 'bar', 'min_ansible_version': '2.9', 'company': 'foo', 'license': 'MIT', 'galaxy_tags': 'database', 'platforms': [{'name': 'Alpine', 'versions': ['all']}]}} is not valid under any of the given schemas",
-      "has_sub_errors": true,
-      "best_match": {
-        "path": "$.galaxy_info",
-        "message": "Additional properties are not allowed ('galaxy_tags', 'min_ansible_version' were unexpected)"
-      },
-      "sub_errors": [
-        {
-          "path": "$.galaxy_info.galaxy_tags",
-          "message": "'database' is not of type 'array'"
-        },
-        {
-          "path": "$.galaxy_info",
-          "message": "Additional properties are not allowed ('galaxy_tags', 'min_ansible_version' were unexpected)"
-        }
-      ]
+      "path": "$.galaxy_info.galaxy_tags",
+      "message": "'database' is not of type 'array'",
+      "has_sub_errors": false
     }
   ],
   "parse_errors": []

diff --git a/negative_test/roles/meta_invalid_collection/meta/main.yml b/negative_test/roles/meta_invalid_collection/meta/main.yml
@@ -1,9 +1,9 @@
+version: 2 # <-- role inside a collection
 collections:
   - foo # invalid pattern
 galaxy_info:
   description: foo
   license: bar
-  min_ansible_version: "2.10"
   platforms:
     - name: Fedora
       versions:

diff --git a/negative_test/roles/meta_invalid_collection/meta/main.yml.md b/negative_test/roles/meta_invalid_collection/meta/main.yml.md
@@ -12,29 +12,13 @@
     "schemaPath": "#/$defs/collections/items/pattern"
   },
   {
-    "instancePath": "/collections/0",
-    "keyword": "pattern",
-    "message": "must match pattern \"^[a-z_]+\\.[a-z_]+$\"",
-    "params": {
-      "pattern": "^[a-z_]+\\.[a-z_]+$"
-    },
-    "schemaPath": "#/$defs/collections/items/pattern"
-  },
-  {
-    "instancePath": "/galaxy_info",
-    "keyword": "additionalProperties",
-    "message": "must NOT have additional properties",
+    "instancePath": "",
+    "keyword": "if",
+    "message": "must match \"else\" schema",
     "params": {
-      "additionalProperty": "min_ansible_version"
+      "failingKeyword": "else"
     },
-    "schemaPath": "#/additionalProperties"
-  },
-  {
-    "instancePath": "",
-    "keyword": "anyOf",
-    "message": "must match a schema in anyOf",
-    "params": {},
-    "schemaPath": "#/anyOf"
+    "schemaPath": "#/if"
   }
 ]
 ```
@@ -49,27 +33,9 @@ stdout:
   "errors": [
     {
       "filename": "negative_test/roles/meta_invalid_collection/meta/main.yml",
-      "path": "$",
-      "message": "{'collections': ['foo'], 'galaxy_info': {'description': 'foo', 'license': 'bar', 'min_ansible_version': '2.10', 'platforms': [{'name': 'Fedora', 'versions': ['all']}]}} is not valid under any of the given schemas",
-      "has_sub_errors": true,
-      "best_match": {
-        "path": "$.galaxy_info",
-        "message": "Additional properties are not allowed ('min_ansible_version' was unexpected)"
-      },
-      "sub_errors": [
-        {
-          "path": "$.collections[0]",
-          "message": "'foo' does not match '^[a-z_]+\\\\.[a-z_]+$'"
-        },
-        {
-          "path": "$.collections[0]",
-          "message": "'foo' does not match '^[a-z_]+\\\\.[a-z_]+$'"
-        },
-        {
-          "path": "$.galaxy_info",
-          "message": "Additional properties are not allowed ('min_ansible_version' was unexpected)"
-        }
-      ]
+      "path": "$.collections[0]",
+      "message": "'foo' does not match '^[a-z_]+\\\\.[a-z_]+$'",
+      "has_sub_errors": false
     }
   ],
   "parse_errors": []

diff --git a/negative_test/roles/meta_invalid_collections/meta/main.yml b/negative_test/roles/meta_invalid_collections/meta/main.yml
@@ -1,9 +1,9 @@
+version: 2 # <-- role inside a collection
 collections:
   - FOO.BAR # invalid pattern, need to use lowercase
 galaxy_info:
   description: foo
   license: bar
-  min_ansible_version: "2.10"
   platforms:
     - name: Fedora
       versions:

diff --git a/negative_test/roles/meta_invalid_collections/meta/main.yml.md b/negative_test/roles/meta_invalid_collections/meta/main.yml.md
@@ -12,29 +12,13 @@
     "schemaPath": "#/$defs/collections/items/pattern"
   },
   {
-    "instancePath": "/collections/0",
-    "keyword": "pattern",
-    "message": "must match pattern \"^[a-z_]+\\.[a-z_]+$\"",
-    "params": {
-      "pattern": "^[a-z_]+\\.[a-z_]+$"
-    },
-    "schemaPath": "#/$defs/collections/items/pattern"
-  },
-  {
-    "instancePath": "/galaxy_info",
-    "keyword": "additionalProperties",
-    "message": "must NOT have additional properties",
+    "instancePath": "",
+    "keyword": "if",
+    "message": "must match \"else\" schema",
     "params": {
-      "additionalProperty": "min_ansible_version"
+      "failingKeyword": "else"
     },
-    "schemaPath": "#/additionalProperties"
-  },
-  {
-    "instancePath": "",
-    "keyword": "anyOf",
-    "message": "must match a schema in anyOf",
-    "params": {},
-    "schemaPath": "#/anyOf"
+    "schemaPath": "#/if"
   }
 ]
 ```
@@ -49,27 +33,9 @@ stdout:
   "errors": [
     {
       "filename": "negative_test/roles/meta_invalid_collections/meta/main.yml",
-      "path": "$",
-      "message": "{'collections': ['FOO.BAR'], 'galaxy_info': {'description': 'foo', 'license': 'bar', 'min_ansible_version': '2.10', 'platforms': [{'name': 'Fedora', 'versions': ['all']}]}} is not valid under any of the given schemas",
-      "has_sub_errors": true,
-      "best_match": {
-        "path": "$.galaxy_info",
-        "message": "Additional properties are not allowed ('min_ansible_version' was unexpected)"
-      },
-      "sub_errors": [
-        {
-          "path": "$.collections[0]",
-          "message": "'FOO.BAR' does not match '^[a-z_]+\\\\.[a-z_]+$'"
-        },
-        {
-          "path": "$.collections[0]",
-          "message": "'FOO.BAR' does not match '^[a-z_]+\\\\.[a-z_]+$'"
-        },
-        {
-          "path": "$.galaxy_info",
-          "message": "Additional properties are not allowed ('min_ansible_version' was unexpected)"
-        }
-      ]
+      "path": "$.collections[0]",
+      "message": "'FOO.BAR' does not match '^[a-z_]+\\\\.[a-z_]+$'",
+      "has_sub_errors": false
     }
   ],
   "parse_errors": []

diff --git a/negative_test/roles/meta_invalid_role_namespace/meta/main.yml b/negative_test/roles/meta_invalid_role_namespace/meta/main.yml
@@ -1,4 +1,5 @@
 ---
+version: 1 # <-- old standalone role
 galaxy_info:
   description: foo
   min_ansible_version: "2.9"