diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index c69bacbcc..c3e8e1098 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -57,6 +57,7 @@ * `snow streamlit deploy` will check for existing streamlit instance before deploying anything. * Fixed `snow git execute` with `/` in name of the branch. * `snow app` commands don't enforce ownership of the objects they manage, and rely on RBAC instead. +* `snow app deploy` for package entity now allows operating on application packages created outside the CLI # v2.8.1 ## Backward incompatibility diff --git a/src/snowflake/cli/_plugins/nativeapp/application_entity.py b/src/snowflake/cli/_plugins/nativeapp/application_entity.py index 5864dfb58..14c9793e9 100644 --- a/src/snowflake/cli/_plugins/nativeapp/application_entity.py +++ b/src/snowflake/cli/_plugins/nativeapp/application_entity.py @@ -146,6 +146,8 @@ def deploy_package(): paths=[], validate=validate, stage_fqn=stage_fqn, + interactive=interactive, + force=force, ) self.deploy( diff --git a/src/snowflake/cli/_plugins/nativeapp/application_package_entity.py b/src/snowflake/cli/_plugins/nativeapp/application_package_entity.py index a5fb58ee6..5476c5d91 100644 --- a/src/snowflake/cli/_plugins/nativeapp/application_package_entity.py +++ b/src/snowflake/cli/_plugins/nativeapp/application_package_entity.py @@ -27,12 +27,19 @@ CouldNotDropApplicationPackageWithVersions, SetupScriptFailedValidation, ) +from snowflake.cli._plugins.nativeapp.policy import ( + AllowAlwaysPolicy, + AskAlwaysPolicy, + DenyAlwaysPolicy, + PolicyBase, +) from snowflake.cli._plugins.nativeapp.utils import ( needs_confirmation, ) from snowflake.cli._plugins.stage.diff import DiffResult from snowflake.cli._plugins.stage.manager import StageManager from snowflake.cli._plugins.workspace.action_context import ActionContext +from snowflake.cli.api.console import cli_console as cc from snowflake.cli.api.console.abc import AbstractConsole from snowflake.cli.api.entities.common import EntityBase, get_sql_executor from snowflake.cli.api.entities.utils import ( @@ -80,12 +87,22 @@ def action_deploy( recursive: bool, paths: List[Path], validate: bool, + interactive: bool, + force: bool, stage_fqn: Optional[str] = None, *args, **kwargs, ): model = self._entity_model package_name = model.fqn.identifier + + if force: + policy = AllowAlwaysPolicy() + elif interactive: + policy = AskAlwaysPolicy() + else: + policy = DenyAlwaysPolicy() + return self.deploy( console=ctx.console, project_root=ctx.project_root, @@ -107,6 +124,7 @@ def action_deploy( ), post_deploy_hooks=model.meta and model.meta.post_deploy, package_scripts=[], # Package scripts are not supported in PDFv2 + policy=policy, ) def action_drop(self, ctx: ActionContext, force_drop: bool, *args, **kwargs): @@ -124,7 +142,9 @@ def action_drop(self, ctx: ActionContext, force_drop: bool, *args, **kwargs): force_drop=force_drop, ) - def action_validate(self, ctx: ActionContext, *args, **kwargs): + def action_validate( + self, ctx: ActionContext, interactive: bool, force: bool, *args, **kwargs + ): model = self._entity_model package_name = model.fqn.identifier stage_fqn = f"{package_name}.{model.stage}" @@ -141,6 +161,8 @@ def deploy_to_scratch_stage_fn(): paths=[], validate=False, stage_fqn=model.scratch_stage, + interactive=interactive, + force=force, ) self.validate_setup_script( @@ -206,6 +228,7 @@ def deploy( stage_fqn: str, post_deploy_hooks: list[PostDeployHook] | None, package_scripts: List[str], + policy: PolicyBase, ) -> DiffResult: # 1. Create a bundle bundle_map = cls.bundle( @@ -218,13 +241,17 @@ def deploy( ) # 2. Create an empty application package, if none exists - cls.create_app_package( - console=console, - package_name=package_name, - package_role=package_role, - package_distribution=package_distribution, - ) - + try: + cls.create_app_package( + console=console, + package_name=package_name, + package_role=package_role, + package_distribution=package_distribution, + ) + except ApplicationPackageAlreadyExistsError as e: + cc.warning(e.message) + if not policy.should_proceed("Proceed with using this package?"): + raise typer.Abort() from e with get_sql_executor().use_role(package_role): if package_scripts: cls.apply_package_scripts( diff --git a/src/snowflake/cli/_plugins/nativeapp/commands.py b/src/snowflake/cli/_plugins/nativeapp/commands.py index ba7a88867..f08d0bd14 100644 --- a/src/snowflake/cli/_plugins/nativeapp/commands.py +++ b/src/snowflake/cli/_plugins/nativeapp/commands.py @@ -406,6 +406,8 @@ def app_deploy( unspecified, the command syncs all local changes to the stage.""" ).strip(), ), + interactive: bool = InteractiveOption, + force: Optional[bool] = ForceOption, validate: bool = ValidateOption, **options, ) -> CommandResult: @@ -416,6 +418,13 @@ def app_deploy( assert_project_type("native_app") + if force: + policy = AllowAlwaysPolicy() + elif interactive: + policy = AskAlwaysPolicy() + else: + policy = DenyAlwaysPolicy() + has_paths = paths is not None and len(paths) > 0 if prune is None and recursive is None and not has_paths: prune = True @@ -442,6 +451,7 @@ def app_deploy( recursive=recursive, local_paths_to_sync=paths, validate=validate, + policy=policy, ) return MessageResult( @@ -452,7 +462,9 @@ def app_deploy( @app.command("validate", requires_connection=True) @with_project_definition() @nativeapp_definition_v2_to_v1() -def app_validate(**options): +def app_validate( + **options, +): """ Validates a deployed Snowflake Native App's setup script. """ diff --git a/src/snowflake/cli/_plugins/nativeapp/manager.py b/src/snowflake/cli/_plugins/nativeapp/manager.py index 0946350b0..eb5bde56f 100644 --- a/src/snowflake/cli/_plugins/nativeapp/manager.py +++ b/src/snowflake/cli/_plugins/nativeapp/manager.py @@ -36,6 +36,7 @@ from snowflake.cli._plugins.nativeapp.exceptions import ( NoEventTableForAccount, ) +from snowflake.cli._plugins.nativeapp.policy import AllowAlwaysPolicy, PolicyBase from snowflake.cli._plugins.nativeapp.project_model import ( NativeAppProjectModel, ) @@ -306,6 +307,7 @@ def deploy( bundle_map: BundleMap, prune: bool, recursive: bool, + policy: PolicyBase, stage_fqn: Optional[str] = None, local_paths_to_sync: List[Path] | None = None, validate: bool = True, @@ -330,6 +332,7 @@ def deploy( package_warehouse=self.package_warehouse, post_deploy_hooks=self.package_post_deploy_hooks, package_scripts=self.package_scripts, + policy=policy, ) def deploy_to_scratch_stage_fn(self): @@ -341,6 +344,7 @@ def deploy_to_scratch_stage_fn(self): stage_fqn=self.scratch_stage_fqn, validate=False, print_diff=False, + policy=AllowAlwaysPolicy(), ) def validate(self, use_scratch_stage: bool = False): diff --git a/src/snowflake/cli/_plugins/nativeapp/policy.py b/src/snowflake/cli/_plugins/nativeapp/policy.py index 5dc5a3869..71fa91584 100644 --- a/src/snowflake/cli/_plugins/nativeapp/policy.py +++ b/src/snowflake/cli/_plugins/nativeapp/policy.py @@ -27,6 +27,9 @@ class PolicyBase(ABC): def should_proceed(self, user_prompt: Optional[str]) -> bool: pass + def __eq__(self, value: object) -> bool: + return self.__class__ == value.__class__ + class AllowAlwaysPolicy(PolicyBase): """Always allow a Snowflake CLI command to continue execution.""" diff --git a/src/snowflake/cli/_plugins/nativeapp/run_processor.py b/src/snowflake/cli/_plugins/nativeapp/run_processor.py index dd9b2eea4..7f93ee676 100644 --- a/src/snowflake/cli/_plugins/nativeapp/run_processor.py +++ b/src/snowflake/cli/_plugins/nativeapp/run_processor.py @@ -151,7 +151,11 @@ def process( ): def deploy_package(): self.deploy( - bundle_map=bundle_map, prune=True, recursive=True, validate=validate + bundle_map=bundle_map, + prune=True, + recursive=True, + validate=validate, + policy=policy, ) def drop_app(): diff --git a/src/snowflake/cli/_plugins/workspace/commands.py b/src/snowflake/cli/_plugins/workspace/commands.py index 99c602e37..0e45c3423 100644 --- a/src/snowflake/cli/_plugins/workspace/commands.py +++ b/src/snowflake/cli/_plugins/workspace/commands.py @@ -179,6 +179,8 @@ def validate( entity_id: str = typer.Option( help=f"""The ID of the entity you want to validate.""", ), + interactive: bool = InteractiveOption, + force: Optional[bool] = ForceOption, **options, ): """Validates the specified entity.""" @@ -191,6 +193,8 @@ def validate( ws.perform_action( entity_id, EntityActions.VALIDATE, + interactive=interactive, + force=force, ) diff --git a/tests/__snapshots__/test_help_messages.ambr b/tests/__snapshots__/test_help_messages.ambr index c7bc75281..1dcad8ebd 100644 --- a/tests/__snapshots__/test_help_messages.ambr +++ b/tests/__snapshots__/test_help_messages.ambr @@ -96,47 +96,70 @@ | syncs all local changes to the stage. | +------------------------------------------------------------------------------+ +- Options --------------------------------------------------------------------+ - | --prune --no-prune Whether to delete specified | - | files from the stage if | - | they don't exist locally. | - | If set, the command deletes | - | files that exist in the | - | stage, but not in the local | - | filesystem. This option | - | cannot be used when paths | - | are specified. | - | [default: no-prune] | - | --recursive -r --no-recursive Whether to traverse and | - | deploy files from | - | subdirectories. If set, the | - | command deploys all files | - | and subdirectories; | - | otherwise, only files in | - | the current directory are | - | deployed. | - | [default: no-recursive] | - | --validate --no-validate When enabled, this option | - | triggers validation of a | - | deployed Snowflake Native | - | App's setup script SQL | - | [default: validate] | - | --package-entity-id TEXT The ID of the package | - | entity on which to operate | - | when definition_version is | - | 2 or higher. | - | --app-entity-id TEXT The ID of the application | - | entity on which to operate | - | when definition_version is | - | 2 or higher. | - | --project -p TEXT Path where Snowflake | - | project resides. Defaults | - | to current working | - | directory. | - | --env TEXT String in format of | - | key=value. Overrides | - | variables from env section | - | used for templates. | - | --help -h Show this message and exit. | + | --prune --no-prune Whether to delete | + | specified files from the | + | stage if they don't exist | + | locally. If set, the | + | command deletes files | + | that exist in the stage, | + | but not in the local | + | filesystem. This option | + | cannot be used when paths | + | are specified. | + | [default: no-prune] | + | --recursive -r --no-recursive Whether to traverse and | + | deploy files from | + | subdirectories. If set, | + | the command deploys all | + | files and subdirectories; | + | otherwise, only files in | + | the current directory are | + | deployed. | + | [default: no-recursive] | + | --interactive --no-interactive When enabled, this option | + | displays prompts even if | + | the standard input and | + | output are not terminal | + | devices. Defaults to True | + | in an interactive shell | + | environment, and False | + | otherwise. | + | --force When enabled, this option | + | causes the command to | + | implicitly approve any | + | prompts that arise. You | + | should enable this option | + | if interactive mode is | + | not specified and if you | + | want perform potentially | + | destructive actions. | + | Defaults to unset. | + | --validate --no-validate When enabled, this option | + | triggers validation of a | + | deployed Snowflake Native | + | App's setup script SQL | + | [default: validate] | + | --package-entity-id TEXT The ID of the package | + | entity on which to | + | operate when | + | definition_version is 2 | + | or higher. | + | --app-entity-id TEXT The ID of the application | + | entity on which to | + | operate when | + | definition_version is 2 | + | or higher. | + | --project -p TEXT Path where Snowflake | + | project resides. Defaults | + | to current working | + | directory. | + | --env TEXT String in format of | + | key=value. Overrides | + | variables from env | + | section used for | + | templates. | + | --help -h Show this message and | + | exit. | +------------------------------------------------------------------------------+ +- Connection configuration ---------------------------------------------------+ | --connection,--environment -c TEXT Name of the connection, as | diff --git a/tests/nativeapp/test_manager.py b/tests/nativeapp/test_manager.py index c423b93b9..52c15119b 100644 --- a/tests/nativeapp/test_manager.py +++ b/tests/nativeapp/test_manager.py @@ -40,6 +40,7 @@ from snowflake.cli._plugins.nativeapp.manager import ( NativeAppManager, ) +from snowflake.cli._plugins.nativeapp.policy import AllowAlwaysPolicy from snowflake.cli._plugins.stage.diff import ( DiffResult, StagePath, @@ -927,6 +928,40 @@ def test_create_app_pkg_internal_distribution_no_special_comment( ) +# Test create_app_package() with existing package without special comment +@mock.patch(APP_PACKAGE_ENTITY_IS_DISTRIBUTION_SAME) +@mock_get_app_pkg_distribution_in_sf() +@mock.patch(APP_PACKAGE_ENTITY_GET_EXISTING_APP_PKG_INFO) +@mock.patch(SQL_EXECUTOR_EXECUTE) +def test_existing_app_pkg_without_special_comment( + mock_execute, + mock_get_existing_app_pkg_info, + mock_get_distribution, + mock_is_distribution_same, + temp_dir, + mock_cursor, +): + mock_get_existing_app_pkg_info.return_value = { + "name": "APP_PKG", + "comment": "NOT_SPECIAL_COMMENT", + "version": LOOSE_FILES_MAGIC_VERSION, + "owner": "package_role", + } + mock_get_distribution.return_value = "internal" + mock_is_distribution_same.return_value = True + + current_working_directory = os.getcwd() + create_named_file( + file_name="snowflake.yml", + dir_name=current_working_directory, + contents=[mock_snowflake_yml_file], + ) + + native_app_manager = _get_na_manager() + with pytest.raises(ApplicationPackageAlreadyExistsError): + native_app_manager.create_app_package() + + @pytest.mark.parametrize( "paths_to_sync,expected_result", [ @@ -1187,6 +1222,7 @@ def test_validate_use_scratch_stage( stage_fqn=native_app_manager.scratch_stage_fqn, validate=False, print_diff=False, + policy=AllowAlwaysPolicy(), ) assert mock_execute.mock_calls == expected @@ -1253,6 +1289,7 @@ def test_validate_failing_drops_scratch_stage( stage_fqn=native_app_manager.scratch_stage_fqn, validate=False, print_diff=False, + policy=AllowAlwaysPolicy(), ) assert mock_execute.mock_calls == expected diff --git a/tests/workspace/test_application_package_entity.py b/tests/workspace/test_application_package_entity.py index 54980190b..34aff9d89 100644 --- a/tests/workspace/test_application_package_entity.py +++ b/tests/workspace/test_application_package_entity.py @@ -135,7 +135,13 @@ def test_deploy( app_pkg, bundle_ctx, mock_console = _get_app_pkg_entity(project_directory) app_pkg.action_deploy( - bundle_ctx, prune=False, recursive=False, paths=["a/b", "c"], validate=True + bundle_ctx, + prune=False, + recursive=False, + paths=["a/b", "c"], + validate=True, + interactive=False, + force=False, ) mock_sync.assert_called_once_with( diff --git a/tests_integration/nativeapp/test_deploy.py b/tests_integration/nativeapp/test_deploy.py index 3f4925964..8312fcd3c 100644 --- a/tests_integration/nativeapp/test_deploy.py +++ b/tests_integration/nativeapp/test_deploy.py @@ -577,3 +577,155 @@ def test_nativeapp_deploy_validate_failing( assert result.exit_code == 1, result.output assert "Snowflake Native App setup script failed validation." in result.output assert "syntax error" in result.output + + +@pytest.mark.integration +@pytest.mark.parametrize( + "test_project", + [ + "napp_init_v1", + "napp_init_v2", + ], +) +def test_nativeapp_deploy_package_no_magic_comment( + runner, + snowflake_session, + default_username, + resource_suffix, + nativeapp_teardown, + snapshot, + nativeapp_project_directory, + test_project, +): + project_name = "myapp" + with nativeapp_project_directory(test_project): + result_create_abort = runner.invoke_with_connection_json(["app", "deploy"]) + assert result_create_abort.exit_code == 0 + + # package exists + package_name = f"{project_name}_pkg_{default_username}{resource_suffix}".upper() + assert contains_row_with( + row_from_snowflake_session( + snowflake_session.execute_string( + f"show application packages like '{package_name}'", + ) + ), + dict(name=package_name), + ) + + assert contains_row_with( + row_from_snowflake_session( + snowflake_session.execute_string( + f"alter application package {package_name} set comment = 'not the magic comment'" + ) + ), + dict(status="Statement executed successfully."), + ) + + # app command - say no + result_create_abort = runner.invoke_with_connection( + ["app", "deploy", "--interactive"], + input="n\n", + ) + assert result_create_abort.exit_code == 1 + assert ( + f"An Application Package {package_name} already exists in account " + "that may have been created without Snowflake CLI.".upper() + in result_create_abort.output.upper() + ) + assert "Aborted." in result_create_abort.output + + # app command - say yes + result_create_yes = runner.invoke_with_connection( + ["app", "deploy", "--interactive"], + input="y\n", + ) + assert result_create_yes.exit_code == 0 + assert ( + f"An Application Package {package_name} already exists in account " + "that may have been created without Snowflake CLI.".upper() + in result_create_yes.output.upper() + ) + + # app command - force + result_create_force = runner.invoke_with_connection( + ["app", "deploy", "--force"] + ) + assert result_create_force.exit_code == 0 + assert ( + f"An Application Package {package_name} already exists in account " + "that may have been created without Snowflake CLI.".upper() + in result_create_force.output.upper() + ) + + +@pytest.mark.integration +def test_ws_deploy_package_no_magic_comment( + runner, + snowflake_session, + default_username, + resource_suffix, + nativeapp_teardown, + snapshot, + nativeapp_project_directory, +): + project_name = "myapp" + with nativeapp_project_directory("napp_init_v2"): + result_create_abort = runner.invoke_with_connection_json(["app", "deploy"]) + assert result_create_abort.exit_code == 0 + + # package exists + package_name = f"{project_name}_pkg_{default_username}{resource_suffix}".upper() + assert contains_row_with( + row_from_snowflake_session( + snowflake_session.execute_string( + f"show application packages like '{package_name}'", + ) + ), + dict(name=package_name), + ) + + assert contains_row_with( + row_from_snowflake_session( + snowflake_session.execute_string( + f"alter application package {package_name} set comment = 'not the magic comment'" + ) + ), + dict(status="Statement executed successfully."), + ) + + # ws command - say no + result_create_abort = runner.invoke_with_connection( + ["ws", "deploy", "--entity-id=pkg", "--interactive"], + input="n\n", + ) + assert result_create_abort.exit_code == 1 + assert ( + f"An Application Package {package_name} already exists in account " + "that may have been created without Snowflake CLI.".upper() + in result_create_abort.output.upper() + ) + assert "Aborted." in result_create_abort.output + + # ws command - say yes + result_create_yes = runner.invoke_with_connection( + ["ws", "deploy", "--entity-id=pkg", "--interactive"], + input="y\n", + ) + assert result_create_yes.exit_code == 0 + assert ( + f"An Application Package {package_name} already exists in account " + "that may have been created without Snowflake CLI.".upper() + in result_create_yes.output.upper() + ) + + # ws command - force + result_create_force = runner.invoke_with_connection( + ["ws", "deploy", "--entity-id=pkg", "--force"] + ) + assert result_create_force.exit_code == 0 + assert ( + f"An Application Package {package_name} already exists in account " + "that may have been created without Snowflake CLI.".upper() + in result_create_force.output.upper() + )