diff --git a/docs/collection_metadata.rst b/docs/collection_metadata.rst index 3732ba1d..9865b245 100644 --- a/docs/collection_metadata.rst +++ b/docs/collection_metadata.rst @@ -35,7 +35,7 @@ If the ``meta/execution-environment.yml`` file is not present, by default, Ansib Dependency introspection ======================== -If any dependencies are given, the introspection is run by Ansible Builder so that the requirements are found and sanitized (deduped) before container image assembly. +If any dependencies are given, the introspection is run by Ansible Builder so that the requirements are found before container image assembly. A user can see the introspection output during the builder intermediate phase using the ``build -v3`` option. @@ -63,13 +63,11 @@ Run the ``introspect`` command against your collection path: :: - ansible-builder introspect --sanitize COLLECTION_PATH + ansible-builder introspect COLLECTION_PATH The default collection path used by the ``ansible-galaxy`` command is ``~/.ansible/collections/``. Read more about collection paths in the `Ansible configuration settings `_ guide. -The ``--sanitize`` option reviews all of the collection requirements and removes duplicates. It also removes any Python requirements that should normally be excluded (see :ref:`python_deps` below). - .. note:: Use the ``-v3`` option to ``introspect`` to see logging messages about requirements that are being excluded. @@ -94,20 +92,28 @@ Then, if the ``ansible_collection`` directory is in your home directory, you can :: - ansible-builder introspect --sanitize ~/ + ansible-builder introspect ~/ .. _python_deps: Python Dependencies ^^^^^^^^^^^^^^^^^^^ -Ansible Builder combines all the Python requirements files from all collections into a single file using the ``requirements-parser`` library. This library supports complex syntax, including references to other files. +Ansible Builder combines all the Python requirements files from all collections into a single file. + +Certain package names are specifically *ignored* by ``ansible-builder``, meaning that Ansible Builder +does not include them in the combined file of Python dependencies, even if a collection lists them as +dependencies. These include test packages and packages that provide Ansible itself. The full list can +be found in ``EXCLUDE_REQUIREMENTS`` in ``src/ansible_builder/_target_scripts/introspect.py``. -If multiple collections require the same *package name*, Ansible Builder combines them into a single entry and combines the constraints. +If you need to include one of these ignored package names, use the ``--user-pip`` option of the +``introspect`` command to list it in the user requirements file. Packages supplied this way are +not processed against the list of excluded Python packages. -Certain package names are specifically *ignored* by ``ansible-builder``, meaning that Ansible Builder does not include them in the combined file of Python dependencies, even if a collection lists them as dependencies. These include test packages and packages that provide Ansible itself. The full list can be found in ``EXCLUDE_REQUIREMENTS`` in ``src/ansible_builder/_target_scripts/introspect.py``. +.. note:: -If you need to include one of these ignored package names, use the ``--user-pip`` option of the ``introspect`` command to list it in the user requirements file. Packages supplied this way are not processed against the list of excluded Python packages. + These dependencies are subject to the same `PEP 508 `_ format + restrictions described for Python requirements in the :ref:`EE definition specification `. System-level Dependencies ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/docs/definition.rst b/docs/definition.rst index 22576854..caa7c6ea 100644 --- a/docs/definition.rst +++ b/docs/definition.rst @@ -50,6 +50,11 @@ Here is a sample version 3 EE file. To use Ansible Builder 3.x, you must specify - six - psutil system: bindep.txt + exclude: + python: + - docker + system: + - python3-Cython images: base_image: @@ -232,10 +237,20 @@ The following keys are valid for this section: ``requirements.yml`` file (see below for examples). Read more about the requirements file format in the `Galaxy user guide `_. + .. _python-pep508: + ``python`` The Python installation requirements. This may either be a filename, or a list of requirements (see below for an example). + .. note:: + + Python requirement specifications are expected to be limited to features defined by + `PEP 508 `_. Hash tag comments will always be allowed. + Any deviation from this specification will be passed through to pip unverified and unaltered, + although this is considered undefined and unsupported behavior. It is not recommended that + you depend on this behavior. + ``python_interpreter`` A dictionary that defines the Python system package name to be installed by ``dnf`` (``package_system``) and/or a path to the Python interpreter to be used @@ -245,6 +260,57 @@ The following keys are valid for this section: The system packages to be installed, in bindep format. This may either be a filename, or a list of requirements (see below for an example). + ``exclude`` + A dictionary defining the Python or system requirements to be excluded from the top-level dependency + requirements of referenced collections. These exclusions will not apply to the user supplied Python or + system dependencies, nor will they apply to dependencies of dependencies (top-level only). + + The following keys are valid for this section: + + * ``python`` - A list of Python dependencies to be excluded. + * ``system`` - A list of system dependencies to be excluded. + * ``all_from_collections`` - If you want to exclude *all* Python and system dependencies from one or + more collections, supply a list of collection names under this key. + + The exclusion feature supports two forms of matching: + + * Simple name matching. + * Advanced name matching using regular expressions. + + For simple name matching, you need only supply the name of the requirement/collection to match. + All values will be compared in a case-insensitive manner. + + For advanced name matching, begin the exclusion string with the tilde (``~``) character to + indicate that the remaining portion of the string is a regular expression to be used to match + a requirement/collection name. The regex should be considered case-insensitive. + + .. note:: + The regular expression must match the full requirement/collection name. For example, ``~foo.`` + does not fully match the name ``foobar``, but ``~foo.+`` does. + + With both forms of matching, the exclusion string will be compared against the *simple* name of + any Python or system requirement. For example, if you need to exclude the system requirement that + appears as ``foo [!platform:gentoo]`` within an included collection, then your exclusion string should be + ``foo``. To exclude the Python requirement ``bar == 1.0.0``, your exclusion string would be ``bar``. + + Example using both simple and advanced matching: + + .. code:: yaml + + dependencies: + exclude: + python: + - docker + system: + - python3-Cython + all_from_collections: + # Regular expression to exclude all from community collections + - ~community\..+ + + .. note:: + The ``exclude`` option requires ``ansible-builder`` version ``3.1`` or newer. + + The following example uses filenames that contain various dependencies: .. code:: yaml diff --git a/setup.cfg b/setup.cfg index e513eeb7..c0ebe410 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,10 +36,9 @@ max-line-length=160 include_package_data = true install_requires = PyYAML - requirements_parser bindep jsonschema - setuptools; python_version >= "3.12" + packaging python_requires = >=3.9 diff --git a/src/ansible_builder/_target_scripts/introspect.py b/src/ansible_builder/_target_scripts/introspect.py index bbd9c95e..43c97825 100644 --- a/src/ansible_builder/_target_scripts/introspect.py +++ b/src/ansible_builder/_target_scripts/introspect.py @@ -1,17 +1,94 @@ +from __future__ import annotations + import argparse -import importlib.metadata import logging import os +import re import sys import yaml -import requirements +from packaging.requirements import InvalidRequirement, Requirement + + +BASE_COLLECTIONS_PATH = '/usr/share/ansible/collections' + + +# regex for a comment at the start of a line, or embedded with leading space(s) +COMMENT_RE = re.compile(r'(?:^|\s+)#.*$') + + +EXCLUDE_REQUIREMENTS = frozenset(( + # obviously already satisfied or unwanted + 'ansible', 'ansible-base', 'python', 'ansible-core', + # general python test requirements + 'tox', 'pycodestyle', 'yamllint', 'pylint', + 'flake8', 'pytest', 'pytest-xdist', 'coverage', 'mock', 'testinfra', + # test requirements highly specific to Ansible testing + 'ansible-lint', 'molecule', 'galaxy-importer', 'voluptuous', + # already present in image for py3 environments + 'yaml', 'pyyaml', 'json', +)) -base_collections_path = '/usr/share/ansible/collections' logger = logging.getLogger(__name__) +class CollectionDefinition: + """ + This class represents the dependency metadata for a collection + should be replaced by logic to hit the Galaxy API if made available + """ + + def __init__(self, collection_path): + self.reference_path = collection_path + + # NOTE: Filenames should match constants.DEAFULT_EE_BASENAME and constants.YAML_FILENAME_EXTENSIONS. + meta_file_base = os.path.join(collection_path, 'meta', 'execution-environment') + ee_exists = False + for ext in ('yml', 'yaml'): + meta_file = f"{meta_file_base}.{ext}" + if os.path.exists(meta_file): + with open(meta_file, 'r') as f: + self.raw = yaml.safe_load(f) + ee_exists = True + break + + if not ee_exists: + self.raw = {'version': 1, 'dependencies': {}} + # Automatically infer requirements for collection + for entry, filename in [('python', 'requirements.txt'), ('system', 'bindep.txt')]: + candidate_file = os.path.join(collection_path, filename) + if has_content(candidate_file): + self.raw['dependencies'][entry] = filename + + def target_dir(self): + namespace, name = self.namespace_name() + return os.path.join( + BASE_COLLECTIONS_PATH, 'ansible_collections', + namespace, name + ) + + def namespace_name(self): + "Returns 2-tuple of namespace and name" + path_parts = [p for p in self.reference_path.split(os.path.sep) if p] + return tuple(path_parts[-2:]) + + def get_dependency(self, entry): + """A collection is only allowed to reference a file by a relative path + which is relative to the collection root + """ + req_file = self.raw.get('dependencies', {}).get(entry) + if req_file is None: + return None + if os.path.isabs(req_file): + raise RuntimeError( + 'Collections must specify relative paths for requirements files. ' + f'The file {req_file} specified by {self.reference_path} violates this.' + ) + + return req_file + + def line_is_empty(line): return bool((not line.strip()) or line.startswith('#')) @@ -60,14 +137,14 @@ def process_collection(path): :param str path: root directory of collection (this would contain galaxy.yml file) """ - CD = CollectionDefinition(path) + col_def = CollectionDefinition(path) - py_file = CD.get_dependency('python') + py_file = col_def.get_dependency('python') pip_lines = [] if py_file: pip_lines = pip_file_data(os.path.join(path, py_file)) - sys_file = CD.get_dependency('system') + sys_file = col_def.get_dependency('system') bindep_lines = [] if sys_file: bindep_lines = bindep_file_data(os.path.join(path, sys_file)) @@ -75,7 +152,36 @@ def process_collection(path): return (pip_lines, bindep_lines) -def process(data_dir=base_collections_path, user_pip=None, user_bindep=None): +def process(data_dir=BASE_COLLECTIONS_PATH, + user_pip=None, + user_bindep=None, + exclude_pip=None, + exclude_bindep=None, + exclude_collections=None): + """ + Build a dictionary of Python and system requirements from any collections + installed in data_dir, and any user specified requirements. + + Excluded requirements, if any, will be inserted into the return dict. + + Example return dict: + { + 'python': { + 'collection.a': ['abc', 'def'], + 'collection.b': ['ghi'], + 'user': ['jkl'], + 'exclude: ['abc'], + }, + 'system': { + 'collection.a': ['ZYX'], + 'user': ['WVU'], + 'exclude': ['ZYX'], + }, + 'excluded_collections': [ + 'a.b', + ] + } + """ paths = [] path_root = os.path.join(data_dir, 'ansible_collections') @@ -97,8 +203,8 @@ def process(data_dir=base_collections_path, user_pip=None, user_bindep=None): sys_req = {} for path in paths: col_pip_lines, col_sys_lines = process_collection(path) - CD = CollectionDefinition(path) - namespace, name = CD.namespace_name() + col_def = CollectionDefinition(path) + namespace, name = col_def.namespace_name() key = f'{namespace}.{name}' if col_pip_lines: @@ -112,16 +218,33 @@ def process(data_dir=base_collections_path, user_pip=None, user_bindep=None): col_pip_lines = pip_file_data(user_pip) if col_pip_lines: py_req['user'] = col_pip_lines + if exclude_pip: + col_pip_exclude_lines = pip_file_data(exclude_pip) + if col_pip_exclude_lines: + py_req['exclude'] = col_pip_exclude_lines if user_bindep: col_sys_lines = bindep_file_data(user_bindep) if col_sys_lines: sys_req['user'] = col_sys_lines + if exclude_bindep: + col_sys_exclude_lines = bindep_file_data(exclude_bindep) + if col_sys_exclude_lines: + sys_req['exclude'] = col_sys_exclude_lines - return { + retval = { 'python': py_req, - 'system': sys_req + 'system': sys_req, } + if exclude_collections: + # This file should just be a newline separated list of collection names, + # so reusing bindep_file_data() to read it should work fine. + excluded_collection_list = bindep_file_data(exclude_collections) + if excluded_collection_list: + retval['excluded_collections'] = excluded_collection_list + + return retval + def has_content(candidate_file): """Beyond checking that the candidate exists, this also assures @@ -135,83 +258,114 @@ def has_content(candidate_file): return bool(content.strip().strip('\n')) -class CollectionDefinition: - """This class represents the dependency metadata for a collection - should be replaced by logic to hit the Galaxy API if made available +def strip_comments(reqs: dict[str, list]) -> dict[str, list]: """ + Filter any comments out of the Python collection requirements input. - def __init__(self, collection_path): - self.reference_path = collection_path + :param dict reqs: A dict of Python requirements, keyed by collection name. - # NOTE: Filenames should match constants.DEAFULT_EE_BASENAME and constants.YAML_FILENAME_EXTENSIONS. - meta_file_base = os.path.join(collection_path, 'meta', 'execution-environment') - ee_exists = False - for ext in ('yml', 'yaml'): - meta_file = f"{meta_file_base}.{ext}" - if os.path.exists(meta_file): - with open(meta_file, 'r') as f: - self.raw = yaml.safe_load(f) - ee_exists = True - break + :return: Same as the input parameter, except with no comment lines. + """ + result: dict[str, list] = {} + for collection, lines in reqs.items(): + for line in lines: + # strip comments + if (base_line := COMMENT_RE.sub('', line.strip())): + result.setdefault(collection, []).append(base_line) - if not ee_exists: - self.raw = {'version': 1, 'dependencies': {}} - # Automatically infer requirements for collection - for entry, filename in [('python', 'requirements.txt'), ('system', 'bindep.txt')]: - candidate_file = os.path.join(collection_path, filename) - if has_content(candidate_file): - self.raw['dependencies'][entry] = filename + return result - def target_dir(self): - namespace, name = self.namespace_name() - return os.path.join( - base_collections_path, 'ansible_collections', - namespace, name - ) - def namespace_name(self): - "Returns 2-tuple of namespace and name" - path_parts = [p for p in self.reference_path.split(os.path.sep) if p] - return tuple(path_parts[-2:]) +def should_be_excluded(value: str, exclusion_list: list[str]) -> bool: + """ + Test if `value` matches against any value in `exclusion_list`. - def get_dependency(self, entry): - """A collection is only allowed to reference a file by a relative path - which is relative to the collection root - """ - req_file = self.raw.get('dependencies', {}).get(entry) - if req_file is None: - return None - if os.path.isabs(req_file): - raise RuntimeError( - 'Collections must specify relative paths for requirements files. ' - f'The file {req_file} specified by {self.reference_path} violates this.' - ) + The exclusion_list values are either strings to be compared in a case-insensitive + manner against value, OR, they are regular expressions to be tested against the + value. A regular expression will contain '~' as the first character. - return req_file + :return: True if the value should be excluded, False otherwise. + """ + for exclude_value in exclusion_list: + if exclude_value[0] == "~": + pattern = exclude_value[1:] + if re.fullmatch(pattern.lower(), value.lower()): + return True + elif exclude_value.lower() == value.lower(): + return True + return False + + +def filter_requirements(reqs: dict[str, list], + exclude: list[str] | None = None, + exclude_collections: list[str] | None = None, + is_python: bool = True) -> list[str]: + """ + Given a dictionary of Python requirement lines keyed off collections, + return a list of cleaned up (no source comments) requirements + annotated with comments indicating the sources based off the collection keys. + + Currently, non-pep508 compliant Python entries are passed through. We also no + longer attempt to normalize names (replace '_' with '-', etc), other than + lowercasing it for exclusion matching, since we no longer are attempting + to combine similar entries. + + :param dict reqs: A dict of either Python or system requirements, keyed by collection name. + :param list exclude: A list of requirements to be excluded from the output. + :param list exclude_collections: A list of collection names from which to exclude all requirements. + :param bool is_python: This should be set to True for Python requirements, as each + will be tested for PEP508 compliance. This should be set to False for system requirements. + + :return: A list of filtered and annotated requirements. + """ + exclusions: list[str] = [] + collection_ignore_list: list[str] = [] + if exclude: + exclusions = exclude.copy() + if exclude_collections: + collection_ignore_list = exclude_collections.copy() -def simple_combine(reqs): - """Given a dictionary of requirement lines keyed off collections, - return a list with the most basic of de-duplication logic, - and comments indicating the sources based off the collection keys - """ - consolidated = [] - fancy_lines = [] - for collection, lines in reqs.items(): - for line in lines: - if line_is_empty(line): - continue + annotated_lines: list[str] = [] + uncommented_reqs = strip_comments(reqs) + + for collection, lines in uncommented_reqs.items(): + # Bypass this collection if we've been told to ignore all requirements from it. + if should_be_excluded(collection, collection_ignore_list): + logger.debug("# Excluding all requirements from collection '%s'", collection) + continue - base_line = line.split('#')[0].strip() - if base_line in consolidated: - i = consolidated.index(base_line) - fancy_lines[i] += f', {collection}' + for line in lines: + # Determine the simple name based on type of requirement + if is_python: + try: + parsed_req = Requirement(line) + name = parsed_req.name + except InvalidRequirement: + logger.warning( + "Passing through non-PEP508 compliant line '%s' from collection '%s'", + line, collection + ) + annotated_lines.append(line) # We intentionally won't annotate these lines (multi-line?) + continue else: - fancy_line = f'{base_line} # from collection {collection}' - consolidated.append(base_line) - fancy_lines.append(fancy_line) + # bindep system requirements have the package name as the first "word" on the line + name = line.split(maxsplit=1)[0] + + if collection.lower() not in {'user', 'exclude'}: + lower_name = name.lower() - return fancy_lines + if lower_name in EXCLUDE_REQUIREMENTS: + logger.debug("# Excluding requirement '%s' from '%s'", name, collection) + continue + + if should_be_excluded(lower_name, exclusions): + logger.debug("# Explicitly excluding requirement '%s' from '%s'", name, collection) + continue + + annotated_lines.append(f'{line} # from collection {collection}') + + return annotated_lines def parse_args(args=None): @@ -235,25 +389,36 @@ def parse_args(args=None): def run_introspect(args, log): - data = process(args.folder, user_pip=args.user_pip, user_bindep=args.user_bindep) - if args.sanitize: - log.info('# Sanitized dependencies for %s', args.folder) - data_for_write = data - data['python'] = sanitize_requirements(data['python']) - data['system'] = simple_combine(data['system']) - else: - log.info('# Dependency data for %s', args.folder) - data_for_write = data.copy() - data_for_write['python'] = simple_combine(data['python']) - data_for_write['system'] = simple_combine(data['system']) + data = process(args.folder, + user_pip=args.user_pip, + user_bindep=args.user_bindep, + exclude_pip=args.exclude_pip, + exclude_bindep=args.exclude_bindep, + exclude_collections=args.exclude_collections) + log.info('# Dependency data for %s', args.folder) + + excluded_collections = data.pop('excluded_collections', None) + + data['python'] = filter_requirements( + data['python'], + exclude=data['python'].pop('exclude', []), + exclude_collections=excluded_collections, + ) + + data['system'] = filter_requirements( + data['system'], + exclude=data['system'].pop('exclude', []), + exclude_collections=excluded_collections, + is_python=False + ) print('---') print(yaml.dump(data, default_flow_style=False)) if args.write_pip and data.get('python'): - write_file(args.write_pip, data_for_write.get('python') + ['']) + write_file(args.write_pip, data.get('python') + ['']) if args.write_bindep and data.get('system'): - write_file(args.write_bindep, data_for_write.get('system') + ['']) + write_file(args.write_bindep, data.get('system') + ['']) sys.exit(0) @@ -269,20 +434,16 @@ def create_introspect_parser(parser): ) ) introspect_parser.add_argument('--sanitize', action='store_true', - help=('Sanitize and de-duplicate requirements. ' - 'This is normally done separately from the introspect script, but this ' - 'option is given to more accurately test collection content.')) + help=argparse.SUPPRESS) introspect_parser.add_argument( - 'folder', default=base_collections_path, nargs='?', + 'folder', default=BASE_COLLECTIONS_PATH, nargs='?', help=( 'Ansible collections path(s) to introspect. ' 'This should have a folder named ansible_collections inside of it.' ) ) - # Combine user requirements and collection requirements into single file - # in the future, could look into passing multilple files to - # python-builder scripts to be fed multiple files as opposed to this + introspect_parser.add_argument( '--user-pip', dest='user_pip', help='An additional file to combine with collection pip requirements.' @@ -291,6 +452,18 @@ def create_introspect_parser(parser): '--user-bindep', dest='user_bindep', help='An additional file to combine with collection bindep requirements.' ) + introspect_parser.add_argument( + '--exclude-bindep-reqs', dest='exclude_bindep', + help='An additional file to exclude specific bindep requirements from collections.' + ) + introspect_parser.add_argument( + '--exclude-pip-reqs', dest='exclude_pip', + help='An additional file to exclude specific pip requirements from collections.' + ) + introspect_parser.add_argument( + '--exclude-collection-reqs', dest='exclude_collections', + help='An additional file to exclude all requirements from the listed collections.' + ) introspect_parser.add_argument( '--write-pip', dest='write_pip', help='Write the combined pip requirements file to this location.' @@ -303,78 +476,6 @@ def create_introspect_parser(parser): return introspect_parser -EXCLUDE_REQUIREMENTS = frozenset(( - # obviously already satisfied or unwanted - 'ansible', 'ansible-base', 'python', 'ansible-core', - # general python test requirements - 'tox', 'pycodestyle', 'yamllint', 'pylint', - 'flake8', 'pytest', 'pytest-xdist', 'coverage', 'mock', 'testinfra', - # test requirements highly specific to Ansible testing - 'ansible-lint', 'molecule', 'galaxy-importer', 'voluptuous', - # already present in image for py3 environments - 'yaml', 'pyyaml', 'json', -)) - - -def sanitize_requirements(collection_py_reqs): - """ - Cleanup Python requirements by removing duplicates and excluded packages. - - The user requirements file will go through the deduplication process, but - skips the special package exclusion process. - - :param dict collection_py_reqs: A dict of lists of Python requirements, keyed - by fully qualified collection name. The special key `user` holds requirements - from the user specified requirements file from the ``--user-pip`` CLI option. - - :returns: A finalized list of sanitized Python requirements. - """ - # de-duplication - consolidated = [] - seen_pkgs = set() - - for collection, lines in collection_py_reqs.items(): - try: - for req in requirements.parse('\n'.join(lines)): - if req.specifier: - req.name = importlib.metadata.Prepared(req.name).normalized - req.collections = [collection] # add backref for later - if req.name is None: - consolidated.append(req) - continue - if req.name in seen_pkgs: - for prior_req in consolidated: - if req.name == prior_req.name: - prior_req.specs.extend(req.specs) - prior_req.collections.append(collection) - break - continue - consolidated.append(req) - seen_pkgs.add(req.name) - except Exception as e: - logger.warning('Warning: failed to parse requirements from %s, error: %s', collection, e) - - # removal of unwanted packages - sanitized = [] - for req in consolidated: - # Exclude packages, unless it was present in the user supplied requirements. - if req.name and req.name.lower() in EXCLUDE_REQUIREMENTS and 'user' not in req.collections: - logger.debug('# Excluding requirement %s from %s', req.name, req.collections) - continue - if req.vcs or req.uri: - # Requirement like git+ or http return as-is - new_line = req.line - elif req.name: - specs = [f'{cmp}{ver}' for cmp, ver in req.specs] - new_line = req.name + ','.join(specs) - else: - raise RuntimeError(f'Could not process {req.line}') - - sanitized.append(f'{new_line} # from collection {",".join(req.collections)}') - - return sanitized - - def write_file(filename: str, lines: list) -> bool: parent_dir = os.path.dirname(filename) if parent_dir and not os.path.exists(parent_dir): diff --git a/src/ansible_builder/constants.py b/src/ansible_builder/constants.py index da37ab3c..87088b32 100644 --- a/src/ansible_builder/constants.py +++ b/src/ansible_builder/constants.py @@ -32,16 +32,16 @@ default_keyring_name = 'keyring.gpg' default_policy_file_name = 'policy.json' +EXCL_COLLECTIONS_FILENAME = 'exclude-collections.txt' +STD_BINDEP_FILENAME = 'bindep.txt' +STD_GALAXY_FILENAME = 'requirements.yml' +STD_PIP_FILENAME = 'requirements.txt' + # Files that need to be moved into the build context, and their naming inside the context CONTEXT_FILES = { - # HACK: hacking in prototype other kinds of deps for dynamic builder - 'python_interpreter': '', - 'ansible_core': '', - 'ansible_runner': '', - - 'galaxy': 'requirements.yml', - 'python': 'requirements.txt', - 'system': 'bindep.txt', + 'galaxy': STD_GALAXY_FILENAME, + 'python': STD_PIP_FILENAME, + 'system': STD_BINDEP_FILENAME, } FINAL_IMAGE_BIN_PATH = "/opt/builder/bin" diff --git a/src/ansible_builder/containerfile.py b/src/ansible_builder/containerfile.py index d9092bfd..c0c70513 100644 --- a/src/ansible_builder/containerfile.py +++ b/src/ansible_builder/containerfile.py @@ -3,6 +3,7 @@ import importlib.resources import logging import os +import tempfile from pathlib import Path @@ -144,7 +145,7 @@ def prepare(self) -> None: self._insert_global_args() if image == "base": - self.steps.append("RUN $PYCMD -m pip install --no-cache-dir bindep pyyaml requirements-parser") + self.steps.append("RUN $PYCMD -m pip install --no-cache-dir bindep pyyaml packaging") else: # For an EE schema earlier than v3 with a custom builder image, we always make sure pip is available. context_dir = Path(self.build_outputs_dir).stem @@ -254,21 +255,36 @@ def _create_folder_copy_files(self) -> None: scripts_dir = str(Path(self.build_outputs_dir) / 'scripts') os.makedirs(scripts_dir, exist_ok=True) + # For the python, system, and galaxy requirements, get a file path to the contents and copy + # it into the context directory with an expected name to later be used during the container builds. + # The get_dep_abs_path() handles parsing the various requirements and any exclusions, if any. for item, new_name in constants.CONTEXT_FILES.items(): - # HACK: new dynamic base/builder - if not new_name: - continue - - requirement_path = self.definition.get_dep_abs_path(item) - if requirement_path is None: - continue - dest = os.path.join( - self.build_context, constants.user_content_subfolder, new_name) - - # Ignore modification time of the requirement file because we could - # be writing it out dynamically (inline EE reqs), and we only care - # about the contents anyway. - copy_file(requirement_path, dest, ignore_mtime=True) + for exclude in (False, True): + if exclude is True: + new_name = f'exclude-{new_name}' + requirement_path = self.definition.get_dep_abs_path(item, exclude=exclude) + if requirement_path is None: + continue + dest = os.path.join( + self.build_context, constants.user_content_subfolder, new_name) + + # Ignore modification time of the requirement file because we could + # be writing it out dynamically (inline EE reqs), and we only care + # about the contents anyway. + copy_file(requirement_path, dest, ignore_mtime=True) + + # We need to handle dependencies.exclude.all_from_collections independently since + # it doesn't follow the same model as the other dependency requirements. + exclude_deps = self.definition.dependencies.get('exclude') + if exclude_deps and 'all_from_collections' in exclude_deps: + collection_ignore_list = exclude_deps['all_from_collections'] + dest = os.path.join(self.build_context, + constants.user_content_subfolder, + constants.EXCL_COLLECTIONS_FILENAME) + with tempfile.NamedTemporaryFile('w') as fp: + fp.write('\n'.join(collection_ignore_list)) + fp.flush() + copy_file(fp.name, dest, ignore_mtime=True) if self.original_galaxy_keyring: copy_file( @@ -384,7 +400,12 @@ def _prepare_label_steps(self) -> None: ]) def _prepare_build_context(self) -> None: - if any(self.definition.get_dep_abs_path(thing) for thing in ('galaxy', 'system', 'python')): + deps: list[str] = [] + for exclude in (False, True): + deps.extend( + self.definition.get_dep_abs_path(thing, exclude=exclude) for thing in ('galaxy', 'system', 'python') + ) + if any(deps): self.steps.extend([ f"COPY {constants.user_content_subfolder} /build", "WORKDIR /build", @@ -393,7 +414,7 @@ def _prepare_build_context(self) -> None: def _prepare_galaxy_install_steps(self) -> None: env = "" - install_opts = (f"-r {constants.CONTEXT_FILES['galaxy']} " + install_opts = (f"-r {constants.STD_GALAXY_FILENAME} " f"--collections-path \"{constants.base_collections_path}\"") if self.galaxy_ignore_signature_status_codes: @@ -415,35 +436,54 @@ def _prepare_galaxy_install_steps(self) -> None: self.steps.append( f"RUN ansible-galaxy role install $ANSIBLE_GALAXY_CLI_ROLE_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']}" + f"-r {constants.STD_GALAXY_FILENAME}" f" --roles-path \"{constants.base_roles_path}\"", ) step = f"RUN {env}ansible-galaxy collection install $ANSIBLE_GALAXY_CLI_COLLECTION_OPTS {install_opts}" self.steps.append(step) + def _add_copy_for_file(self, filename: str) -> bool: + """ + If the given file exists within the context build directory, add a COPY command to the + instruction file steps. + + :param str filename: The base requirement filename to check. + + :return: True if file exists and COPY command was added, False otherwise. + """ + file_exists = os.path.exists(os.path.join(self.build_outputs_dir, filename)) + if file_exists: + relative_path = os.path.join(constants.user_content_subfolder, filename) + # WORKDIR is /build, so we use the (shorter) relative paths there + self.steps.append(f"COPY {relative_path} {filename}") + return True + return False + def _prepare_introspect_assemble_steps(self) -> None: # The introspect/assemble block is valid if there are any form of requirements - if any(self.definition.get_dep_abs_path(thing) for thing in ('galaxy', 'system', 'python')): - - introspect_cmd = "RUN $PYCMD /output/scripts/introspect.py introspect --sanitize" - - requirements_file_exists = os.path.exists(os.path.join( - self.build_outputs_dir, constants.CONTEXT_FILES['python'] - )) - - if requirements_file_exists: - relative_requirements_path = os.path.join( - constants.user_content_subfolder, - constants.CONTEXT_FILES['python'] - ) - self.steps.append(f"COPY {relative_requirements_path} {constants.CONTEXT_FILES['python']}") - # WORKDIR is /build, so we use the (shorter) relative paths there - introspect_cmd += f" --user-pip={constants.CONTEXT_FILES['python']}" - bindep_exists = os.path.exists(os.path.join(self.build_outputs_dir, constants.CONTEXT_FILES['system'])) - if bindep_exists: - relative_bindep_path = os.path.join(constants.user_content_subfolder, constants.CONTEXT_FILES['system']) - self.steps.append(f"COPY {relative_bindep_path} {constants.CONTEXT_FILES['system']}") - introspect_cmd += f" --user-bindep={constants.CONTEXT_FILES['system']}" + deps: list[str] = [] + for exclude in (False, True): + deps.extend( + self.definition.get_dep_abs_path(thing, exclude=exclude) for thing in ('galaxy', 'system', 'python') + ) + + if any(deps): + introspect_cmd = "RUN $PYCMD /output/scripts/introspect.py introspect" + + for option, exc_option, req_file in ( + ('--user-pip', '--exclude-pip-reqs', constants.STD_PIP_FILENAME), + ('--user-bindep', '--exclude-bindep-reqs', constants.STD_BINDEP_FILENAME) + ): + if self._add_copy_for_file(req_file): + introspect_cmd += f" {option}={req_file}" + + exclude_req_file = f"exclude-{req_file}" + + if self._add_copy_for_file(exclude_req_file): + introspect_cmd += f" {exc_option}={exclude_req_file}" + + if self._add_copy_for_file(constants.EXCL_COLLECTIONS_FILENAME): + introspect_cmd += f" --exclude-collection-reqs={constants.EXCL_COLLECTIONS_FILENAME}" introspect_cmd += " --write-bindep=/tmp/src/bindep.txt --write-pip=/tmp/src/requirements.txt" diff --git a/src/ansible_builder/ee_schema.py b/src/ansible_builder/ee_schema.py index 9cdda75b..dcb71439 100644 --- a/src/ansible_builder/ee_schema.py +++ b/src/ansible_builder/ee_schema.py @@ -272,6 +272,33 @@ {"required": ["package_pip"]}, ], }, + "exclude": { + "type": "object", + "additionalProperties": False, + "properties": { + "python": { + "description": "A list of python package names", + "type": "array", + "items": { + "type": "string" + } + }, + "system": { + "description": "A list of system package names", + "type": "array", + "items": { + "type": "string" + } + }, + "all_from_collections": { + "description": "A list of collection names", + "type": "array", + "items": { + "type": "string" + } + }, + }, + }, }, }, diff --git a/src/ansible_builder/user_definition.py b/src/ansible_builder/user_definition.py index 399283d7..eb78b521 100644 --- a/src/ansible_builder/user_definition.py +++ b/src/ansible_builder/user_definition.py @@ -1,5 +1,6 @@ from __future__ import annotations +import functools import logging import os import textwrap @@ -179,12 +180,22 @@ def container_init(self): def options(self): return self.raw.get('options', {}) - def get_dep_abs_path(self, entry): + @property + def dependencies(self): + return self.raw.get('dependencies', {}) + + # This is the size of galaxy, python, system * exclude=True/False + @functools.lru_cache(maxsize=6) + def get_dep_abs_path(self, entry, exclude=False): """Unique to the user EE definition, files can be referenced by either an absolute path or a path relative to the EE definition folder This method will return the absolute path. """ - req_file = self.raw.get('dependencies', {}).get(entry) + deps = self.dependencies + if exclude: + deps = deps.get('exclude', {}) + + req_file = deps.get(entry) if not req_file: return None @@ -236,14 +247,12 @@ def validate(self): """ validate_schema(self.raw) - for item, value in constants.CONTEXT_FILES.items(): - # HACK: non-file deps for dynamic base/builder - if not value: - continue - requirement_path = self.get_dep_abs_path(item) - if requirement_path: - if not os.path.exists(requirement_path): - raise DefinitionError(f"Dependency file {requirement_path} does not exist.") + for item in constants.CONTEXT_FILES: + for exclude in (False, True): + requirement_path = self.get_dep_abs_path(item, exclude=exclude) + if requirement_path: + if not os.path.exists(requirement_path): + raise DefinitionError(f"Dependency file {requirement_path} does not exist.") # Validate and set any user-specified build arguments build_arg_defaults = self.raw.get('build_arg_defaults') diff --git a/test/data/v3/complete/ee.yml b/test/data/v3/complete/ee.yml index a0bf3ee6..534002d5 100644 --- a/test/data/v3/complete/ee.yml +++ b/test/data/v3/complete/ee.yml @@ -32,6 +32,13 @@ dependencies: system: - python311 - mysql + exclude: + python: + - foo + system: + - bar + all_from_collections: + - a.b additional_build_files: - src: files/random.cfg diff --git a/test/integration/test_create.py b/test/integration/test_create.py index 155ce974..7e1f7dbf 100644 --- a/test/integration/test_create.py +++ b/test/integration/test_create.py @@ -542,3 +542,59 @@ def test_v3_additional_build_files(cli, build_dir_and_ee_yml): cfg.touch() cli(f'ansible-builder create -c {tmpdir} -f {eeyml} --output-filename Containerfile') + + +def test_exclude_files_created(cli, build_dir_and_ee_yml): + """ + Test that the bindep and Python requirement exclude files are created. + """ + tmpdir, eeyml = build_dir_and_ee_yml( + """ + version: 3 + images: + base_image: + name: quay.io/ansible/awx-ee:latest + dependencies: + galaxy: + collections: + - name: community.crypto + exclude: + python: + - pyyaml + system: + - openssh-client + all_from_collections: + - a.b + """ + ) + + cli(f'ansible-builder create -c {tmpdir} -f {eeyml} --output-filename Containerfile') + + # Validate that the bindep exclude file is created with proper content + bindep_exclude = tmpdir / constants.user_content_subfolder / f"exclude-{constants.STD_BINDEP_FILENAME}" + assert bindep_exclude.exists() + text = bindep_exclude.read_text() + assert text == "openssh-client" + + # Validate that the Python requirements exclude file is created with proper content + pyreq_exclude = tmpdir / constants.user_content_subfolder / f"exclude-{constants.STD_PIP_FILENAME}" + assert pyreq_exclude.exists() + text = pyreq_exclude.read_text() + assert text == "pyyaml" + + # Validate that the collections exclude file is created with proper content + coll_exclude = tmpdir / constants.user_content_subfolder / f"{constants.EXCL_COLLECTIONS_FILENAME}" + assert coll_exclude.exists() + text = coll_exclude.read_text() + assert text == "a.b" + + # Validate that the EE will use the exclude files + containerfile = tmpdir / "Containerfile" + assert containerfile.exists() + text = containerfile.read_text() + expected_introspect_command = "RUN $PYCMD /output/scripts/introspect.py introspect" \ + f" --exclude-pip-reqs=exclude-{constants.STD_PIP_FILENAME}" \ + f" --exclude-bindep-reqs=exclude-{constants.STD_BINDEP_FILENAME}" \ + f" --exclude-collection-reqs={constants.EXCL_COLLECTIONS_FILENAME}" \ + " --write-bindep=/tmp/src/bindep.txt --write-pip=/tmp/src/requirements.txt\n" + assert expected_introspect_command in text diff --git a/test/integration/test_help.py b/test/integration/test_help.py index 73c349d0..1748d437 100644 --- a/test/integration/test_help.py +++ b/test/integration/test_help.py @@ -38,5 +38,5 @@ def test_create_help(cli): def test_introspect_help(cli): result = cli('ansible-builder introspect --help', check=False) help_text = result.stdout - assert 'usage: ansible-builder introspect [-h] [--sanitize]' in help_text + assert 'usage: ansible-builder introspect [-h]' in help_text assert re.search(r'Loops over collections in folder', help_text) diff --git a/test/integration/test_introspect_cli.py b/test/integration/test_introspect_cli.py index 72449600..18716e89 100644 --- a/test/integration/test_introspect_cli.py +++ b/test/integration/test_introspect_cli.py @@ -6,15 +6,7 @@ def test_introspect_write(cli, data_dir): data = yaml.safe_load(r.stdout) # assure that output is valid YAML assert 'python' in data assert 'system' in data - assert 'pytz' in data['python']['test.reqfile'] - - -def test_introspect_with_sanitize(cli, data_dir): - r = cli(f'ansible-builder introspect --sanitize {data_dir}') - data = yaml.safe_load(r.stdout) # assure that output is valid YAML - assert 'python' in data - assert 'system' in data - assert '# from collection test.bindep' in r.stdout # should have comments + assert 'pytz # from collection test.reqfile' in r.stdout def test_introspect_write_bindep(cli, data_dir, tmp_path): @@ -43,31 +35,54 @@ def test_introspect_write_python(cli, data_dir, tmp_path): ]) -def test_introspect_write_python_and_sanitize(cli, data_dir, tmp_path): - dest_file = tmp_path / 'req.txt' - cli(f'ansible-builder introspect {data_dir} --write-pip={dest_file} --sanitize') - - assert dest_file.read_text() == '\n'.join([ - 'pyvcloud>=14,>=18.0.10 # from collection test.metadata,test.reqfile', - 'pytz # from collection test.reqfile', - 'python_dateutil>=2.8.2 # from collection test.reqfile', - 'jinja2>=3.0 # from collection test.reqfile', - 'tacacs_plus # from collection test.reqfile', - '', - ]) - - -def test_introspect_with_sanitize_user_reqs(cli, data_dir, tmp_path): +def test_introspect_with_user_reqs(cli, data_dir, tmp_path): user_file = tmp_path / 'requirements.txt' user_file.write_text("ansible\npytest\n") - r = cli(f'ansible-builder introspect --sanitize --user-pip={user_file} {data_dir}') + r = cli(f'ansible-builder introspect --user-pip={user_file} {data_dir}') data = yaml.safe_load(r.stdout) # assure that output is valid YAML assert 'python' in data assert 'system' in data - assert 'pytz # from collection test.reqfile' in data['python'] - + assert 'pytz # from collection test.reqfile' in r.stdout # 'ansible' allowed in user requirements - assert 'ansible # from collection user' in data['python'] + assert 'ansible # from collection user' in r.stdout # 'pytest' allowed in user requirements - assert 'pytest # from collection user' in data['python'] + assert 'pytest # from collection user' in r.stdout + + +def test_introspect_exclude_python(cli, data_dir, tmp_path): + exclude_file = tmp_path / 'exclude.txt' + exclude_file.write_text("pytz\npython-dateutil\n") + + r = cli(f'ansible-builder introspect {data_dir} --exclude-pip-reqs={exclude_file}') + data = yaml.safe_load(r.stdout) + + assert 'python' in data + assert 'system' in data + assert 'pytz' not in r.stdout + assert 'python-dateutil' not in r.stdout + + +def test_introspect_exclude_system(cli, data_dir, tmp_path): + exclude_file = tmp_path / 'exclude.txt' + exclude_file.write_text("subversion\n") + + r = cli(f'ansible-builder introspect {data_dir} --exclude-bindep-reqs={exclude_file}') + data = yaml.safe_load(r.stdout) + + assert 'python' in data + assert 'system' in data + assert 'subversion' not in r.stdout + + +def test_introspect_exclude_collections(cli, data_dir, tmp_path): + exclude_file = tmp_path / 'exclude.txt' + exclude_file.write_text("test.reqfile\ntest.bindep\n") + + r = cli(f'ansible-builder introspect {data_dir} --exclude-collection-reqs={exclude_file}') + data = yaml.safe_load(r.stdout) + + assert 'python' in data + assert 'system' in data + assert 'from collection test.reqfile' not in r.stdout + assert 'from collection test.bindep' not in r.stdout diff --git a/test/unit/test_containerfile.py b/test/unit/test_containerfile.py index d988b4e8..5cf37cbe 100644 --- a/test/unit/test_containerfile.py +++ b/test/unit/test_containerfile.py @@ -51,9 +51,9 @@ def test_prepare_galaxy_install_steps(build_dir_and_ee_yml): c._prepare_galaxy_install_steps() expected = [ f"RUN ansible-galaxy role install $ANSIBLE_GALAXY_CLI_ROLE_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']} --roles-path \"{constants.base_roles_path}\"", + f"-r {constants.STD_GALAXY_FILENAME} --roles-path \"{constants.base_roles_path}\"", f"RUN ANSIBLE_GALAXY_DISABLE_GPG_VERIFY=1 ansible-galaxy collection install " - f"$ANSIBLE_GALAXY_CLI_COLLECTION_OPTS -r {constants.CONTEXT_FILES['galaxy']} " + f"$ANSIBLE_GALAXY_CLI_COLLECTION_OPTS -r {constants.STD_GALAXY_FILENAME} " f"--collections-path \"{constants.base_collections_path}\"" ] assert c.steps == expected @@ -69,9 +69,9 @@ def test_prepare_galaxy_install_steps_with_keyring(build_dir_and_ee_yml): c._prepare_galaxy_install_steps() expected = [ f"RUN ansible-galaxy role install $ANSIBLE_GALAXY_CLI_ROLE_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']} --roles-path \"{constants.base_roles_path}\"", + f"-r {constants.STD_GALAXY_FILENAME} --roles-path \"{constants.base_roles_path}\"", f"RUN ansible-galaxy collection install $ANSIBLE_GALAXY_CLI_COLLECTION_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']} " + f"-r {constants.STD_GALAXY_FILENAME} " f"--collections-path \"{constants.base_collections_path}\" --keyring \"{constants.default_keyring_name}\"" ] assert c.steps == expected @@ -90,9 +90,9 @@ def test_prepare_galaxy_install_steps_with_sigcount(build_dir_and_ee_yml): c._prepare_galaxy_install_steps() expected = [ f"RUN ansible-galaxy role install $ANSIBLE_GALAXY_CLI_ROLE_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']} --roles-path \"{constants.base_roles_path}\"", + f"-r {constants.STD_GALAXY_FILENAME} --roles-path \"{constants.base_roles_path}\"", f"RUN ansible-galaxy collection install $ANSIBLE_GALAXY_CLI_COLLECTION_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']} " + f"-r {constants.STD_GALAXY_FILENAME} " f"--collections-path \"{constants.base_collections_path}\" " f"--required-valid-signature-count {sig_count} --keyring \"{constants.default_keyring_name}\"" ] @@ -112,9 +112,9 @@ def test_prepare_galaxy_install_steps_with_ignore_code(build_dir_and_ee_yml): c._prepare_galaxy_install_steps() expected = [ f"RUN ansible-galaxy role install $ANSIBLE_GALAXY_CLI_ROLE_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']} --roles-path \"{constants.base_roles_path}\"", + f"-r {constants.STD_GALAXY_FILENAME} --roles-path \"{constants.base_roles_path}\"", f"RUN ansible-galaxy collection install $ANSIBLE_GALAXY_CLI_COLLECTION_OPTS " - f"-r {constants.CONTEXT_FILES['galaxy']} " + f"-r {constants.STD_GALAXY_FILENAME} " f"--collections-path \"{constants.base_collections_path}\" " f"--ignore-signature-status-code {codes[0]} --ignore-signature-status-code {codes[1]} " f"--keyring \"{constants.default_keyring_name}\"" @@ -329,3 +329,45 @@ def test_v2_builder_image_default(build_dir_and_ee_yml): c.prepare() assert "FROM base as builder" in c.steps assert "COPY _build/scripts/pip_install /output/scripts/pip_install" not in c.steps + + +def test_prepare_introspect_assemble_steps(build_dir_and_ee_yml): + """ + Test that the introspect command is built as expected. + """ + + ee_data = """ + version: 3 + images: + base_image: + name: quay.io/user/mycustombaseimage:latest + dependencies: + python: + - six + system: + - git + galaxy: + collections: + - name: community.windows + exclude: + python: + - aaa + system: + - bbb + all_from_collections: + - a.b + """ + + tmpdir, ee_path = build_dir_and_ee_yml(ee_data) + c = make_containerfile(tmpdir, ee_path, run_validate=True) + c._create_folder_copy_files() + c._prepare_introspect_assemble_steps() + + expected_introspect_command = "RUN $PYCMD /output/scripts/introspect.py introspect" \ + f" --user-pip={constants.STD_PIP_FILENAME}" \ + f" --exclude-pip-reqs=exclude-{constants.STD_PIP_FILENAME}" \ + f" --user-bindep={constants.STD_BINDEP_FILENAME}" \ + f" --exclude-bindep-reqs=exclude-{constants.STD_BINDEP_FILENAME}" \ + f" --exclude-collection-reqs={constants.EXCL_COLLECTIONS_FILENAME}" \ + " --write-bindep=/tmp/src/bindep.txt --write-pip=/tmp/src/requirements.txt" + assert expected_introspect_command in c.steps diff --git a/test/unit/test_introspect.py b/test/unit/test_introspect.py index 36ea823e..3e57cc63 100644 --- a/test/unit/test_introspect.py +++ b/test/unit/test_introspect.py @@ -1,33 +1,72 @@ import os import pytest -from ansible_builder._target_scripts.introspect import process, process_collection -from ansible_builder._target_scripts.introspect import simple_combine, sanitize_requirements -from ansible_builder._target_scripts.introspect import parse_args +from ansible_builder._target_scripts.introspect import (parse_args, + process, + process_collection, + filter_requirements, + strip_comments) def test_multiple_collection_metadata(data_dir): - files = process(data_dir) - files['python'] = sanitize_requirements(files['python']) - files['system'] = simple_combine(files['system']) + files['python'] = filter_requirements(files['python']) + files['system'] = filter_requirements(files['system'], is_python=False) assert files == {'python': [ - 'pyvcloud>=14,>=18.0.10 # from collection test.metadata,test.reqfile', + 'pyvcloud>=14 # from collection test.metadata', 'pytz # from collection test.reqfile', - # python-dateutil should appear only once even though referenced in - # multiple places, once with a dash and another with an underscore in the name. - 'python_dateutil>=2.8.2 # from collection test.reqfile', - # jinja2 should appear only once even though referenced in multiple - # places, once with uppercase and another with lowercase in the name. + 'python-dateutil>=2.8.2 # from collection test.reqfile', 'jinja2>=3.0 # from collection test.reqfile', - 'tacacs_plus # from collection test.reqfile' + 'tacacs_plus # from collection test.reqfile', + 'pyvcloud>=18.0.10 # from collection test.reqfile' ], 'system': [ 'subversion [platform:rpm] # from collection test.bindep', 'subversion [platform:dpkg] # from collection test.bindep' ]} +def test_process_returns_excluded_python(data_dir, tmp_path): + """ + Test that process() return value is properly formatted for excluded Python reqs. + """ + pip_ignore_file = tmp_path / "exclude-requirements.txt" + pip_ignore_file.write_text("req1\nreq2") + + retval = process(data_dir, exclude_pip=str(pip_ignore_file)) + + assert 'python' in retval + assert 'exclude' in retval['python'] + assert retval['python']['exclude'] == ['req1', 'req2'] + + +def test_process_returns_excluded_system(data_dir, tmp_path): + """ + Test that process() return value is properly formatted for excluded system reqs. + """ + bindep_ignore_file = tmp_path / "exclude-bindep.txt" + bindep_ignore_file.write_text("req1\nreq2") + + retval = process(data_dir, exclude_bindep=str(bindep_ignore_file)) + + assert 'system' in retval + assert 'exclude' in retval['system'] + assert retval['system']['exclude'] == ['req1', 'req2'] + + +def test_process_returns_excluded_collections(data_dir, tmp_path): + """ + Test that process() return value is properly formatted for excluded collections. + """ + col_ignore_file = tmp_path / "ignored_collections" + col_ignore_file.write_text("a.b\nc.d") + + retval = process(data_dir, exclude_collections=str(col_ignore_file)) + + assert 'excluded_collections' in retval + assert retval['excluded_collections'] == ['a.b', 'c.d'] + + def test_single_collection_metadata(data_dir): col_path = os.path.join(data_dir, 'ansible_collections', 'test', 'metadata') @@ -53,7 +92,7 @@ def test_parse_args_default_action(): parser = parse_args( [ - action, '--sanitize', + action, f'--user-pip={user_pip}', f'--user-bindep={user_bindep}', f'--write-pip={write_pip}', @@ -62,7 +101,6 @@ def test_parse_args_default_action(): ) assert parser.action == action - assert parser.sanitize assert parser.user_pip == user_pip assert parser.user_bindep == user_bindep assert parser.write_pip == write_pip @@ -83,3 +121,308 @@ def test_yaml_extension(data_dir): 'python': {'test_collection.test_yaml_extension': ['python-six']}, 'system': {}, } + + +def test_filter_requirements_pep508(): + reqs = { + 'a.b': [ + 'foo[ext1,ext3] == 1', + 'bar; python_version < "2.7"', + 'A', + "name", + ], + 'c.d': [ + 'FOO >= 1', + 'bar; python_version < "3.6"', + "name<=1", + ], + 'e.f': [ + 'foo[ext2] @ git+http://github.com/foo/foo.git', + "name>=3", + ], + 'g.h': [ + "name>=3,<2", + ], + 'i.j': [ + "name@http://foo.com", + ], + 'k.l': [ + "name [fred,bar] @ http://foo.com ; python_version=='2.7'", + ], + 'm.n': [ + "name[quux, strange];python_version<'2.7' and platform_version=='2'", + ], + } + + expected = [ + 'foo[ext1,ext3] == 1 # from collection a.b', + 'bar; python_version < "2.7" # from collection a.b', + 'A # from collection a.b', + 'name # from collection a.b', + 'FOO >= 1 # from collection c.d', + 'bar; python_version < "3.6" # from collection c.d', + 'name<=1 # from collection c.d', + 'foo[ext2] @ git+http://github.com/foo/foo.git # from collection e.f', + 'name>=3 # from collection e.f', + 'name>=3,<2 # from collection g.h', + 'name@http://foo.com # from collection i.j', + "name [fred,bar] @ http://foo.com ; python_version=='2.7' # from collection k.l", + "name[quux, strange];python_version<'2.7' and platform_version=='2' # from collection m.n" + ] + + assert filter_requirements(reqs) == expected + + +def test_comment_parsing(): + """ + Test that filter_requirements() does not remove embedded URL anchors due to comment parsing. + """ + reqs = { + 'a.b': [ + '# comment 1', + 'git+https://git.repo/some_pkg.git#egg=SomePackage', + 'git+https://git.repo/some_pkg.git#egg=SomeOtherPackage # inline comment', + 'git+https://git.repo/some_pkg.git#egg=AlsoSomePackage #inline comment that hates leading spaces', + ' # crazy indented comment (waka waka!)', + '####### something informative' + ' ', + '', + ] + } + + expected = [ + 'git+https://git.repo/some_pkg.git#egg=SomePackage', + 'git+https://git.repo/some_pkg.git#egg=SomeOtherPackage', + 'git+https://git.repo/some_pkg.git#egg=AlsoSomePackage', + ] + + assert filter_requirements(reqs) == expected + + +def test_strip_comments(): + """ + Test that strip_comments() properly removes comments from Python requirements input. + """ + reqs = { + 'a.b': [ + '# comment 1', + 'git+https://git.repo/some_pkg.git#egg=SomePackage', + 'git+https://git.repo/some_pkg.git#egg=SomeOtherPackage # inline comment', + 'git+https://git.repo/some_pkg.git#egg=AlsoSomePackage #inline comment that hates leading spaces', + ' # crazy indented comment (waka waka!)', + '####### something informative' + ' ', + '', + ], + 'c.d': [ + '# comment 2', + 'git', + ] + } + + expected = { + 'a.b': [ + 'git+https://git.repo/some_pkg.git#egg=SomePackage', + 'git+https://git.repo/some_pkg.git#egg=SomeOtherPackage', + 'git+https://git.repo/some_pkg.git#egg=AlsoSomePackage', + ], + 'c.d': [ + 'git', + ] + } + + assert strip_comments(reqs) == expected + + +def test_python_pass_thru(): + """ + Test that filter_requirements() will pass through non-pep508 data. + """ + reqs = { + # various VCS and URL options + 'a.b': [ + 'git+https://git.repo/some_pkg.git#egg=SomePackage', + 'svn+svn://svn.repo/some_pkg/trunk/#egg=SomePackage', + 'https://example.com/foo/foo-0.26.0-py2.py3-none-any.whl', + 'http://my.package.repo/SomePackage-1.0.4.zip', + ], + + # various 'pip install' options + 'c.d': [ + '-i https://pypi.org/simple', + '--extra-index-url http://my.package.repo/simple', + '--no-clean', + '-e svn+http://svn.example.com/svn/MyProject/trunk@2019#egg=MyProject', + ] + } + + expected = [ + 'git+https://git.repo/some_pkg.git#egg=SomePackage', + 'svn+svn://svn.repo/some_pkg/trunk/#egg=SomePackage', + 'https://example.com/foo/foo-0.26.0-py2.py3-none-any.whl', + 'http://my.package.repo/SomePackage-1.0.4.zip', + '-i https://pypi.org/simple', + '--extra-index-url http://my.package.repo/simple', + '--no-clean', + '-e svn+http://svn.example.com/svn/MyProject/trunk@2019#egg=MyProject', + ] + + assert filter_requirements(reqs) == expected + + +def test_excluded_system_requirements(): + reqs = { + 'a.b': [ + 'libxml2-dev [platform:dpkg]', + 'dev-libs/libxml2', + 'python3-lxml [(platform:redhat platform:base-py3)]', + 'foo [platform:bar]', + ], + 'c.d': [ + '# python is in EXCLUDED_REQUIREMENTS', + 'python [platform:brew] ==3.7.3', + 'libxml2-dev [platform:dpkg]', + 'python3-all-dev [platform:dpkg !platform:ubuntu-precise]', + ], + 'user': [ + 'foo', # should never exclude from user reqs + ] + } + + excluded = ['python3-lxml', 'foo'] + + expected = [ + 'libxml2-dev [platform:dpkg] # from collection a.b', + 'dev-libs/libxml2 # from collection a.b', + 'libxml2-dev [platform:dpkg] # from collection c.d', + 'python3-all-dev [platform:dpkg !platform:ubuntu-precise] # from collection c.d', + 'foo # from collection user', + ] + + assert filter_requirements(reqs, exclude=excluded, is_python=False) == expected + + +def test_excluded_python_requirements(): + reqs = { + "a.b": [ + "req1", + "req2==0.1.0", + "req4 ; python_version<='3.9'", + "git+https://git.repo/some_pkg.git#egg=SomePackage", + ], + "c.d": [ + "req1<=2.0.0", + "req3", + ], + "user": [ + "req1" # should never exclude from user reqs + ] + } + + excluded = [ + "req1", + "req4", + "git", + ] + + expected = [ + "req2==0.1.0 # from collection a.b", + "git+https://git.repo/some_pkg.git#egg=SomePackage", + "req3 # from collection c.d", + "req1 # from collection user", + ] + + assert filter_requirements(reqs, excluded) == expected + + +def test_filter_requirements_excludes_collections(): + """ + Test that excluding all requirements from a list of collections works in filter_requirements(). + """ + reqs = { + "a.b": [ + "req1", + "req2==0.1.0", + "req4 ; python_version<='3.9'", + "git+https://git.repo/some_pkg.git#egg=SomePackage", + ], + "c.d": [ + "req1<=2.0.0", + "req3", + ], + "e.f": [ + "req5", + ], + "user": [ + "req1" # should never exclude from user reqs + ] + } + + excluded_collections = [ + 'a.b', + 'e.f', + ] + + expected = [ + "req1<=2.0.0 # from collection c.d", + "req3 # from collection c.d", + "req1 # from collection user", + ] + + assert filter_requirements(reqs, exclude_collections=excluded_collections) == expected + + +def test_requirement_regex_exclusions(): + reqs = { + "a.b": [ + "foo", + "shimmy", + "kungfoo", + "aaab", + ], + "c.d": [ + "foobar", + "shake", + "ab", + ] + } + + excluded = [ + "Foo", # straight string comparison (case shouldn't matter) + "foo.", # straight string comparison (shouldn't match) + "~foo.", # regex (shouldn't match b/c not full string match) + "~Sh.*", # regex (case shouldn't matter) + "~^.+ab", # regex + ] + + expected = [ + "kungfoo # from collection a.b", + "foobar # from collection c.d", + "ab # from collection c.d" + ] + + assert filter_requirements(reqs, excluded) == expected + + +def test_collection_regex_exclusions(): + reqs = { + "a.b": ["foo"], + "c.d": ["bar"], + "ab.cd": ["foobar"], + "e.f": ["baz"], + "be.fun": ["foobaz"], + } + + excluded_collections = [ + r"~A\..+", # regex (case shouldn't matter) + "E.F", # straight string comparison (case shouldn't matter) + "~b.c", # regex (shouldn't match b/c not full string match) + ] + + expected = [ + "bar # from collection c.d", + "foobar # from collection ab.cd", + "foobaz # from collection be.fun", + ] + + assert filter_requirements(reqs, exclude_collections=excluded_collections) == expected diff --git a/test/unit/test_requirements.py b/test/unit/test_requirements.py deleted file mode 100644 index 42cb017c..00000000 --- a/test/unit/test_requirements.py +++ /dev/null @@ -1,58 +0,0 @@ -from ansible_builder._target_scripts.introspect import sanitize_requirements - - -def test_combine_entries(): - assert sanitize_requirements({ - 'foo.bar': ['foo>1.0'], - 'bar.foo': ['foo>=2.0'] - }) == ['foo>1.0,>=2.0 # from collection foo.bar,bar.foo'] - - -def test_remove_unwanted_requirements(): - assert sanitize_requirements({ - 'foo.bar': [ - 'foo', - 'ansible', - 'bar', - ], - 'bar.foo': [ - 'pytest', - 'bar', - 'zoo' - ] - }) == [ - 'foo # from collection foo.bar', - 'bar # from collection foo.bar,bar.foo', - 'zoo # from collection bar.foo' - ] - - -def test_skip_bad_formats(): - """A single incorrectly formatted requirement should warn, but not block other reqs""" - assert sanitize_requirements({'foo.bar': [ - 'foo', - 'bar' - ], 'foo.bad': ['zizzer zazzer zuzz'] # not okay - }) == ['foo # from collection foo.bar', 'bar # from collection foo.bar'] - - -def test_sanitize_requirements_do_not_exclude(): - py_reqs = { - 'foo.bar': [ - 'foo', - 'ansible', # should not appear - 'bar', - ], - 'user': [ - 'pytest', # should appear - 'bar', - 'zoo' - ] - } - - assert sanitize_requirements(py_reqs) == [ - 'foo # from collection foo.bar', - 'bar # from collection foo.bar,user', - 'pytest # from collection user', - 'zoo # from collection user', - ] diff --git a/test/unit/test_user_definition.py b/test/unit/test_user_definition.py index db21b9bd..3928f862 100644 --- a/test/unit/test_user_definition.py +++ b/test/unit/test_user_definition.py @@ -267,6 +267,27 @@ def test_v3_set_tags(self, exec_env_definition_file): value = definition.raw.get('options', {}).get('tags') assert value == ['ee_test:latest'] + def test_v3_dependencies_property(self, exec_env_definition_file): + deps = { + 'python': ['req1', 'req2'], + 'system': ['sysreq1', 'sysreq2'], + 'exclude': { + 'all_from_collections': ['a.b', 'c.d'], + } + } + + ee_content = { + 'version': 3, + 'dependencies': deps, + } + + path = exec_env_definition_file(content=ee_content) + definition = UserDefinition(path) + definition.validate() + + value = definition.dependencies + assert value == deps + class TestImageDescription: