commit 41f247b8650c5a80d97411daeaf724e0615e6a4c
author Brian Norris <briannorris@chromium.org> Fri Jun 30 11:09:40 2023 -0700
committer Chromeos LUCI <chromeos-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Jun 30 22:58:35 2023 +0000
tree c42e5d78305c18d4800f76d5cb6b756e6d09cb38
parent d3e391e626ab3b086ce7c0f95050ff000300973f

api: controller: Fix pb2_path_to_pathlib_path() chroot assumptions

It's not valid to do path concatenation or subtraction using
chroot.path. Use chroot_lib instead.

BUG=b:265885353
TEST=./run_tests

Change-Id: I1f5be1b97a16bdb0f91ee66b0e657aa71c9c501d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/4661950
Reviewed-by: Greg Edelston <gredelston@google.com>
Tested-by: Brian Norris <briannorris@chromium.org>
Commit-Queue: Brian Norris <briannorris@chromium.org>

api/controller/sdk_unittest.py

diff --git a/api/controller/sdk_unittest.py b/api/controller/sdk_unittest.py
index 014e281..0ca944f 100644
--- a/api/controller/sdk_unittest.py
+++ b/api/controller/sdk_unittest.py
@@ -5,7 +5,7 @@
 """SDK tests."""
 
 import os
-import pathlib
+from pathlib import Path
 from typing import List, Optional
 from unittest import mock
 
@@ -13,6 +13,7 @@
 from chromite.api.controller import sdk as sdk_controller
 from chromite.api.gen.chromite.api import sdk_pb2
 from chromite.api.gen.chromiumos import common_pb2
+from chromite.lib import chroot_lib
 from chromite.lib import constants
 from chromite.lib import cros_build_lib
 from chromite.lib import cros_test_lib
@@ -369,16 +370,17 @@
 ):
     """Test the SdkService/CreateManifestFromSdk endpoint."""
 
-    _chroot_path = "/path/to/chroot"
-    _sdk_path_relative = "build/my_sdk"
+    _sdk_path = "/build/my_sdk"
     _dest_dir = "/build"
     _manifest_path = "/build/my_sdk.Manifest"
 
     def _NewRequest(self, inside: bool) -> sdk_pb2.CreateManifestFromSdkRequest:
         return sdk_pb2.CreateManifestFromSdkRequest(
-            chroot=common_pb2.Chroot(path=self._chroot_path),
+            chroot=common_pb2.Chroot(
+                path=self.chroot.path, out_path=str(self.chroot.out_path)
+            ),
             sdk_path=common_pb2.Path(
-                path="/%s" % self._sdk_path_relative,
+                path=self._sdk_path,
                 location=common_pb2.Path.Location.INSIDE
                 if inside
                 else common_pb2.Path.Location.OUTSIDE,
@@ -392,6 +394,14 @@
     def _NewResponse(self) -> sdk_pb2.CreateManifestFromSdkResponse:
         return sdk_pb2.CreateManifestFromSdkResponse()
 
+    def setUp(self):
+        self.PatchObject(cros_build_lib, "IsInsideChroot", return_value=False)
+
+        self.chroot = chroot_lib.Chroot(
+            path=Path("/path/to/chroot"),
+            out_path=Path("/path/to/out"),
+        )
+
     def testValidateOnly(self):
         """Check that a validate only call does not execute any logic."""
         impl_patch = self.PatchObject(sdk_service, "CreateManifestFromSdk")
@@ -407,7 +417,7 @@
         impl_patch = self.PatchObject(
             sdk_service,
             "CreateManifestFromSdk",
-            return_value=pathlib.Path(self._manifest_path),
+            return_value=Path(self._manifest_path),
         )
         request = self._NewRequest(inside=False)
         response = self._NewResponse()
@@ -417,8 +427,8 @@
             self.api_config,
         )
         impl_patch.assert_called_with(
-            pathlib.Path("/", self._sdk_path_relative),
-            pathlib.Path(self._dest_dir),
+            Path(self._sdk_path),
+            Path(self._dest_dir),
         )
         self.assertEqual(
             response.manifest_path.location, common_pb2.Path.Location.OUTSIDE
@@ -430,7 +440,7 @@
         impl_patch = self.PatchObject(
             sdk_service,
             "CreateManifestFromSdk",
-            return_value=pathlib.Path(self._manifest_path),
+            return_value=Path(self._manifest_path),
         )
         request = self._NewRequest(inside=True)
         response = self._NewResponse()
@@ -440,8 +450,8 @@
             self.api_config,
         )
         impl_patch.assert_called_with(
-            pathlib.Path(self._chroot_path, self._sdk_path_relative),
-            pathlib.Path(self._dest_dir),
+            Path(self.chroot.full_path(self._sdk_path)),
+            Path(self._dest_dir),
         )
         self.assertEqual(
             response.manifest_path.location, common_pb2.Path.Location.OUTSIDE