From 4125642e53ba646edee7b29f849e0422ca75df93 Mon Sep 17 00:00:00 2001 From: Francois Campbell Date: Tue, 13 Aug 2024 14:40:14 -0400 Subject: [PATCH] Append SNOWFLAKE_CLI_RESOURCE_SUFFIX value to app and package name --- .../cli/_plugins/nativeapp/project_model.py | 71 +++++++++++--- src/snowflake/cli/api/project/definition.py | 10 -- tests/nativeapp/test_project_model.py | 87 ++++++++++++++++- tests_integration/nativeapp/test_deploy.py | 82 ++++++++++++++++ tests_integration/nativeapp/test_init_run.py | 95 +++++++++++++++++++ 5 files changed, 319 insertions(+), 26 deletions(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/project_model.py b/src/snowflake/cli/_plugins/nativeapp/project_model.py index e129ab46cc..c7052cc452 100644 --- a/src/snowflake/cli/_plugins/nativeapp/project_model.py +++ b/src/snowflake/cli/_plugins/nativeapp/project_model.py @@ -14,6 +14,7 @@ from __future__ import annotations +import os from functools import cached_property from pathlib import Path from typing import List, Optional @@ -21,19 +22,23 @@ from snowflake.cli._plugins.nativeapp.artifacts import resolve_without_follow from snowflake.cli._plugins.nativeapp.bundle_context import BundleContext from snowflake.cli.api.cli_global_context import get_cli_context -from snowflake.cli.api.project.definition import ( - default_app_package, - default_application, - default_role, -) +from snowflake.cli.api.project.definition import DEFAULT_USERNAME, default_role from snowflake.cli.api.project.schemas.native_app.application import ( PostDeployHook, ) from snowflake.cli.api.project.schemas.native_app.native_app import NativeApp from snowflake.cli.api.project.schemas.native_app.path_mapping import PathMapping -from snowflake.cli.api.project.util import extract_schema, to_identifier +from snowflake.cli.api.project.util import ( + clean_identifier, + concat_identifiers, + extract_schema, + get_env_username, + to_identifier, +) from snowflake.connector import DictCursor +RESOURCE_SUFFIX_VAR = "SNOWFLAKE_CLI_TEST_RESOURCE_SUFFIX" + def current_role() -> str: conn = get_cli_context().connection @@ -129,12 +134,24 @@ def project_identifier(self) -> str: # sometimes strip out double quotes, so we try to get them back here. return to_identifier(self.definition.name) - @cached_property + @property def package_name(self) -> str: + # If an explicit package name is set, use it and append the resource suffix + # In this case, if the suffix is empty we don't append the default suffix + # since we want to honor the user's chosen package name + suffix = resource_suffix() if self.definition.package and self.definition.package.name: - return to_identifier(self.definition.package.name) - else: - return to_identifier(default_app_package(self.project_identifier)) + return concat_identifiers( + [to_identifier(self.definition.package.name), suffix] + ) + + # If there's no explicit package name set in the project definition, + # generate a name for the package from the project identifier and + # append the resource suffix + # If we don't have a resource suffix specified, use the default one + return concat_identifiers( + [self.project_identifier, "_pkg", suffix or default_resource_suffix()] + ) @cached_property def package_role(self) -> str: @@ -150,12 +167,24 @@ def package_distribution(self) -> str: else: return "internal" - @cached_property + @property def app_name(self) -> str: + # If an explicit app name is set, use it and append the resource suffix + # In this case, if the suffix is empty we don't append the default suffix + # since we want to honor the user's chosen app name + suffix = resource_suffix() if self.definition.application and self.definition.application.name: - return to_identifier(self.definition.application.name) - else: - return to_identifier(default_application(self.project_identifier)) + return concat_identifiers( + [to_identifier(self.definition.application.name), suffix] + ) + + # If there's no explicit package name set in the project definition, + # generate a name for the package from the project identifier and + # append the resource suffix. + # If we don't have a resource suffix specified, use the default one + return concat_identifiers( + [self.project_identifier, suffix or default_resource_suffix()] + ) @cached_property def app_role(self) -> str: @@ -206,3 +235,17 @@ def get_bundle_context(self) -> BundleContext: deploy_root=self.deploy_root, generated_root=self.generated_root, ) + + +def resource_suffix() -> str: + """ + A suffix that should be added to account-level Native App resources. + + This is an internal concern that is currently only used in tests. + """ + return os.environ.get(RESOURCE_SUFFIX_VAR, "") + + +def default_resource_suffix(): + user = clean_identifier(get_env_username() or DEFAULT_USERNAME) + return f"_{user}" diff --git a/src/snowflake/cli/api/project/definition.py b/src/snowflake/cli/api/project/definition.py index 318dec4760..586764ce9b 100644 --- a/src/snowflake/cli/api/project/definition.py +++ b/src/snowflake/cli/api/project/definition.py @@ -102,16 +102,6 @@ def generate_local_override_yml( return project.update_from_dict(local) -def default_app_package(project_name: str): - user = clean_identifier(get_env_username() or DEFAULT_USERNAME) - return append_to_identifier(to_identifier(project_name), f"_pkg_{user}") - - def default_role(): conn = get_cli_context().connection return conn.role - - -def default_application(project_name: str): - user = clean_identifier(get_env_username() or DEFAULT_USERNAME) - return append_to_identifier(to_identifier(project_name), f"_{user}") diff --git a/tests/nativeapp/test_project_model.py b/tests/nativeapp/test_project_model.py index 8b7f7e8a88..fcaccb534d 100644 --- a/tests/nativeapp/test_project_model.py +++ b/tests/nativeapp/test_project_model.py @@ -24,7 +24,10 @@ import yaml from snowflake.cli._plugins.nativeapp.bundle_context import BundleContext from snowflake.cli._plugins.nativeapp.project_model import NativeAppProjectModel -from snowflake.cli.api.project.definition import default_app_package, load_project +from snowflake.cli.api.project.definition import ( + RESOURCE_SUFFIX_VAR, + load_project, +) from snowflake.cli.api.project.schemas.native_app.application import SqlScriptHookType from snowflake.cli.api.project.schemas.native_app.path_mapping import PathMapping from snowflake.cli.api.project.schemas.project_definition import ( @@ -79,6 +82,31 @@ def test_project_model_all_defaults( assert project.debug_mode is None +@pytest.mark.parametrize("project_definition_files", ["minimal"], indirect=True) +@mock.patch("snowflake.cli._app.snow_connector.connect_to_snowflake") +@mock.patch.dict( + os.environ, + {"USER": "test_user", RESOURCE_SUFFIX_VAR: "_suffix!"}, + clear=True, +) +def test_project_model_default_package_app_name_with_suffix( + mock_connect, project_definition_files: List[Path], mock_ctx +): + ctx = mock_ctx() + mock_connect.return_value = ctx + + project_defn = load_project(project_definition_files).project_definition + + project_dir = Path().resolve() + project = NativeAppProjectModel( + project_definition=project_defn.native_app, + project_root=project_dir, + ) + + assert project.package_name == '"minimal_pkg_suffix!"' + assert project.app_name == '"minimal_suffix!"' + + @mock.patch("snowflake.cli._app.snow_connector.connect_to_snowflake") @mock.patch.dict(os.environ, {"USER": "test_user"}, clear=True) def test_project_model_all_explicit(mock_connect, mock_ctx): @@ -153,6 +181,61 @@ def test_project_model_all_explicit(mock_connect, mock_ctx): assert project.debug_mode is False +@pytest.mark.parametrize("project_definition_files", ["minimal"], indirect=True) +@mock.patch("snowflake.cli._app.snow_connector.connect_to_snowflake") +@mock.patch.dict( + os.environ, + {"USER": "test_user", RESOURCE_SUFFIX_VAR: "_suffix!"}, + clear=True, +) +def test_project_model_explicit_package_app_name_with_suffix( + mock_connect, project_definition_files: List[Path], mock_ctx +): + ctx = mock_ctx() + mock_connect.return_value = ctx + + project_defition_file_yml = dedent( + f""" + definition_version: 1.1 + native_app: + name: minimal + + artifacts: + - setup.sql + - README.md + + package: + name: minimal_test_pkg + role: PkgRole + distribution: external + warehouse: PkgWarehouse + scripts: + - scripts/package_setup.sql + + application: + name: minimal_test_app + warehouse: AppWarehouse + role: AppRole + debug: false + post_deploy: + - sql_script: scripts/app_setup.sql + + """ + ) + + project_defn = build_project_definition( + **yaml.load(project_defition_file_yml, Loader=yaml.BaseLoader) + ) + project_dir = Path().resolve() + project = NativeAppProjectModel( + project_definition=project_defn.native_app, + project_root=project_dir, + ) + + assert project.package_name == '"minimal_test_pkg_suffix!"' + assert project.app_name == '"minimal_test_app_suffix!"' + + @pytest.mark.parametrize("project_definition_files", ["minimal"], indirect=True) @mock.patch("snowflake.cli._app.snow_connector.connect_to_snowflake") @mock.patch.dict(os.environ, {"USER": "test_user"}, clear=True) @@ -188,7 +271,7 @@ def test_bundle_context_from_project_model(project_definition_files: List[Path]) actual_bundle_ctx = project.get_bundle_context() expected_bundle_ctx = BundleContext( - package_name=default_app_package("minimal"), + package_name=project.package_name, artifacts=[ PathMapping(src="setup.sql", dest=None), PathMapping(src="README.md", dest=None), diff --git a/tests_integration/nativeapp/test_deploy.py b/tests_integration/nativeapp/test_deploy.py index 44787578b6..575c8de87f 100644 --- a/tests_integration/nativeapp/test_deploy.py +++ b/tests_integration/nativeapp/test_deploy.py @@ -15,6 +15,7 @@ import os import uuid +from snowflake.cli._plugins.nativeapp.project_model import RESOURCE_SUFFIX_VAR from snowflake.cli.api.project.util import generate_user_env @@ -112,6 +113,87 @@ def test_nativeapp_deploy( assert result.exit_code == 0 +@pytest.mark.integration +@enable_definition_v2_feature_flag +@pytest.mark.parametrize("test_project", ["napp_init_v1", "napp_init_v2"]) +def test_nativeapp_deploy_with_resource_suffix( + test_project, + project_directory, + runner, + snowflake_session, +): + suffix = f"_some_suffix_{uuid.uuid4().hex}" + test_env_with_suffix = TEST_ENV | {RESOURCE_SUFFIX_VAR: suffix} + with project_directory(test_project): + result = runner.invoke_with_connection( + ["app", "deploy"], + env=test_env_with_suffix, + ) + assert result.exit_code == 0 + + try: + # package exist + assert row_from_snowflake_session( + snowflake_session.execute_string( + f"show application packages like '%{suffix}'", + ) + ) + + # make sure we always delete the app + result = runner.invoke_with_connection_json( + ["app", "teardown"], + env=test_env_with_suffix, + ) + assert result.exit_code == 0 + finally: + # teardown is idempotent, so we can execute it again with no ill effects + result = runner.invoke_with_connection_json( + ["app", "teardown", "--force"], + env=test_env_with_suffix, + ) + assert result.exit_code == 0 + + +@pytest.mark.integration +@enable_definition_v2_feature_flag +@pytest.mark.parametrize("test_project", ["napp_init_v1", "napp_init_v2"]) +def test_nativeapp_deploy_with_resource_suffix_quoted( + test_project, + project_directory, + runner, + snowflake_session, +): + suffix = f"_must.be.quoted!!!_{uuid.uuid4().hex}" + test_env_with_quoted_suffix = TEST_ENV | {RESOURCE_SUFFIX_VAR: suffix} + with project_directory(test_project): + result = runner.invoke_with_connection( + ["app", "deploy"], + env=test_env_with_quoted_suffix, + ) + assert result.exit_code == 0 + + try: + # package exist + assert row_from_snowflake_session( + snowflake_session.execute_string( + f"show application packages like '%{suffix}'", + ) + ) + # make sure we always delete the app + result = runner.invoke_with_connection_json( + ["app", "teardown"], + env=test_env_with_quoted_suffix, + ) + assert result.exit_code == 0 + finally: + # teardown is idempotent, so we can execute it again with no ill effects + result = runner.invoke_with_connection_json( + ["app", "teardown", "--force"], + env=test_env_with_quoted_suffix, + ) + assert result.exit_code == 0 + + @pytest.mark.integration @enable_definition_v2_feature_flag @pytest.mark.parametrize( diff --git a/tests_integration/nativeapp/test_init_run.py b/tests_integration/nativeapp/test_init_run.py index 5f274ed0ed..2e48f713fb 100644 --- a/tests_integration/nativeapp/test_init_run.py +++ b/tests_integration/nativeapp/test_init_run.py @@ -15,6 +15,7 @@ import os import uuid +from snowflake.cli._plugins.nativeapp.project_model import RESOURCE_SUFFIX_VAR from snowflake.cli.api.project.util import generate_user_env from snowflake.cli.api.secure_path import SecurePath from snowflake.cli._plugins.nativeapp.init import OFFICIAL_TEMPLATES_GITHUB_URL @@ -90,6 +91,100 @@ def test_nativeapp_init_run_without_modifications( assert result.exit_code == 0 +@pytest.mark.integration +@enable_definition_v2_feature_flag +@pytest.mark.parametrize("test_project", ["napp_init_v1", "napp_init_v2"]) +def test_nativeapp_init_run_with_resource_suffix( + test_project, + project_directory, + runner, + snowflake_session, +): + suffix = f"_some_suffix_{uuid.uuid4().hex}" + test_env_with_suffix = TEST_ENV | {RESOURCE_SUFFIX_VAR: suffix} + with project_directory(test_project): + result = runner.invoke_with_connection_json( + ["app", "run"], + env=test_env_with_suffix, + ) + assert result.exit_code == 0 + + try: + # app + package exist + assert row_from_snowflake_session( + snowflake_session.execute_string( + f"show application packages like '%{suffix}'", + ) + ) + assert row_from_snowflake_session( + snowflake_session.execute_string( + f"show applications like '%{suffix}'", + ) + ) + + # make sure we always delete the app + result = runner.invoke_with_connection_json( + ["app", "teardown"], + env=test_env_with_suffix, + ) + assert result.exit_code == 0 + + finally: + # teardown is idempotent, so we can execute it again with no ill effects + result = runner.invoke_with_connection_json( + ["app", "teardown", "--force"], + env=test_env_with_suffix, + ) + assert result.exit_code == 0 + + +@pytest.mark.integration +@enable_definition_v2_feature_flag +@pytest.mark.parametrize("test_project", ["napp_init_v1", "napp_init_v2"]) +def test_nativeapp_init_run_with_resource_suffix_quoted( + test_project, + project_directory, + runner, + snowflake_session, +): + suffix = f"_must.be.quoted!!!_{uuid.uuid4().hex}" + test_env_with_quoted_suffix = TEST_ENV | {RESOURCE_SUFFIX_VAR: suffix} + with project_directory(test_project): + result = runner.invoke_with_connection_json( + ["app", "run"], + env=test_env_with_quoted_suffix, + ) + assert result.exit_code == 0 + + try: + # app + package exist + assert row_from_snowflake_session( + snowflake_session.execute_string( + f"show application packages like '%{suffix}'", + ) + ) + assert row_from_snowflake_session( + snowflake_session.execute_string( + f"show applications like '%{suffix}'", + ) + ) + + # make sure we always delete the app + result = runner.invoke_with_connection_json( + ["app", "teardown"], + env=test_env_with_quoted_suffix, + ) + assert result.exit_code == 0 + + finally: + # teardown is idempotent, so we can execute it again with no ill effects + result = runner.invoke_with_connection_json( + ["app", "teardown", "--force"], + env=test_env_with_quoted_suffix, + ) + assert result.exit_code == 0 + + # Tests a simple flow of an existing project, but executing snow app run and teardown, all with distribution=internal @pytest.mark.integration @enable_definition_v2_feature_flag