From 98061275803ccb57acf4c047c746c012f06e761a Mon Sep 17 00:00:00 2001 From: Francois Campbell Date: Mon, 7 Oct 2024 11:25:45 -0400 Subject: [PATCH] Enforce that paths used for BundleMap and NativeAppCompiler are always absolute --- RELEASE-NOTES.md | 1 + .../cli/_plugins/nativeapp/artifacts.py | 12 ++++++ .../_plugins/nativeapp/codegen/compiler.py | 7 ++++ .../nativeapp/entities/application_package.py | 40 ++++++++++++------- tests/nativeapp/codegen/test_compiler.py | 15 ++++++- tests/nativeapp/test_artifacts.py | 16 ++++++++ 6 files changed, 75 insertions(+), 16 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 4024302a81..d3a3e21aea 100644 --- a/RELEASE-NOTES.md +++ b/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 diff --git a/src/snowflake/cli/_plugins/nativeapp/artifacts.py b/src/snowflake/cli/_plugins/nativeapp/artifacts.py index 0c2ce25bfb..10a0d169ac 100644 --- a/src/snowflake/cli/_plugins/nativeapp/artifacts.py +++ b/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) diff --git a/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py b/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py index 7f89588b69..05456503fe 100644 --- a/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py +++ b/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. diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index 55f9be8cce..8a1b8b1e56 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/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, diff --git a/tests/nativeapp/codegen/test_compiler.py b/tests/nativeapp/codegen/test_compiler.py index c0ec538445..e35ec0ce95 100644 --- a/tests/nativeapp/codegen/test_compiler.py +++ b/tests/nativeapp/codegen/test_compiler.py @@ -11,7 +11,7 @@ # 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. - +from pathlib import Path import pytest from snowflake.cli._plugins.nativeapp.codegen.artifact_processor import ( @@ -62,6 +62,19 @@ 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=f"{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 diff --git a/tests/nativeapp/test_artifacts.py b/tests/nativeapp/test_artifacts.py index ffe5c400fc..cde18430e3 100644 --- a/tests/nativeapp/test_artifacts.py +++ b/tests/nativeapp/test_artifacts.py @@ -166,6 +166,22 @@ def test_empty_bundle_map(bundle_map): ) +def test_bundle_map_requires_absolute_project_root(): + project_root = Path() + with pytest.raises( + AssertionError, match=f"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=f"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"))