169 Calculate and Change the SHA-256 hash for http/https Artifacts (#246

)

* Modifies the from_recipe() factory to return the path in the recipe file a source was found in

* Adds missing unit test cases for from_recipe()

* Adds initial vision for the _update_sha256() function in bump_recipe.py

* Minor improvement to _update_sha256()

* Minor refactor of the recipe bumper tool that separates update concerns

* Adds comment and only updates the sha256 field when the build flag is unspecified

* Fixes bug in from_recipe() function that reports the wrong source path for non-list source sections

* Adds unit test edge case for from_recipe() that parses recipe files with a single value in a source list

* Adds a new python plugin to fail a test if it reaches out to the network

* Adds a new python plugin to fail a test if it reaches out to the network

* Adds HTTP mocking to test_bump_recipe_cli

* Adds missing test dependency

.pytest.ini

@@ -14,4 +14,4 @@ filterwarnings =
     # Deprecation warnings from `conda`
     ignore::DeprecationWarning:boltons.*
     ignore::DeprecationWarning:xdist.*
-addopts = --ignore=tests/test_aux_files
+addopts = --ignore=tests/test_aux_files --disable-socket

conda_recipe_manager/commands/bump_recipe.py

@@ -12,22 +12,34 @@
 
 from conda_recipe_manager.commands.utils.print import print_err
 from conda_recipe_manager.commands.utils.types import ExitCode
+from conda_recipe_manager.fetcher.artifact_fetcher import from_recipe as af_from_recipe
+from conda_recipe_manager.fetcher.http_artifact_fetcher import HttpArtifactFetcher
 from conda_recipe_manager.parser.recipe_parser import RecipeParser
+from conda_recipe_manager.parser.recipe_reader import RecipeReader
 from conda_recipe_manager.types import JsonPatchType
 
-# TODO Improve. In order for `click` to play nice with `pyfakefs`, we set `path_type=str` and delay converting to a
-# `Path` instance. This is caused by how `click` uses decorators. See these links for more detail:
-# - https://pytest-pyfakefs.readthedocs.io/en/latest/troubleshooting.html#pathlib-path-objects-created-outside-of-tests
-# - https://github.com/pytest-dev/pyfakefs/discussions/605
 
+def _exit_on_failed_patch(recipe_parser: RecipeParser, patch_blob: JsonPatchType) -> None:
+    """
+    Convenience function that exits the program when a patch operation fails. This standardizes how we handle patch
+    failures across all patch operations performed in this program.
+
+    :param recipe_parser: Recipe file to update.
+    :param patch_blob: Recipe patch to execute.
+    """
+    if recipe_parser.patch(patch_blob):
+        return
+
+    print_err(f"Couldn't perform the patch: {patch_blob}.")
+    sys.exit(ExitCode.PATCH_ERROR)
 
-def _get_required_patch_blob(recipe_parser: RecipeParser, increment_build_num: bool) -> JsonPatchType:
+
+def _update_build_num(recipe_parser: RecipeParser, increment_build_num: bool) -> None:
     """
-    Returns the required JSON Patch Blob
+    Attempts to update the build number in a recipe file.
 
     :param recipe_parser: Recipe file to update.
     :param increment_build_num: Increments the `/build/number` field by 1 if set to `True`. Otherwise resets to 0.
-    :returns: A JSON Patch blob to add or modify the build number
     """
 
     # Try to get "build" key from the recipe, exit if not found
