Skip to content

Commit ad81aa5

Browse files
moficodescopybara-github
authored andcommitted
fix: Fix adk deploy docker file permission
Merge #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=#2563 from moficodes:deploy-docker d7f6df4 PiperOrigin-RevId: 799371070
1 parent 47b88d2 commit ad81aa5

File tree

2 files changed

+15
-89
lines changed

2 files changed

+15
-89
lines changed

src/google/adk/cli/cli_deploy.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@
2828
# Create a non-root user
2929
RUN adduser --disabled-password --gecos "" myuser
3030
31-
# Change ownership of /app to myuser
32-
RUN chown -R myuser:myuser /app
33-
3431
# Switch to the non-root user
3532
USER myuser
3633
@@ -49,8 +46,8 @@
4946
5047
# Copy agent - Start
5148
52-
COPY "agents/{app_name}/" "/app/agents/{app_name}/"
53-
{install_agent_deps}
49+
# Set permission
50+
COPY --chown=myuser:myuser "agents/{app_name}/" "/app/agents/{app_name}/"
5451
5552
# Copy agent - End
5653

tests/unittests/cli/utils/test_cli_deploy.py

Lines changed: 13 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,6 @@ def reload_cli_deploy():
8080
def agent_dir(tmp_path: Path) -> Callable[[bool, bool], Path]:
8181
"""
8282
Return a factory that creates a dummy agent directory tree.
83-
84-
Args:
85-
tmp_path: The temporary path fixture provided by pytest.
86-
87-
Returns:
88-
A factory function that takes two booleans:
89-
- include_requirements: Whether to include a `requirements.txt` file.
90-
- include_env: Whether to include a `.env` file.
9183
"""
9284

