diff --git a/changelog/1057.changed.md b/changelog/1057.changed.md new file mode 100644 index 000000000..b129be2fc --- /dev/null +++ b/changelog/1057.changed.md @@ -0,0 +1 @@ +`infrahubctl marketplace get --collection` now downloads each member schema individually into a `//.yml` layout (for example `schemas/base-schemas/dcim.yml`), instead of dumping version-suffixed files flat into the output directory. Filenames no longer carry the version, matching single-schema downloads, so re-downloading a collection overwrites cleanly rather than accumulating stale versions. If two members share a schema name across namespaces, those members are written to `///.yml` instead of overwriting each other. diff --git a/infrahub_sdk/ctl/marketplace.py b/infrahub_sdk/ctl/marketplace.py index df1cc9a77..1b12e2794 100644 --- a/infrahub_sdk/ctl/marketplace.py +++ b/infrahub_sdk/ctl/marketplace.py @@ -1,9 +1,7 @@ from __future__ import annotations import asyncio -import io import sys -import zipfile from enum import Enum from pathlib import Path from typing import Any, Literal, NamedTuple, NoReturn @@ -73,7 +71,7 @@ def _schema_url(base_url: str, namespace: str, name: str, version: str | None = def _collection_url(base_url: str, namespace: str, name: str) -> str: - return f"{base_url}/api/v1/collections/{namespace}/{name}/download" + return f"{base_url}/api/v1/collections/{namespace}/{name}" def _is_transport_failure(r: object) -> bool: @@ -158,6 +156,7 @@ async def _download_schema( stdout: bool, prefetched: httpx.Response | None = None, schema_confirmed_exists: bool = False, + needs_separator: bool = False, ) -> None: """Download a single schema and write it to disk or stdout. @@ -166,6 +165,9 @@ async def _download_schema( ``schema_confirmed_exists`` signals that the schema is known to exist (e.g. from the auto-detect probe), so a 404 on a versioned URL is reported as version-not-found rather than the generic not-found message. + ``needs_separator`` inserts a ``---`` document separator before the content in + stdout mode when it is missing, so multiple schemas streamed back-to-back (e.g. + from a collection) form a valid multi-document YAML stream. """ if prefetched is not None and version is None: resp = prefetched @@ -188,6 +190,8 @@ async def _download_schema( resolved_version = version or resp.headers.get("x-schema-version", "latest") if stdout: + if needs_separator and not resp.text.lstrip().startswith("---"): + sys.stdout.write("---\n") sys.stdout.write(resp.text) if not resp.text.endswith("\n"): sys.stdout.write("\n") @@ -212,11 +216,17 @@ async def _download_collection( stdout: bool, prefetched: httpx.Response | None = None, ) -> None: - """Fetch all schemas in a collection, writing to disk or stdout. - - The marketplace returns a ZIP archive of ``{namespace}-{name}-{semver}.yml`` files. + """Fetch every schema in a collection, writing to disk or stdout. + + The collection metadata endpoint lists each member schema along with its latest + published version. Each member is downloaded individually via :func:`_download_schema` + so naming, versioning, and error handling stay identical to single-schema downloads. + On disk, members land in ``output_dir//.yml``. If two + members share a name across namespaces, those members are disambiguated into + ``output_dir///.yml`` instead of silently + overwriting each other. When ``prefetched`` is supplied (from the auto-detect probe), reuses that response - instead of re-fetching the collection download URL. + instead of re-fetching the collection metadata. """ if prefetched: resp = prefetched @@ -230,34 +240,45 @@ async def _download_collection( resp.raise_for_status() try: - archive = zipfile.ZipFile(io.BytesIO(resp.content)) - except zipfile.BadZipFile: + payload = resp.json() + except ValueError: _fail( _ErrorClass.NETWORK, - f"Response from {_collection_url(base_url, namespace, name)} is not a valid ZIP archive", + f"Response from {_collection_url(base_url, namespace, name)} is not valid JSON", ) - schema_names = [info.filename for info in archive.infolist() if not info.is_dir()] + items = payload.get("items", []) if isinstance(payload, dict) else [] + schemas = [item.get("schema") for item in items if isinstance(item, dict)] + members: list[dict[str, Any]] = [schema for schema in schemas if isinstance(schema, dict)] status = _status_console(stdout) + target_dir = output_dir / name + + member_names = [schema.get("name") for schema in members if schema.get("namespace") and schema.get("name")] + duplicated_names = {member_name for member_name in member_names if member_names.count(member_name) > 1} + + downloaded = 0 + for schema in members: + member_namespace = schema.get("namespace") + member_name = schema.get("name") + if not member_namespace or not member_name: + status.print("[yellow]Warning: skipping a collection member with missing namespace or name.") + continue + version = (schema.get("latest_version") or {}).get("semver") + member_dir = target_dir / member_namespace if member_name in duplicated_names else target_dir + await _download_schema( + client=client, + base_url=base_url, + namespace=member_namespace, + name=member_name, + version=version, + output_dir=member_dir, + stdout=stdout, + schema_confirmed_exists=True, + needs_separator=downloaded > 0, + ) + downloaded += 1 - if stdout: - for index, schema_name in enumerate(schema_names): - content = archive.read(schema_name).decode("utf-8") - if index > 0 and not content.lstrip().startswith("---"): - sys.stdout.write("---\n") - sys.stdout.write(content) - if not content.endswith("\n"): - sys.stdout.write("\n") - err_console.print(f"[green]Fetched {schema_name}") - else: - _mkdir_or_fail(output_dir) - for schema_name in schema_names: - content = archive.read(schema_name).decode("utf-8") - file_path = output_dir / schema_name - file_path.write_text(content, encoding="utf-8") - console.print(f"[green]Downloaded {schema_name} -> {file_path}") - - status.print(f"\n[green]Collection {namespace}/{name}: {len(schema_names)} schemas downloaded") + status.print(f"\n[green]Collection {namespace}/{name}: {downloaded} schemas downloaded") @app.command() diff --git a/tests/unit/ctl/test_marketplace_app.py b/tests/unit/ctl/test_marketplace_app.py index 8edcc6efa..4f302a510 100644 --- a/tests/unit/ctl/test_marketplace_app.py +++ b/tests/unit/ctl/test_marketplace_app.py @@ -1,5 +1,3 @@ -import io -import zipfile from pathlib import Path import httpx @@ -21,12 +19,17 @@ """ -def _make_zip(files: dict[str, str]) -> bytes: - buf = io.BytesIO() - with zipfile.ZipFile(buf, "w") as zf: - for filename, content in files.items(): - zf.writestr(filename, content) - return buf.getvalue() +def _collection_json(members: list[tuple[str, str, str]]) -> dict: + """Build collection metadata mimicking the marketplace endpoint. + + ``members`` is a list of ``(namespace, name, semver)`` tuples. + """ + return { + "items": [ + {"schema": {"namespace": ns, "name": name, "latest_version": {"semver": semver}}} + for ns, name, semver in members + ] + } def test_download_schema_specific_version(httpx_mock: HTTPXMock, tmp_path: Path) -> None: @@ -39,7 +42,7 @@ def test_download_schema_specific_version(httpx_mock: HTTPXMock, tmp_path: Path) ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -61,22 +64,27 @@ def test_download_schema_specific_version(httpx_mock: HTTPXMock, tmp_path: Path) def test_download_collection(httpx_mock: HTTPXMock, tmp_path: Path) -> None: httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack/download", - content=_make_zip( - { - "acme-network-base-1.0.0.yml": SCHEMA_YAML, - "acme-dcim-2.1.0.yml": SCHEMA_YAML, - } - ), + url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack", + json=_collection_json([("acme", "network-base", "1.0.0"), ("acme", "dcim", "2.1.0")]), + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/network-base/versions/1.0.0/download", + text=SCHEMA_YAML, + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/dcim/versions/2.1.0/download", + text=SCHEMA_YAML, ) result = runner.invoke(app, ["get", "acme/starter-pack", "-c", "-o", str(tmp_path)]) assert result.exit_code == 0 - assert "Downloaded acme-network-base-1.0.0.yml" in result.output - assert "Downloaded acme-dcim-2.1.0.yml" in result.output + assert "Downloaded schema acme/network-base v1.0.0" in result.output + assert "Downloaded schema acme/dcim v2.1.0" in result.output assert "2 schemas downloaded" in result.output - assert (tmp_path / "acme-network-base-1.0.0.yml").exists() - assert (tmp_path / "acme-dcim-2.1.0.yml").exists() + assert (tmp_path / "starter-pack" / "network-base.yml").exists() + assert (tmp_path / "starter-pack" / "dcim.yml").exists() def test_download_not_found(httpx_mock: HTTPXMock, tmp_path: Path) -> None: @@ -88,7 +96,7 @@ def test_download_not_found(httpx_mock: HTTPXMock, tmp_path: Path) -> None: ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/nonexistent/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/nonexistent", status_code=404, json={"detail": "Collection not found"}, ) @@ -115,7 +123,7 @@ def test_download_custom_marketplace_url(httpx_mock: HTTPXMock, tmp_path: Path) ) httpx_mock.add_response( method="GET", - url="http://localhost:8000/api/v1/collections/acme/test/download", + url="http://localhost:8000/api/v1/collections/acme/test", status_code=404, json={"detail": "Collection not found"}, ) @@ -141,7 +149,7 @@ def test_marketplace_url_from_env(httpx_mock: HTTPXMock, tmp_path: Path, monkeyp ) httpx_mock.add_response( method="GET", - url="http://staging.example.com/api/v1/collections/acme/network-base/download", + url="http://staging.example.com/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -160,7 +168,7 @@ def test_autodetect_schema(httpx_mock: HTTPXMock, tmp_path: Path) -> None: ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -180,15 +188,20 @@ def test_autodetect_collection(httpx_mock: HTTPXMock, tmp_path: Path) -> None: ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack/download", - content=_make_zip({"acme-network-base-1.0.0.yml": SCHEMA_YAML}), + url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack", + json=_collection_json([("acme", "network-base", "1.0.0")]), + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/network-base/versions/1.0.0/download", + text=SCHEMA_YAML, ) result = runner.invoke(app, ["get", "acme/starter-pack", "-o", str(tmp_path)]) assert result.exit_code == 0 assert "Collection acme/starter-pack" in result.output assert "1 schemas downloaded" in result.output - assert (tmp_path / "acme-network-base-1.0.0.yml").exists() + assert (tmp_path / "starter-pack" / "network-base.yml").exists() def test_autodetect_collision_schema_wins(httpx_mock: HTTPXMock, tmp_path: Path) -> None: @@ -200,8 +213,8 @@ def test_autodetect_collision_schema_wins(httpx_mock: HTTPXMock, tmp_path: Path) ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network/download", - content=_make_zip({}), + url="https://marketplace.infrahub.app/api/v1/collections/acme/network", + json=_collection_json([]), ) result = runner.invoke(app, ["get", "acme/network", "-o", str(tmp_path)]) @@ -230,7 +243,7 @@ def test_version_not_found(httpx_mock: HTTPXMock, tmp_path: Path) -> None: ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -257,23 +270,33 @@ def test_version_ignored_on_autodetected_collection(httpx_mock: HTTPXMock, tmp_p ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack/download", - content=_make_zip({"acme-network-base-1.0.0.yml": SCHEMA_YAML}), + url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack", + json=_collection_json([("acme", "network-base", "1.0.0")]), + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/network-base/versions/1.0.0/download", + text=SCHEMA_YAML, ) result = runner.invoke(app, ["get", "acme/starter-pack", "-v", "1.0.0", "-o", str(tmp_path)]) assert result.exit_code == 0 assert "Warning: --version is ignored" in result.output - assert (tmp_path / "acme-network-base-1.0.0.yml").exists() + assert (tmp_path / "starter-pack" / "network-base.yml").exists() def test_collection_flag_overrides_autodetect(httpx_mock: HTTPXMock, tmp_path: Path) -> None: httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack/download", - content=_make_zip({"acme-network-base-1.0.0.yml": SCHEMA_YAML}), + url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack", + json=_collection_json([("acme", "network-base", "1.0.0")]), + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/network-base/versions/1.0.0/download", + text=SCHEMA_YAML, ) - # No schema endpoint mock — if the implementation probes it, pytest-httpx + # No schema-detect endpoint mock — if the implementation probes it, pytest-httpx # will raise "request not expected". result = runner.invoke(app, ["get", "acme/starter-pack", "-c", "-o", str(tmp_path)]) @@ -290,7 +313,7 @@ def test_output_dir_creates_nested_missing_parents(httpx_mock: HTTPXMock, tmp_pa ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -310,7 +333,7 @@ def test_output_dir_default_is_schemas(httpx_mock: HTTPXMock, tmp_path: Path, mo ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -330,7 +353,7 @@ def test_output_dir_permission_error(httpx_mock: HTTPXMock, tmp_path: Path, monk ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -351,17 +374,61 @@ def raising_mkdir(self: Path, *args: object, **kwargs: object) -> None: assert "unwritable" in result.output -def test_download_collection_with_skipped(httpx_mock: HTTPXMock, tmp_path: Path) -> None: +def test_download_collection_skips_members_missing_identity(httpx_mock: HTTPXMock, tmp_path: Path) -> None: + """A member entry missing namespace/name is skipped rather than aborting the download.""" + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/collections/acme/mixed", + json={ + "items": [ + {"schema": {"namespace": "acme", "name": "good", "latest_version": {"semver": "1.0.0"}}}, + {"schema": {"name": "orphan", "latest_version": {"semver": "1.0.0"}}}, + ] + }, + ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/mixed/download", - content=_make_zip({"acme-good-1.0.0.yml": SCHEMA_YAML}), + url="https://marketplace.infrahub.app/api/v1/schemas/acme/good/versions/1.0.0/download", + text=SCHEMA_YAML, ) result = runner.invoke(app, ["get", "acme/mixed", "-c", "-o", str(tmp_path)]) assert result.exit_code == 0 + assert "Downloaded schema acme/good v1.0.0" in result.output + assert "Warning: skipping a collection member" in result.output assert "1 schemas downloaded" in result.output - assert (tmp_path / "acme-good-1.0.0.yml").exists() + assert (tmp_path / "mixed" / "good.yml").exists() + + +def test_download_collection_duplicate_names_across_namespaces(httpx_mock: HTTPXMock, tmp_path: Path) -> None: + """Members sharing a name across namespaces land in namespace subdirectories instead of overwriting.""" + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/collections/acme/clash", + json=_collection_json([("acme", "dcim", "1.0.0"), ("other", "dcim", "2.0.0"), ("acme", "ipam", "1.0.0")]), + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/dcim/versions/1.0.0/download", + text=SCHEMA_YAML, + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/other/dcim/versions/2.0.0/download", + text=SCHEMA_YAML, + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/ipam/versions/1.0.0/download", + text=SCHEMA_YAML, + ) + result = runner.invoke(app, ["get", "acme/clash", "-c", "-o", str(tmp_path)]) + + assert result.exit_code == 0 + assert "3 schemas downloaded" in result.output + assert (tmp_path / "clash" / "acme" / "dcim.yml").exists() + assert (tmp_path / "clash" / "other" / "dcim.yml").exists() + assert (tmp_path / "clash" / "ipam.yml").exists() def test_autodetect_partial_probe_failure_is_network(httpx_mock: HTTPXMock, tmp_path: Path) -> None: @@ -374,7 +441,7 @@ def test_autodetect_partial_probe_failure_is_network(httpx_mock: HTTPXMock, tmp_ ) httpx_mock.add_exception( httpx.ConnectError("connection refused"), - url="https://marketplace.infrahub.app/api/v1/collections/acme/foo/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/foo", ) result = runner.invoke(app, ["get", "acme/foo", "-o", str(tmp_path)]) @@ -392,7 +459,7 @@ def test_versioned_download_network_error(httpx_mock: HTTPXMock, tmp_path: Path) ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -410,7 +477,7 @@ def test_collection_flag_network_error(httpx_mock: HTTPXMock, tmp_path: Path) -> """A network failure on the explicit --collection fetch should exit with code 2.""" httpx_mock.add_exception( httpx.ConnectError("connection refused"), - url="https://marketplace.infrahub.app/api/v1/collections/acme/foo/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/foo", ) result = runner.invoke(app, ["get", "acme/foo", "-c", "-o", str(tmp_path)]) @@ -422,7 +489,7 @@ def test_network_error_empty_message_shows_exception_type(httpx_mock: HTTPXMock, """When an httpx exception has no message (e.g. ReadTimeout), the type name is shown.""" httpx_mock.add_exception( httpx.ReadTimeout(""), - url="https://marketplace.infrahub.app/api/v1/collections/acme/foo/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/foo", ) result = runner.invoke(app, ["get", "acme/foo", "-c", "-o", str(tmp_path)]) @@ -439,7 +506,7 @@ def test_get_schema_stdout(httpx_mock: HTTPXMock, tmp_path: Path) -> None: ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, ) @@ -454,20 +521,25 @@ def test_get_schema_stdout(httpx_mock: HTTPXMock, tmp_path: Path) -> None: def test_get_collection_stdout(httpx_mock: HTTPXMock, tmp_path: Path) -> None: httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack/download", - content=_make_zip( - { - "acme-network-base-1.0.0.yml": SCHEMA_YAML, - "acme-dcim-2.1.0.yml": SCHEMA_YAML, - } - ), + url="https://marketplace.infrahub.app/api/v1/collections/acme/starter-pack", + json=_collection_json([("acme", "network-base", "1.0.0"), ("acme", "dcim", "2.1.0")]), + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/network-base/versions/1.0.0/download", + text=SCHEMA_YAML, + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/dcim/versions/2.1.0/download", + text=SCHEMA_YAML, ) result = runner.invoke(app, ["get", "acme/starter-pack", "-c", "--stdout", "-o", str(tmp_path)]) assert result.exit_code == 0 assert SCHEMA_YAML in result.output - assert "Fetched acme-network-base-1.0.0.yml" in result.output - assert "Fetched acme-dcim-2.1.0.yml" in result.output + assert "Fetched schema acme/network-base v1.0.0" in result.output + assert "Fetched schema acme/dcim v2.1.0" in result.output assert "2 schemas downloaded" in result.output assert not any(tmp_path.iterdir()) @@ -477,8 +549,18 @@ def test_get_collection_stdout_separator(httpx_mock: HTTPXMock, tmp_path: Path) bare_yaml = 'version: "1.0"\nnodes: []\n' httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/bare/download", - content=_make_zip({"acme-a-1.0.0.yml": bare_yaml, "acme-b-1.0.0.yml": bare_yaml}), + url="https://marketplace.infrahub.app/api/v1/collections/acme/bare", + json=_collection_json([("acme", "a", "1.0.0"), ("acme", "b", "1.0.0")]), + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/a/versions/1.0.0/download", + text=bare_yaml, + ) + httpx_mock.add_response( + method="GET", + url="https://marketplace.infrahub.app/api/v1/schemas/acme/b/versions/1.0.0/download", + text=bare_yaml, ) result = runner.invoke(app, ["get", "acme/bare", "-c", "--stdout", "-o", str(tmp_path)]) @@ -498,7 +580,7 @@ async def test_collection_false_autodetects_schema(httpx_mock: HTTPXMock, tmp_pa ) httpx_mock.add_response( method="GET", - url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base/download", + url="https://marketplace.infrahub.app/api/v1/collections/acme/network-base", status_code=404, json={"detail": "Collection not found"}, )