From 3df80722e95eec031daca1c9ce4957345943e461 Mon Sep 17 00:00:00 2001 From: RonaldHensbergen Date: Sun, 23 Aug 2026 23:45:29 +0200 Subject: [PATCH 1/2] feat: download cds get profiles from GitHub instead of local checkout By design, `cds get` now fetches its source repository from GitHub rather than copying files from a local checkout: - Default (no flags): downloads this project's upstream repo at `main` via the GitHub tarball API (no `git` binary required). - `--remote ` or a github.com URL, plus optional `--ref `, fetches a specific fork/revision. - `--local ` explicitly opts into using an existing local directory instead (mutually exclusive with --remote/--ref), for offline/dev workflows. Updates cli/getter.py, cli/main.py, README.md, and tests accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 18 +++-- cli/getter.py | 169 +++++++++++++++++++++++++++++++++---------- cli/main.py | 19 ++++- tests/test_getter.py | 167 ++++++++++++++++++++++++++++++++++++++---- tests/test_main.py | 4 +- 5 files changed, 313 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index 18bfd6fa..d5b643ba 100644 --- a/README.md +++ b/README.md @@ -606,7 +606,7 @@ before and never look for an `environments/` directory. |Command|Description| |---|---| -|cds get \ [--remote \] [--into \]|Fetch a profile plus its dependent module/runtime assets into a local CDS layout| +|cds get \ [--remote \] [--ref \] [--local \] [--into \]|Fetch a profile plus its dependent module/runtime assets from GitHub into a local CDS layout| |cds init [profile]|Generate a project `.env` template from profile secret definitions| |cds validate [profile]|Validate modules and contracts| |cds preflight [profile]|Check runtime tools, required environment values, and host ports without starting services| @@ -627,12 +627,16 @@ all accept `--environment ` (or `-e `) to merge `cds get` copies the selected `profiles//` tree, every referenced module directory, and any local build-context assets referenced by those modules' -Dockerfiles. By default it reads from the current repository and writes into the -current working directory; use `--remote` to point at another checked-out -repository, `--into` to choose a destination root, `--dry-run` to inspect the -copy plan first, and `--force` to replace conflicting local files. Successful -fetches record tracking metadata in `.cds/get-manifest.json` for future update -workflows. +Dockerfiles. By design it downloads from GitHub rather than a local checkout: +by default it fetches this project's upstream repository at the `main` branch, +downloading a tarball via the GitHub API (no `git` binary required). Use +`--remote ` (or a `github.com/...` URL) to fetch a fork, and +`--ref ` to select a specific revision. Pass `--local ` +to use an existing local directory instead of downloading (mutually exclusive +with `--remote`/`--ref`) for offline/dev workflows. Use `--into` to choose a +destination root, `--dry-run` to inspect the copy plan first, and `--force` to +replace conflicting local files. Successful fetches record tracking metadata +in `.cds/get-manifest.json` for future update workflows. `[profile]` accepts: diff --git a/cli/getter.py b/cli/getter.py index 444cb3e7..a10f12ca 100644 --- a/cli/getter.py +++ b/cli/getter.py @@ -2,17 +2,34 @@ import json import os +import re import shlex import shutil +import tarfile +import tempfile +from collections.abc import Iterator +from contextlib import contextmanager from dataclasses import dataclass from datetime import UTC, datetime from pathlib import Path from typing import Any +from urllib.error import HTTPError, URLError +from urllib.request import Request, urlopen from .loader import load_yaml_file, resolve_module_dir from .planner import MaxNestingDepthExceeded, apply_defaults, substitute_string +# The upstream repository `cds get` downloads from when no `--remote` is +# given. Keep in sync with the `Repository` URL in pyproject.toml. +DEFAULT_REMOTE = "RonaldHensbergen/composable-data-stack" +DEFAULT_REF = "main" + +_GITHUB_URL_PATTERN = re.compile( + r"^(?:https?://|git@)?(?:www\.)?github\.com[/:](?P[^/]+)/(?P[^/]+?)(?:\.git)?/?$" +) +_GITHUB_SHORTHAND_PATTERN = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") + _TRACKING_FILE = Path(".cds") / "get-manifest.json" _SKIP_DIRS = { ".cds", @@ -49,41 +66,46 @@ def fetch_profile( profile: str, *, remote: str | None = None, + ref: str = DEFAULT_REF, + local: str | None = None, destination_root: Path | None = None, force: bool = False, dry_run: bool = False, ) -> tuple[list[CopyAction], Path]: - source_repo = _resolve_source_repository(remote) target_root = (destination_root or Path.cwd()).expanduser().resolve() - profile_path = _resolve_source_profile_path(source_repo, profile) - asset_roots = _collect_asset_roots(source_repo, profile_path) - actions = _build_copy_plan(source_repo, asset_roots, target_root) - if dry_run: - return actions, target_root / _TRACKING_FILE + with _prepare_source_repository(remote, ref, local) as source_repo: + profile_path = _resolve_source_profile_path(source_repo, profile) + asset_roots = _collect_asset_roots(source_repo, profile_path) - conflicts = _find_conflicts(actions) - if conflicts and not force: - rendered = ", ".join(conflicts[:5]) - extra = "" if len(conflicts) <= 5 else f" (+{len(conflicts) - 5} more)" - raise GetError( - "Refusing to overwrite existing files without --force: " - f"{rendered}{extra}" - ) + actions = _build_copy_plan(source_repo, asset_roots, target_root) + if dry_run: + return actions, target_root / _TRACKING_FILE - for action in actions: - action.destination.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(action.source, action.destination) - - _write_tracking_manifest( - target_root=target_root, - requested_profile=profile, - source_repo=source_repo, - profile_path=profile_path, - remote=remote, - actions=actions, - asset_roots=asset_roots, - ) + conflicts = _find_conflicts(actions) + if conflicts and not force: + rendered = ", ".join(conflicts[:5]) + extra = "" if len(conflicts) <= 5 else f" (+{len(conflicts) - 5} more)" + raise GetError( + "Refusing to overwrite existing files without --force: " + f"{rendered}{extra}" + ) + + for action in actions: + action.destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(action.source, action.destination) + + _write_tracking_manifest( + target_root=target_root, + requested_profile=profile, + source_repo=source_repo, + profile_path=profile_path, + remote=remote, + ref=ref, + local=local, + actions=actions, + asset_roots=asset_roots, + ) return actions, target_root / _TRACKING_FILE @@ -99,8 +121,84 @@ def format_get_plan(actions: list[CopyAction], *, destination_root: Path) -> str return "\n".join(lines) -def _resolve_source_repository(remote: str | None) -> Path: - candidate = Path(remote).expanduser() if remote else _find_project_root() +@contextmanager +def _prepare_source_repository( + remote: str | None, ref: str, local: str | None +) -> Iterator[Path]: + """Resolve the source repository containing a `profiles/` tree. + + By design, `cds get` downloads its source from GitHub: a bare `remote` + defaults to this project's upstream repository, and any `owner/repo` or + `github.com/...` value is fetched as a tarball for `ref`. Pass `local` to + explicitly use an existing local directory instead (e.g. an offline/dev + checkout) -- `remote`/`ref` are ignored in that case. + """ + if local is not None: + if remote is not None: + raise GetError("Specify only one of --remote and --local") + yield _validate_source_repository(Path(local).expanduser()) + return + + candidate = remote or DEFAULT_REMOTE + parsed = _parse_github_remote(candidate) + if parsed is None: + raise GetError( + f'Could not resolve remote "{candidate}": expected an "owner/repo" ' + 'GitHub reference or a github.com URL. Use --local for an existing ' + "local directory instead." + ) + owner, repo = parsed + with tempfile.TemporaryDirectory(prefix="cds-get-") as tmp_dir: + extracted = _download_github_repository(owner, repo, ref, Path(tmp_dir)) + yield _validate_source_repository(extracted) + + +def _parse_github_remote(remote: str) -> tuple[str, str] | None: + candidate = remote.strip() + match = _GITHUB_URL_PATTERN.match(candidate) + if match: + return match.group("owner"), match.group("repo") + if _GITHUB_SHORTHAND_PATTERN.match(candidate): + owner, repo = candidate.split("/", 1) + return owner, repo + return None + + +def _download_github_repository(owner: str, repo: str, ref: str, work_dir: Path) -> Path: + url = f"https://api.github.com/repos/{owner}/{repo}/tarball/{ref}" + request = Request(url, headers={"User-Agent": "composable-data-stack-cds-get"}) + try: + with urlopen(request, timeout=30) as response: # noqa: S310 - fixed https GitHub API host + archive_bytes = response.read() + except HTTPError as exc: + raise GetError( + f"Could not download {owner}/{repo}@{ref} from GitHub: HTTP {exc.code}" + ) from exc + except URLError as exc: + raise GetError( + f"Could not download {owner}/{repo}@{ref} from GitHub: {exc.reason}" + ) from exc + + archive_path = work_dir / "repository.tar.gz" + archive_path.write_bytes(archive_bytes) + + extract_root = work_dir / "extracted" + extract_root.mkdir(parents=True, exist_ok=True) + try: + with tarfile.open(archive_path) as archive: + archive.extractall(extract_root, filter="data") + except tarfile.TarError as exc: + raise GetError( + f"Could not extract archive for {owner}/{repo}@{ref}: {exc}" + ) from exc + + extracted_entries = [entry for entry in extract_root.iterdir() if entry.is_dir()] + if len(extracted_entries) != 1: + raise GetError(f"Unexpected archive layout for {owner}/{repo}@{ref}") + return extracted_entries[0] + + +def _validate_source_repository(candidate: Path) -> Path: resolved = candidate.resolve() if not resolved.exists(): raise GetError(f"Source repository does not exist: {resolved}") @@ -113,14 +211,6 @@ def _resolve_source_repository(remote: str | None) -> Path: return resolved -def _find_project_root(start: Path | None = None) -> Path: - current = (start or Path.cwd()).resolve() - for directory in [current, *current.parents]: - if (directory / "pyproject.toml").exists() or (directory / ".git").exists(): - return directory - return current - - def _resolve_source_profile_path(source_repo: Path, profile: str) -> Path: profile_selector = Path(profile) candidates = [ @@ -544,6 +634,8 @@ def _write_tracking_manifest( source_repo: Path, profile_path: Path, remote: str | None, + ref: str, + local: str | None, actions: list[CopyAction], asset_roots: list[Path], ) -> None: @@ -553,7 +645,8 @@ def _write_tracking_manifest( entry = { "requestedProfile": requested_profile, "sourceProfile": profile_path.relative_to(source_repo).as_posix(), - "remote": remote or str(source_repo), + "remote": local or remote or DEFAULT_REMOTE, + "ref": None if local else ref, "fetchedAt": datetime.now(UTC).isoformat(), "assetRoots": [ _asset_root_relative_path(asset_root, source_repo) for asset_root in asset_roots diff --git a/cli/main.py b/cli/main.py index db80e872..4d8a6e47 100644 --- a/cli/main.py +++ b/cli/main.py @@ -853,7 +853,22 @@ def main() -> int: ) get_parser.add_argument( "--remote", - help="Path to the source repository root (default: current repository)", + help=( + "GitHub repository to fetch from, as 'owner/repo' or a github.com URL " + "(default: the upstream composable-data-stack repository)" + ), + ) + get_parser.add_argument( + "--ref", + default="main", + help="Branch, tag, or commit to fetch from --remote (default: main)", + ) + get_parser.add_argument( + "--local", + help=( + "Use an existing local directory as the source repository instead of " + "downloading from GitHub (mutually exclusive with --remote/--ref)" + ), ) get_parser.add_argument( "--into", @@ -1279,6 +1294,8 @@ def _begin_log_tail(_up_exit_code: int) -> None: actions, manifest_path = fetch_profile( args.profile, remote=args.remote, + ref=args.ref, + local=args.local, destination_root=Path(args.into) if args.into else None, force=args.force, dry_run=args.dry_run, diff --git a/tests/test_getter.py b/tests/test_getter.py index f340d82a..b8925498 100644 --- a/tests/test_getter.py +++ b/tests/test_getter.py @@ -1,12 +1,15 @@ +import io import json import os import stat import sys +import tarfile import tempfile import unittest from pathlib import Path +from unittest.mock import patch -from cli.getter import GetError, fetch_profile +from cli.getter import DEFAULT_REMOTE, GetError, _parse_github_remote, fetch_profile def _write(path: Path, content: str) -> None: @@ -74,7 +77,7 @@ def test_fetch_profile_copies_profile_module_and_build_assets(self) -> None: actions, manifest_path = fetch_profile( "demo", - remote=str(source_root), + local=str(source_root), destination_root=destination_root, ) @@ -101,16 +104,16 @@ def test_fetch_profile_requires_force_for_conflicting_files(self) -> None: destination_root = Path(dest_dir) _make_source_repo(source_root) - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) profile_file = destination_root / "profiles" / "demo" / "profile.yaml" profile_file.write_text("changed\n", encoding="utf-8") with self.assertRaises(GetError): - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) fetch_profile( "demo", - remote=str(source_root), + local=str(source_root), destination_root=destination_root, force=True, ) @@ -122,13 +125,13 @@ def test_fetch_profile_dry_run_ignores_conflicting_files(self) -> None: destination_root = Path(dest_dir) _make_source_repo(source_root) - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) profile_file = destination_root / "profiles" / "demo" / "profile.yaml" profile_file.write_text("changed\n", encoding="utf-8") actions, manifest_path = fetch_profile( "demo", - remote=str(source_root), + local=str(source_root), destination_root=destination_root, dry_run=True, ) @@ -198,7 +201,7 @@ def test_fetch_profile_resolves_templated_dockerfile_from_module_config_defaults _write(source_root / ".github" / "workflows" / "ci.yml", "name: CI\n") _write(source_root / ".env.example", "EXAMPLE=true\n") - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) self.assertTrue( (destination_root / "images" / "demo" / "hardened" / "Dockerfile").exists() @@ -243,7 +246,7 @@ def test_fetch_profile_supports_single_file_profiles(self) -> None: _, manifest_path = fetch_profile( "demo", - remote=str(source_root), + local=str(source_root), destination_root=destination_root, ) @@ -294,7 +297,7 @@ def test_fetch_profile_reports_invalid_json_copy_instruction_as_get_error(self) ) with self.assertRaises(GetError) as ctx: - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) self.assertIn("Could not parse COPY sources", str(ctx.exception)) @@ -341,7 +344,7 @@ def test_fetch_profile_reports_invalid_shell_copy_instruction_as_get_error(self) ) with self.assertRaises(GetError) as ctx: - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) self.assertIn("Could not parse COPY sources", str(ctx.exception)) @@ -388,7 +391,7 @@ def test_fetch_profile_preserves_hash_characters_inside_quoted_copy_sources(self ) _write(source_root / "shared" / "file#1.txt", "ok\n") - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) self.assertTrue((destination_root / "shared" / "file#1.txt").exists()) @@ -438,7 +441,7 @@ def test_fetch_profile_skips_copy_from_stage_sources(self) -> None: ) _write(source_root / "shared" / "python" / "__init__.py", "") - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) self.assertTrue((destination_root / "images" / "demo" / "Dockerfile").exists()) self.assertTrue((destination_root / "shared" / "python" / "__init__.py").exists()) @@ -466,7 +469,7 @@ def test_fetch_profile_rejects_profile_paths_outside_source_repo(self) -> None: with self.assertRaises(GetError) as ctx: fetch_profile( f"../{outside_root.name}/evil.yaml", - remote=str(source_root), + local=str(source_root), destination_root=destination_root, ) @@ -509,7 +512,7 @@ def test_fetch_profile_rejects_absolute_dockerfile_outside_source_repo(self) -> ) with self.assertRaises(GetError) as ctx: - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) self.assertIn('build.dockerfile "/etc/passwd" resolves outside the source repository', str(ctx.exception)) @@ -523,12 +526,144 @@ def test_fetch_profile_preserves_executable_permissions(self) -> None: entrypoint = source_root / "images" / "demo" / "entrypoint.sh" os.chmod(entrypoint, 0o755) - fetch_profile("demo", remote=str(source_root), destination_root=destination_root) + fetch_profile("demo", local=str(source_root), destination_root=destination_root) destination_entrypoint = destination_root / "images" / "demo" / "entrypoint.sh" mode = stat.S_IMODE(destination_entrypoint.stat().st_mode) self.assertEqual(mode, 0o755) +class GitHubRemoteTest(unittest.TestCase): + def _make_tarball(self, root: Path) -> bytes: + buffer = io.BytesIO() + with tarfile.open(fileobj=buffer, mode="w:gz") as archive: + archive.add(root, arcname="owner-demo-repo-abcdef1") + return buffer.getvalue() + + def test_parse_github_remote_accepts_shorthand_and_urls(self) -> None: + self.assertEqual(_parse_github_remote("owner/repo"), ("owner", "repo")) + self.assertEqual( + _parse_github_remote("https://github.com/owner/repo"), ("owner", "repo") + ) + self.assertEqual( + _parse_github_remote("https://github.com/owner/repo.git"), ("owner", "repo") + ) + self.assertEqual( + _parse_github_remote("git@github.com:owner/repo.git"), ("owner", "repo") + ) + self.assertIsNone(_parse_github_remote("not a remote at all")) + + def test_fetch_profile_downloads_default_remote_when_no_remote_given(self) -> None: + with tempfile.TemporaryDirectory() as source_dir, tempfile.TemporaryDirectory() as dest_dir: + source_root = Path(source_dir) + destination_root = Path(dest_dir) + _make_source_repo(source_root) + archive_bytes = self._make_tarball(source_root) + + class _FakeResponse: + def __enter__(self_inner): + return self_inner + + def __exit__(self_inner, *exc_info): + return False + + def read(self_inner): + return archive_bytes + + captured_urls: list[str] = [] + + def _fake_urlopen(request, timeout=30): + captured_urls.append(request.full_url) + return _FakeResponse() + + with patch("cli.getter.urlopen", side_effect=_fake_urlopen): + actions, manifest_path = fetch_profile( + "demo", + destination_root=destination_root, + ) + + self.assertEqual( + captured_urls, + [f"https://api.github.com/repos/{DEFAULT_REMOTE}/tarball/main"], + ) + self.assertGreater(len(actions), 0) + self.assertTrue((destination_root / "profiles" / "demo" / "profile.yaml").exists()) + + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + entry = manifest["profiles"]["demo"] + self.assertEqual(entry["remote"], DEFAULT_REMOTE) + self.assertEqual(entry["ref"], "main") + + def test_fetch_profile_downloads_explicit_owner_repo_and_ref(self) -> None: + with tempfile.TemporaryDirectory() as source_dir, tempfile.TemporaryDirectory() as dest_dir: + source_root = Path(source_dir) + destination_root = Path(dest_dir) + _make_source_repo(source_root) + archive_bytes = self._make_tarball(source_root) + + class _FakeResponse: + def __enter__(self_inner): + return self_inner + + def __exit__(self_inner, *exc_info): + return False + + def read(self_inner): + return archive_bytes + + def _fake_urlopen(request, timeout=30): + return _FakeResponse() + + with patch("cli.getter.urlopen", side_effect=_fake_urlopen): + actions, _ = fetch_profile( + "demo", + remote="RonaldHensbergen/composable-data-stack", + ref="v1.2.3", + destination_root=destination_root, + ) + + self.assertGreater(len(actions), 0) + self.assertTrue((destination_root / "modules" / "apps" / "demo" / "module.yaml").exists()) + + def test_fetch_profile_raises_get_error_on_download_failure(self) -> None: + from urllib.error import URLError + + def _fake_urlopen(request, timeout=30): + raise URLError("network unreachable") + + with tempfile.TemporaryDirectory() as dest_dir: + with patch("cli.getter.urlopen", side_effect=_fake_urlopen): + with self.assertRaises(GetError) as ctx: + fetch_profile("demo", destination_root=Path(dest_dir)) + + self.assertIn("Could not download", str(ctx.exception)) + + def test_fetch_profile_rejects_unresolvable_remote(self) -> None: + with tempfile.TemporaryDirectory() as dest_dir: + with self.assertRaises(GetError) as ctx: + fetch_profile( + "demo", + remote="this is not a remote", + destination_root=Path(dest_dir), + ) + self.assertIn("Could not resolve remote", str(ctx.exception)) + + + def test_fetch_profile_rejects_both_remote_and_local(self) -> None: + with tempfile.TemporaryDirectory() as source_dir, tempfile.TemporaryDirectory() as dest_dir: + source_root = Path(source_dir) + destination_root = Path(dest_dir) + _make_source_repo(source_root) + + with self.assertRaises(GetError) as ctx: + fetch_profile( + "demo", + remote="owner/repo", + local=str(source_root), + destination_root=destination_root, + ) + self.assertIn("Specify only one of --remote and --local", str(ctx.exception)) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_main.py b/tests/test_main.py index 2068f2f3..1094a3f4 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -475,7 +475,7 @@ def write(path: Path, content: str) -> None: "cds", "get", "demo", - "--remote", + "--local", str(source_root), "--into", str(destination_root), @@ -500,7 +500,7 @@ def test_get_command_reports_errors_on_stderr(self): "cds", "get", "missing-profile", - "--remote", + "--local", str(Path(tempfile.gettempdir()) / "cds-missing-source-repo"), ], ), contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): From 60d2895edb13af10f638f2ec7cd56f9b45a3fe03 Mon Sep 17 00:00:00 2001 From: RonaldHensbergen Date: Sun, 23 Aug 2026 23:54:13 +0200 Subject: [PATCH 2/2] fix: use bandit-recognized nosec comment for GitHub tarball urlopen Bandit (B310) doesn't recognize ruff's `# noqa: S310` suppression syntax; it needs its own `# nosec B310` marker, matching the existing convention in cli/image_updates.py. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/getter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/getter.py b/cli/getter.py index a10f12ca..c4434e1f 100644 --- a/cli/getter.py +++ b/cli/getter.py @@ -168,7 +168,7 @@ def _download_github_repository(owner: str, repo: str, ref: str, work_dir: Path) url = f"https://api.github.com/repos/{owner}/{repo}/tarball/{ref}" request = Request(url, headers={"User-Agent": "composable-data-stack-cds-get"}) try: - with urlopen(request, timeout=30) as response: # noqa: S310 - fixed https GitHub API host + with urlopen(request, timeout=30) as response: # nosec B310 - fixed https GitHub API host archive_bytes = response.read() except HTTPError as exc: raise GetError(