diff --git a/src/google/adk/cli/cli_deploy.py b/src/google/adk/cli/cli_deploy.py index 2a2166c107f..e977f608083 100644 --- a/src/google/adk/cli/cli_deploy.py +++ b/src/google/adk/cli/cli_deploy.py @@ -43,7 +43,9 @@ from .deployers import DeployerFactory from .deployers._dockerfile_template import _render_dockerfile from .deployers._dockerfile_template import _render_install_agent_deps +from .deployers._dockerfile_template import _to_exec_form from .deployers._dockerfile_template import _validate_app_name +from .deployers._dockerfile_template import _validate_dockerfile_literal from .utils import _onboarding logger = logging.getLogger('google_adk.' + __name__) @@ -1054,6 +1056,23 @@ def _get_ignore_patterns_func( return shutil.ignore_patterns(*patterns) +def _validate_extra_package_name(base: str) -> str: + """Validates an extra_packages basename before writing it into a Dockerfile.""" + _validate_dockerfile_literal('extra_packages', base) + if ( + not base + or base in ('.', '..') + or '"' in base + or '\\' in base + or '/' in base + ): + raise click.ClickException( + f'Invalid extra_packages entry {base!r}. The basename must not contain' + ' quotes, backslashes, or path separators.' + ) + return base + + def _stage_extra_packages( requested_extra_packages: Sequence[tuple[str, str]], build_context: str, @@ -1082,7 +1101,9 @@ def _stage_extra_packages( pkg_src = os.path.abspath(pkg_src) if not os.path.exists(pkg_src): raise click.ClickException(f'extra_packages path not found: {pkg}') - base = os.path.basename(os.path.normpath(pkg_src)) + base = _validate_extra_package_name( + os.path.basename(os.path.normpath(pkg_src)) + ) dst = os.path.join(build_context, base) if os.path.exists(dst) or base in reserved_names: raise click.ClickException( @@ -1097,19 +1118,79 @@ def _stage_extra_packages( return staged_extra_packages +_INSTALLABLE_ARCHIVE_SUFFIXES: Final[tuple[str, ...]] = ( + '.whl', + '.tar.gz', + '.tgz', + '.zip', +) + + def _get_extra_packages_copy( staged_extra_packages: Sequence[str], build_context: str ) -> str: - """Returns the Dockerfile lines that copy staged extra packages.""" + """Returns the Dockerfile lines that copy and install staged extra packages. + + Each staged entry is handled one of two ways: + + * An installable distribution (a file ending in one of + `_INSTALLABLE_ARCHIVE_SUFFIXES`, or a directory containing + `pyproject.toml` or `setup.py`) is copied to `/tmp/extra_packages/` + and installed with `pip install`, so its modules land in site-packages + together with the dependencies it declares. + * Anything else (a plain module or a folder of modules) is copied to + `/app/` and `/app` is added to PYTHONPATH. + + Installable entries are deliberately kept out of `/app`: a flat-layout + package such as `my_pkg/pyproject.toml` + `my_pkg/my_pkg/__init__.py` copied + to `/app/my_pkg` would otherwise resolve as a PEP 420 namespace package + ahead of its own site-packages install. For the same reason PYTHONPATH is + only set when at least one plain entry is present. + + Args: + staged_extra_packages: Basenames returned by `_stage_extra_packages`. + build_context: The folder the entries were staged into. + + Returns: + The `COPY`, `ENV`, and `RUN` lines to splice into the Dockerfile, or an + empty string when nothing was staged. + """ if not staged_extra_packages: return '' - copy_lines = [ - f'COPY --chown=myuser:myuser "{base}/" "/app/{base}/"' - if os.path.isdir(os.path.join(build_context, base)) - else f'COPY --chown=myuser:myuser "{base}" "/app/{base}"' - for base in staged_extra_packages - ] - copy_lines.append('ENV PYTHONPATH="/app:$PYTHONPATH"') + copy_lines: list[str] = [] + installable_targets: list[str] = [] + has_plain_packages = False + for base in staged_extra_packages: + _validate_extra_package_name(base) + staged_path = os.path.join(build_context, base) + if os.path.isdir(staged_path): + if os.path.isfile( + os.path.join(staged_path, 'pyproject.toml') + ) or os.path.isfile(os.path.join(staged_path, 'setup.py')): + copy_lines.append( + f'COPY --chown=myuser:myuser "{base}/"' + f' "/tmp/extra_packages/{base}/"' + ) + installable_targets.append(f'/tmp/extra_packages/{base}') + else: + copy_lines.append( + f'COPY --chown=myuser:myuser "{base}/" "/app/{base}/"' + ) + has_plain_packages = True + elif base.endswith(_INSTALLABLE_ARCHIVE_SUFFIXES): + copy_lines.append( + f'COPY --chown=myuser:myuser "{base}" "/tmp/extra_packages/{base}"' + ) + installable_targets.append(f'/tmp/extra_packages/{base}') + else: + copy_lines.append(f'COPY --chown=myuser:myuser "{base}" "/app/{base}"') + has_plain_packages = True + if has_plain_packages: + copy_lines.append('ENV PYTHONPATH="/app:$PYTHONPATH"') + if installable_targets: + copy_lines.append( + _to_exec_form('RUN', ['pip', 'install', *installable_targets]) + ) return '\n'.join(copy_lines) @@ -1182,8 +1263,11 @@ def run( with_cloud_run_sandbox: Whether to enable the Cloud Run sandbox for code execution. extra_packages: Additional local file or directory paths to stage alongside - the agent and make importable in the image. A relative path is resolved - against the current working directory. + the agent and make importable in the image. A wheel, source archive, or + a directory containing `pyproject.toml` or `setup.py` is installed with + pip; any other entry is copied to `/app/` and `/app` is added + to PYTHONPATH. A relative path is resolved against the current working + directory. """ _validate_trigger_options( trigger_sources, trigger_oidc_audience, trigger_oidc_service_accounts @@ -1487,7 +1571,10 @@ def to_agent_engine( deployment. If not specified, the version in the dev environment will be used. extra_packages (list[str]): Optional. Additional local file or directory - paths to stage alongside the agent and make importable in the image. + paths to stage alongside the agent and make importable in the image. A + wheel, source archive, or a directory containing `pyproject.toml` or + `setup.py` is installed with pip; any other entry is copied to + `/app/` and `/app` is added to PYTHONPATH. worker_pool (str): Optional. Full Cloud Build private worker pool resource name (`projects/{project}/locations/{location}/workerPools/{pool}`). @@ -1975,8 +2062,11 @@ def to_gke( use_local_storage: Whether to use local .adk storage in the container. service_type: The Kubernetes Service type (default: ClusterIP). extra_packages: Additional local file or directory paths to stage alongside - the agent and make importable in the image. A relative path is resolved - against the current working directory. + the agent and make importable in the image. A wheel, source archive, or + a directory containing `pyproject.toml` or `setup.py` is installed with + pip; any other entry is copied to `/app/` and `/app` is added + to PYTHONPATH. A relative path is resolved against the current working + directory. """ _validate_trigger_options( trigger_sources, trigger_oidc_audience, trigger_oidc_service_accounts diff --git a/src/google/adk/cli/cli_tools_click.py b/src/google/adk/cli/cli_tools_click.py index 165f8cb400a..dc79e2f20a6 100644 --- a/src/google/adk/cli/cli_tools_click.py +++ b/src/google/adk/cli/cli_tools_click.py @@ -2667,10 +2667,12 @@ async def _lifespan(app: FastAPI) -> AsyncIterator[None]: help=( "Optional. Additional local package paths (a file or directory) to" " stage and deploy alongside the agent, and make importable in the" - " deployed image. Each entry is placed at `/app/` and `/app`" - " is added to PYTHONPATH, so a top-level name that matches an installed" - " dependency will shadow it at runtime; pick distinct names." - " Repeatable." + " deployed image. A wheel (.whl), source archive (.tar.gz, .tgz," + " .zip), or a directory containing pyproject.toml or setup.py is" + " installed with pip. Any other entry is placed at `/app/`" + " and `/app` is added to PYTHONPATH, so a top-level name that matches" + " an installed dependency will shadow it at runtime; pick distinct" + " names. Repeatable." ), ) @deploy_options @@ -3086,10 +3088,12 @@ def cli_migrate_session( help=( "Optional. Additional local package paths (a file or directory) to" " stage and deploy alongside the agent, and make importable in the" - " deployed image. Each entry is placed at `/app/` and `/app`" - " is added to PYTHONPATH, so a top-level name that matches an installed" - " dependency will shadow it at runtime; pick distinct names." - " Repeatable." + " deployed image. A wheel (.whl), source archive (.tar.gz, .tgz," + " .zip), or a directory containing pyproject.toml or setup.py is" + " installed with pip. Any other entry is placed at `/app/`" + " and `/app` is added to PYTHONPATH, so a top-level name that matches" + " an installed dependency will shadow it at runtime; pick distinct" + " names. Repeatable." ), ) @click.option( @@ -3377,10 +3381,12 @@ def cli_deploy_agent_engine( help=( "Optional. Additional local package paths (a file or directory) to" " stage and deploy alongside the agent, and make importable in the" - " deployed image. Each entry is placed at `/app/` and `/app`" - " is added to PYTHONPATH, so a top-level name that matches an installed" - " dependency will shadow it at runtime; pick distinct names." - " Repeatable." + " deployed image. A wheel (.whl), source archive (.tar.gz, .tgz," + " .zip), or a directory containing pyproject.toml or setup.py is" + " installed with pip. Any other entry is placed at `/app/`" + " and `/app` is added to PYTHONPATH, so a top-level name that matches" + " an installed dependency will shadow it at runtime; pick distinct" + " names. Repeatable." ), ) @adk_services_options(default_use_local_storage=False) diff --git a/tests/unittests/cli/utils/test_cli_deploy.py b/tests/unittests/cli/utils/test_cli_deploy.py index 07a2b46850f..5d5f1b95ec0 100644 --- a/tests/unittests/cli/utils/test_cli_deploy.py +++ b/tests/unittests/cli/utils/test_cli_deploy.py @@ -3053,3 +3053,136 @@ def test_cli_deploy_agent_engine_passes_service_account( mock_to_agent_engine.assert_called_once() _, kwargs = mock_to_agent_engine.call_args assert kwargs["service_account"] == _VALID_SERVICE_ACCOUNT + + +def test_extra_packages_pip_installs_installable_packages_and_wheels( + monkeypatch: pytest.MonkeyPatch, + agent_dir: Callable[[bool, bool], Path], + tmp_path: Path, +) -> None: + """Installable extra_packages (pyproject.toml, setup.py, .whl) are pip-installed.""" + src_dir = agent_dir(False, False) + build_dir = tmp_path / "build" + + schemas_dir = tmp_path / "libs" / "schemas" + (schemas_dir / "agent_schemas").mkdir(parents=True) + (schemas_dir / "pyproject.toml").write_text( + '[project]\nname = "agent-schemas"\nversion = "0.1.0"\n' + ) + (schemas_dir / "agent_schemas" / "__init__.py").write_text("SCHEMA = {}\n") + + wheel_file = tmp_path / "pkg.whl" + wheel_file.write_bytes(b"fake-wheel-bytes") + + plain_dir = tmp_path / "common" + plain_dir.mkdir() + (plain_dir / "helper.py").write_text("VALUE = 1\n") + + def mock_subprocess_run(*args, **kwargs): + del kwargs + if args[0][0:2] == ["kubectl", "apply"]: + return types.SimpleNamespace(stdout="service/gke-svc created") + return None + + monkeypatch.setattr(subprocess, "run", mock_subprocess_run) + monkeypatch.setattr(shutil, "rmtree", _Recorder()) + + cli_deploy.to_gke( + agent_folder=str(src_dir), + project="gke-proj", + region="us-east1", + cluster_name="my-gke-cluster", + service_name="gke-svc", + app_name="agent", + temp_folder=str(build_dir), + port=9090, + trace_to_cloud=False, + otel_to_cloud=False, + with_ui=False, + log_level="debug", + adk_version="1.2.0", + extra_packages=[str(schemas_dir), str(wheel_file), str(plain_dir)], + ) + + dockerfile_content = (build_dir / "Dockerfile").read_text() + assert ( + 'COPY --chown=myuser:myuser "schemas/" "/tmp/extra_packages/schemas/"' + in dockerfile_content + ) + assert ( + 'COPY --chown=myuser:myuser "pkg.whl" "/tmp/extra_packages/pkg.whl"' + in dockerfile_content + ) + assert ( + 'COPY --chown=myuser:myuser "common/" "/app/common/"' + in dockerfile_content + ) + assert 'ENV PYTHONPATH="/app:$PYTHONPATH"' in dockerfile_content + assert ( + 'RUN ["pip", "install", "/tmp/extra_packages/schemas",' + ' "/tmp/extra_packages/pkg.whl"]' + in dockerfile_content + ) + + +def test_get_extra_packages_copy_pip_installs_setup_py_package( + tmp_path: Path, +) -> None: + """A staged directory with setup.py is installed without setting PYTHONPATH.""" + legacy_pkg = tmp_path / "legacy_pkg" + legacy_pkg.mkdir() + (legacy_pkg / "setup.py").write_text( + "from setuptools import setup; setup()\n" + ) + + rendered = cli_deploy._get_extra_packages_copy(["legacy_pkg"], str(tmp_path)) + + assert ( + 'COPY --chown=myuser:myuser "legacy_pkg/"' + ' "/tmp/extra_packages/legacy_pkg/"' + in rendered + ) + assert "PYTHONPATH" not in rendered + assert 'RUN ["pip", "install", "/tmp/extra_packages/legacy_pkg"]' in rendered + + +@pytest.mark.parametrize( + "archive_name", + ["pkg-0.1.0.tar.gz", "pkg-0.1.0.tgz", "pkg-0.1.0.zip"], +) +def test_get_extra_packages_copy_pip_installs_source_archives( + tmp_path: Path, + archive_name: str, +) -> None: + """Source distribution archives (.tar.gz, .tgz, .zip) are pip-installed.""" + (tmp_path / archive_name).write_bytes(b"fake-archive-bytes") + + rendered = cli_deploy._get_extra_packages_copy([archive_name], str(tmp_path)) + + assert ( + f'COPY --chown=myuser:myuser "{archive_name}"' + f' "/tmp/extra_packages/{archive_name}"' + in rendered + ) + assert "PYTHONPATH" not in rendered + assert ( + f'RUN ["pip", "install", "/tmp/extra_packages/{archive_name}"]' + in rendered + ) + + +@pytest.mark.parametrize( + "unsafe_base", + [ + 'pkg"\nRUN touch /tmp/pwned', + 'pkg" "/tmp/evil', + "../outside", + ], +) +def test_get_extra_packages_copy_rejects_unsafe_basename( + tmp_path: Path, + unsafe_base: str, +) -> None: + """Unsafe extra_packages basenames cannot break out of Dockerfile COPY.""" + with pytest.raises(click.ClickException): + cli_deploy._get_extra_packages_copy([unsafe_base], str(tmp_path))