From 19eb58fb8f0a2c9234c25fc5c69e3b54723b912b Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Tue, 3 Oct 2023 13:00:20 +0100 Subject: [PATCH] Correctly recognize specific syntax on GitHub workflows Fixes: #3790 --- .config/dictionary.txt | 1 + .github/workflows/release.yml | 2 +- src/ansiblelint/config.py | 5 ++- src/ansiblelint/file_utils.py | 16 +++++---- src/ansiblelint/schemas/__store__.json | 2 +- src/ansiblelint/utils.py | 47 ++++++++++++++++++-------- src/ansiblelint/yaml_utils.py | 6 +++- 7 files changed, 53 insertions(+), 26 deletions(-) diff --git a/.config/dictionary.txt b/.config/dictionary.txt index fb30b67ac94..13c11f2116f 100644 --- a/.config/dictionary.txt +++ b/.config/dictionary.txt @@ -311,6 +311,7 @@ rulesdir rulesdirs ruleset runas +ruyaml sarif scalarint schemafile diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 444630e4d46..8d4e9c962eb 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -2,7 +2,7 @@ # cspell:ignore mislav name: release -"on": +on: release: types: [published] workflow_dispatch: diff --git a/src/ansiblelint/config.py b/src/ansiblelint/config.py index 39188574bdf..8586bf4ad51 100644 --- a/src/ansiblelint/config.py +++ b/src/ansiblelint/config.py @@ -37,7 +37,10 @@ # Do not sort this list, order matters. {"jinja2": "**/*.j2"}, # jinja2 templates are not always parsable as something else {"jinja2": "**/*.j2.*"}, - {"yaml": ".github/**/*.{yaml,yml}"}, # github workflows + { + "github-workflow": "**/.github/workflows/*.{yaml,yml}", + }, # github workflows (YAML 1.2) + {"yaml": "**/.github/**/*.{yaml,yml}"}, # github related files {"text": "**/templates/**/*.*"}, # templates are likely not validable {"execution-environment": "**/execution-environment.yml"}, {"ansible-lint-config": "**/.ansible-lint"}, diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index a39a7de8057..8e61bbfa148 100644 --- a/src/ansiblelint/file_utils.py +++ b/src/ansiblelint/file_utils.py @@ -275,18 +275,20 @@ def __del__(self) -> None: if hasattr(self, "file"): self.file.close() - def _guess_kind(self) -> None: + def _guess_kind(self, data: Any = None) -> None: + if data is None: + data = self.data if self.kind == "yaml": if ( - isinstance(self.data, list) - and len(self.data) > 0 + isinstance(data, list) + and len(data) > 0 and ( - "hosts" in self.data[0] - or "import_playbook" in self.data[0] - or "ansible.builtin.import_playbook" in self.data[0] + "hosts" in data[0] + or "import_playbook" in data[0] + or "ansible.builtin.import_playbook" in data[0] ) ): - if "rules" not in self.data[0]: + if "rules" not in data[0]: self.kind = "playbook" else: self.kind = "rulebook" diff --git a/src/ansiblelint/schemas/__store__.json b/src/ansiblelint/schemas/__store__.json index 7d7a01dd0ad..d2d0c845857 100644 --- a/src/ansiblelint/schemas/__store__.json +++ b/src/ansiblelint/schemas/__store__.json @@ -24,7 +24,7 @@ "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/inventory.json" }, "meta": { - "etag": "d1f66f20b45ecb814d9be900f01451d8035059b312de4a19edc64e29811a7f39", + "etag": "80d83d5c61044aa67496ea22c74aef08b0216c20e318400d3c939927a2761d51", "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/meta.json" }, "meta-runtime": { diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 85e91177e3b..99e57784878 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -33,6 +33,7 @@ from pathlib import Path from typing import Any +import ruamel.yaml.parser import yaml from ansible.errors import AnsibleError, AnsibleParserError from ansible.module_utils.parsing.convert_bool import boolean @@ -44,6 +45,7 @@ from ansible.plugins.loader import add_all_plugin_dirs from ansible.template import Templar from ansible.utils.collection_loader import AnsibleCollectionConfig +from ruamel.yaml.main import YAML from yaml.composer import Composer from yaml.representer import RepresenterError @@ -885,26 +887,41 @@ def construct_mapping( return mapping try: - kwargs = {} - if "vault_password" in inspect.getfullargspec(AnsibleLoader.__init__).args: - kwargs["vault_password"] = DEFAULT_VAULT_PASSWORD - loader = AnsibleLoader(lintable.content, **kwargs) - loader.compose_node = compose_node - loader.construct_mapping = construct_mapping - # while Ansible only accepts single documents, we also need to load - # multi-documents, as we attempt to load any YAML file, not only - # Ansible managed ones. - while True: - data = loader.get_data() - if data is None: - break - result.append(data) + # Using Ansible loader for loading YAML file that are not owned by Ansible is risky as + # that uses pyyaml which is unable to load YAML 1.2 documents correctly. + # https://github.com/yaml/pyyaml/issues/116 + if lintable.kind in ("yaml", "github-workflow"): + ruyaml = YAML() + ruyaml.allow_duplicate_keys = True # compatibility with pyyaml + result = list(ruyaml.load_all(lintable.content)) + # breakpoint() + # result.append(data) + lintable._guess_kind( # noqa: SLF001 + data=result, + ) # this might change file type from yaml to something else + if lintable.kind not in ("yaml", "github-workflow"): + result = [] # we need to reset result as we will be reloading the file + kwargs = {} + if "vault_password" in inspect.getfullargspec(AnsibleLoader.__init__).args: + kwargs["vault_password"] = DEFAULT_VAULT_PASSWORD + loader = AnsibleLoader(lintable.content, **kwargs) + loader.compose_node = compose_node + loader.construct_mapping = construct_mapping + # while Ansible only accepts single documents, we also need to load + # multi-documents, as we attempt to load any YAML file, not only + # Ansible managed ones. + while True: + data = loader.get_data() + if data is None: + break + result.append(data) except ( yaml.parser.ParserError, yaml.scanner.ScannerError, yaml.constructor.ConstructorError, + ruamel.yaml.parser.ParserError, ) as exc: - msg = "Failed to load YAML file" + msg = f"Failed to load YAML file: {lintable.path}" raise RuntimeError(msg) from exc if len(result) == 0: diff --git a/src/ansiblelint/yaml_utils.py b/src/ansiblelint/yaml_utils.py index 4d2490f4d90..d8d72c666b3 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -816,7 +816,7 @@ def __init__( - name: Task """ # Default to reading/dumping YAML 1.1 (ruamel.yaml defaults to 1.2) - self._yaml_version_default: tuple[int, int] = (1, 1) + self._yaml_version_default: tuple[int, int] = (1, 2) self._yaml_version: str | tuple[int, int] = self._yaml_version_default super().__init__(typ=typ, pure=pure, output=output, plug_ins=plug_ins) @@ -1073,6 +1073,10 @@ def _post_process_yaml(text: str) -> str: Make sure null list items don't end in a space. """ + # remove YAML directive + if text.startswith("%YAML"): + text = text.split("\n", 1)[1] + text = text.rstrip("\n") + "\n" lines = text.splitlines(keepends=True)