Skip to content

Commit

Permalink
Enforce that paths used for BundleMap and NativeAppCompiler are alway…
Browse files Browse the repository at this point in the history
…s absolute
  • Loading branch information
sfc-gh-fcampbell committed Oct 7, 2024
1 parent dda3ce5 commit 9806127
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 16 deletions.
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 12 additions & 0 deletions src/snowflake/cli/_plugins/nativeapp/artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 14 additions & 1 deletion tests/nativeapp/codegen/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions tests/nativeapp/test_artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down

0 comments on commit 9806127

Please sign in to comment.