Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds experimental stdin support #1355

Merged
merged 1 commit into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
import logging
import os
import subprocess
import sys
from argparse import Namespace
from collections import OrderedDict
from contextlib import contextmanager
from pathlib import Path
from tempfile import NamedTemporaryFile
from typing import TYPE_CHECKING, Any, Dict, Iterator, List, Optional, Set, Union

import wcmatch.pathlib
Expand Down Expand Up @@ -83,6 +85,9 @@ def kind_from_path(path: Path):
return k
if path.is_dir():
return "role"

if str(path) == '/dev/stdin':
return "playbook"
raise RuntimeError("Unable to determine file type for %s" % pathex)


Expand All @@ -100,6 +105,10 @@ def __init__(
kind: Optional[FileType] = None,
):
"""Create a Lintable instance."""
# Filename is effective file on disk, for stdin is a namedtempfile
self.filename: str = str(name)
self.dir: str = ""

if isinstance(name, str):
self.name = normpath(name)
self.path = Path(self.name)
Expand All @@ -118,12 +127,24 @@ def __init__(
if role.exists:
self.role = role.name

self.kind = kind or kind_from_path(self.path)
# We store absolute directory in dir
if self.kind == "role":
self.dir = str(self.path.resolve())
if str(self.path) in ['/dev/stdin', '-']:
self.file = NamedTemporaryFile(mode="w+", suffix="playbook.yml")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be kept in-memory instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope because I do need to feed ansible-playbook --syntax-check a file. The rest of the linter code could work fine with in-memory files, but not ansible itself.

self.filename = self.file.name
self._content = sys.stdin.read()
self.file.write(self._content)
self.file.flush()
self.path = Path(self.file.name)
self.name = 'stdin'
self.kind = 'playbook'
self.dir = '/'
else:
self.dir = str(self.path.parent.resolve())
self.kind = kind or kind_from_path(self.path)
# We store absolute directory in dir
if not self.dir:
if self.kind == "role":
self.dir = str(self.path.resolve())
else:
self.dir = str(self.path.parent.resolve())

def __getitem__(self, item):
"""Provide compatibility subscriptable support."""
Expand Down
10 changes: 4 additions & 6 deletions src/ansiblelint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ def matchtasks(self, file: Lintable) -> List[MatchError]:
if not self.matchtask or file.kind == 'meta':
return matches

yaml = ansiblelint.utils.parse_yaml_linenumbers(file.content, file.path)
yaml = ansiblelint.utils.parse_yaml_linenumbers(file)
if not yaml:
return matches

yaml = append_skipped_rules(yaml, file.content, file.kind)
yaml = append_skipped_rules(yaml, file)

try:
tasks = ansiblelint.utils.get_normalized_tasks(yaml, file)
Expand Down Expand Up @@ -138,7 +138,7 @@ def matchyaml(self, file: Lintable) -> List[MatchError]:
if not self.matchplay:
return matches

yaml = ansiblelint.utils.parse_yaml_linenumbers(file.content, file.path)
yaml = ansiblelint.utils.parse_yaml_linenumbers(file)
# yaml returned can be an AnsibleUnicode (a string) when the yaml
# file contains a single string. YAML spec allows this but we consider
# this an fatal error.
Expand All @@ -150,9 +150,7 @@ def matchyaml(self, file: Lintable) -> List[MatchError]:
if isinstance(yaml, dict):
yaml = [yaml]

yaml = ansiblelint.skip_utils.append_skipped_rules(
yaml, file.content, file.kind
)
yaml = ansiblelint.skip_utils.append_skipped_rules(yaml, file)

for play in yaml:

Expand Down
18 changes: 13 additions & 5 deletions src/ansiblelint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class LintResult:
"""Class that tracks result of linting."""

matches: List[MatchError]
files: Set[str]
files: Set[Lintable]


class Runner:
Expand All @@ -41,7 +41,7 @@ def __init__(
skip_list: List[str] = [],
exclude_paths: List[str] = [],
verbosity: int = 0,
checked_files: Optional[Set[str]] = None
checked_files: Optional[Set[Lintable]] = None
) -> None:
"""Initialize a Runner instance."""
self.rules = rules
Expand Down Expand Up @@ -127,7 +127,7 @@ def worker(lintable: Lintable) -> List[MatchError]:
files = [value for n, value in enumerate(files) if value not in files[:n]]

for file in self.lintables:
if str(file.path) in self.checked_files:
if file in self.checked_files:
continue
_logger.debug(
"Examining %s of type %s",
Expand All @@ -140,7 +140,7 @@ def worker(lintable: Lintable) -> List[MatchError]:
)

# update list of checked files
self.checked_files.update([str(x.path) for x in self.lintables])
self.checked_files.update(self.lintables)

# remove any matches made inside excluded files
matches = list(
Expand Down Expand Up @@ -174,7 +174,7 @@ def _get_matches(rules: "RulesCollection", options: "Namespace") -> LintResult:
lintables = ansiblelint.utils.get_lintables(options=options, args=options.lintables)

matches = list()
checked_files: Set[str] = set()
checked_files: Set[Lintable] = set()
runner = Runner(
*lintables,
rules=rules,
Expand All @@ -189,4 +189,12 @@ def _get_matches(rules: "RulesCollection", options: "Namespace") -> LintResult:
# Assure we do not print duplicates and the order is consistent
matches = sorted(set(matches))

# Convert reported filenames into human redable ones, so we hide the
# fact we used temporary files when processing input from stdin.
for match in matches:
for lintable in lintables:
Comment on lines +194 to +195
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using an itertools.product() here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really as I do also have the break.

if match.filename == lintable.filename:
match.filename = lintable.name
break

return LintResult(matches=matches, files=checked_files)
29 changes: 16 additions & 13 deletions src/ansiblelint/skip_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,17 @@
import logging
from functools import lru_cache
from itertools import product
from typing import Any, Generator, List, Optional, Sequence
from typing import TYPE_CHECKING, Any, Generator, List, Sequence

import ruamel.yaml

from ansiblelint.config import used_old_tags
from ansiblelint.constants import RENAMED_TAGS, FileType
from ansiblelint.constants import RENAMED_TAGS
from ansiblelint.file_utils import Lintable

if TYPE_CHECKING:
from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject


_logger = logging.getLogger(__name__)

Expand All @@ -46,7 +51,7 @@ def get_rule_skips_from_line(line: str) -> List:


def append_skipped_rules(
pyyaml_data: str, file_text: str, file_type: Optional[FileType] = None
pyyaml_data: "AnsibleBaseYAMLObject", lintable: Lintable
) -> Sequence:
"""Append 'skipped_rules' to individual tasks or single metadata block.

Expand All @@ -61,7 +66,7 @@ def append_skipped_rules(
to individual tasks, or added to the single metadata block.
"""
try:
yaml_skip = _append_skipped_rules(pyyaml_data, file_text, file_type)
yaml_skip = _append_skipped_rules(pyyaml_data, lintable)
except RuntimeError:
# Notify user of skip error, do not stop, do not change exit code
_logger.error('Error trying to append skipped rules', exc_info=True)
Expand All @@ -82,21 +87,19 @@ def load_data(file_text: str) -> Any:
return yaml.load(file_text)


def _append_skipped_rules(
pyyaml_data: Sequence[Any], file_text: str, file_type: Optional[FileType] = None
) -> Sequence:
def _append_skipped_rules(pyyaml_data: Sequence[Any], lintable: Lintable) -> Sequence:
# parse file text using 2nd parser library
ruamel_data = load_data(file_text)
ruamel_data = load_data(lintable.content)

if file_type == 'meta':
if lintable.kind == 'meta':
pyyaml_data[0]['skipped_rules'] = _get_rule_skips_from_yaml(ruamel_data)
return pyyaml_data

# create list of blocks of tasks or nested tasks
if file_type in ('tasks', 'handlers'):
if lintable.kind in ('tasks', 'handlers'):
ruamel_task_blocks = ruamel_data
pyyaml_task_blocks = pyyaml_data
elif file_type in ('playbook', 'pre_tasks', 'post_tasks'):
elif lintable.kind in ('playbook', 'pre_tasks', 'post_tasks'):
try:
pyyaml_task_blocks = _get_task_blocks_from_playbook(pyyaml_data)
ruamel_task_blocks = _get_task_blocks_from_playbook(ruamel_data)
Expand All @@ -105,10 +108,10 @@ def _append_skipped_rules(
# assume it is a playbook, check needs to be added higher in the
# call stack, and can remove this except
return pyyaml_data
elif file_type in ['yaml', 'requirements', 'vars']:
elif lintable.kind in ['yaml', 'requirements', 'vars']:
return pyyaml_data
else:
raise RuntimeError('Unexpected file type: {}'.format(file_type))
raise RuntimeError('Unexpected file type: {}'.format(lintable.kind))

# get tasks from blocks of tasks
pyyaml_tasks = _get_tasks_from_blocks(pyyaml_task_blocks)
Expand Down
10 changes: 5 additions & 5 deletions src/ansiblelint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
from ansible.parsing.splitter import split_args
from ansible.parsing.yaml.constructor import AnsibleConstructor
from ansible.parsing.yaml.loader import AnsibleLoader
from ansible.parsing.yaml.objects import AnsibleSequence
from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleSequence
from ansible.plugins.loader import add_all_plugin_dirs
from ansible.template import Templar

Expand Down Expand Up @@ -704,7 +704,7 @@ def get_normalized_tasks(yaml, file: Lintable) -> List[Dict[str, Any]]:


@lru_cache(maxsize=128)
def parse_yaml_linenumbers(data, filename):
def parse_yaml_linenumbers(lintable: Lintable) -> AnsibleBaseYAMLObject:
"""Parse yaml as ansible.utils.parse_yaml but with linenumbers.

The line numbers are stored in each node's LINE_NUMBER_KEY key.
Expand All @@ -723,19 +723,19 @@ def construct_mapping(node, deep=False):
mapping[LINE_NUMBER_KEY] = node.__line__
else:
mapping[LINE_NUMBER_KEY] = mapping._line_number
mapping[FILENAME_KEY] = filename
mapping[FILENAME_KEY] = lintable.path
return mapping

try:
kwargs = {}
if 'vault_password' in inspect.getfullargspec(AnsibleLoader.__init__).args:
kwargs['vault_password'] = DEFAULT_VAULT_PASSWORD
loader = AnsibleLoader(data, **kwargs)
loader = AnsibleLoader(lintable.content, **kwargs)
loader.compose_node = compose_node
loader.construct_mapping = construct_mapping
data = loader.get_single_data()
except (yaml.parser.ParserError, yaml.scanner.ScannerError) as e:
raise SystemExit("Failed to parse YAML in %s: %s" % (filename, str(e)))
raise SystemExit("Failed to parse YAML in %s: %s" % (lintable.path, str(e)))
return data


Expand Down
2 changes: 1 addition & 1 deletion test/TestRunner.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def test_runner_with_directory(default_rules_collection, directory_name) -> None


def test_files_not_scanned_twice(default_rules_collection) -> None:
checked_files: Set[str] = set()
checked_files: Set[Lintable] = set()

filename = os.path.abspath('examples/playbooks/common-include-1.yml')
runner = Runner(
Expand Down