diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 23251bae08..e943cd190a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -34,7 +34,7 @@ repos: - id: check-useless-excludes - repo: https://github.com/pre-commit/mirrors-prettier # keep it before yamllint - rev: v3.0.3 + rev: v3.1.0 hooks: - id: prettier # Temporary excludes so we can gradually normalize the formatting diff --git a/examples/playbooks/blockincludes.yml b/examples/playbooks/blockincludes.yml index b8387a8bdd..31317a7940 100644 --- a/examples/playbooks/blockincludes.yml +++ b/examples/playbooks/blockincludes.yml @@ -14,7 +14,7 @@ - name: Block level 3 block: - name: Include under block level 3 # noqa: deprecated-module - ansible.builtin.include: "{{ varset }}.yml" + ansible.builtin.include_tasks: "{{ varset }}.yml" - name: Block level 4 block: - name: INCLUDE under block level 4 diff --git a/examples/playbooks/removed-include.yml b/examples/playbooks/removed-include.yml new file mode 100644 index 0000000000..4f0ba58fd4 --- /dev/null +++ b/examples/playbooks/removed-include.yml @@ -0,0 +1,6 @@ +--- +- name: Invalid playbook + hosts: localhost + tasks: + - name: Foo + include: tasks/simple_task.yml # <-- include was removed in 2.16 diff --git a/src/ansiblelint/schemas/ansible.json b/src/ansiblelint/schemas/ansible.json index ae204254f9..efba283e9c 100644 --- a/src/ansiblelint/schemas/ansible.json +++ b/src/ansiblelint/schemas/ansible.json @@ -1,5 +1,10 @@ { "$defs": { + "removed-include-module": { + "markdownDescription": "See [include module](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/include_module.html)", + "not": {}, + "title": "Replace 'include' with either 'ansible.builtin.include_tasks' or 'ansible.builtin.import_tasks'" + }, "ansible.builtin.import_playbook": { "additionalProperties": false, "oneOf": [ @@ -831,6 +836,15 @@ "title": "Action", "type": "string" }, + "ansible.builtin.include": { + "$ref": "#/$defs/removed-include-module" + }, + "include": { + "$ref": "#/$defs/removed-include-module" + }, + "ansible.legacy.include": { + "$ref": "#/$defs/removed-include-module" + }, "any_errors_fatal": { "$ref": "#/$defs/templated-boolean", "title": "Any Errors Fatal" diff --git a/src/ansiblelint/schemas/main.py b/src/ansiblelint/schemas/main.py index ab146cbb87..c7467d12de 100644 --- a/src/ansiblelint/schemas/main.py +++ b/src/ansiblelint/schemas/main.py @@ -55,19 +55,29 @@ def validate_file_schema(file: Lintable) -> list[str]: return [] if error.context: error = find_best_deep_match(error) - message = f"{error.json_path} {error.message}" + # determine if we want to use our own messages embedded into schemas inside title/markdownDescription fields + if "not" in error.schema and len(error.schema["not"]) == 0: + message = error.schema["title"] + schema = error.schema + else: + message = f"{error.json_path} {error.message}" documentation_url = "" - for k in ("description", "markdownDescription"): - if k in schema: - # Find standalone URLs and also markdown urls. - match = re.search( - r"\[.*?\]\((https?://[^\s]+)\)|https?://[^\s]+", - schema[k], - ) - if match: - documentation_url = match[0] if match[0] else match[1] - break + for json_schema in (error.schema, schema): + for k in ("description", "markdownDescription"): + if k in json_schema: + # Find standalone URLs and also markdown urls. + match = re.search( + r"\[.*?\]\((?Phttps?://[^\s]+)\)|(?Phttps?://[^\s]+)", + json_schema[k], + ) + if match: + documentation_url = next( + x for x in match.groups() if x is not None + ) + break + if documentation_url: + break if documentation_url: if not message.endswith("."): message += "." @@ -86,7 +96,7 @@ def validate_file_schema(file: Lintable) -> list[str]: schema[k], ) if match: - documentation_url = match[0] if match[0] else match[1] + documentation_url = match.groups()[0] break if documentation_url: if not message.endswith("."): diff --git a/src/ansiblelint/schemas/playbook.json b/src/ansiblelint/schemas/playbook.json index 7ab56c2798..7ed7edd0ce 100644 --- a/src/ansiblelint/schemas/playbook.json +++ b/src/ansiblelint/schemas/playbook.json @@ -796,6 +796,11 @@ "title": "play-role", "type": "object" }, + "removed-include-module": { + "markdownDescription": "See [include module](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/include_module.html)", + "not": {}, + "title": "Replace 'include' with either 'ansible.builtin.include_tasks' or 'ansible.builtin.import_tasks'" + }, "tags": { "anyOf": [ { @@ -847,6 +852,12 @@ "title": "Action", "type": "string" }, + "ansible.builtin.include": { + "$ref": "#/$defs/removed-include-module" + }, + "ansible.legacy.include": { + "$ref": "#/$defs/removed-include-module" + }, "any_errors_fatal": { "$ref": "#/$defs/templated-boolean", "title": "Any Errors Fatal" @@ -932,6 +943,9 @@ "title": "Ignore Unreachable", "type": "boolean" }, + "include": { + "$ref": "#/$defs/removed-include-module" + }, "listen": { "anyOf": [ { diff --git a/src/ansiblelint/schemas/tasks.json b/src/ansiblelint/schemas/tasks.json index ec7f85dbc7..e0273fd50b 100644 --- a/src/ansiblelint/schemas/tasks.json +++ b/src/ansiblelint/schemas/tasks.json @@ -238,6 +238,11 @@ "markdownDescription": "Use for protecting sensitive data. See [no_log](https://docs.ansible.com/ansible/latest/reference_appendices/logging.html)", "title": "no_log" }, + "removed-include-module": { + "markdownDescription": "See [include module](https://docs.ansible.com/ansible/latest/collections/ansible/builtin/include_module.html)", + "not": {}, + "title": "Replace 'include' with either 'ansible.builtin.include_tasks' or 'ansible.builtin.import_tasks'" + }, "tags": { "anyOf": [ { @@ -289,6 +294,12 @@ "title": "Action", "type": "string" }, + "ansible.builtin.include": { + "$ref": "#/$defs/removed-include-module" + }, + "ansible.legacy.include": { + "$ref": "#/$defs/removed-include-module" + }, "any_errors_fatal": { "$ref": "#/$defs/templated-boolean", "title": "Any Errors Fatal" @@ -374,6 +385,9 @@ "title": "Ignore Unreachable", "type": "boolean" }, + "include": { + "$ref": "#/$defs/removed-include-module" + }, "listen": { "anyOf": [ { diff --git a/test/schemas/negative_test/playbooks/include.yml b/test/schemas/negative_test/playbooks/include.yml new file mode 100644 index 0000000000..5504e13386 --- /dev/null +++ b/test/schemas/negative_test/playbooks/include.yml @@ -0,0 +1,3 @@ +- hosts: localhost + tasks: + - include: foo.yml # <-- removed in Ansible 2.16 diff --git a/test/schemas/negative_test/playbooks/include.yml.md b/test/schemas/negative_test/playbooks/include.yml.md new file mode 100644 index 0000000000..c577d849f8 --- /dev/null +++ b/test/schemas/negative_test/playbooks/include.yml.md @@ -0,0 +1,142 @@ +# ajv errors + +```json +[ + { + "instancePath": "/0", + "keyword": "required", + "message": "must have required property 'ansible.builtin.import_playbook'", + "params": { + "missingProperty": "ansible.builtin.import_playbook" + }, + "schemaPath": "#/oneOf/0/required" + }, + { + "instancePath": "/0", + "keyword": "required", + "message": "must have required property 'import_playbook'", + "params": { + "missingProperty": "import_playbook" + }, + "schemaPath": "#/oneOf/1/required" + }, + { + "instancePath": "/0", + "keyword": "oneOf", + "message": "must match exactly one schema in oneOf", + "params": { + "passingSchemas": null + }, + "schemaPath": "#/oneOf" + }, + { + "instancePath": "/0", + "keyword": "additionalProperties", + "message": "must NOT have additional properties", + "params": { + "additionalProperty": "hosts" + }, + "schemaPath": "#/additionalProperties" + }, + { + "instancePath": "/0", + "keyword": "additionalProperties", + "message": "must NOT have additional properties", + "params": { + "additionalProperty": "tasks" + }, + "schemaPath": "#/additionalProperties" + }, + { + "instancePath": "/0/tasks/0", + "keyword": "required", + "message": "must have required property 'block'", + "params": { + "missingProperty": "block" + }, + "schemaPath": "#/required" + }, + { + "instancePath": "/0/tasks/0/include", + "keyword": "not", + "message": "must NOT be valid", + "params": {}, + "schemaPath": "#/$defs/removed-include-module/not" + }, + { + "instancePath": "/0/tasks/0", + "keyword": "anyOf", + "message": "must match a schema in anyOf", + "params": {}, + "schemaPath": "#/items/anyOf" + }, + { + "instancePath": "/0", + "keyword": "oneOf", + "message": "must match exactly one schema in oneOf", + "params": { + "passingSchemas": null + }, + "schemaPath": "#/items/oneOf" + } +] +``` + +# check-jsonschema + +stdout: + +```json +{ + "status": "fail", + "successes": [], + "errors": [ + { + "filename": "negative_test/playbooks/include.yml", + "path": "$[0]", + "message": "{'hosts': 'localhost', 'tasks': [{'include': 'foo.yml'}]} is not valid under any of the given schemas", + "has_sub_errors": true, + "best_match": { + "path": "$[0]", + "message": "'hosts', 'tasks' do not match any of the regexes: '^(ansible\\\\.builtin\\\\.)?import_playbook$', 'name', 'tags', 'vars', 'when'" + }, + "best_deep_match": { + "path": "$[0].tasks[0].include", + "message": "'foo.yml' should not be valid under {}" + }, + "num_sub_errors": 6, + "sub_errors": [ + { + "path": "$[0]", + "message": "'hosts', 'tasks' do not match any of the regexes: '^(ansible\\\\.builtin\\\\.)?import_playbook$', 'name', 'tags', 'vars', 'when'" + }, + { + "path": "$[0]", + "message": "{'hosts': 'localhost', 'tasks': [{'include': 'foo.yml'}]} is not valid under any of the given schemas" + }, + { + "path": "$[0]", + "message": "'ansible.builtin.import_playbook' is a required property" + }, + { + "path": "$[0]", + "message": "'import_playbook' is a required property" + }, + { + "path": "$[0].tasks[0]", + "message": "{'include': 'foo.yml'} is not valid under any of the given schemas" + }, + { + "path": "$[0].tasks[0]", + "message": "'block' is a required property" + }, + { + "path": "$[0].tasks[0].include", + "message": "'foo.yml' should not be valid under {}" + } + ] + } + ], + "parse_errors": [] +} +``` diff --git a/test/test_task_includes.py b/test/test_task_includes.py index 9f962132c1..80b5856d7e 100644 --- a/test/test_task_includes.py +++ b/test/test_task_includes.py @@ -1,5 +1,4 @@ """Tests related to task inclusions.""" -import sys import pytest @@ -14,9 +13,7 @@ pytest.param( "examples/playbooks/blockincludes.yml", 4, - 3 - if sys.version_info >= (3, 10, 0) - else 4, # 3 with py310/ansible2.16, or 4 with py39/ansible2.15, + 3, id="blockincludes", ), pytest.param(