meta: refactor schema versioning to improve errors

ssbarnea · ssbarnea · commit a2f119441f9b · 2022-09-14T10:53:31.000+01:00
This greatly improves the quality of the validating error messages. Related: python-jsonschema/jsonschema#1002
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"
diff --git a/negative_test/roles/meta_invalid_role_namespace/meta/main.yml.md b/negative_test/roles/meta_invalid_role_namespace/meta/main.yml.md
diff --git a/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml b/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml
diff --git a/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml.md b/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml.md
diff --git a/test/roles/empty-meta/meta/main.yml.md b/test/roles/empty-meta/meta/main.yml.md

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+version: 1`
`1`	`2`	`galaxy_info:`
`2`	`3`	`description: bar`
`3`	`4`	`min_ansible_version: "2.9"`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`---`
	`2`	`+version: 1 # <-- old standalone role`
`2`	`3`	`galaxy_info:`
`3`	`4`	`description: foo`
`4`	`5`	`min_ansible_version: "2.9"`