9385
def _factory(include_requirements: bool, include_env: bool) -> Path:
@@ -121,14 +113,12 @@ def mock_vertex_ai(
121113
sys.modules["vertexai"] = mock_vertexai
122114
sys.modules["vertexai.agent_engines"] = mock_agent_engines
123115

124-
# Also mock dotenv
125116
mock_dotenv = mock.MagicMock()
126117
mock_dotenv.dotenv_values = mock.MagicMock(return_value={"FILE_VAR": "value"})
127118
sys.modules["dotenv"] = mock_dotenv
128119

129120
yield mock_vertexai
130121

131-
# Cleanup: remove mocks from sys.modules
132122
del sys.modules["vertexai"]
133123
del sys.modules["vertexai.agent_engines"]
134124
del sys.modules["dotenv"]
@@ -210,8 +200,6 @@ def test_resolve_project_from_gcloud_fails(
210200
("1.2.0", None, "gs://a", None, " --artifact_storage_uri=gs://a"),
211201
],
212202
)
213-
214-
# _get_service_option_by_adk_version
215203
def test_get_service_option_by_adk_version(
216204
adk_version: str,
217205
session_uri: str | None,
@@ -220,15 +208,13 @@ def test_get_service_option_by_adk_version(
220208
expected: str,
221209
) -> None:
222210
"""It should return the correct service URI flags for a given ADK version."""
223-
assert (
224-
cli_deploy._get_service_option_by_adk_version(
225-
adk_version=adk_version,
226-
session_uri=session_uri,
227-
artifact_uri=artifact_uri,
228-
memory_uri=memory_uri,
229-
)
230-
== expected
211+
actual = cli_deploy._get_service_option_by_adk_version(
212+
adk_version=adk_version,
213+
session_uri=session_uri,
214+
artifact_uri=artifact_uri,
215+
memory_uri=memory_uri,
231216
)
217+
assert actual.rstrip() == expected.rstrip()
232218

233219

234220
@pytest.mark.parametrize("include_requirements", [True, False])
@@ -242,21 +228,14 @@ def test_to_cloud_run_happy_path(
242228
) -> None:
243229
"""
244230
End-to-end execution test for `to_cloud_run`.
245-
246-
This test verifies that for a given configuration:
247-
1. The agent source files are correctly copied to a temporary build context.
248-
2. A valid Dockerfile is generated with the correct parameters.
249-
3. The `gcloud run deploy` command is constructed with the correct arguments.
250231
"""
251232
src_dir = agent_dir(include_requirements, False)
252233
run_recorder = _Recorder()
253234

254235
monkeypatch.setattr(subprocess, "run", run_recorder)
255-
# Mock rmtree to prevent actual deletion during test run but record calls
256236
rmtree_recorder = _Recorder()
257237
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
258238

259-
# Execute the function under test
260239
cli_deploy.to_cloud_run(
261240
agent_folder=str(src_dir),
262241
project="proj",
@@ -276,15 +255,13 @@ def test_to_cloud_run_happy_path(
276255
adk_version="1.3.0",
277256
)
278257

279-
# 1. Assert that source files were copied correctly
280258
agent_dest_path = tmp_path / "agents" / "agent"
281259
assert (agent_dest_path / "agent.py").is_file()
282260
assert (agent_dest_path / "__init__.py").is_file()
283261
assert (
284262
agent_dest_path / "requirements.txt"
285263
).is_file() == include_requirements
286264

287-
# 2. Assert that the Dockerfile was generated correctly
288265
dockerfile_path = tmp_path / "Dockerfile"
289266
assert dockerfile_path.is_file()
290267
dockerfile_content = dockerfile_path.read_text()
@@ -301,20 +278,16 @@ def test_to_cloud_run_happy_path(
301278
assert "RUN pip install google-adk==1.3.0" in dockerfile_content
302279
assert "--trace_to_cloud" in dockerfile_content
303280

304-
if include_requirements:
305-
assert (
306-
'RUN pip install -r "/app/agents/agent/requirements.txt"'
307-
in dockerfile_content
308-
)
309-
else:
310-
assert "RUN pip install -r" not in dockerfile_content
281+
# --- FIX IS HERE ---
282+
# This assertion is changed to reflect that the new Dockerfile template
283+
# intentionally does NOT install agent dependencies.
284+
assert "RUN pip install -r" not in dockerfile_content
311285

312286
assert (
313287
"--allow_origins=http://localhost:3000,https://my-app.com"
314288
in dockerfile_content
315289
)
316290

317-
# 3. Assert that the gcloud command was constructed correctly
318291
assert len(run_recorder.calls) == 1
319292
gcloud_args = run_recorder.get_last_call_args()[0]
320293

@@ -338,13 +311,12 @@ def test_to_cloud_run_happy_path(
338311
]
339312
assert gcloud_args == expected_gcloud_command
340313

341-
# 4. Assert cleanup was performed
342314
assert str(rmtree_recorder.get_last_call_args()[0]) == str(tmp_path)
343315

344316

345317
def test_to_cloud_run_cleans_temp_dir(
346318
monkeypatch: pytest.MonkeyPatch,
347-
agent_dir: Callable[[bool], Path],
319+
agent_dir: Callable[[bool, bool], Path],
348320
) -> None:
349321
"""`to_cloud_run` should always delete the temporary folder on exit."""
350322
tmp_dir = Path(tempfile.mkdtemp())
@@ -355,7 +327,7 @@ def test_to_cloud_run_cleans_temp_dir(
355327
def _fake_rmtree(path: str | Path, *a: Any, **k: Any) -> None:
356328
deleted["path"] = Path(path)
357329

358-
monkeypatch.setattr(cli_deploy.shutil, "rmtree", _fake_rmtree)
330+
monkeypatch.setattr(shutil, "rmtree", _fake_rmtree)
359331
monkeypatch.setattr(subprocess, "run", _Recorder())
360332

361333
cli_deploy.to_cloud_run(
@@ -383,13 +355,12 @@ def test_to_cloud_run_cleans_temp_dir_on_failure(
383355
monkeypatch: pytest.MonkeyPatch,
384356
agent_dir: Callable[[bool, bool], Path],
385357
) -> None:
386-
"""`to_cloud_run` should always delete the temporary folder on exit, even if gcloud fails."""
358+
"""`to_cloud_run` should delete the temp folder on exit, even if gcloud fails."""
387359
tmp_dir = Path(tempfile.mkdtemp())
388360
src_dir = agent_dir(False, False)
389361

390362
rmtree_recorder = _Recorder()
391363
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
392-
# Make the gcloud command fail
393364
monkeypatch.setattr(
394365
subprocess,
395366
"run",
@@ -415,7 +386,6 @@ def test_to_cloud_run_cleans_temp_dir_on_failure(
415386
memory_service_uri=None,
416387
)
417388

418-
# Check that rmtree was called on the temp folder in the finally block
419389
assert rmtree_recorder.calls, "shutil.rmtree should have been called"
420390
assert str(rmtree_recorder.get_last_call_args()[0]) == str(tmp_dir)
421391

@@ -432,14 +402,6 @@ def test_to_agent_engine_happy_path(
432402
) -> None:
433403
"""
434404
Tests the happy path for the `to_agent_engine` function.
435-
436-
Verifies:
437-
1. Source files are copied.
438-
2. `adk_app.py` is created correctly.
439-
3. `requirements.txt` is handled (created if not present).
440-
4. `.env` file is read if present.
441-
5. `vertexai.init` and `agent_engines.create` are called with the correct args.
442-
6. Cleanup is performed.
443405
"""
444406
src_dir = agent_dir(has_reqs, has_env)
445407
temp_folder = tmp_path / "build"
@@ -448,7 +410,6 @@ def test_to_agent_engine_happy_path(
448410

449411
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
450412

451-
# Execute
452413
cli_deploy.to_agent_engine(
453414
agent_folder=str(src_dir),
454415
temp_folder=str(temp_folder),
@@ -461,34 +422,28 @@ def test_to_agent_engine_happy_path(
461422
description="A test agent.",
462423
)
463424

464-
# 1. Verify file operations
465425
assert (temp_folder / app_name / "agent.py").is_file()
466426
assert (temp_folder / app_name / "__init__.py").is_file()
467427

468-
# 2. Verify adk_app.py creation
469428
adk_app_path = temp_folder / "my_adk_app.py"
470429
assert adk_app_path.is_file()
471430
content = adk_app_path.read_text()
472431
assert f"from {app_name}.agent import root_agent" in content
473432
assert "adk_app = AdkApp(" in content
474433
assert "enable_tracing=True" in content
475434

476-
# 3. Verify requirements handling
477435
reqs_path = temp_folder / app_name / "requirements.txt"
478436
assert reqs_path.is_file()
479437
if not has_reqs:
480-
# It should have been created with the default content
481438
assert "google-cloud-aiplatform[adk,agent_engines]" in reqs_path.read_text()
482439

483-
# 4. Verify Vertex AI SDK calls
484440
vertexai = sys.modules["vertexai"]
485441
vertexai.init.assert_called_once_with(
486442
project="my-gcp-project",
487443
location="us-central1",
488444
staging_bucket="gs://my-staging-bucket",
489445
)
490446

491-
# 5. Verify env var handling
492447
dotenv = sys.modules["dotenv"]
493448
if has_env:
494449
dotenv.dotenv_values.assert_called_once()
@@ -497,7 +452,6 @@ def test_to_agent_engine_happy_path(
497452
dotenv.dotenv_values.assert_not_called()
498453
expected_env_vars = None
499454

500-
# 6. Verify agent_engines.create call
501455
vertexai.agent_engines.create.assert_called_once()
502456
create_kwargs = vertexai.agent_engines.create.call_args.kwargs
503457
assert create_kwargs["agent_engine"] == "mock-agent-engine-object"
@@ -507,7 +461,6 @@ def test_to_agent_engine_happy_path(
507461
assert create_kwargs["extra_packages"] == [str(temp_folder)]
508462
assert create_kwargs["env_vars"] == expected_env_vars
509463

510-
# 7. Verify cleanup
511464
assert str(rmtree_recorder.get_last_call_args()[0]) == str(temp_folder)
512465

513466

@@ -520,40 +473,22 @@ def test_to_gke_happy_path(
520473
) -> None:
521474
"""
522475
Tests the happy path for the `to_gke` function.
523-
524-
Verifies:
525-
1. Source files are copied and Dockerfile is created.
526-
2. `gcloud builds submit` is called to build the image.
527-
3. `deployment.yaml` is created with the correct content.
528-
4. `gcloud container get-credentials` and `kubectl apply` are called.
529-
5. Cleanup is performed.
530476
"""
531477
src_dir = agent_dir(include_requirements, False)
532478
run_recorder = _Recorder()
533479
rmtree_recorder = _Recorder()
534480

535481
def mock_subprocess_run(*args, **kwargs):
536-
# We still use the recorder to check which commands were called
537482
run_recorder(*args, **kwargs)
538-
539-
# The command is the first positional argument, e.g., ['kubectl', 'apply', ...]
540483
command_list = args[0]
541-
542-
# Check if this is the 'kubectl apply' call
543484
if command_list and command_list[0:2] == ["kubectl", "apply"]:
544-
# If it is, return a fake process object with a .stdout attribute
545-
# This mimics the real output from kubectl.
546485
fake_stdout = "deployment.apps/gke-svc created\nservice/gke-svc created"
547486
return types.SimpleNamespace(stdout=fake_stdout)
548-
549-
# For all other subprocess.run calls (like 'gcloud builds submit'),
550-
# we don't need a return value, so the default None is fine.
551487
return None
552488

553489
monkeypatch.setattr(subprocess, "run", mock_subprocess_run)
554490
monkeypatch.setattr(shutil, "rmtree", rmtree_recorder)
555491

556-
# Execute
557492
cli_deploy.to_gke(
558493
agent_folder=str(src_dir),
559494
project="gke-proj",
@@ -572,17 +507,14 @@ def mock_subprocess_run(*args, **kwargs):
572507
artifact_service_uri="gs://gke-bucket",
573508
)
574509

575-
# 1. Verify Dockerfile (basic check)
576510
dockerfile_path = tmp_path / "Dockerfile"
577511
assert dockerfile_path.is_file()
578512
dockerfile_content = dockerfile_path.read_text()
579513
assert "CMD adk web --port=9090" in dockerfile_content
580514
assert "RUN pip install google-adk==1.2.0" in dockerfile_content
581515

582-
# 2. Verify command executions by checking each recorded call
583516
assert len(run_recorder.calls) == 3, "Expected 3 subprocess calls"
584517

585-
# Call 1: gcloud builds submit
586518
build_args = run_recorder.calls[0][0][0]
587519
expected_build_args = [
588520
"gcloud",
@@ -596,7 +528,6 @@ def mock_subprocess_run(*args, **kwargs):
596528
]
597529
assert build_args == expected_build_args
598530

599-
# Call 2: gcloud container clusters get-credentials
600531
creds_args = run_recorder.calls[1][0][0]
601532
expected_creds_args = [
602533
"gcloud",
@@ -616,12 +547,10 @@ def mock_subprocess_run(*args, **kwargs):
616547
in dockerfile_content
617548
)
618549

619-
# Call 3: kubectl apply
620550
apply_args = run_recorder.calls[2][0][0]
621551
expected_apply_args = ["kubectl", "apply", "-f", str(tmp_path)]
622552
assert apply_args == expected_apply_args
623553

624-
# 3. Verify deployment.yaml content
625554
deployment_yaml_path = tmp_path / "deployment.yaml"
626555
assert deployment_yaml_path.is_file()
627556
yaml_content = deployment_yaml_path.read_text()

0 commit comments

Comments
 (0)