Skip to content

Commit

Permalink
Avoid creating temporary YAML files inside source tree (#3819)
Browse files Browse the repository at this point in the history
Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>
  • Loading branch information
Qalthos and ssbarnea committed Oct 11, 2023
1 parent 9ec6f59 commit 4a36d80
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 47 deletions.
3 changes: 1 addition & 2 deletions src/ansiblelint/errors.py
Expand Up @@ -98,8 +98,7 @@ def __post_init__(self) -> None:
msg = "MatchError called incorrectly as column numbers start with 1"
raise RuntimeError(msg)

offset = getattr(self.lintable, "_line_offset", 0)
self.lineno += offset
self.lineno += self.lintable.line_offset

@functools.cached_property
def level(self) -> str:
Expand Down
38 changes: 3 additions & 35 deletions src/ansiblelint/file_utils.py
@@ -1,7 +1,6 @@
"""Utility functions related to file operations."""
from __future__ import annotations

import ast
import copy
import logging
import os
Expand All @@ -15,7 +14,6 @@
import pathspec
import wcmatch.pathlib
import wcmatch.wcmatch
from ansible.parsing.plugin_docs import read_docstring
from yaml.error import YAMLError

from ansiblelint.config import BASE_KINDS, Options, options
Expand Down Expand Up @@ -199,6 +197,9 @@ def __init__(
self.exc: Exception | None = None # Stores data loading exceptions
self.parent = parent
self.explicit = False # Indicates if the file was explicitly provided or was indirectly included.
self.line_offset = (
0 # Amount to offset line numbers by to get accurate position
)

if isinstance(name, str):
name = Path(name)
Expand Down Expand Up @@ -256,20 +257,6 @@ def __init__(
if self.kind == "yaml":
_ = self.data

if self.kind == "plugin":
# pylint: disable=consider-using-with
self.file = NamedTemporaryFile(
mode="w+",
suffix=f"_{name.name}.yaml",
dir=self.dir,
)
self.filename = self.file.name
self._content = self.parse_examples_from_plugin()
self.file.write(self._content)
self.file.flush()
self.path = Path(self.file.name)
self.base_kind = "text/yaml"

def __del__(self) -> None:
"""Clean up temporary files when the instance is cleaned up."""
if hasattr(self, "file"):
Expand Down Expand Up @@ -399,25 +386,6 @@ def __repr__(self) -> str:
"""Return user friendly representation of a lintable."""
return f"{self.name} ({self.kind})"

def parse_examples_from_plugin(self) -> str:
"""Parse yaml inside plugin EXAMPLES string.
Store a line number offset to realign returned line numbers later
"""
parsed = ast.parse(self.content)
for child in parsed.body:
if isinstance(child, ast.Assign):
label = child.targets[0]
if isinstance(label, ast.Name) and label.id == "EXAMPLES":
self._line_offset = child.lineno - 1
break

docs = read_docstring(str(self.path))
examples = docs["plainexamples"]
# Ignore the leading newline and lack of document start
# as including those in EXAMPLES would be weird.
return f"---{examples}" if examples else ""

@property
def data(self) -> Any:
"""Return loaded data representation for current file, if possible."""
Expand Down
30 changes: 30 additions & 0 deletions src/ansiblelint/runner.py
Expand Up @@ -15,6 +15,7 @@
from fnmatch import fnmatch
from functools import cache
from pathlib import Path
from tempfile import NamedTemporaryFile
from typing import TYPE_CHECKING, Any

from ansible.errors import AnsibleError
Expand All @@ -41,6 +42,7 @@
_include_children,
_roles_children,
_taskshandlers_children,
parse_examples_from_plugin,
template,
)

Expand Down Expand Up @@ -465,6 +467,8 @@ def find_children(self, lintable: Lintable) -> list[Lintable]:
add_all_plugin_dirs(playbook_dir or ".")
if lintable.kind == "role":
playbook_ds = AnsibleMapping({"roles": [{"role": str(lintable.path)}]})
elif lintable.kind == "plugin":
return self.plugin_children(lintable)
elif lintable.kind not in ("playbook", "tasks"):
return []
else:
Expand Down Expand Up @@ -543,6 +547,32 @@ def play_children(
return delegate_map[k](str(basedir), k, v, parent_type)
return []

def plugin_children(self, lintable: Lintable) -> list[Lintable]:
"""Collect lintable sections from plugin file."""
offset, content = parse_examples_from_plugin(lintable)
if not content:
# No examples, nothing to see here
return []
examples = Lintable(
name=lintable.name,
content=content,
kind="yaml",
base_kind="text/yaml",
parent=lintable,
)
examples.line_offset = offset

# pylint: disable=consider-using-with
examples.file = NamedTemporaryFile(
mode="w+",
suffix=f"_{lintable.path.name}.yaml",
)
examples.file.write(content)
examples.file.flush()
examples.filename = examples.file.name
examples.path = Path(examples.file.name)
return [examples]


@cache
def threads() -> int:
Expand Down
24 changes: 24 additions & 0 deletions src/ansiblelint/utils.py
Expand Up @@ -22,6 +22,7 @@
"""Generic utility helpers."""
from __future__ import annotations

import ast
import contextlib
import inspect
import logging
Expand All @@ -38,6 +39,7 @@
from ansible.module_utils.parsing.convert_bool import boolean
from ansible.parsing.dataloader import DataLoader
from ansible.parsing.mod_args import ModuleArgsParser
from ansible.parsing.plugin_docs import read_docstring
from ansible.parsing.yaml.constructor import AnsibleConstructor, AnsibleMapping
from ansible.parsing.yaml.loader import AnsibleLoader
from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleSequence
Expand Down Expand Up @@ -1021,3 +1023,25 @@ def _extend_with_roles(lintables: list[Lintable]) -> None:
def convert_to_boolean(value: Any) -> bool:
"""Use Ansible to convert something to a boolean."""
return bool(boolean(value))


def parse_examples_from_plugin(lintable: Lintable) -> tuple[int, str]:
"""Parse yaml inside plugin EXAMPLES string.
Store a line number offset to realign returned line numbers later
"""
offset = 1
parsed = ast.parse(lintable.content)
for child in parsed.body:
if isinstance(child, ast.Assign):
label = child.targets[0]
if isinstance(label, ast.Name) and label.id == "EXAMPLES":
offset = child.lineno - 1
break

docs = read_docstring(str(lintable.path))
examples = docs["plainexamples"]

# Ignore the leading newline and lack of document start
# as including those in EXAMPLES would be weird.
return offset, (f"---{examples}" if examples else "")
10 changes: 0 additions & 10 deletions test/test_file_utils.py
Expand Up @@ -541,13 +541,3 @@ def test_bug_2513(
results = Runner(filename, rules=default_rules_collection).run()
assert len(results) == 1
assert results[0].rule.id == "name"


def test_examples_content() -> None:
"""Test that a module loads the correct content."""
filename = Path("plugins/modules/fake_module.py")
lintable = Lintable(filename)
# Lintable is now correctly purporting to be a YAML file
assert lintable.base_kind == "text/yaml"
# Lintable content should be contents of EXAMPLES
assert lintable.content == "---" + BASIC_PLAYBOOK
17 changes: 17 additions & 0 deletions test/test_utils.py
Expand Up @@ -447,3 +447,20 @@ def test_task_in_list(file: str, names: list[str], positions: list[str]) -> None
for index, task in enumerate(tasks):
assert task.name == names[index]
assert task.position == positions[index]


def test_find_children_in_module(default_rules_collection: RulesCollection) -> None:
"""Verify correct function of find_children() in tasks."""
lintable = Lintable("plugins/modules/fake_module.py")
children = Runner(
rules=default_rules_collection,
).find_children(lintable)
assert len(children) == 1
child = children[0]

# Parent is a python file
assert lintable.base_kind == "text/python"

# Child correctly looks like a YAML file
assert child.base_kind == "text/yaml"
assert child.content.startswith("---")

0 comments on commit 4a36d80

Please sign in to comment.