@@ -48,15 +60,60 @@ def _get_required_patch_blob(recipe_parser: RecipeParser, increment_build_num: b
                 print_err("Build number is not an integer.")
                 sys.exit(ExitCode.ILLEGAL_OPERATION)
 
-            return cast(JsonPatchType, {"op": "replace", "path": "/build/number", "value": build_number + 1})
+            _exit_on_failed_patch(
+                recipe_parser,
+                cast(JsonPatchType, {"op": "replace", "path": "/build/number", "value": build_number + 1}),
+            )
+            return
     except KeyError:
         if increment_build_num:
             print_err("`/build/number` key could not be found in the recipe.")
             sys.exit(ExitCode.ILLEGAL_OPERATION)
 
-    return cast(JsonPatchType, {"op": "add", "path": "/build/number", "value": 0})
+    _exit_on_failed_patch(recipe_parser, cast(JsonPatchType, {"op": "add", "path": "/build/number", "value": 0}))
 
 
+def _update_sha256(recipe_parser: RecipeParser) -> None:
+    """
+    Attempts to update the SHA-256 hash(s) in the `/source` section of a recipe file, if applicable. Note that this is
+    only required for build artifacts that are hosted as compressed software archives. If this field must be updated,
+    a lengthy network request may be required to calculate the new hash.
+
+    NOTE: For this to make any meaningful changes, the `version` field will need to be updated first.
+
+    :param recipe_parser: Recipe file to update.
+    """
+    fetcher_lst = af_from_recipe(recipe_parser, True)
+    if not fetcher_lst:
+        # TODO log
+        return
+
+    # TODO handle case where SHA is stored in one or more variables (see cctools-ld64.yaml)
+    # TODO handle case where SHA is a variable
+
+    # TODO Future: Figure out
+    # NOTE: Each source _might_ have a different SHA-256 hash. This is the case for the `cctools-ld64` feedstock. That
+    # project has a different implementation per architecture. However, in other circumstances, mirrored sources with
+    # different hashes might imply there is a security threat.
+    for src_path, fetcher in fetcher_lst.items():
+        if not isinstance(fetcher, HttpArtifactFetcher):
+            continue
+
+        # TODO retry mechanism
+        # TODO attempt fetch in the background, especially if multiple fetch() calls are required.
+        fetcher.fetch()
+        sha = fetcher.get_archive_sha256()
+        sha_path = RecipeReader.append_to_path(src_path, "/sha256")
+
+        # Guard against the unlikely scenario that the `sha256` field is missing.
+        patch_op = "replace" if recipe_parser.contains_value(sha_path) else "add"
+        _exit_on_failed_patch(recipe_parser, {"op": patch_op, "path": sha_path, "value": sha})
+
+
+# TODO Improve. In order for `click` to play nice with `pyfakefs`, we set `path_type=str` and delay converting to a
+# `Path` instance. This is caused by how `click` uses decorators. See these links for more detail:
+# - https://pytest-pyfakefs.readthedocs.io/en/latest/troubleshooting.html#pathlib-path-objects-created-outside-of-tests
+# - https://github.com/pytest-dev/pyfakefs/discussions/605
 @click.command(short_help="Bumps a recipe file to a new version.")
 @click.argument("recipe_file_path", type=click.Path(exists=True, path_type=str))
 @click.option(
@@ -82,11 +139,10 @@ def bump_recipe(recipe_file_path: str, build_num: bool) -> None:
         print_err("An error occurred while parsing the recipe file contents.")
         sys.exit(ExitCode.PARSE_EXCEPTION)
 
-    required_patch_blob = _get_required_patch_blob(recipe_parser, build_num)
-
-    if not recipe_parser.patch(required_patch_blob):
-        print_err(f"Couldn't perform the patch: {required_patch_blob}.")
-        sys.exit(ExitCode.PARSE_EXCEPTION)
+    # Attempt to update fields
+    _update_build_num(recipe_parser, build_num)
+    if not build_num:
+        _update_sha256(recipe_parser)
 
     Path(recipe_file_path).write_text(recipe_parser.render(), encoding="utf-8")
     sys.exit(ExitCode.SUCCESS)

conda_recipe_manager/commands/utils/types.py

@@ -35,6 +35,9 @@ class ExitCode(IntEnum):
     PRE_PROCESS_EXCEPTION = 105
     ILLEGAL_OPERATION = 106
 
+    # bump-recipe
+    PATCH_ERROR = 107
+
     ## rattler-bulk-build ##
     # NOTE: There may be overlap with rattler-build
 

conda_recipe_manager/fetcher/artifact_fetcher.py

@@ -45,7 +45,7 @@ def _render_git_key(recipe: RecipeReader, key: str) -> str:
                     raise FetchUnsupportedError(f"The following key is not supported for git sources: {key}")
 
 
-def from_recipe(recipe: RecipeReader, ignore_unsupported: bool = False) -> list[BaseArtifactFetcher]:
+def from_recipe(recipe: RecipeReader, ignore_unsupported: bool = False) -> dict[str, BaseArtifactFetcher]:
     """
     Parses and constructs a list of artifact-fetching objects based on the contents of a recipe.
 
@@ -60,17 +60,19 @@ def from_recipe(recipe: RecipeReader, ignore_unsupported: bool = False) -> list[
     :param ignore_unsupported: (Optional) If set to `True`, ignore currently unsupported artifacts found in the source
         section and return the list of supported sources. Otherwise, throw an exception.
     :raises FetchUnsupportedError: If an unsupported source format is found.
-    :returns: A list containing one Artifact Fetcher instance per source found in the recipe file.
+    :returns: A map containing one path and Artifact Fetcher instance pair per source found in the recipe file.
     """
-    sources: list[BaseArtifactFetcher] = []
+    sources: dict[str, BaseArtifactFetcher] = {}
     parsed_sources = cast(
         dict[str, Primitives] | list[dict[str, Primitives]], recipe.get_value("/source", sub_vars=True, default=[])
     )
     # TODO Handle selector evaluation/determine how common it is to have a selector in `/source`
 
     # Normalize to a list to handle both single and multi-source cases.
+    is_src_lst = True
     if not isinstance(parsed_sources, list):
         parsed_sources = [parsed_sources]
+        is_src_lst = False
 
     recipe_name = recipe.get_recipe_name()
     if recipe_name is None:
@@ -85,19 +87,19 @@ def from_recipe(recipe: RecipeReader, ignore_unsupported: bool = False) -> list[
 
         src_name = recipe_name if len(parsed_sources) == 1 else f"{recipe_name}_{i}"
 
+        # If the source section is not a list, it contains one "flag" source object.
+        src_path = f"/source/{i}" if is_src_lst else "/source"
         if url is not None:
-            sources.append(HttpArtifactFetcher(src_name, url))
+            sources[src_path] = HttpArtifactFetcher(src_name, url)
         elif git_url is not None:
-            sources.append(
-                GitArtifactFetcher(
-                    src_name,
-                    git_url,
-                    branch=optional_str(parsed_source.get(_render_git_key(recipe, "git_branch"))),
-                    tag=optional_str(parsed_source.get(_render_git_key(recipe, "git_tag"))),
-                    rev=optional_str(parsed_source.get(_render_git_key(recipe, "git_rev"))),
-                )
+            sources[src_path] = GitArtifactFetcher(
+                src_name,
+                git_url,
+                branch=optional_str(parsed_source.get(_render_git_key(recipe, "git_branch"))),
+                tag=optional_str(parsed_source.get(_render_git_key(recipe, "git_tag"))),
+                rev=optional_str(parsed_source.get(_render_git_key(recipe, "git_rev"))),
             )
         elif not ignore_unsupported:
-            raise FetchUnsupportedError(f"{recipe_name} contains an unsupported source object at `/source/{i}`.")
+            raise FetchUnsupportedError(f"{recipe_name} contains an unsupported source object at `{src_path}`.")
 
     return sources

environment.yaml

@@ -10,6 +10,7 @@ dependencies:
   - python >=3.11
   - pytest
   - pytest-cov
+  - conda-forge::pytest-socket
   - pytest-xdist
   - pyfakefs
   # Match version with .pre-commit-config.yaml to ensure consistent rules with `make lint`.

recipe/meta.yaml

@@ -46,6 +46,7 @@ test:
     - pip
     - pytest
     - pytest-xdist
+    - pytest-socket
     - pyfakefs
   commands:
     - pip check

tests/commands/test_bump_recipe.py

@@ -2,19 +2,73 @@
 :Description: Tests the `bump-recipe` CLI
 """
 
-from typing import Final
+from typing import Final, cast
+from unittest.mock import patch
 
 import pytest
 from click.testing import CliRunner
 from pyfakefs.fake_filesystem import FakeFilesystem
 
 from conda_recipe_manager.commands import bump_recipe
 from conda_recipe_manager.commands.utils.types import ExitCode
+from conda_recipe_manager.fetcher.http_artifact_fetcher import HttpArtifactFetcher
 from conda_recipe_manager.parser.recipe_parser import RecipeParser
 from tests.file_loading import get_test_path, load_recipe
+from tests.http_mocking import MockHttpStreamResponse
 from tests.smoke_testing import assert_cli_usage
 
 
+class MockUrl:
+    """
+    Namespace for mocked URLs
+    """
+
+    # URL endpoint(s) used by the `types-toml.yaml` file
+    TYPES_TOML_V0_10_8_20240310_URL: Final[str] = (
+        "https://pypi.io/packages/source/t/types-toml/types-toml-0.10.8.20240310.tar.gz"
+    )
+
+    # Failed URL
+    PYPI_HTTP_500: Final[str] = "https://pypi.io/error_500.html"
+
+
+@pytest.fixture(name="http_fetcher_types_toml")
+def fixture_http_fetcher_types_toml() -> HttpArtifactFetcher:
+    """
+    `HttpArtifactFetcher` test fixture for the `types-toml` project.
+    """
+    return HttpArtifactFetcher("types-toml", MockUrl.TYPES_TOML_V0_10_8_20240310_URL)
+
+
+@pytest.fixture(name="http_pypi_failure")
+def fixture_http_pypi_failure() -> HttpArtifactFetcher:
+    """
+    Single-instance `HttpArtifactFetcher` test fixture. This can be used for error cases that don't need multiple tests
+    to be run or need to simulate a failed HTTP request.
+    """
+    return HttpArtifactFetcher("pypi_failure", MockUrl.PYPI_HTTP_500)
+
+
+def mock_requests_get(*args: tuple[str], **_: dict[str, str | int]) -> MockHttpStreamResponse:
+    """
+    Mocking function for HTTP requests made in this test file.
+
+    NOTE: The artifacts provided are not the real build artifacts.
+
+    :param args: Arguments passed to the `requests.get()`
+    :param _: Name-specified arguments passed to `requests.get()` (Unused)
+    """
+    endpoint = cast(str, args[0])
+    match endpoint:
+        case MockUrl.TYPES_TOML_V0_10_8_20240310_URL:
+            return MockHttpStreamResponse(200, "archive_files/dummy_project_01.tar.gz")
+        case MockUrl.PYPI_HTTP_500:
+            return MockHttpStreamResponse(500, "archive_files/dummy_project_01.tar.gz")
+        case _:
+            # TODO fix: pyfakefs does include `/dev/null` by default, but this actually points to `<temp_dir>/dev/null`
+            return MockHttpStreamResponse(404, "/dev/null")
+
+
 def test_usage() -> None:
     """
     Smoke test that ensures rendering of the help menu
@@ -23,10 +77,13 @@ def test_usage() -> None:
 
 
 @pytest.mark.parametrize(
-    "recipe_file, increment_build_num, expected_recipe_file",
+    "recipe_file,increment_build_num,expected_recipe_file",
     [
-        ("simple-recipe.yaml", True, "bump_recipe/build_num_1.yaml"),
-        ("simple-recipe.yaml", False, "simple-recipe.yaml"),
+        ("types-toml.yaml", True, "bump_recipe/types-toml_build_num_1.yaml"),
+        # TODO: Re-enable test when version bumping is supported.
+        # ("types-toml.yaml", False, "bump_recipe/types-toml_version_bump.yaml"),
+        # NOTE: These have no source section, therefore all SHA-256 update attempts (and associated network requests)
+        # should be skipped.
         ("bump_recipe/build_num_1.yaml", True, "bump_recipe/build_num_2.yaml"),
         ("bump_recipe/build_num_1.yaml", False, "simple-recipe.yaml"),
         ("bump_recipe/build_num_42.yaml", True, "bump_recipe/build_num_43.yaml"),
@@ -36,12 +93,16 @@ def test_usage() -> None:
     ],
 )
 def test_bump_recipe_cli(
-    fs: FakeFilesystem, recipe_file: str, increment_build_num: bool, expected_recipe_file: str
+    fs: FakeFilesystem,
+    recipe_file: str,
+    increment_build_num: bool,
+    expected_recipe_file: str,
 ) -> None:
     """
     Test that the recipe file is successfully updated/bumped.
 
     :param fs: `pyfakefs` Fixture used to replace the file system
+    :param request: Pytest fixture request object.
     """
     runner = CliRunner()
     fs.add_real_directory(get_test_path(), read_only=False)
@@ -52,7 +113,9 @@ def test_bump_recipe_cli(
     cli_args: Final[list[str]] = (
         ["--build-num", str(recipe_file_path)] if increment_build_num else [str(recipe_file_path)]
     )
-    result = runner.invoke(bump_recipe.bump_recipe, cli_args)
+
+    with patch("requests.get", new=mock_requests_get):
+        result = runner.invoke(bump_recipe.bump_recipe, cli_args)
 
     parser = load_recipe(recipe_file_path, RecipeParser)
     expected_parser = load_recipe(expected_recipe_file_path, RecipeParser)