diff --git a/.config/dictionary.txt b/.config/dictionary.txt index fb30b67ac9..13c11f2116 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 444630e4d4..8d4e9c962e 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 39188574bd..4fb6632f3a 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"}, @@ -92,6 +95,7 @@ {"text/rst": "**/*.rst"}, # https://en.wikipedia.org/wiki/ReStructuredText {"text/ini": "**/*.ini"}, # YAML has no official IANA assignation + {"text/yaml1.2": "**/.github/workflows/*.{yaml,yml}"}, {"text/yaml": "**/{.ansible-lint,.yamllint}"}, {"text/yaml": "**/*.{yaml,yml}"}, {"text/yaml": "**/.*.{yaml,yml}"}, diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index a39a7de805..8e61bbfa14 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/rules/yaml_rule.py b/src/ansiblelint/rules/yaml_rule.py index 0784706da8..10b03e181f 100644 --- a/src/ansiblelint/rules/yaml_rule.py +++ b/src/ansiblelint/rules/yaml_rule.py @@ -188,7 +188,7 @@ def _fetch_skips(data: Any, collector: dict[int, set[str]]) -> dict[int, set[str ), pytest.param( "examples/yamllint/.github/workflows/ci.yml", - "yaml", + "github-workflow", [], id="rule-yaml-github-workflow", ), diff --git a/src/ansiblelint/schemas/__store__.json b/src/ansiblelint/schemas/__store__.json index 7d7a01dd0a..d2d0c84585 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/transformer.py b/src/ansiblelint/transformer.py index 5e7d828d2e..653372dec5 100644 --- a/src/ansiblelint/transformer.py +++ b/src/ansiblelint/transformer.py @@ -79,7 +79,7 @@ def run(self) -> None: for file, matches in self.matches_per_file.items(): # str() convinces mypy that "text/yaml" is a valid Literal. # Otherwise, it thinks base_kind is one of playbook, meta, tasks, ... - file_is_yaml = str(file.base_kind) == "text/yaml" + file_is_yaml = str(file.base_kind).startswith("text/yaml") try: data: str = file.content @@ -94,8 +94,11 @@ def run(self) -> None: # stores intermediate state during load which could affect loading # any other files. (Based on suggestion from ruamel.yaml author) yaml = FormattedYAML() - + if file.base_kind == "text/yaml1.2": + yaml._yaml_version_default = (1, 2) + yaml._yaml_version = (1, 2) ruamel_data = yaml.loads(data) + if not isinstance(ruamel_data, (CommentedMap, CommentedSeq)): # This is an empty vars file or similar which loads as None. # It is not safe to write this file or data-loss is likely. diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 85e91177e3..99e5778487 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 4d2490f4d9..4ebeb47646 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -968,6 +968,7 @@ def dumps(self, data: Any) -> str: """Dump YAML document to string (including its preamble_comment).""" preamble_comment: str | None = getattr(data, "preamble_comment", None) self._prevent_wrapping_flow_style(data) + self._yaml_version = (1, 2) with StringIO() as stream: if preamble_comment: stream.write(preamble_comment) @@ -1073,6 +1074,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)