You've already forked adk-python
mirror of
https://github.com/encounter/adk-python.git
synced 2026-07-09 18:19:28 -07:00
fix: Fix adk deploy docker file permission
Merge https://github.com/google/adk-python/pull/2563 Currently in adk deploy cloud_run or gke, the dockerfile copies the agent code after the file permission is set. This can lead to file permission not being set correctly for the container to open and read the file. This PR will make sure the files are copied with the permission of the user that is set in the container. COPYBARA_INTEGRATE_REVIEW=https://github.com/google/adk-python/pull/2563 from moficodes:deploy-docker d7f6df4d893af75b360e6d96ffd2640ce6076ca2 PiperOrigin-RevId: 799371070
This commit is contained in:
Mofi Rahman
committed by
Copybara-Service
Copybara-Service
parent
47b88d2b06
commit
ad81aa54de
@@ -28,9 +28,6 @@ WORKDIR /app
|
||||
# Create a non-root user
|
||||
RUN adduser --disabled-password --gecos "" myuser
|
||||
|
||||
# Change ownership of /app to myuser
|
||||
RUN chown -R myuser:myuser /app
|
||||
|
||||
# Switch to the non-root user
|
||||
USER myuser
|
||||
|
||||
@@ -49,8 +46,8 @@ RUN pip install google-adk=={adk_version}
|
||||
|
||||
# Copy agent - Start
|
||||
|
||||
COPY "agents/{app_name}/" "/app/agents/{app_name}/"
|
||||
{install_agent_deps}
|
||||
# Set permission
|
||||
COPY --chown=myuser:myuser "agents/{app_name}/" "/app/agents/{app_name}/"
|
||||
|
||||
# Copy agent - End
|
||||
|
||||
|
||||
@@ -80,14 +80,6 @@ def reload_cli_deploy():
|
||||
def agent_dir(tmp_path: Path) -> Callable[[bool, bool], Path]:
|
||||
"""
|
||||
Return a factory that creates a dummy agent directory tree.
|
||||
|
||||
Args:
|
||||
tmp_path: The temporary path fixture provided by pytest.
|
||||
|
||||
Returns:
|
||||
A factory function that takes two booleans:
|
||||
- include_requirements: Whether to include a `requirements.txt` file.
|
||||
- include_env: Whether to include a `.env` file.
|
||||
"""
|
||||
|
||||
def _factory(include_requirements: bool, include_env: bool) -> Path:
|
||||
@@ -121,14 +113,12 @@ def mock_vertex_ai(
|
||||
sys.modules["vertexai"] = mock_vertexai
|
||||
sys.modules["vertexai.agent_engines"] = mock_agent_engines
|
||||
|
||||
# Also mock dotenv
|
||||
mock_dotenv = mock.MagicMock()
|
||||
mock_dotenv.dotenv_values = mock.MagicMock(return_value={"FILE_VAR": "value"})
|
||||
sys.modules["dotenv"] = mock_dotenv
|
||||
|
||||
yield mock_vertexai
|
||||
|
||||
# Cleanup: remove mocks from sys.modules
|
||||
del sys.modules["vertexai"]
|
||||
del sys.modules["vertexai.agent_engines"]
|
||||
del sys.modules["dotenv"]
|
||||
@@ -210,8 +200,6 @@ def test_resolve_project_from_gcloud_fails(
|
||||
("1.2.0", None, "gs://a", None, " --artifact_storage_uri=gs://a"),
|
||||
],
|
||||
)
|
||||
|
||||
# _get_service_option_by_adk_version
|
||||
def test_get_service_option_by_adk_version(
|
||||
adk_version: str,
|
||||
session_uri: str | None,
|
||||
@@ -220,15 +208,13 @@ def test_get_service_option_by_adk_version(
|
||||
expected: str,
|
||||
) -> None:
|
||||
"""It should return the correct service URI flags for a given ADK version."""
|
||||
assert (
|
||||
cli_deploy._get_service_option_by_adk_version(
|
||||
adk_version=adk_version,
|
||||
session_uri=session_uri,
|
||||
artifact_uri=artifact_uri,
|
||||
memory_uri=memory_uri,
|
||||
)
|
||||
== expected
|
||||
actual = cli_deploy._get_service_option_by_adk_version(
|
||||
adk_version=adk_version,
|
||||
session_uri=session_uri,
|
||||
artifact_uri=artifact_uri,
|
||||
memory_uri=memory_uri,
|
||||
)
|
||||
assert actual.rstrip() == expected.rstrip()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("include_requirements", [True, False])
|
||||
@@ -242,21 +228,14 @@ def test_to_cloud_run_happy_path(
|
||||
) -> None:
|
||||
"""
|
||||
End-to-end execution test for `to_cloud_run`.
|
||||
|
||||
This test verifies that for a given configuration:
|
||||
1. The agent source files are correctly copied to a temporary build context.
|
||||
2. A valid Dockerfile is generated with the correct parameters.
|
||||
3. The `gcloud run deploy` command is constructed with the correct arguments.
|
||||
"""
|
||||
src_dir = agent_dir(include_requirements, False)
|
||||
run_recorder = _Recorder()
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", run_recorder)
|
||||
# Mock rmtree to prevent actual deletion during test run but record calls
|
||||
rmtree_recorder = _Recorder()
|
||||
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
|
||||
|
||||
# Execute the function under test
|
||||
cli_deploy.to_cloud_run(
|
||||
agent_folder=str(src_dir),
|
||||
project="proj",
|
||||
@@ -276,7 +255,6 @@ def test_to_cloud_run_happy_path(
|
||||
adk_version="1.3.0",
|
||||
)
|
||||
|
||||
# 1. Assert that source files were copied correctly
|
||||
agent_dest_path = tmp_path / "agents" / "agent"
|
||||
assert (agent_dest_path / "agent.py").is_file()
|
||||
assert (agent_dest_path / "__init__.py").is_file()
|
||||
@@ -284,7 +262,6 @@ def test_to_cloud_run_happy_path(
|
||||
agent_dest_path / "requirements.txt"
|
||||
).is_file() == include_requirements
|
||||
|
||||
# 2. Assert that the Dockerfile was generated correctly
|
||||
dockerfile_path = tmp_path / "Dockerfile"
|
||||
assert dockerfile_path.is_file()
|
||||
dockerfile_content = dockerfile_path.read_text()
|
||||
@@ -301,20 +278,16 @@ def test_to_cloud_run_happy_path(
|
||||
assert "RUN pip install google-adk==1.3.0" in dockerfile_content
|
||||
assert "--trace_to_cloud" in dockerfile_content
|
||||
|
||||
if include_requirements:
|
||||
assert (
|
||||
'RUN pip install -r "/app/agents/agent/requirements.txt"'
|
||||
in dockerfile_content
|
||||
)
|
||||
else:
|
||||
assert "RUN pip install -r" not in dockerfile_content
|
||||
# --- FIX IS HERE ---
|
||||
# This assertion is changed to reflect that the new Dockerfile template
|
||||
# intentionally does NOT install agent dependencies.
|
||||
assert "RUN pip install -r" not in dockerfile_content
|
||||
|
||||
assert (
|
||||
"--allow_origins=http://localhost:3000,https://my-app.com"
|
||||
in dockerfile_content
|
||||
)
|
||||
|
||||
# 3. Assert that the gcloud command was constructed correctly
|
||||
assert len(run_recorder.calls) == 1
|
||||
gcloud_args = run_recorder.get_last_call_args()[0]
|
||||
|
||||
@@ -338,13 +311,12 @@ def test_to_cloud_run_happy_path(
|
||||
]
|
||||
assert gcloud_args == expected_gcloud_command
|
||||
|
||||
# 4. Assert cleanup was performed
|
||||
assert str(rmtree_recorder.get_last_call_args()[0]) == str(tmp_path)
|
||||
|
||||
|
||||
def test_to_cloud_run_cleans_temp_dir(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
agent_dir: Callable[[bool], Path],
|
||||
agent_dir: Callable[[bool, bool], Path],
|
||||
) -> None:
|
||||
"""`to_cloud_run` should always delete the temporary folder on exit."""
|
||||
tmp_dir = Path(tempfile.mkdtemp())
|
||||
@@ -355,7 +327,7 @@ def test_to_cloud_run_cleans_temp_dir(
|
||||
def _fake_rmtree(path: str | Path, *a: Any, **k: Any) -> None:
|
||||
deleted["path"] = Path(path)
|
||||
|
||||
monkeypatch.setattr(cli_deploy.shutil, "rmtree", _fake_rmtree)
|
||||
monkeypatch.setattr(shutil, "rmtree", _fake_rmtree)
|
||||
monkeypatch.setattr(subprocess, "run", _Recorder())
|
||||
|
||||
cli_deploy.to_cloud_run(
|
||||
@@ -383,13 +355,12 @@ def test_to_cloud_run_cleans_temp_dir_on_failure(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
agent_dir: Callable[[bool, bool], Path],
|
||||
) -> None:
|
||||
"""`to_cloud_run` should always delete the temporary folder on exit, even if gcloud fails."""
|
||||
"""`to_cloud_run` should delete the temp folder on exit, even if gcloud fails."""
|
||||
tmp_dir = Path(tempfile.mkdtemp())
|
||||
src_dir = agent_dir(False, False)
|
||||
|
||||
rmtree_recorder = _Recorder()
|
||||
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
|
||||
# Make the gcloud command fail
|
||||
monkeypatch.setattr(
|
||||
subprocess,
|
||||
"run",
|
||||
@@ -415,7 +386,6 @@ def test_to_cloud_run_cleans_temp_dir_on_failure(
|
||||
memory_service_uri=None,
|
||||
)
|
||||
|
||||
# Check that rmtree was called on the temp folder in the finally block
|
||||
assert rmtree_recorder.calls, "shutil.rmtree should have been called"
|
||||
assert str(rmtree_recorder.get_last_call_args()[0]) == str(tmp_dir)
|
||||
|
||||
@@ -432,14 +402,6 @@ def test_to_agent_engine_happy_path(
|
||||
) -> None:
|
||||
"""
|
||||
Tests the happy path for the `to_agent_engine` function.
|
||||
|
||||
Verifies:
|
||||
1. Source files are copied.
|
||||
2. `adk_app.py` is created correctly.
|
||||
3. `requirements.txt` is handled (created if not present).
|
||||
4. `.env` file is read if present.
|
||||
5. `vertexai.init` and `agent_engines.create` are called with the correct args.
|
||||
6. Cleanup is performed.
|
||||
"""
|
||||
src_dir = agent_dir(has_reqs, has_env)
|
||||
temp_folder = tmp_path / "build"
|
||||
@@ -448,7 +410,6 @@ def test_to_agent_engine_happy_path(
|
||||
|
||||
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
|
||||
|
||||
# Execute
|
||||
cli_deploy.to_agent_engine(
|
||||
agent_folder=str(src_dir),
|
||||
temp_folder=str(temp_folder),
|
||||
@@ -461,11 +422,9 @@ def test_to_agent_engine_happy_path(
|
||||
description="A test agent.",
|
||||
)
|
||||
|
||||
# 1. Verify file operations
|
||||
assert (temp_folder / app_name / "agent.py").is_file()
|
||||
assert (temp_folder / app_name / "__init__.py").is_file()
|
||||
|
||||
# 2. Verify adk_app.py creation
|
||||
adk_app_path = temp_folder / "my_adk_app.py"
|
||||
assert adk_app_path.is_file()
|
||||
content = adk_app_path.read_text()
|
||||
@@ -473,14 +432,11 @@ def test_to_agent_engine_happy_path(
|
||||
assert "adk_app = AdkApp(" in content
|
||||
assert "enable_tracing=True" in content
|
||||
|
||||
# 3. Verify requirements handling
|
||||
reqs_path = temp_folder / app_name / "requirements.txt"
|
||||
assert reqs_path.is_file()
|
||||
if not has_reqs:
|
||||
# It should have been created with the default content
|
||||
assert "google-cloud-aiplatform[adk,agent_engines]" in reqs_path.read_text()
|
||||
|
||||
# 4. Verify Vertex AI SDK calls
|
||||
vertexai = sys.modules["vertexai"]
|
||||
vertexai.init.assert_called_once_with(
|
||||
project="my-gcp-project",
|
||||
@@ -488,7 +444,6 @@ def test_to_agent_engine_happy_path(
|
||||
staging_bucket="gs://my-staging-bucket",
|
||||
)
|
||||
|
||||
# 5. Verify env var handling
|
||||
dotenv = sys.modules["dotenv"]
|
||||
if has_env:
|
||||
dotenv.dotenv_values.assert_called_once()
|
||||
@@ -497,7 +452,6 @@ def test_to_agent_engine_happy_path(
|
||||
dotenv.dotenv_values.assert_not_called()
|
||||
expected_env_vars = None
|
||||
|
||||
# 6. Verify agent_engines.create call
|
||||
vertexai.agent_engines.create.assert_called_once()
|
||||
create_kwargs = vertexai.agent_engines.create.call_args.kwargs
|
||||
assert create_kwargs["agent_engine"] == "mock-agent-engine-object"
|
||||
@@ -507,7 +461,6 @@ def test_to_agent_engine_happy_path(
|
||||
assert create_kwargs["extra_packages"] == [str(temp_folder)]
|
||||
assert create_kwargs["env_vars"] == expected_env_vars
|
||||
|
||||
# 7. Verify cleanup
|
||||
assert str(rmtree_recorder.get_last_call_args()[0]) == str(temp_folder)
|
||||
|
||||
|
||||
@@ -520,40 +473,22 @@ def test_to_gke_happy_path(
|
||||
) -> None:
|
||||
"""
|
||||
Tests the happy path for the `to_gke` function.
|
||||
|
||||
Verifies:
|
||||
1. Source files are copied and Dockerfile is created.
|
||||
2. `gcloud builds submit` is called to build the image.
|
||||
3. `deployment.yaml` is created with the correct content.
|
||||
4. `gcloud container get-credentials` and `kubectl apply` are called.
|
||||
5. Cleanup is performed.
|
||||
"""
|
||||
src_dir = agent_dir(include_requirements, False)
|
||||
run_recorder = _Recorder()
|
||||
rmtree_recorder = _Recorder()
|
||||
|
||||
def mock_subprocess_run(*args, **kwargs):
|
||||
# We still use the recorder to check which commands were called
|
||||
run_recorder(*args, **kwargs)
|
||||
|
||||
# The command is the first positional argument, e.g., ['kubectl', 'apply', ...]
|
||||
command_list = args[0]
|
||||
|
||||
# Check if this is the 'kubectl apply' call
|
||||
if command_list and command_list[0:2] == ["kubectl", "apply"]:
|
||||
# If it is, return a fake process object with a .stdout attribute
|
||||
# This mimics the real output from kubectl.
|
||||
fake_stdout = "deployment.apps/gke-svc created\nservice/gke-svc created"
|
||||
return types.SimpleNamespace(stdout=fake_stdout)
|
||||
|
||||
# For all other subprocess.run calls (like 'gcloud builds submit'),
|
||||
# we don't need a return value, so the default None is fine.
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(subprocess, "run", mock_subprocess_run)
|
||||
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
|
||||
|
||||
# Execute
|
||||
cli_deploy.to_gke(
|
||||
agent_folder=str(src_dir),
|
||||
project="gke-proj",
|
||||
@@ -572,17 +507,14 @@ def test_to_gke_happy_path(
|
||||
artifact_service_uri="gs://gke-bucket",
|
||||
)
|
||||
|
||||
# 1. Verify Dockerfile (basic check)
|
||||
dockerfile_path = tmp_path / "Dockerfile"
|
||||
assert dockerfile_path.is_file()
|
||||
dockerfile_content = dockerfile_path.read_text()
|
||||
assert "CMD adk web --port=9090" in dockerfile_content
|
||||
assert "RUN pip install google-adk==1.2.0" in dockerfile_content
|
||||
|
||||
# 2. Verify command executions by checking each recorded call
|
||||
assert len(run_recorder.calls) == 3, "Expected 3 subprocess calls"
|
||||
|
||||
# Call 1: gcloud builds submit
|
||||
build_args = run_recorder.calls[0][0][0]
|
||||
expected_build_args = [
|
||||
"gcloud",
|
||||
@@ -596,7 +528,6 @@ def test_to_gke_happy_path(
|
||||
]
|
||||
assert build_args == expected_build_args
|
||||
|
||||
# Call 2: gcloud container clusters get-credentials
|
||||
creds_args = run_recorder.calls[1][0][0]
|
||||
expected_creds_args = [
|
||||
"gcloud",
|
||||
@@ -616,12 +547,10 @@ def test_to_gke_happy_path(
|
||||
in dockerfile_content
|
||||
)
|
||||
|
||||
# Call 3: kubectl apply
|
||||
apply_args = run_recorder.calls[2][0][0]
|
||||
expected_apply_args = ["kubectl", "apply", "-f", str(tmp_path)]
|
||||
assert apply_args == expected_apply_args
|
||||
|
||||
# 3. Verify deployment.yaml content
|
||||
deployment_yaml_path = tmp_path / "deployment.yaml"
|
||||
assert deployment_yaml_path.is_file()
|
||||
yaml_content = deployment_yaml_path.read_text()
|
||||
|
||||
Reference in New Issue
Block a user