From 2342afa96ba39a0bd56ff2ffade590afb90bbe4d Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 29 Oct 2026 21:53:10 -0700 Subject: [PATCH 1/3] feat(bundle): a registry image with its build context beside it, and bundle init --image-ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An image key can be `{ ref = "/:@sha256:…", dockerfile = "Dockerfile" }`, and an image folder can hold a Dockerfile beside its ref. A run pulls the image by its digest and never builds it; the context is kept for reading or rebuilding, which a deploy does only when it can't pull. put_ref given a context and no digest pins through the registry and records name and digest apart, as a published image does; the ledger covers the context's files. `agent-env bundle init --image-ref ID=REF` (image_refs= on import_task / import_eval) writes the stored image ID as that digest-pinned registry image with its pruned context beside it, skipping version pinning, and refuses a ref that isn't pinned or names no image the import writes. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- .../artifact/artifacts/docker_image.py | 42 +++---- src/agent_env/bundle/_images.py | 4 +- src/agent_env/bundle/_import_report.py | 25 ++-- src/agent_env/bundle/importer.py | 56 +++++++-- src/agent_env/bundle/ledger.py | 7 +- src/agent_env/bundle/materialize.py | 22 ++-- src/agent_env/bundle/resolve.py | 47 +++++--- src/agent_env/bundle/run.py | 4 +- src/agent_env/cli/bundle.py | 15 ++- tst/unit/bundle/importer_image_refs_test.py | 112 ++++++++++++++++++ tst/unit/bundle/referenced_context_test.py | 107 +++++++++++++++++ tst/unit/bundle/resolve_test.py | 27 ++++- 13 files changed, 393 insertions(+), 77 deletions(-) create mode 100644 tst/unit/bundle/importer_image_refs_test.py create mode 100644 tst/unit/bundle/referenced_context_test.py diff --git a/AGENTS.md b/AGENTS.md index a847e7a7a..aa06e65c1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -122,7 +122,7 @@ and deserialized through a registry. | `openenv/` | `build_app(bundle)`: the [OpenEnv](https://github.com/openenv/OpenEnv) server for a bundle's tasks, each session playing one episode through the Episode API, with the envs' MCP tools, an added `end_turn` tool and the task's verifier score as the reward. It prepares bundles as `agent-env run` does (`prepare_bundle(played=True)`, without the agent the client plays) and keeps namespace routing on for the app's life. `openenv` comes with the `openenv` extra, a floor with no cap; this package and `training/trl.py` are the only ones that import it. Each task's description in the Task API names its `toolset` (the played agent's envs, role, and whether it gets `bash`). | | `training/` | One module per outside trainer, each behind its own extra, and `_tools.py`, which turns an env's tools into typed, documented Python methods from their JSON Schema, as transformers' `get_json_schema` reads them. `trl.py` (`trl` extra): `AgentEnv(url)` over a server `openenv/` makes, with `dataset` (a row per task) and `factory` (TRL's `environment_factory`: a generated class per toolset whose methods are the tools, a background `reset` and an owned `get_reward`); `AgentEnv.local(bundle)` serves the bundle in process. | | `runner/` | The `[runner]` seam: `Runner.submit()` returns a `RunHandle` (`run_id`, `instance_id`) before the run starts; `LocalRunner` is built in. `runner/store.py` is the run index every runner writes and the explorer reads. | -| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image) so it runs with no store; `agent-env bundle register` writes a folder into the configured stores under registry ids (``), each task and eval naming the versions it reads and each image it builds pushed, through a ledger in the target store so registering again writes only what changed. | +| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image, or, given `--image-ref ID=REF`, as that registry image pinned by digest with its context beside it, `image = { ref, dockerfile }`, which a run pulls and builds only when it can't) so it runs with no store; `agent-env bundle register` writes a folder into the configured stores under registry ids (``), each task and eval naming the versions it reads and each image it builds pushed, through a ledger in the target store so registering again writes only what changed. | | `eval/` | `Eval` (`eval`), a versioned list of task references, and its store. | | `plugins/` | Entry-point plugins (see Extension points): the group names, `inventory()`, which `agent-env plugin list/show/check` report, `load_failures` and `settings`. The top level is public; the underscored modules are internal. | | `explorer/` | Optional local web UI: `agent-env up`, binds loopback `:8234`. | diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index af595cb71..0f921d595 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -217,8 +217,9 @@ def put_ref(cls, id: str, *, description: str, image_name: str, earlier_pin: str dockerfile_path: str | None = None) -> "DockerImageArtifact": """Register ``image_name``, an image already in a registry, which a sandbox pulls; nothing is built, pushed or saved. A tag is pinned to the digest the registry serves now, as ``name:tag@sha256:...`` (``earlier_pin`` kept - when it pins the same ref). Given the image's build context and ``image_digest``, the tag stays the name, the - digest is recorded beside it, and the context is uploaded for a sandbox that can't pull the image to build it. + when it pins the same ref). Given the image's build context, the name is recorded without its digest, the + digest beside it (``image_digest``, or the one the registry pins it to), and the context is uploaded for a + sandbox that can't pull the image to build it. ``image_name`` must name its registry, read with the configured image store's credentials where it holds them; one on this machine is refused unless ``id``'s image store is the local registry.""" @@ -235,25 +236,26 @@ def put_ref(cls, id: str, *, description: str, image_name: str, earlier_pin: str and not isinstance(config.get_image_store_for(id), LocalRegistryImageStore)): raise ValueError(f"{image_name} is in a registry on this machine, which only an image in the local " "registry's store, such as an @local one, can name") - if context_path is not None: - context = _context_of(Path(context_path), dockerfile_path) - store = get_artifact_store() - prefix = store.attempt_prefix("docker_image", id) - return store.put_document(cls( - id=id, - description=description, - image_name=image_name, - image_digest=image_digest, - build_context_object_url=_upload_context(config.get_object_store_to_write(prefix, id), prefix, context), - dockerfile_path=context.dockerfile, - source_digest=context.source_digest(None), - )) - if earlier_pin is not None and pinned_from(earlier_pin, image_name): - pinned = earlier_pin - else: - pinned = pin_digest(image_name, config.get_image_store().auth(image_name)) + if context_path is None or image_digest is None: + if earlier_pin is not None and pinned_from(earlier_pin, image_name): + pinned = earlier_pin + else: + pinned = pin_digest(image_name, config.get_image_store().auth(image_name)) + if context_path is None: + return get_artifact_store().put_document(cls(id=id, description=description, image_name=pinned)) + image_name, _, image_digest = pinned.rpartition("@") + context = _context_of(Path(context_path), dockerfile_path) store = get_artifact_store() - return store.put_document(cls(id=id, description=description, image_name=pinned)) + prefix = store.attempt_prefix("docker_image", id) + return store.put_document(cls( + id=id, + description=description, + image_name=image_name, + image_digest=image_digest, + build_context_object_url=_upload_context(config.get_object_store_to_write(prefix, id), prefix, context), + dockerfile_path=context.dockerfile, + source_digest=context.source_digest(None), + )) @classmethod def put_context(cls, id: str, *, description: str, context_path: str, dockerfile_path: str, diff --git a/src/agent_env/bundle/_images.py b/src/agent_env/bundle/_images.py index 6ef2d9f7c..2935b1f8d 100644 --- a/src/agent_env/bundle/_images.py +++ b/src/agent_env/bundle/_images.py @@ -34,6 +34,7 @@ class ImageJob: problem: str | None = None # why it can't be written locked: Locked | None = None unlocked: str | None = None # why its versions weren't pinned + ref: str | None = None # the registry image the folder names, given for it, its context beside it def context_groups(jobs: list[ImageJob]) -> list[list[ImageJob]]: @@ -89,7 +90,8 @@ def write_images(groups: list[list[ImageJob]], staging: Path, scratch: Path, # Pinning rewrites a Dockerfile, which another image placed in the same folder may hold too: so only once all are. sharing: dict[tuple[str, str], list[ImageJob]] = {} for job in placed: - sharing.setdefault((job.folder, job.pruned.dockerfile), []).append(job) + if job.ref is None: # a run pulls an image given its ref, so nothing pins its context's installs + sharing.setdefault((job.folder, job.pruned.dockerfile), []).append(job) for (folder, dockerfile), jobs in sharing.items(): _lock(jobs, staging / folder, dockerfile) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index 8aa286f3a..59b4c4af9 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -75,6 +75,7 @@ class WrittenImage: pruned: Pruned # what its folder holds of the context locked: Locked | None # what was pinned to the stored image's versions unlocked: str | None # why nothing was + ref: str | None = None # the registry image a run pulls, given for it, the context kept beside it @dataclass(frozen=True) @@ -156,6 +157,7 @@ def provenance(source: dict, imported: BundleImport) -> str: "written": [{"folder": entity.folder, "kind": entity.kind.value, "type": entity.type, "id": entity.id, "version": entity.version} for entity in imported.written], "images": [{"folder": image.folder, "key": image.key, "id": image.artifact, "version": image.version, + **({"ref": image.ref} if image.ref else {}), "dockerfile": image.pruned.dockerfile, "dockerfile_found": image.found} for image in imported.images], "store": [{"where": pin.where, "kind": pin.kind.value, "id": pin.id, "version": pin.version, "why": pin.why} @@ -170,8 +172,10 @@ def readme(description: str, imported: BundleImport) -> str: held = ("Every env, agent and artifact it uses is written here, so `agent-env run` runs it with no store" + (", except what *Still needs the store* lists, which it reads from the store this was imported from." if imported.pinned else ".")) + built = not imported.images or any(image.ref is None for image in imported.images) + pulled = [_PULLED_IMAGES] if any(image.ref for image in imported.images) else [] lines = [description, "", f"Imported with agent-env {package_version('agentenv-framework')} on {stamp}. {held}", "", - *_RUN_IT, *_needs_lines(imported), "", *_SHARE_IT, ""] + *_RUN_IT, *_needs_lines(imported), "", *([_BUILT_IMAGES] if built else []), *pulled, "", *_SHARE_IT, ""] contents = _sections([("Tasks", [f"{task.path}: task {task.id} v{task.version}" for task in imported.tasks]), ("Eval", [f"{imported.eval.path}: eval {imported.eval.id} v{imported.eval.version}"] if imported.eval else [])]) @@ -186,8 +190,11 @@ def _image_line(image: WrittenImage) -> str: found = " (found: the image records no Dockerfile)" if image.found else "" largest = ", ".join(f"{name} ({size_text(size)})" for name, size in pruned.largest) whole = f"; all of its build context, since {pruned.whole}" if pruned.whole else "" - return (f"{image.folder}: {image.key} built from {pruned.dockerfile}{found}, {pruned.files} file(s), " - f"{size_text(pruned.size)}{whole}; largest: {largest}; {_locked_text(image)}") + held = f"{pruned.files} file(s), {size_text(pruned.size)}{whole}; largest: {largest}" + if image.ref: + return (f"{image.folder}: {image.key} pulled as {image.ref}, its build context beside it from " + f"{pruned.dockerfile}{found}, {held}") + return f"{image.folder}: {image.key} built from {pruned.dockerfile}{found}, {held}; {_locked_text(image)}" def _locked_text(image: WrittenImage) -> str: @@ -223,11 +230,14 @@ def _listed(items: list[str]) -> str: f"- A model endpoint for its agents and judges: `{ENV_BASE_URL}` and `{ENV_API_KEY}`, or `[model] base_url` and " "`api_key` in `.agentenv/config.toml`.", "- An agent where a step names none: the configured default (`[agents] default_a2a_agent_id`).", - "", +] +_BUILT_IMAGES = ( "Its images are built from the build contexts in their folders on the machine that runs it, pinned to the " "Python and npm versions of the images the store held, as *Images* says. To build one unpinned, take out the " - f"COPY and two ARG lines naming {posixpath.basename(IN_IMAGE)} after each FROM in its Dockerfile.", -] + f"COPY and two ARG lines naming {posixpath.basename(IN_IMAGE)} after each FROM in its Dockerfile.") +_PULLED_IMAGES = ( + "An image *Images* says is pulled is pulled by its digest, never built; the build context beside it is there to " + "read or rebuild, and a deploy builds it only when it can't pull it.") _SHARE_IT = [ "## Share it", "", @@ -247,7 +257,8 @@ def _needs_lines(imported: BundleImport) -> list[str]: lines = [f"- The plugins it uses: `{'`, `'.join(plugins)}`."] if plugins else [] if configured: lines.append("- The configuration that registers what *Needs beyond core* says configuration registers.") - registries = sorted({host for pulled in imported.pulled if (host := registry_host_from_ref(pulled.image))}) + refs = [pulled.image for pulled in imported.pulled] + [image.ref for image in imported.images if image.ref] + registries = sorted({host for ref in refs if (host := registry_host_from_ref(ref))}) if registries: lines.append(f"- Access to the registries its pulled images are in: `{'`, `'.join(registries)}`. agent-env reads " "a private one only through its configured image store, such as an ECR store for an ECR registry.") diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index d00824c3f..294791b8e 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -74,11 +74,13 @@ def import_task(task_id: str, dest: Path | str, *, version: int | None = None, - include_scopes: Collection[FieldScope] = (), + include_scopes: Collection[FieldScope] = (), image_refs: Mapping[str, str] | None = None, progress: Callable[[str], None] | None = None) -> BundleImport: """Write the stored task ``task_id``, at ``version`` or its latest, and every entity it uses as a bundle at - ``dest``. Its steps' scoped fields are left out unless ``include_scopes`` names their scope.""" - importer = _Importer(Path(dest), include_scopes, progress) + ``dest``. Its steps' scoped fields are left out unless ``include_scopes`` names their scope. An image whose store id + ``image_refs`` maps to a registry reference pinned by digest is written as that reference, which a run pulls, with + its build context beside it.""" + importer = _Importer(Path(dest), include_scopes, progress, image_refs) task = Task.get(task_id, version) importer.add_task(task) return importer.write(f"Imported from the store's task {task.id} v{task.version}.", {"task": task.id, @@ -86,11 +88,11 @@ def import_task(task_id: str, dest: Path | str, *, version: int | None = None, def import_eval(eval_id: str, dest: Path | str, *, version: int | None = None, - include_scopes: Collection[FieldScope] = (), + include_scopes: Collection[FieldScope] = (), image_refs: Mapping[str, str] | None = None, progress: Callable[[str], None] | None = None) -> BundleImport: """Write the stored eval ``eval_id``, at ``version`` or its latest, the tasks it names and every entity they use as - a bundle, leaving out their steps' scoped fields as ``import_task`` does.""" - importer = _Importer(Path(dest), include_scopes, progress) + a bundle, leaving out their steps' scoped fields and writing ``image_refs`` as ``import_task`` does.""" + importer = _Importer(Path(dest), include_scopes, progress, image_refs) stored = Eval.get(eval_id, version) names, seen = [], set() for entry in stored.tasks: @@ -155,9 +157,11 @@ def left_out(self, field: str, why: str) -> None: class _Importer: def __init__(self, dest: Path, include_scopes: Collection[FieldScope] = (), - progress: Callable[[str], None] | None = None): + progress: Callable[[str], None] | None = None, image_refs: Mapping[str, str] | None = None): self.dest = Path(os.path.abspath(dest.expanduser())) # so ``.`` has a name, and a sibling to stage in self.include_scopes = frozenset({FieldScope.TASK, *include_scopes}) + self.image_refs = dict(image_refs or {}) # the registry image to write for an image, by its store id + self.refs_used: set[str] = set() self.progress = progress or (lambda message: None) self.files: dict[str, str] = {} self.taken: dict[str, set[str]] = {kind.value: set() for kind in BundleKind} # folded names, per folder @@ -259,11 +263,23 @@ def local(self, kind: EntityKind, entity: Any, where: str) -> str | None: def image(self, out: _EntityOut, artifact: DockerImageArtifact, key: str) -> str | dict: """The toml value of an image a folder is built from: its stored context, pruned into the folder, else its - registry ref, else a pinned store image.""" + registry ref, else a pinned store image. Given a registry reference for it, that reference, with the context + kept beside it when it has one.""" + ref = self.image_refs.get(artifact.id) + if ref is not None: + self.refs_used.add(artifact.id) + if problem := _given_ref_problem(ref): + self.problems.append(f"{out.folder}: {key}: the reference given for {artifact.id}, {ref!r}, {problem}") + return {"ref": ref} if artifact.build_context_object_url: - job = ImageJob(artifact, out.folder, key, {"dockerfile": artifact.dockerfile_path or ""}, out.toml_name) + toml = {"ref": ref} if ref is not None else {} + job = ImageJob(artifact, out.folder, key, {**toml, "dockerfile": artifact.dockerfile_path or ""}, + out.toml_name, ref=ref) out.images.append(job) return job.toml + if ref is not None: + out.pulled.append(Pulled(out.folder, key, ref)) + return {"ref": ref} if names_registry(artifact.image_name): out.pulled.append(Pulled(out.folder, key, artifact.image_name)) return {"ref": artifact.image_name} @@ -274,6 +290,8 @@ def image(self, out: _EntityOut, artifact: DockerImageArtifact, key: str) -> str def write(self, description: str, source: dict) -> BundleImport: """Write the folder, with ``description`` as its README's first line, and say what was written.""" + if unused := sorted(self.image_refs.keys() - self.refs_used): + self.problems.append(f"a reference was given for {', '.join(unused)}, which the import writes no image of") if self.problems: raise BundleError(self.problems) staging = self.dest.parent / f".{self.dest.name}.import-{uuid.uuid4().hex[:8]}" @@ -317,7 +335,7 @@ def write(self, description: str, source: dict) -> BundleImport: def _download(self, staging: Path, scratch: Path) -> int: """Download every file and image the folder holds into ``staging``, saying how much first; return the bytes.""" groups = context_groups(self.jobs) - saved = [job.artifact.tar_gz_object_url for job in self.jobs if job.artifact.tar_gz_object_url] + saved = [job.artifact.tar_gz_object_url for job in self.jobs if job.artifact.tar_gz_object_url and not job.ref] with ThreadPoolExecutor(_DOWNLOADS) as pool: sizes = [*pool.map(_size, self.downloads.values()), *pool.map(_saved_size, saved)] total = sum(size or 0 for size in sizes) + sum(group[0].size or 0 for group in groups) @@ -338,12 +356,17 @@ def _download(self, staging: Path, scratch: Path) -> int: else: self.images.append(WrittenImage(job.folder, job.key, job.artifact.id, job.artifact.version, found=not job.artifact.dockerfile_path, pruned=job.pruned, - locked=job.locked, unlocked=job.unlocked)) + locked=job.locked, unlocked=job.unlocked, ref=job.ref)) return total def _image_left_in_store(self, job: ImageJob) -> None: """An image whose context the folder can't hold: its registry ref when it names one, else a problem, or a pinned - store image when the import keeps one.""" + store image when the import keeps one; one given a registry reference, that reference.""" + if job.ref is not None: + job.toml.clear() + job.toml["ref"] = job.ref + self.pulled.append(Pulled(job.folder, job.key, job.ref, job.problem)) + return if names_registry(job.artifact.image_name): job.toml.clear() job.toml["ref"] = job.artifact.image_name @@ -519,6 +542,15 @@ def _kind_of(entity: Any) -> EntityKind: raise TypeError(f"{entity!r} is no env, agent or artifact") +def _given_ref_problem(ref: str) -> str | None: + """Why ``ref``, given for an image, can't be written: it must name its registry and pin a digest.""" + if not isinstance(ref, str) or not names_registry(ref): + return "doesn't name its registry" + if "@sha256:" not in ref: + return "doesn't pin a digest, as /:@sha256: does" + return None + + def _size(url: str) -> int | None: metadata = get_config().get_object_store_at(url).get_object_metadata_at(url) return metadata.size if metadata is not None else None diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index 715a3f098..a491a84e9 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -133,8 +133,9 @@ def digest(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Digest | "files": {}, "needs": {}, "store_refs": {}} from_folder = write.kind in (BundleKind.ARTIFACT, BundleKind.SKILL) or ( write.kind is BundleKind.ENV and env_folder_writer(get_env_registry().get(_type(write)))) - if from_folder and not isinstance(write.source, ReferencedImage): - built = isinstance(write.source, BuiltImage) + built = isinstance(write.source, BuiltImage) or ( + isinstance(write.source, ReferencedImage) and write.source.dockerfile is not None) + if from_folder and (built or not isinstance(write.source, ReferencedImage)): listing = build_context_files if built else folder_files for key, path in listing(self._plan.bundle_of(write.source.entry), write.source.entry).items(): inputs["files"][key] = _file_sha256(path) + (_mode(path) if built else "") @@ -256,6 +257,8 @@ def _config(write: Write) -> Any: if not isinstance(source, BundleImage): return source.config config = {"dockerfile": source.dockerfile} if isinstance(source, BuiltImage) else {"ref": source.ref} + if isinstance(source, ReferencedImage) and source.dockerfile is not None: + config["dockerfile"] = source.dockerfile if source.description is not None: # only an image folder's, so an image key's digest is as before config["description"] = source.description return config diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index 4366fa053..f8db7387c 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -244,22 +244,30 @@ def _tail(message: str) -> str: def _write_referenced_image(plan: Plan, write: Write) -> int: """Register the registry image an entry's image key or image folder names, its tag pinned to the digest the - registry serves now. Written again with the same ref, as for a new description, it keeps the digest it has.""" + registry serves now, with the folder kept as its build context when it names a Dockerfile. Written again with the + same ref, as for a new description, it keeps the digest it has.""" + image, entry = write.source, write.source.entry try: - return DockerImageArtifact.put_ref( - write.id, description=_image_description(plan, write), image_name=write.source.ref, - earlier_pin=_latest_image_name(write.id)).version + if image.dockerfile is None: + return DockerImageArtifact.put_ref( + write.id, description=_image_description(plan, write), image_name=image.ref, + earlier_pin=_latest_pin(write.id)).version + with staged_build_context(plan.bundle_of(entry), entry, image.dockerfile) as staged: + return DockerImageArtifact.put_ref( + write.id, description=_image_description(plan, write), image_name=image.ref, + earlier_pin=_latest_pin(write.id), context_path=str(staged.path), + dockerfile_path=staged.dockerfile).version except ValueError as e: # the registry's answer, or a registry on this machine the image's store can't name raise BundleError([f"{_path(plan, write)}: {e}"]) from None -def _latest_image_name(image_id: str) -> str | None: - """The image name ``image_id``'s latest version records, when that version is an image: a folder can change type.""" +def _latest_pin(image_id: str) -> str | None: + """What ``image_id``'s latest version pulls, when that version is an image: a folder can change type.""" try: latest = Artifact.get(image_id) except NotFoundError: return None - return latest.image_name if isinstance(latest, DockerImageArtifact) else None + return latest.pinned_name if isinstance(latest, DockerImageArtifact) else None def _image_description(plan: Plan, write: Write) -> str: diff --git a/src/agent_env/bundle/resolve.py b/src/agent_env/bundle/resolve.py index 5b7e59e1b..828f3b71d 100644 --- a/src/agent_env/bundle/resolve.py +++ b/src/agent_env/bundle/resolve.py @@ -61,9 +61,11 @@ class BuiltImage(BundleImage): @dataclass(frozen=True) class ReferencedImage(BundleImage): - """An image already in a registry, which the key names as ``{ ref = "" }``.""" + """An image already in a registry, which the key names as ``{ ref = "" }``; with ``dockerfile``, the folder + is kept beside it as its build context, which a deploy builds only when it can't pull the image.""" ref: str + dockerfile: str | None = None @dataclass(frozen=True) @@ -251,17 +253,15 @@ def _task(self, entry: BundleEntry) -> ResolvedEntry: def _image_folder(self, entry: BundleEntry) -> bool: """Record the image a docker_image artifact folder is: built from its Dockerfile, or the registry image its - artifact.toml's ref names. False for any other entry.""" + artifact.toml's ref names, with the folder as its build context when it holds a Dockerfile. False for any + other entry.""" cls = get_artifact_registry().get(canonical_type(entry.type)) if entry.kind is BundleKind.ARTIFACT else None if cls is not DockerImageArtifact: return False ctx = AuthoringContext(self.bundle, entry) fields, problems = ctx.accepted(entry.config, description=str, ref=str) ref, built = fields.get("ref"), (entry.path / DOCKERFILE).is_file() - if ref is not None and built: - problems.append(ctx.config_problem(f"ref names a registry image, so this folder builds none; remove ref " - f"or its {DOCKERFILE}")) - elif ref is not None and (problem := _registry_problem("ref", ref)): + if ref is not None and (problem := _registry_problem("ref", ref)): problems.append(ctx.config_problem(problem)) elif "ref" not in entry.config and not built: problems.append(f"an image folder needs a {DOCKERFILE}, or ref = \"\" in its artifact.toml") @@ -270,8 +270,8 @@ def _image_folder(self, entry: BundleEntry) -> bool: self._problem(entry, problem) return True description = fields.get("description") - self.images.append(ReferencedImage(entry.id, entry, ref, description=description) if ref is not None - else BuiltImage(entry.id, entry, DOCKERFILE, description=description)) + self.images.append(ReferencedImage(entry.id, entry, ref, DOCKERFILE if built else None, description=description) + if ref is not None else BuiltImage(entry.id, entry, DOCKERFILE, description=description)) return True # Parts of a config @@ -292,8 +292,9 @@ def _toml_class(self, entry: BundleEntry) -> type: def _image(self, entry: BundleEntry, config: dict, ref: EntityRef, references: list[Reference]) -> bool: """Record the image an image key has the bundle write: the folder's Dockerfile when a top-level ``image`` is - left out, ``{ dockerfile = "" }``, or a registry image, ``{ ref = "" }``. A key in a table, such as - a computer's ``forms.container.image``, says which. False when the key names an image artifact instead.""" + left out, ``{ dockerfile = "" }``, or a registry image, ``{ ref = "" }``, with the folder kept as + its build context when it also gives a ``dockerfile``. A key in a table, such as a computer's + ``forms.container.image``, says which. False when the key names an image artifact instead.""" if ref.artifact_type != "docker_image" or "[]" in ref.path: return False *tables, key = ref.path.split(".") @@ -305,14 +306,18 @@ def _image(self, entry: BundleEntry, config: dict, ref: EntityRef, references: l value = owner.get(key) role = f"{entry.kind.store}_image" if ref.path == "image" else ref.path.replace(".", "_") if isinstance(value, dict) and "ref" in value: - name = value["ref"] - if value.keys() != {"ref"}: - extra = ", ".join(sorted(value.keys() - {"ref"})) - self._problem(entry, f"{ref.path}: a registry image takes only ref, not {extra}") + name, dockerfile = value["ref"], value.get("dockerfile") + if extra := value.keys() - {"ref", "dockerfile"}: + self._problem(entry, f"{ref.path}: a registry image takes ref and dockerfile, not " + f"{', '.join(sorted(extra))}") elif problem := _registry_problem(ref.path, name): self._problem(entry, problem) + elif dockerfile is not None and not _folder_file(entry, dockerfile): + self._problem(entry, f"{ref.path}: there is no {dockerfile!r} in this folder to keep as its build " + "context") else: - self._implied(ReferencedImage(derive_id(entry.id, role), entry, name), owner, key, ref, references) + self._implied(ReferencedImage(derive_id(entry.id, role), entry, name, dockerfile), owner, key, ref, + references) return True if value is None: dockerfile = default_dockerfile(ref.path) @@ -324,9 +329,7 @@ def _image(self, entry: BundleEntry, config: dict, ref: EntityRef, references: l return True else: return False - path = entry.path / dockerfile if isinstance(dockerfile, str) and dockerfile else None - inside = path is not None and not Path(dockerfile).is_absolute() and path.resolve().is_relative_to(entry.path.resolve()) - if not (inside and path.is_file()): + if not _folder_file(entry, dockerfile): self._problem(entry, f"{ref.path}: there is no {dockerfile!r} in this folder to build") return True self._implied(BuiltImage(derive_id(entry.id, role), entry, dockerfile), owner, key, ref, references) @@ -482,6 +485,14 @@ def entity_class(entry: BundleEntry) -> type | None: return get_artifact_registry().get(canonical_type(entry.type)) +def _folder_file(entry: BundleEntry, name: Any) -> bool: + """Whether ``name`` is a relative path to a file inside ``entry``'s folder.""" + if not isinstance(name, str) or not name or Path(name).is_absolute(): + return False + path = entry.path / name + return path.resolve().is_relative_to(entry.path.resolve()) and path.is_file() + + def _registry_problem(key: str, name: Any) -> str | None: """Why ``name``, a registry image ``key`` gives, can't be pulled as written.""" if isinstance(name, str) and names_registry(name): diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index b3b8e11be..4dc89a175 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -168,6 +168,8 @@ def images(self) -> tuple[str, ...]: return tuple( f"{_label(plan, done.write)} v{done.version}: " + ("pulled from its registry, its tag pinned to a digest when it's written" + + (", with its build context beside it, which a deploy builds only when it can't pull it" + if done.write.source.dockerfile else "") if isinstance(done.write.source, ReferencedImage) else f"{plan.published[done.write.id].ref}, published for its sources, which each deploy pulls by its " "digest, or builds from its build context when it can't" @@ -578,7 +580,7 @@ def _pinned(done: Materialized) -> str: if not isinstance(done.write.source, ReferencedImage) or "@" in done.write.source.ref: return "" try: - name = DockerImageArtifact.get(done.write.id, done.version).image_name + name = DockerImageArtifact.get(done.write.id, done.version).pinned_name except NotFoundError: # a dry run's image, not yet written return "" algorithm, _, digest = name.rpartition("@")[2].partition(":") diff --git a/src/agent_env/cli/bundle.py b/src/agent_env/cli/bundle.py index 5277f11e5..74c85641c 100644 --- a/src/agent_env/cli/bundle.py +++ b/src/agent_env/cli/bundle.py @@ -29,8 +29,12 @@ def bundle(): type=click.Choice([scope.value for scope in FieldScope if scope is not FieldScope.TASK]), help="Keep the fields of this scope as stored, and leave in the store what the folder can't hold, for a " "folder run against the same store. Repeatable.") +@click.option("--image-ref", "image_refs", multiple=True, metavar="ID=REF", + help="Write the stored image ID as the registry image REF, pinned by its digest " + "(/:@sha256:), which a run pulls, with its build context beside it. " + "Repeatable.") def init(folder: Path, task_id: str | None, eval_id: str | None, version: int | None, - include_scopes: tuple[str, ...]): + include_scopes: tuple[str, ...], image_refs: tuple[str, ...]): """Write a stored task, or an eval and its tasks, as a bundle at FOLDER, which must be new or empty. Each task is written as the store holds it, and every env, agent and artifact it uses is written into a folder of @@ -42,11 +46,16 @@ def init(folder: Path, task_id: str | None, eval_id: str | None, version: int | import that fails writes nothing.""" if (task_id is None) == (eval_id is None): raise click.UsageError("give one of --from-task or --from-eval") + if bad := [given for given in image_refs if "=" not in given]: + raise click.UsageError(f"--image-ref takes ID=REF, not {bad[0]!r}") scopes = [FieldScope(scope) for scope in include_scopes] + refs = dict(given.split("=", 1) for given in image_refs) with interruptible(): - imported = (import_task(task_id, folder, version=version, include_scopes=scopes, progress=click.echo) + imported = (import_task(task_id, folder, version=version, include_scopes=scopes, image_refs=refs, + progress=click.echo) if task_id is not None - else import_eval(eval_id, folder, version=version, include_scopes=scopes, progress=click.echo)) + else import_eval(eval_id, folder, version=version, include_scopes=scopes, image_refs=refs, + progress=click.echo)) _report(imported, folder) diff --git a/tst/unit/bundle/importer_image_refs_test.py b/tst/unit/bundle/importer_image_refs_test.py new file mode 100644 index 000000000..1824a2a91 --- /dev/null +++ b/tst/unit/bundle/importer_image_refs_test.py @@ -0,0 +1,112 @@ +"""``bundle init --image-ref``: an image given a registry reference pinned by digest is written as that reference, +which a run pulls, with the part of its stored build context its Dockerfile reads kept beside it.""" + +import json +import tomllib + +import pytest +from click.testing import CliRunner + +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.artifact.store import get_artifact_store +from agent_env.bundle import BundleError +from agent_env.bundle.importer import import_task +from agent_env.bundle.materialize import materialize +from agent_env.bundle.resolve import build_step +from agent_env.cli import cli +from agent_env.env import Env +from agent_env.env.envs.mcp_server import MCPServerEnv +from agent_env.task import Task +from tst.unit.bundle._support import context_image, local_builds, plan_of + +REF = "123456789012.dkr.ecr.us-west-2.amazonaws.com/delivery/crm:t1@sha256:" + "c" * 64 +DEPLOY = {"id": "env", "type": "deploy_env", "env_id": "crm"} + + +@pytest.fixture +def home(local_stores, tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + return tmp_path + + +def _stored(image): + MCPServerEnv.put(id="crm", docker_image_artifact=image, environment_name="crm") + Task.put(id="t", steps=[build_step(dict(DEPLOY))]) + + +def _toml(root, path): + return tomllib.loads((root / path).read_text()) + + +def test_an_image_given_a_ref_is_written_as_it_with_its_pruned_context_beside_it(home): + _stored(context_image(home, "crm-image")) + + imported = import_task("t", home / "t", image_refs={"crm-image": REF}) + + assert _toml(imported.root, "envs/crm/env.toml")["image"] == {"ref": REF, "dockerfile": "Dockerfile"} + assert sorted(path.name for path in (imported.root / "envs/crm").iterdir()) == ["Dockerfile", "app.py", "env.toml"] + (image,) = imported.images + assert (image.ref, image.locked, image.unlocked, imported.pulled) == (REF, None, None, ()) + assert dict(imported.report())["Images"] == [ + f"envs/crm: image pulled as {REF}, its build context beside it from Dockerfile, 2 file(s), 61 B; largest: " + "Dockerfile (42 B), app.py (19 B)"] + readme = (imported.root / "README.md").read_text() + assert "An image *Images* says is pulled is pulled by its digest, never built" in readme + assert "Its images are built from the build contexts" not in readme + assert "`123456789012.dkr.ecr.us-west-2.amazonaws.com`" in readme + assert json.loads((imported.root / "provenance.json").read_text())["images"][0]["ref"] == REF + + +def test_an_imported_ref_is_written_by_a_run_as_that_image_pinned_with_its_context_kept(home, registry, builds, + cli_routing): + _stored(context_image(home, "crm-image")) + imported = import_task("t", home / "t", image_refs={"crm-image": REF}) + + plan = plan_of(imported.root) + done = materialize(plan, build_here=local_builds(plan)) + + env = "@local/~/t/crm" + image = Env.get(env, done.version_of("env", env)).docker_image_artifact + assert (image.pinned_name, image.dockerfile_path, bool(image.build_context_object_url)) == (REF, "Dockerfile", True) + assert (registry, builds) == ([REF], []) + + +def test_an_image_with_no_context_given_a_ref_is_written_as_the_ref_alone(home): + _stored(get_artifact_store().put_document(DockerImageArtifact( + id="crm-image", description="d", image_name="crm-image:v1", tar_gz_s3_url="file:///nowhere/crm.tar.gz"))) + + imported = import_task("t", home / "t", image_refs={"crm-image": REF}) + + assert _toml(imported.root, "envs/crm/env.toml")["image"] == {"ref": REF} + assert [(pull.folder, pull.image) for pull in imported.pulled] == [("envs/crm", REF)] + + +@pytest.mark.parametrize("refs, problem", [ + ({"crm-image": REF.split("@")[0]}, + "envs/crm: image: the reference given for crm-image, '" + REF.split("@")[0] + "', doesn't pin a digest, as " + "/:@sha256: does"), + ({"crm-image": "crm@sha256:" + "c" * 64}, "doesn't name its registry"), + ({"crm-image": REF, "other-image": REF}, + "a reference was given for other-image, which the import writes no image of"), +], ids=["no digest", "no registry", "unused"]) +def test_a_ref_that_cant_be_written_fails_the_import_and_writes_nothing(home, refs, problem): + _stored(context_image(home, "crm-image")) + + with pytest.raises(BundleError) as caught: + import_task("t", home / "t", image_refs=refs) + + assert any(problem in found for found in caught.value.problems) + assert not (home / "t").exists() + + +def test_the_cli_takes_each_ref_as_id_equals_ref(home): + _stored(context_image(home, "crm-image")) + runner = CliRunner() + + written = runner.invoke(cli, ["bundle", "init", "--from-task", "t", "--image-ref", f"crm-image={REF}", + str(home / "t")]) + refused = runner.invoke(cli, ["bundle", "init", "--from-task", "t", "--image-ref", REF, str(home / "u")]) + + assert written.exit_code == 0, written.output + assert _toml(home / "t", "envs/crm/env.toml")["image"]["ref"] == REF + assert refused.exit_code == 2 and "--image-ref takes ID=REF" in refused.output diff --git a/tst/unit/bundle/referenced_context_test.py b/tst/unit/bundle/referenced_context_test.py new file mode 100644 index 000000000..29ef2b41e --- /dev/null +++ b/tst/unit/bundle/referenced_context_test.py @@ -0,0 +1,107 @@ +"""A registry image with its build context beside it: written as the image the registry serves, pinned by digest, +never built, with the folder kept as its context, which a deploy builds only when it can't pull the image.""" + +import io +import json +import tarfile + +import pytest + +from agent_env.a2a_agent import A2AAgent +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.bundle import dry_run_bundle +from agent_env.bundle.materialize import materialize +from agent_env.config import get_config +from agent_env.env import Env +from tst.unit.bundle._support import DIGEST, layout, local_builds, plan_of + +ROOT = "@local/~/triage" +IMAGE = f"{ROOT}/crm-image" +WEB = f"{ROOT}/web" +PINNED = "ghcr.io/acme/web:1@sha256:" + "b" * 64 +FOLDER = { + "artifacts/crm-image/Dockerfile": "FROM scratch\nCOPY server.py /app/\n", + "artifacts/crm-image/server.py": "print('crm')\n", + "artifacts/crm-image/artifact.toml": 'type = "docker_image"\nref = "ghcr.io/acme/crm:1"\n', + "envs/crm/env.toml": 'image = "crm-image"\nenvironment_name = "crm"\n', + "agents/solver/agent.toml": 'image = "crm-image"\n', + "tasks/t.json": json.dumps([ + {"id": "crm", "type": "deploy_env", "env_id": "crm"}, + {"id": "agent", "type": "deploy_agent", "env_ids": [], "a2a_agent_id": "solver"}, + ]), +} +WEB_FILES = { + "envs/web/Dockerfile": "FROM scratch\nCOPY server.py /app/\n", + "envs/web/server.py": "print('web')\n", + "envs/web/env.toml": f'environment_name = "web"\nimage = {{ ref = "{PINNED}", dockerfile = "Dockerfile" }}\n', + "tasks/t.json": json.dumps([{"id": "web", "type": "deploy_env", "env_id": "web"}]), +} + + +@pytest.fixture +def bundle_dir(tmp_path, monkeypatch, local_stores, cli_routing): + monkeypatch.setenv("HOME", str(tmp_path)) + return tmp_path / "triage" + + +def _run(root): + plan = plan_of(root) + return materialize(plan, build_here=local_builds(plan)) + + +def _context(image): + url = image.build_context_object_url + with get_config().get_object_store_at(url).open(url) as stream: + data = stream.read() + with tarfile.open(fileobj=io.BytesIO(data)) as tar: + return sorted(member.name.removeprefix("./") for member in tar.getmembers() if member.isfile()) + + +def test_an_image_folder_naming_a_registry_image_keeps_its_dockerfile_as_its_context_and_builds_nothing( + bundle_dir, builds, registry): + layout(bundle_dir, FOLDER) + + done = _run(bundle_dir) + + image = DockerImageArtifact.get(IMAGE) + assert (image.image_name, image.image_digest, image.pinned_name, image.dockerfile_path) == ( + "ghcr.io/acme/crm:1", DIGEST, f"ghcr.io/acme/crm:1@{DIGEST}", "Dockerfile") + assert _context(image) == ["Dockerfile", "artifact.toml", "server.py"] + env = Env.get(f"{ROOT}/crm", done.version_of("env", f"{ROOT}/crm")).docker_image_artifact + agent = A2AAgent.get(f"{ROOT}/solver", done.version_of("agent", f"{ROOT}/solver")).docker_image_artifact + assert [(each.id, each.version) for each in (env, agent)] == [(IMAGE, 1)] * 2 + assert (builds, registry) == ([], ["ghcr.io/acme/crm:1"]) + + +def test_a_rerun_reuses_it_and_an_edit_to_its_context_rewrites_it_keeping_the_digest_it_was_pinned_to( + bundle_dir, builds, registry): + layout(bundle_dir, FOLDER) + _run(bundle_dir) + rerun = {done.write.id: (done.version, done.reused) for done in _run(bundle_dir).writes} + (bundle_dir / "artifacts/crm-image/server.py").write_text("print('crm v2')\n") + + edited = {done.write.id: (done.version, done.reused) for done in _run(bundle_dir).writes} + + assert (rerun[IMAGE], edited[IMAGE]) == ((1, True), (2, False)) + assert DockerImageArtifact.get(IMAGE).pinned_name == f"ghcr.io/acme/crm:1@{DIGEST}" + assert (builds, registry) == ([], ["ghcr.io/acme/crm:1"]) + + +def test_an_image_key_naming_a_digest_and_a_dockerfile_records_that_digest_and_the_folder_as_its_context( + bundle_dir, builds, registry): + layout(bundle_dir, WEB_FILES) + + done = _run(bundle_dir) + + image = Env.get(WEB, done.version_of("env", WEB)).docker_image_artifact + assert (image.id, image.image_name, image.pinned_name) == (f"{WEB}__env_image", "ghcr.io/acme/web:1", PINNED) + assert _context(image) == ["Dockerfile", "env.toml", "server.py"] + assert (builds, registry) == ([], [PINNED]) + + +def test_a_dry_run_says_the_image_is_pulled_and_its_context_kept(bundle_dir, docker_on_path): + layout(bundle_dir, WEB_FILES) + + assert dry_run_bundle(bundle_dir, sandbox="local").images() == ( + f"envs/web ({PINNED} image) v1: pulled from its registry, its tag pinned to a digest when it's written, with " + "its build context beside it, which a deploy builds only when it can't pull it",) diff --git a/tst/unit/bundle/resolve_test.py b/tst/unit/bundle/resolve_test.py index d8440f6a0..7505fa9b4 100644 --- a/tst/unit/bundle/resolve_test.py +++ b/tst/unit/bundle/resolve_test.py @@ -415,6 +415,24 @@ def test_an_image_key_naming_a_registry_image_has_the_bundle_reference_it(make): assert refs(judge) == [(EntityKind.ARTIFACT, f"{ROOT}/judge__agent_image", None, "ghcr.io/acme/judge:2")] +def test_a_registry_image_beside_its_dockerfile_keeps_the_folder_as_its_build_context(make): + result = resolve_bundle(make(files={ + "envs/web/Dockerfile": "FROM scratch\n", + "envs/web/env.toml": 'type = "composite_test"\nimage = { ref = "ghcr.io/acme/web:1@sha256:' + "a" * 64 + '", ' + 'dockerfile = "Dockerfile" }\nbackend_image = { ref = "ghcr.io/acme/api:1" }\n', + "artifacts/crm/Dockerfile": "FROM scratch\n", + "artifacts/crm/artifact.toml": 'ref = "ghcr.io/acme/crm:2"\n', + })) + web = next(e for e in result.entries if e.entry.name == "web") + crm = next(entry for entry in result.bundle.entries if entry.name == "crm") + + assert web.config["image"] == f"{ROOT}/web__env_image" + assert [image for image in result.images if image.entry in (web.entry, crm)] == [ + ReferencedImage(f"{ROOT}/web__env_image", web.entry, "ghcr.io/acme/web:1@sha256:" + "a" * 64, "Dockerfile"), + ReferencedImage(f"{ROOT}/web__backend_image", web.entry, "ghcr.io/acme/api:1"), + ReferencedImage(f"{ROOT}/crm", crm, "ghcr.io/acme/crm:2", "Dockerfile"), + ] + def test_an_image_folder_is_the_image_built_from_its_dockerfile_or_the_registry_image_its_toml_names(make): result = resolve_bundle(make(files={ "artifacts/base-mcp/artifact.toml": 'description = "the shared server image"\n', @@ -437,9 +455,6 @@ def test_an_image_folder_is_the_image_built_from_its_dockerfile_or_the_registry_ @pytest.mark.parametrize("files, problem", [ ({"artifacts/img/artifact.toml": 'type = "docker_image"\n'}, 'artifacts/img: an image folder needs a Dockerfile, or ref = "" in its artifact.toml'), - ({"artifacts/img/Dockerfile": "FROM scratch\n", "artifacts/img/artifact.toml": 'ref = "ghcr.io/acme/img:1"\n'}, - "artifacts/img: artifact.toml: ref names a registry image, so this folder builds none; remove ref or its " - "Dockerfile"), ({"artifacts/img/artifact.toml": 'type = "docker_image"\nref = "img:1"\n'}, "artifacts/img: artifact.toml: ref: 'img:1' doesn't name its registry; spell it out, as in " "docker.io/library/img:1"), @@ -448,7 +463,7 @@ def test_an_image_folder_is_the_image_built_from_its_dockerfile_or_the_registry_ "and id"), ({"artifacts/img/Dockerfile": "FROM scratch\n", "artifacts/img/artifact.toml": "description = 1\n"}, "artifacts/img: artifact.toml: description must be a string, not 1"), -], ids=["nothing to build", "both", "no registry", "unknown key", "description type"]) +], ids=["nothing to build", "no registry", "unknown key", "description type"]) def test_an_image_folder_its_toml_cant_describe_is_a_problem(make, files, problem): assert problem in problems(make(files=files)) @@ -470,7 +485,9 @@ def test_an_agent_names_a_store_image_or_builds_its_folder(make): ('image = "base-mcp"\nbackend_image = { dockerfile = "Dockerfile.missing" }', "backend_image: there is no 'Dockerfile.missing' in " "this folder to build"), ('image = { ref = "ghcr.io/acme/web:1", platform = "linux/amd64" }', - "image: a registry image takes only ref, not platform"), + "image: a registry image takes ref and dockerfile, not platform"), + ('image = { ref = "ghcr.io/acme/web:1", dockerfile = "Dockerfile.web" }', + "image: there is no 'Dockerfile.web' in this folder to keep as its build context"), ('image = { ref = "web:1" }', "image: 'web:1' doesn't name its registry; spell it out, as in docker.io/library/web:1"), ('image = "greeting"', "image: 'greeting' is this bundle's file_artifact_universe, but this field takes " From d24f27950be7f41436a3c8cb5fdb1d1b282b69fa Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 29 Oct 2026 22:08:51 -0700 Subject: [PATCH 2/3] fix(bundle): leave a Dockerfile a pulled image shares unpinned, and refuse what can't be written - A Dockerfile an image given a ref shares with another image is no longer pinned to the other image's versions, since a deploy that can't pull the ref'd image rebuilds it from that file. - A context in a store this config can't read no longer aborts the import: its download says why, and an image given a ref is written as the ref alone. - A given ref must pin a whole sha256 digest (64 hex digits). - The README gives the build paragraph only when an image was written from its context, and the pulled one whenever an image is pulled, ref-only ones included. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/_images.py | 18 ++++++++--- src/agent_env/bundle/_import_report.py | 10 +++---- src/agent_env/bundle/importer.py | 9 ++++-- tst/unit/bundle/importer_entities_test.py | 21 +++++++++++++ tst/unit/bundle/importer_image_refs_test.py | 33 +++++++++++++++++++-- 5 files changed, 77 insertions(+), 14 deletions(-) diff --git a/src/agent_env/bundle/_images.py b/src/agent_env/bundle/_images.py index 2935b1f8d..1b7717142 100644 --- a/src/agent_env/bundle/_images.py +++ b/src/agent_env/bundle/_images.py @@ -39,11 +39,15 @@ class ImageJob: def context_groups(jobs: list[ImageJob]) -> list[list[ImageJob]]: """``jobs`` grouped by the stored context they're built from: the same object when the store tells its content - (an ETag), else the same URL.""" + (an ETag), else the same URL. A context in a store this config can't read is grouped by its URL, and its download + says why it can't be written.""" groups: dict[tuple, list[ImageJob]] = {} for job in jobs: url = job.artifact.build_context_object_url - metadata = get_config().get_object_store_at(url).get_object_metadata_at(url) + try: + metadata = get_config().get_object_store_at(url).get_object_metadata_at(url) + except ValueError: + metadata = None job.size = metadata.size if metadata is not None else None key = ("etag", metadata.etag, metadata.size) if metadata is not None and metadata.etag else ("url", url) groups.setdefault(key, []).append(job) @@ -90,9 +94,15 @@ def write_images(groups: list[list[ImageJob]], staging: Path, scratch: Path, # Pinning rewrites a Dockerfile, which another image placed in the same folder may hold too: so only once all are. sharing: dict[tuple[str, str], list[ImageJob]] = {} for job in placed: - if job.ref is None: # a run pulls an image given its ref, so nothing pins its context's installs - sharing.setdefault((job.folder, job.pruned.dockerfile), []).append(job) + sharing.setdefault((job.folder, job.pruned.dockerfile), []).append(job) for (folder, dockerfile), jobs in sharing.items(): + if pulled := [job.key for job in jobs if job.ref is not None]: + # A run pulls an image given its ref, and a deploy that can't rebuilds it from this Dockerfile as written. + for job in jobs: + if job.ref is None: + job.unlocked = (f"its Dockerfile is also the build context of {', '.join(pulled)}, which a run " + "pulls, so a build resolves them afresh") + continue _lock(jobs, staging / folder, dockerfile) diff --git a/src/agent_env/bundle/_import_report.py b/src/agent_env/bundle/_import_report.py index 59b4c4af9..c1468766f 100644 --- a/src/agent_env/bundle/_import_report.py +++ b/src/agent_env/bundle/_import_report.py @@ -172,10 +172,10 @@ def readme(description: str, imported: BundleImport) -> str: held = ("Every env, agent and artifact it uses is written here, so `agent-env run` runs it with no store" + (", except what *Still needs the store* lists, which it reads from the store this was imported from." if imported.pinned else ".")) - built = not imported.images or any(image.ref is None for image in imported.images) - pulled = [_PULLED_IMAGES] if any(image.ref for image in imported.images) else [] + built = [_BUILT_IMAGES] if any(image.ref is None for image in imported.images) else [] + pulled = [_PULLED_IMAGES] if imported.pulled or any(image.ref for image in imported.images) else [] lines = [description, "", f"Imported with agent-env {package_version('agentenv-framework')} on {stamp}. {held}", "", - *_RUN_IT, *_needs_lines(imported), "", *([_BUILT_IMAGES] if built else []), *pulled, "", *_SHARE_IT, ""] + *_RUN_IT, *_needs_lines(imported), "", *built, *pulled, "", *_SHARE_IT, ""] contents = _sections([("Tasks", [f"{task.path}: task {task.id} v{task.version}" for task in imported.tasks]), ("Eval", [f"{imported.eval.path}: eval {imported.eval.id} v{imported.eval.version}"] if imported.eval else [])]) @@ -236,8 +236,8 @@ def _listed(items: list[str]) -> str: "Python and npm versions of the images the store held, as *Images* says. To build one unpinned, take out the " f"COPY and two ARG lines naming {posixpath.basename(IN_IMAGE)} after each FROM in its Dockerfile.") _PULLED_IMAGES = ( - "An image *Images* says is pulled is pulled by its digest, never built; the build context beside it is there to " - "read or rebuild, and a deploy builds it only when it can't pull it.") + "An image *Images* says is pulled comes from its registry and is never built here. A build context beside one is " + "there to read or rebuild, and a deploy builds from it only when it can't pull the image.") _SHARE_IT = [ "## Share it", "", diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index 294791b8e..40c499bdc 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -6,6 +6,7 @@ import contextlib import json import os +import re import shutil import unicodedata import uuid @@ -542,12 +543,16 @@ def _kind_of(entity: Any) -> EntityKind: raise TypeError(f"{entity!r} is no env, agent or artifact") +# A reference pinned to a whole sha256 digest, at its end. +_PINNED = re.compile(r"@sha256:[0-9a-f]{64}$") + + def _given_ref_problem(ref: str) -> str | None: """Why ``ref``, given for an image, can't be written: it must name its registry and pin a digest.""" if not isinstance(ref, str) or not names_registry(ref): return "doesn't name its registry" - if "@sha256:" not in ref: - return "doesn't pin a digest, as /:@sha256: does" + if not _PINNED.search(ref): + return "doesn't pin a digest, as /:@sha256:<64 hex digits> does" return None diff --git a/tst/unit/bundle/importer_entities_test.py b/tst/unit/bundle/importer_entities_test.py index 64bc0a58a..a57291202 100644 --- a/tst/unit/bundle/importer_entities_test.py +++ b/tst/unit/bundle/importer_entities_test.py @@ -443,6 +443,27 @@ def test_two_images_of_one_folder_built_from_one_context_are_both_written_and_pi assert (imported.pulled, imported.pinned) == ((), ()) +def test_a_dockerfile_an_image_given_a_ref_shares_is_left_unpinned_for_the_image_beside_it(home): + """The pulled image's context is that Dockerfile, which a deploy that can't pull it builds as written.""" + dockerfile = "FROM python:3.12-slim\nRUN pip install requests\nCOPY app.py /app.py\n" + context = context_image(home, "web-context", {"Dockerfile": dockerfile, "app.py": "app\n"}) + site = "usr/local/lib/python3.12/site-packages" + images = [_stored_image(home, f"web-{side}", context, "Dockerfile", + {f"{site}/requests-2.31.0.dist-info/METADATA": "Name: requests\nVersion: 2.31.0\n"}) + for side in ("backend", "frontend")] + WebsiteEnv.put(id="web", backend_docker_image_artifact=images[0], frontend_docker_image_artifact=images[1], + environment_name="web") + _task([{"id": "env", "type": "deploy_env", "env_id": "web"}]) + ref = "ghcr.io/acme/web-backend:1@sha256:" + "d" * 64 + + imported = import_task("t", home / "t", image_refs={"web-backend": ref}) + + assert (imported.root / "envs/web/Dockerfile").read_text() == dockerfile + assert [(image.key, image.ref, image.locked, image.unlocked) for image in imported.images] == [ + ("backend_image", ref, None, None), + ("frontend_image", None, None, "its Dockerfile is also the build context of backend_image, which a run pulls, " + "so a build resolves them afresh")] + @pytest.mark.parametrize("registry", [True, False], ids=["names-a-registry", "names-none"]) def test_an_image_whose_stored_context_is_gone_is_pulled_by_its_ref_or_fails_naming_it_or_stays_pinned(home, registry): diff --git a/tst/unit/bundle/importer_image_refs_test.py b/tst/unit/bundle/importer_image_refs_test.py index 1824a2a91..a4af15e59 100644 --- a/tst/unit/bundle/importer_image_refs_test.py +++ b/tst/unit/bundle/importer_image_refs_test.py @@ -14,6 +14,7 @@ from agent_env.bundle.materialize import materialize from agent_env.bundle.resolve import build_step from agent_env.cli import cli +from agent_env.config import get_config from agent_env.env import Env from agent_env.env.envs.mcp_server import MCPServerEnv from agent_env.task import Task @@ -51,7 +52,7 @@ def test_an_image_given_a_ref_is_written_as_it_with_its_pruned_context_beside_it f"envs/crm: image pulled as {REF}, its build context beside it from Dockerfile, 2 file(s), 61 B; largest: " "Dockerfile (42 B), app.py (19 B)"] readme = (imported.root / "README.md").read_text() - assert "An image *Images* says is pulled is pulled by its digest, never built" in readme + assert "An image *Images* says is pulled comes from its registry and is never built here." in readme assert "Its images are built from the build contexts" not in readme assert "`123456789012.dkr.ecr.us-west-2.amazonaws.com`" in readme assert json.loads((imported.root / "provenance.json").read_text())["images"][0]["ref"] == REF @@ -84,11 +85,14 @@ def test_an_image_with_no_context_given_a_ref_is_written_as_the_ref_alone(home): @pytest.mark.parametrize("refs, problem", [ ({"crm-image": REF.split("@")[0]}, "envs/crm: image: the reference given for crm-image, '" + REF.split("@")[0] + "', doesn't pin a digest, as " - "/:@sha256: does"), + "/:@sha256:<64 hex digits> does"), + ({"crm-image": REF.split("@")[0] + "@sha256:"}, "doesn't pin a digest"), + ({"crm-image": REF[:-1]}, "doesn't pin a digest"), + ({"crm-image": REF[:-1] + "X"}, "doesn't pin a digest"), ({"crm-image": "crm@sha256:" + "c" * 64}, "doesn't name its registry"), ({"crm-image": REF, "other-image": REF}, "a reference was given for other-image, which the import writes no image of"), -], ids=["no digest", "no registry", "unused"]) +], ids=["no digest", "empty digest", "short digest", "not hex", "no registry", "unused"]) def test_a_ref_that_cant_be_written_fails_the_import_and_writes_nothing(home, refs, problem): _stored(context_image(home, "crm-image")) @@ -99,6 +103,29 @@ def test_a_ref_that_cant_be_written_fails_the_import_and_writes_nothing(home, re assert not (home / "t").exists() +def test_an_image_whose_context_no_configured_store_can_read_is_written_as_its_ref_alone(home, monkeypatch): + _stored(get_artifact_store().put_document(DockerImageArtifact( + id="crm-image", description="d", image_name="crm-image:v1", + build_context_s3_url="ftp://elsewhere/crm/build-context.tar.gz", dockerfile_path="Dockerfile"))) + config_type, store_at = type(get_config()), type(get_config()).get_object_store_at + + def refuse_ftp(self, url): + if url.startswith("ftp://"): + raise ValueError(f"no configured object store reads {url}") + return store_at(self, url) + + monkeypatch.setattr(config_type, "get_object_store_at", refuse_ftp) + + imported = import_task("t", home / "t", image_refs={"crm-image": REF}) + + assert _toml(imported.root, "envs/crm/env.toml")["image"] == {"ref": REF} + assert [(pull.image, pull.why) for pull in imported.pulled] == [ + (REF, "its build context can't be downloaded: the configured stores can't read it")] + readme = (imported.root / "README.md").read_text() + assert "comes from its registry and is never built here" in readme + assert "Its images are built from the build contexts" not in readme + + def test_the_cli_takes_each_ref_as_id_equals_ref(home): _stored(context_image(home, "crm-image")) runner = CliRunner() From 68e436614f7a99949a7e0c1410b3a6d58d320782 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Fri, 30 Oct 2026 08:24:56 -0700 Subject: [PATCH 3/3] feat(bundle): bundle init --image-ref pins a tag to its digest A ref given for an image may now name a tag alone: the import pins it to the digest its registry serves now, with the configured image store's credentials, once per image, and writes that. A ref giving a digest is written as given without asking the registry; one whose digest isn't whole, or a tag its registry can't pin, still fails the import. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/importer.py | 44 +++++++++----- src/agent_env/cli/bundle.py | 4 +- tst/unit/bundle/importer_image_refs_test.py | 66 ++++++++++++++++++--- 3 files changed, 89 insertions(+), 25 deletions(-) diff --git a/src/agent_env/bundle/importer.py b/src/agent_env/bundle/importer.py index 40c499bdc..32aae2675 100644 --- a/src/agent_env/bundle/importer.py +++ b/src/agent_env/bundle/importer.py @@ -64,6 +64,7 @@ from agent_env.store.base import NotFoundError from agent_env.store.ids import LOCAL_PREFIX from agent_env.store.image_store.oci_registry_credentials import names_registry +from agent_env.store.image_store.registry_api import pin_digest from agent_env.task.task import Task from agent_env.task_step.registry import get_task_step_registry from agent_env.utils.paths import os_reason @@ -79,8 +80,8 @@ def import_task(task_id: str, dest: Path | str, *, version: int | None = None, progress: Callable[[str], None] | None = None) -> BundleImport: """Write the stored task ``task_id``, at ``version`` or its latest, and every entity it uses as a bundle at ``dest``. Its steps' scoped fields are left out unless ``include_scopes`` names their scope. An image whose store id - ``image_refs`` maps to a registry reference pinned by digest is written as that reference, which a run pulls, with - its build context beside it.""" + ``image_refs`` maps to a registry reference is written as that reference, which a run pulls, its tag pinned to the + digest its registry serves now when it gives none, with its build context beside it.""" importer = _Importer(Path(dest), include_scopes, progress, image_refs) task = Task.get(task_id, version) importer.add_task(task) @@ -163,6 +164,7 @@ def __init__(self, dest: Path, include_scopes: Collection[FieldScope] = (), self.include_scopes = frozenset({FieldScope.TASK, *include_scopes}) self.image_refs = dict(image_refs or {}) # the registry image to write for an image, by its store id self.refs_used: set[str] = set() + self.refs_pinned: dict[str, str] = {} # each given ref as pinned to its digest, by image store id self.progress = progress or (lambda message: None) self.files: dict[str, str] = {} self.taken: dict[str, set[str]] = {kind.value: set() for kind in BundleKind} # folded names, per folder @@ -264,13 +266,15 @@ def local(self, kind: EntityKind, entity: Any, where: str) -> str | None: def image(self, out: _EntityOut, artifact: DockerImageArtifact, key: str) -> str | dict: """The toml value of an image a folder is built from: its stored context, pruned into the folder, else its - registry ref, else a pinned store image. Given a registry reference for it, that reference, with the context - kept beside it when it has one.""" + registry ref, else a pinned store image. Given a registry reference for it, that reference, its tag pinned to + the digest the registry serves now when it gives none, with the context kept beside it when it has one.""" ref = self.image_refs.get(artifact.id) if ref is not None: self.refs_used.add(artifact.id) - if problem := _given_ref_problem(ref): - self.problems.append(f"{out.folder}: {key}: the reference given for {artifact.id}, {ref!r}, {problem}") + try: + ref = self._pinned(artifact.id, ref) + except ValueError as e: + self.problems.append(f"{out.folder}: {key}: the reference given for {artifact.id}, {ref!r}, {e}") return {"ref": ref} if artifact.build_context_object_url: toml = {"ref": ref} if ref is not None else {} @@ -289,6 +293,25 @@ def image(self, out: _EntityOut, artifact: DockerImageArtifact, key: str) -> str self._unwritable((EntityKind.ARTIFACT, artifact.id, artifact.version), f"{out.folder}: {key}", why) return {EntityKind.ARTIFACT.value: artifact.id, "version": artifact.version} + def _pinned(self, image_id: str, ref: str) -> str: + """``ref``, given for ``image_id``, pinned to a digest: as given when it pins one, else its tag pinned to the + digest its registry serves now, once per image. Raises ValueError when it names no registry, gives a digest + that isn't whole, or its registry can't pin it.""" + if image_id not in self.refs_pinned: + if not isinstance(ref, str) or not names_registry(ref): + raise ValueError("doesn't name its registry") + if "@" in ref: + if not _PINNED.search(ref): + raise ValueError("doesn't give a whole digest, as /:@sha256:<64 hex digits> " + "does") + self.refs_pinned[image_id] = ref + else: + try: + self.refs_pinned[image_id] = pin_digest(ref, get_config().get_image_store().auth(ref)) + except ValueError as e: + raise ValueError(f"can't be pinned to a digest: {e}") from None + return self.refs_pinned[image_id] + def write(self, description: str, source: dict) -> BundleImport: """Write the folder, with ``description`` as its README's first line, and say what was written.""" if unused := sorted(self.image_refs.keys() - self.refs_used): @@ -547,15 +570,6 @@ def _kind_of(entity: Any) -> EntityKind: _PINNED = re.compile(r"@sha256:[0-9a-f]{64}$") -def _given_ref_problem(ref: str) -> str | None: - """Why ``ref``, given for an image, can't be written: it must name its registry and pin a digest.""" - if not isinstance(ref, str) or not names_registry(ref): - return "doesn't name its registry" - if not _PINNED.search(ref): - return "doesn't pin a digest, as /:@sha256:<64 hex digits> does" - return None - - def _size(url: str) -> int | None: metadata = get_config().get_object_store_at(url).get_object_metadata_at(url) return metadata.size if metadata is not None else None diff --git a/src/agent_env/cli/bundle.py b/src/agent_env/cli/bundle.py index 74c85641c..43d6a02ce 100644 --- a/src/agent_env/cli/bundle.py +++ b/src/agent_env/cli/bundle.py @@ -30,8 +30,8 @@ def bundle(): help="Keep the fields of this scope as stored, and leave in the store what the folder can't hold, for a " "folder run against the same store. Repeatable.") @click.option("--image-ref", "image_refs", multiple=True, metavar="ID=REF", - help="Write the stored image ID as the registry image REF, pinned by its digest " - "(/:@sha256:), which a run pulls, with its build context beside it. " + help="Write the stored image ID as the registry image REF, which a run pulls, with its build context " + "beside it. A tag is pinned to the digest its registry serves now; a digest is written as given. " "Repeatable.") def init(folder: Path, task_id: str | None, eval_id: str | None, version: int | None, include_scopes: tuple[str, ...], image_refs: tuple[str, ...]): diff --git a/tst/unit/bundle/importer_image_refs_test.py b/tst/unit/bundle/importer_image_refs_test.py index a4af15e59..e0e0bd2f6 100644 --- a/tst/unit/bundle/importer_image_refs_test.py +++ b/tst/unit/bundle/importer_image_refs_test.py @@ -1,5 +1,6 @@ -"""``bundle init --image-ref``: an image given a registry reference pinned by digest is written as that reference, -which a run pulls, with the part of its stored build context its Dockerfile reads kept beside it.""" +"""``bundle init --image-ref``: an image given a registry reference is written as that reference, its tag pinned to the +digest its registry serves now, which a run pulls, with the part of its stored build context its Dockerfile reads kept +beside it.""" import json import tomllib @@ -10,6 +11,7 @@ from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.artifact.store import get_artifact_store from agent_env.bundle import BundleError +from agent_env.bundle import importer as importer_module from agent_env.bundle.importer import import_task from agent_env.bundle.materialize import materialize from agent_env.bundle.resolve import build_step @@ -21,6 +23,7 @@ from tst.unit.bundle._support import context_image, local_builds, plan_of REF = "123456789012.dkr.ecr.us-west-2.amazonaws.com/delivery/crm:t1@sha256:" + "c" * 64 +TAG = REF.split("@")[0] DEPLOY = {"id": "env", "type": "deploy_env", "env_id": "crm"} @@ -30,6 +33,19 @@ def home(local_stores, tmp_path, monkeypatch): return tmp_path +@pytest.fixture +def pins(monkeypatch): + """Each ref the import asks its registry to pin; a tag pins to REF's digest.""" + asked = [] + + def pin(ref, auth): + asked.append(ref) + return f"{ref}@{REF.split('@')[1]}" + + monkeypatch.setattr(importer_module, "pin_digest", pin) + return asked + + def _stored(image): MCPServerEnv.put(id="crm", docker_image_artifact=image, environment_name="crm") Task.put(id="t", steps=[build_step(dict(DEPLOY))]) @@ -82,17 +98,51 @@ def test_an_image_with_no_context_given_a_ref_is_written_as_the_ref_alone(home): assert [(pull.folder, pull.image) for pull in imported.pulled] == [("envs/crm", REF)] +def test_a_tag_given_for_an_image_is_pinned_to_the_digest_its_registry_serves_once_per_image(home, pins): + image = context_image(home, "crm-image") + MCPServerEnv.put(id="crm", docker_image_artifact=image, environment_name="crm") + MCPServerEnv.put(id="crm-eu", docker_image_artifact=image, environment_name="crm") + Task.put(id="t", steps=[build_step(dict(DEPLOY)), build_step({**DEPLOY, "id": "env2", "env_id": "crm-eu"})]) + + imported = import_task("t", home / "t", image_refs={"crm-image": TAG}) + + assert [_toml(imported.root, f"envs/{name}/env.toml")["image"]["ref"] for name in ("crm", "crm-eu")] == [REF] * 2 + assert pins == [TAG] + + +def test_a_digest_given_for_an_image_is_written_as_given_without_asking_its_registry(home, pins): + _stored(context_image(home, "crm-image")) + + import_task("t", home / "t", image_refs={"crm-image": REF}) + + assert pins == [] + + +def test_a_tag_its_registry_cant_pin_fails_the_import_naming_the_image(home, monkeypatch): + _stored(context_image(home, "crm-image")) + + def refuse(ref, auth): + raise ValueError(f"{ref}: the registry has no such tag") + + monkeypatch.setattr(importer_module, "pin_digest", refuse) + + with pytest.raises(BundleError) as caught: + import_task("t", home / "t", image_refs={"crm-image": TAG}) + + assert caught.value.problems == (f"envs/crm: image: the reference given for crm-image, {TAG!r}, can't be pinned to " + f"a digest: {TAG}: the registry has no such tag",) + + @pytest.mark.parametrize("refs, problem", [ - ({"crm-image": REF.split("@")[0]}, - "envs/crm: image: the reference given for crm-image, '" + REF.split("@")[0] + "', doesn't pin a digest, as " + ({"crm-image": TAG + "@sha256:"}, + "envs/crm: image: the reference given for crm-image, '" + TAG + "@sha256:', doesn't give a whole digest, as " "/:@sha256:<64 hex digits> does"), - ({"crm-image": REF.split("@")[0] + "@sha256:"}, "doesn't pin a digest"), - ({"crm-image": REF[:-1]}, "doesn't pin a digest"), - ({"crm-image": REF[:-1] + "X"}, "doesn't pin a digest"), + ({"crm-image": REF[:-1]}, "doesn't give a whole digest"), + ({"crm-image": REF[:-1] + "X"}, "doesn't give a whole digest"), ({"crm-image": "crm@sha256:" + "c" * 64}, "doesn't name its registry"), ({"crm-image": REF, "other-image": REF}, "a reference was given for other-image, which the import writes no image of"), -], ids=["no digest", "empty digest", "short digest", "not hex", "no registry", "unused"]) +], ids=["empty digest", "short digest", "not hex", "no registry", "unused"]) def test_a_ref_that_cant_be_written_fails_the_import_and_writes_nothing(home, refs, problem): _stored(context_image(home, "crm-image"))