Enforce that paths used for BundleMap and NativeAppCompiler are alway…

…s absolute (#1678)

Subtle cwd-dependent bugs can creep in if we work with relative paths too much. Let's enforce that all paths used to initialize `BundleMap` and `NativeAppCompiler` should be absolute so there's no doubt about where files will be read from or written to. Paths to individual files within these roots are still allowed to be relative.

RELEASE-NOTES.md

@@ -23,6 +23,7 @@
 
 
 ## Fixes and improvements
+* Fixed a bug that would cause the `deploy_root`, `bundle_root`, and `generated_root` directories to be created in the current working directory instead of the project root when invoking commands with the `--project` flag from a different directory.
 
 # v3.0.0
 

src/snowflake/cli/_plugins/nativeapp/artifacts.py

@@ -240,9 +240,21 @@ class BundleMap:
     """
     Computes the mapping between project directory artifacts (aka source artifacts) to their deploy root location
     (aka destination artifact). This information is primarily used when bundling a native applications project.
+
+    :param project_root: The root directory of the project and base for all relative paths. Must be an absolute path.
+    :param deploy_root: The directory where artifacts should be copied to. Must be an absolute path.
     """
 
     def __init__(self, *, project_root: Path, deploy_root: Path):
+        # If a relative path ends up here, it's a bug in the app and can lead to other
+        # subtle bugs as paths would be resolved relative to the current working directory.
+        assert (
+            project_root.is_absolute()
+        ), f"Project root {project_root} must be an absolute path."
+        assert (
+            deploy_root.is_absolute()
+        ), f"Deploy root {deploy_root} must be an absolute path."
+
         self._project_root: Path = resolve_without_follow(project_root)
         self._deploy_root: Path = resolve_without_follow(deploy_root)
         self._artifact_map = _ArtifactPathMap(project_root=self._project_root)

src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py

@@ -65,10 +65,17 @@ def __init__(
         self,
         bundle_ctx: BundleContext,
     ):
+        self._assert_absolute_paths(bundle_ctx)
         self._bundle_ctx = bundle_ctx
         # dictionary of all processors created and shared between different artifact objects.
         self.cached_processors: Dict[str, ArtifactProcessor] = {}
 
+    @staticmethod
+    def _assert_absolute_paths(bundle_ctx: BundleContext):
+        for name in ["Project", "Deploy", "Bundle", "Generated"]:
+            path = getattr(bundle_ctx, f"{name.lower()}_root")
+            assert path.is_absolute(), f"{name} root {path} must be an absolute path."
+
     def compile_artifacts(self):
         """
         Go through every artifact object in the project definition of a native app, and execute processors in order of specification for each of the artifact object.

src/snowflake/cli/_plugins/nativeapp/entities/application_package.py

@@ -155,9 +155,11 @@ def action_bundle(self, action_ctx: ActionContext, *args, **kwargs):
         workspace_ctx = self._workspace_ctx
         return self.bundle(
             project_root=workspace_ctx.project_root,
-            deploy_root=Path(model.deploy_root),
-            bundle_root=Path(model.bundle_root),
-            generated_root=Path(model.generated_root),
+            deploy_root=workspace_ctx.project_root / model.deploy_root,
+            bundle_root=workspace_ctx.project_root / model.bundle_root,
+            generated_root=(
+                workspace_ctx.project_root / model.deploy_root / model.generated_root
+            ),
             package_name=model.identifier,
             artifacts=model.artifacts,
         )
@@ -189,9 +191,11 @@ def action_deploy(
         return self.deploy(
             console=workspace_ctx.console,
             project_root=workspace_ctx.project_root,
-            deploy_root=Path(model.deploy_root),
-            bundle_root=Path(model.bundle_root),
-            generated_root=Path(model.generated_root),
+            deploy_root=workspace_ctx.project_root / model.deploy_root,
+            bundle_root=workspace_ctx.project_root / model.bundle_root,
+            generated_root=(
+                workspace_ctx.project_root / model.deploy_root / model.generated_root
+            ),
             artifacts=model.artifacts,
             bundle_map=None,
             package_name=package_name,
@@ -243,9 +247,11 @@ def action_validate(
         self.validate_setup_script(
             console=workspace_ctx.console,
             project_root=workspace_ctx.project_root,
-            deploy_root=Path(model.deploy_root),
-            bundle_root=Path(model.bundle_root),
-            generated_root=Path(model.generated_root),
+            deploy_root=workspace_ctx.project_root / model.deploy_root,
+            bundle_root=workspace_ctx.project_root / model.bundle_root,
+            generated_root=(
+                workspace_ctx.project_root / model.deploy_root / model.generated_root
+            ),
             artifacts=model.artifacts,
             package_name=package_name,
             package_role=(model.meta and model.meta.role) or workspace_ctx.default_role,
@@ -292,9 +298,11 @@ def action_version_create(
         return self.version_create(
             console=workspace_ctx.console,
             project_root=workspace_ctx.project_root,
-            deploy_root=Path(model.deploy_root),
-            bundle_root=Path(model.bundle_root),
-            generated_root=Path(model.generated_root),
+            deploy_root=workspace_ctx.project_root / model.deploy_root,
+            bundle_root=workspace_ctx.project_root / model.bundle_root,
+            generated_root=(
+                workspace_ctx.project_root / model.deploy_root / model.generated_root
+            ),
             artifacts=model.artifacts,
             package_name=package_name,
             package_role=(model.meta and model.meta.role) or workspace_ctx.default_role,
@@ -332,9 +340,11 @@ def action_version_drop(
         return self.version_drop(
             console=workspace_ctx.console,
             project_root=workspace_ctx.project_root,
-            deploy_root=Path(model.deploy_root),
-            bundle_root=Path(model.bundle_root),
-            generated_root=Path(model.generated_root),
+            deploy_root=workspace_ctx.project_root / model.deploy_root,
+            bundle_root=workspace_ctx.project_root / model.bundle_root,
+            generated_root=(
+                workspace_ctx.project_root / model.deploy_root / model.generated_root
+            ),
             artifacts=model.artifacts,
             package_name=package_name,
             package_role=(model.meta and model.meta.role) or workspace_ctx.default_role,

src/snowflake/cli/api/project/definition_conversion.py

@@ -243,7 +243,7 @@ def _find_manifest():
         # manifest file from the resultant BundleMap, since the bundle process ensures
         # that only a single source path can map to the corresponding destination path
         bundle_map = BundleMap(
-            project_root=project_root, deploy_root=Path(native_app.deploy_root)
+            project_root=project_root, deploy_root=project_root / native_app.deploy_root
         )
         for artifact in native_app.artifacts:
             bundle_map.add(artifact)

tests/nativeapp/codegen/test_compiler.py

@@ -11,7 +11,8 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
+import re
+from pathlib import Path
 
 import pytest
 from snowflake.cli._plugins.nativeapp.codegen.artifact_processor import (
@@ -62,6 +63,20 @@ def test_compiler(test_proj_def):
     return NativeAppCompiler(na_project.get_bundle_context())
 
 
+@pytest.mark.parametrize("name", ["Project", "Deploy", "Bundle", "Generated"])
+def test_compiler_requires_absolute_paths(test_proj_def, name):
+    na_project = create_native_app_project_model(test_proj_def.native_app)
+    bundle_context = na_project.get_bundle_context()
+
+    path = Path()
+    setattr(bundle_context, f"{name.lower()}_root", path)
+    with pytest.raises(
+        AssertionError,
+        match=re.escape(rf"{name} root {path} must be an absolute path."),
+    ):
+        NativeAppCompiler(bundle_context)
+
+
 def test_try_create_processor_returns_none(test_proj_def, test_compiler):
     artifact_to_process = test_proj_def.native_app.artifacts[2]
     result = test_compiler._try_create_processor(  # noqa: SLF001

tests/nativeapp/test_artifacts.py

@@ -15,6 +15,7 @@
 from __future__ import annotations
 
 import os
+import re
 from pathlib import Path
 from typing import Dict, Iterable, List, Optional, Union
 
@@ -166,6 +167,24 @@ def test_empty_bundle_map(bundle_map):
     )
 
 
+def test_bundle_map_requires_absolute_project_root():
+    project_root = Path()
+    with pytest.raises(
+        AssertionError,
+        match=re.escape(rf"Project root {project_root} must be an absolute path."),
+    ):
+        BundleMap(project_root=project_root, deploy_root=Path("output/deploy"))
+
+
+def test_bundle_map_requires_absolute_deploy_root():
+    deploy_root = Path("output/deploy")
+    with pytest.raises(
+        AssertionError,
+        match=re.escape(rf"Deploy root {deploy_root} must be an absolute path."),
+    ):
+        BundleMap(project_root=Path().resolve(), deploy_root=deploy_root)
+
+
 def test_bundle_map_handles_file_to_file_mappings(bundle_map):
     bundle_map.add(PathMapping(src="README.md", dest="deployed_readme.md"))
     bundle_map.add(PathMapping(src="app/setup.sql", dest="app_setup.sql"))

tests/workspace/test_application_package_entity.py

@@ -150,7 +150,9 @@ def test_deploy(
 
     mock_sync.assert_called_once_with(
         console=mock_console,
-        deploy_root=Path("output/deploy"),
+        deploy_root=(
+            app_pkg._workspace_ctx.project_root / Path("output/deploy")  # noqa SLF001
+        ),
         package_name="pkg",
         stage_schema="app_src",
         bundle_map=mock.ANY,