Skip to content

Commit

Permalink
Correctly recognize specific syntax on GitHub workflows
Browse files Browse the repository at this point in the history
Fixes: #3790
  • Loading branch information
ssbarnea committed Oct 3, 2023
1 parent 487fdfb commit 31fd04e
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 28 deletions.
1 change: 1 addition & 0 deletions .config/dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ rulesdir
rulesdirs
ruleset
runas
ruyaml
sarif
scalarint
schemafile
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# cspell:ignore mislav
name: release

"on":
on:
release:
types: [published]
workflow_dispatch:
Expand Down
6 changes: 5 additions & 1 deletion src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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}"},
Expand Down
16 changes: 9 additions & 7 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/yaml_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
),
Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/schemas/__store__.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
7 changes: 5 additions & 2 deletions src/ansiblelint/transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
47 changes: 32 additions & 15 deletions src/ansiblelint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions src/ansiblelint/yaml_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 31fd04e

Please sign in to comment.