diff --git a/docs/test-coverage.md b/docs/test-coverage.md index 8e393e3..3163acd 100644 --- a/docs/test-coverage.md +++ b/docs/test-coverage.md @@ -29,6 +29,7 @@ file listing is its own index. | What | Test | Step | CI runs | |---|---|---|---| | `fetch repo`, `prepare`, `build containers` | [`tests/smoke-test/run-smoke-test.sh`](../tests/smoke-test/run-smoke-test.sh) | whole script | compose, per-PR | +| `prepare` from a working checkout the tool did not clone (colocated container recipes) | [`tests/smoke-test/run-smoke-test.sh`](../tests/smoke-test/run-smoke-test.sh) | `prepare from local checkout` | compose, per-PR | | Rebuild giving a dirty checkout a `stackdev-` identity | [`tests/app-deploy/run-test.sh`](../tests/app-deploy/run-test.sh) | `deploy update content` | compose + kind per-PR; remote + remote-compose weekly | | `webapp` wrapper (build and serve a Vite/React app) | [`tests/webapp-test/run-webapp-test.sh`](../tests/webapp-test/run-webapp-test.sh) | whole script | compose, per-PR and weekly | | `static-content` wrapper | [`tests/static-content-test/run-static-content-test.sh`](../tests/static-content-test/run-static-content-test.sh) | whole script | compose, per-PR and weekly | diff --git a/src/stack/build/build_containers.py b/src/stack/build/build_containers.py index d736441..d8f2d68 100644 --- a/src/stack/build/build_containers.py +++ b/src/stack/build/build_containers.py @@ -137,11 +137,20 @@ def process_container(build_context: BuildContext) -> bool: # If the build script filename is not explicitly provided, we try to infer it # DBDB this code seems not to work because we use the bare stack name rather than a directory # We go looking for a "containers" directory in the root of the container's repo. - container_build_script_dir = (fs_path_for_repo(building_container.ref) + if building_container.ref: + recipe_repo_dir = _repo_dir_for_ref(building_container.ref, stack) + if not recipe_repo_dir: + error_exit(f"Cannot locate the repo {building_container.ref}, which should hold the build " + f"recipe for {building_container.name}") + else: + # A stack with no derivable repo ref (a checkout whose origin names no + # remote we can parse) has only the tree it was loaded from to offer. + recipe_repo_dir = Path(stack.repo_path) if stack.repo_path else None + container_build_script_dir = (recipe_repo_dir .joinpath(constants.stack_files_directory_name) - .joinpath(constants.containers_directory_name)) + .joinpath(constants.containers_directory_name)) if recipe_repo_dir else None log_debug(f"Looking for build script in this directory: {container_build_script_dir}") - if os.path.exists(container_build_script_dir): + if container_build_script_dir and os.path.exists(container_build_script_dir): temp_build_dir = container_build_script_dir.joinpath(building_container.name.replace("/", "-")) temp_build_script_filename = temp_build_dir.joinpath("build.sh") # Now check if the container exists in the external stack. @@ -179,7 +188,8 @@ def process_container(build_context: BuildContext) -> bool: build_envs["STACK_REPO_STACK_DIR"] = str(stack.repo_path) if stack.repo_path else "" build_envs["STACK_REPO_CONTAINER_DIR"] = (str(build_context.container.repo_path) if building_container.repo_path else build_envs["STACK_REPO_STACK_DIR"]) - build_envs["STACK_REPO_SOURCE_DIR"] = (str(fs_path_for_repo(building_container.ref)) if building_container.ref + source_repo_dir = _source_repo_dir(building_container, stack) + build_envs["STACK_REPO_SOURCE_DIR"] = (str(source_repo_dir) if building_container.ref and source_repo_dir else build_envs["STACK_REPO_CONTAINER_DIR"]) # The default build uses this as its context; a build script has to opt in, so hand it over. @@ -273,18 +283,30 @@ def _resolve_wrapper_for_container(building_container, wrapper_pin: dict): return wrapper +def _repo_dir_for_ref(ref, stack): + """The local tree holding the repo `ref` names. + + A ref naming the stack's own repo resolves to the tree the stack was loaded from, + which may be a checkout outside the dev root -- and is never cloned into it, so the + dev root is the wrong place to look for anything colocated with that stack. Any + other ref resolves to its dev root clone, or to None when the ref names no repo we + can locate.""" + if not ref: + return None + if stack is not None and getattr(stack, "repo_path", None) and same_repo_ref(ref, stack.get_repo_ref()): + return Path(stack.repo_path) + return fs_path_for_repo(ref) + + def _source_repo_dir(building_container, stack): """The repo whose source this container is built from. - `ref` names it. A ref naming the stack's own repo resolves to the tree the stack was - loaded from (possibly a checkout outside the dev root), keeping the built content and - the image identity in the same tree. A container.yml that omits `ref` means "the repo - this descriptor lives in", which is what `repo_path` records. Failing both, the - stack's own repo.""" + `ref` names it, and resolves as `_repo_dir_for_ref` describes, keeping the built + content and the image identity in the same tree. A container.yml that omits `ref` + means "the repo this descriptor lives in", which is what `repo_path` records. + Failing both, the stack's own repo.""" if building_container.ref: - if stack is not None and getattr(stack, "repo_path", None) and same_repo_ref(building_container.ref, stack.get_repo_ref()): - return stack.repo_path - return fs_path_for_repo(building_container.ref) + return _repo_dir_for_ref(building_container.ref, stack) if building_container.repo_path: return building_container.repo_path return stack.repo_path diff --git a/tests/smoke-test/run-smoke-test.sh b/tests/smoke-test/run-smoke-test.sh index 7ee6e32..12146e6 100755 --- a/tests/smoke-test/run-smoke-test.sh +++ b/tests/smoke-test/run-smoke-test.sh @@ -8,6 +8,29 @@ setup_test_dir smoke-test-dir # We must delete any instances of the test-container in the local registory # otherwise we'll skip building it below remove_local_images bozemanpass/test-container +# First, the same stack from a working checkout the tool did not clone -- a developer +# tree, or a CI actions/checkout. This phase must stay ahead of the fetch below: once +# the repo is under STACK_REPO_BASE_DIR the recipe resolves from there and the phase +# tests nothing (bozemanpass/stack#297). +checkout_dir=$STACK_TEST_DIR/checkout +git clone https://github.com/bozemanpass/stack-test-stacks $checkout_dir +# A path, not a name: a name is only resolvable through the repo base dir, so the path +# form is the only way to reach a stack that was never cloned. +$TEST_TARGET_STACK prepare --stack $checkout_dir/stack-files/stacks/test-stack +if ! docker image inspect bozemanpass/test-container:stack > /dev/null 2>&1; then + fail "prepare from local checkout: FAILED - image not built" +fi +# The stack's colocated container recipe has to come from the checkout itself. Assert +# that it was not instead obtained by quietly cloning the repo: a build that reaches the +# recipe by way of the repo base dir is the bug, whether or not an image comes out of it. +if [ -d "$STACK_REPO_BASE_DIR/github.com/bozemanpass/stack-test-stacks" ]; then + fail "prepare from local checkout: FAILED - stack repo was cloned into $STACK_REPO_BASE_DIR" +fi +echo "prepare from local checkout: PASSED" +# The checkout and the clone below are the same commit, so they yield the same image +# identity: leaving this image in place would make the build that follows a no-op. +remove_local_images bozemanpass/test-container + # Fetch the test stacks echo "Fetching test stac repo into: $STACK_REPO_BASE_DIR" $TEST_TARGET_STACK fetch repo github.com/bozemanpass/stack-test-stacks diff --git a/tests/unit/test_container_recipe_lookup.py b/tests/unit/test_container_recipe_lookup.py new file mode 100644 index 0000000..5bb5638 --- /dev/null +++ b/tests/unit/test_container_recipe_lookup.py @@ -0,0 +1,127 @@ +# Copyright © 2026 Bozeman Pass, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. + +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Tests for finding a container's build recipe (issue #297). + +A stack's colocated containers -- those with no `ref` of their own, whose recipes sit +beside it in its own repo -- have to be found in the tree the stack was loaded from. +Looking for them under the dev root instead finds nothing when the stack came from a +working checkout, and the miss is silent: the build falls through to the default build +with the repo root as its context and fails on a Dockerfile that was never involved. +""" + +import subprocess +import textwrap + +from pathlib import Path + +import pytest + +from stack.build.build_containers import process_container +from stack.build.build_types import BuildContext +from stack.build.build_util import compute_image_identity, get_containers_in_scope +from stack.command_types import CommandOptions +from stack.deploy.stack import Stack +from stack.opts import opts + + +DEFAULT_CONTAINER_BASE_DIR = Path(__file__).absolute().parent.parent.parent.joinpath( + "src", "stack", "data", "container-build" +) + + +def make_checkout(base_dir, origin, with_recipe=True, name="teststack"): + """A repo laid out the way a stack repo is: stacks and container recipes side by side. + + `origin` is the remote the checkout is given, since whether a repo ref can be derived + from it is the thing under test. Returns the stack's directory. + """ + repo_dir = base_dir / "checkout" + stack_dir = repo_dir / "stack-files" / "stacks" / name + pod_dir = stack_dir / "web" + pod_dir.mkdir(parents=True) + (stack_dir / "stack.yml").write_text( + textwrap.dedent( + f"""\ + name: {name} + description: "test stack" + containers: + - example/web + pods: + - name: web + path: ./web + """ + ) + ) + (pod_dir / "composefile.yml").write_text("services:\n web:\n image: example/web:stack\n") + if with_recipe: + recipe_dir = repo_dir / "stack-files" / "containers" / "example-web" + recipe_dir.mkdir(parents=True) + (recipe_dir / "build.sh").write_text("#!/usr/bin/env bash\nexit 0\n") + subprocess.run(["git", "init", "-q", str(repo_dir)], check=True) + subprocess.run(["git", "-C", str(repo_dir), "remote", "add", "origin", origin], check=True) + subprocess.run(["git", "-C", str(repo_dir), "add", "-A"], check=True) + # An image's identity is the recipe repo's commit hash, so the checkout needs one. + subprocess.run(["git", "-C", str(repo_dir), "-c", "user.email=test@example.com", + "-c", "user.name=test", "commit", "-q", "-m", "initial"], check=True) + return stack_dir + + +def resolve_build(stack_dir, dev_root): + """Run the build of the stack's one container as a dry run, returning its environment. + + The build environment is where the resolved recipe directory shows up: STACK_BUILD_DIR + is the directory the build script was found in, or the default build's context when no + script was found. + """ + stack = Stack().init_from_file(stack_dir / "stack.yml") + stack_container = get_containers_in_scope(stack)[0] + identity = compute_image_identity(stack, stack_container, dev_root) + build_env = {} + context = BuildContext(stack, identity.container_spec, DEFAULT_CONTAINER_BASE_DIR, build_env, dev_root) + assert process_container(context) + return build_env + + +@pytest.fixture(autouse=True) +def dry_run_opts(): + """Resolve the build without running it: the recipe lookup is what is under test.""" + saved = opts.o + opts.o = CommandOptions(dry_run=True) + yield + opts.o = saved + + +def test_colocated_recipe_found_in_local_checkout(tmp_path): + # The repo is not under the dev root -- which is what standing in a working checkout + # (or a CI actions/checkout) looks like -- so only the checkout can supply the recipe. + stack_dir = make_checkout(tmp_path, "https://github.com/example/teststack.git") + build_env = resolve_build(stack_dir, tmp_path / "empty-dev-root") + assert Path(build_env["STACK_BUILD_DIR"]) == tmp_path / "checkout" / "stack-files" / "containers" / "example-web" + + +def test_colocated_recipe_found_when_no_repo_ref_can_be_derived(tmp_path): + # A checkout whose origin is a local path names no repo we can resolve, so there is + # no ref to look up at all. The checkout is still all the recipe can be in. + stack_dir = make_checkout(tmp_path, str(tmp_path / "upstream.git")) + build_env = resolve_build(stack_dir, tmp_path / "empty-dev-root") + assert Path(build_env["STACK_BUILD_DIR"]) == tmp_path / "checkout" / "stack-files" / "containers" / "example-web" + + +def test_missing_recipe_still_falls_back_to_the_default_build(tmp_path): + # No recipe anywhere: the default build gets the repo as its context, as before. + stack_dir = make_checkout(tmp_path, "https://github.com/example/teststack.git", with_recipe=False) + build_env = resolve_build(stack_dir, tmp_path / "empty-dev-root") + assert Path(build_env["STACK_BUILD_DIR"]) == tmp_path / "checkout"