From d4ba6161bc3dbb052a992bc688780b8446b42f2c Mon Sep 17 00:00:00 2001 From: Pawel Job Date: Tue, 29 Oct 2024 15:17:44 +0100 Subject: [PATCH 1/7] Changes --- src/snowflake/cli/_plugins/git/commands.py | 85 ++++++++++++++++++++-- 1 file changed, 77 insertions(+), 8 deletions(-) diff --git a/src/snowflake/cli/_plugins/git/commands.py b/src/snowflake/cli/_plugins/git/commands.py index c53c089881..3758bc7412 100644 --- a/src/snowflake/cli/_plugins/git/commands.py +++ b/src/snowflake/cli/_plugins/git/commands.py @@ -18,7 +18,7 @@ import logging from os import path from pathlib import Path -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional import typer from click import ClickException @@ -31,6 +31,7 @@ from snowflake.cli.api.commands.common import OnErrorType from snowflake.cli.api.commands.flags import ( ExecuteVariablesOption, + IdentifierType, OnErrorOption, PatternOption, identifier_argument, @@ -39,6 +40,7 @@ from snowflake.cli.api.commands.snow_typer import SnowTyperFactory from snowflake.cli.api.console.console import cli_console from snowflake.cli.api.constants import ObjectType +from snowflake.cli.api.exceptions import IncompatibleParametersError from snowflake.cli.api.output.types import CollectionResult, CommandResult, QueryResult from snowflake.cli.api.utils.path_utils import is_stage_path from snowflake.connector import DictCursor @@ -120,6 +122,47 @@ def _unique_new_object_name( @app.command("setup", requires_connection=True) def setup( repository_name: FQN = RepoNameArgument, + url: Optional[str] = typer.Option( + None, + "--url", + help="Origin URL.", + show_default=False, + click_type=IdentifierType(), + ), + no_secret: Optional[bool] = typer.Option( + False, + "--no-secret", + help="Mark that your repository does not require a secret.", + is_flag=True, + show_default=False, + ), + provided_secret_identifier: Optional[FQN] = typer.Option( + None, + "--secret", + help="The identifier of the secret (will be created if not exists).", + show_default=False, + click_type=IdentifierType(), + ), + new_secret_user: Optional[str] = typer.Option( + None, + "--new-secret-user", + help="An user being a part of a new secret definition.", + show_default=False, + ), + new_secret_password: Optional[str] = typer.Option( + None, + "--new-secret-password", + "--new-secret-token", + help="A password or a token being a part of new a secret definition.", + show_default=False, + ), + provided_api_integration_identifier: Optional[FQN] = typer.Option( + None, + "--api-integration", + help="The identifier of the API integration (will be created if not exists).", + show_default=False, + click_type=IdentifierType(), + ), **options, ) -> CommandResult: """ @@ -127,7 +170,7 @@ def setup( ## Usage notes - You will be prompted for: + You can use options to specify details, otherwise you will be prompted for: * url - address of repository to be used for git clone operation @@ -136,14 +179,20 @@ def setup( * API integration - object allowing Snowflake to interact with git repository. """ + _validate_exclusivity_of_git_setup_params( + no_secret, provided_secret_identifier, new_secret_user, new_secret_password + ) + manager = GitManager() om = ObjectManager() _assure_repository_does_not_exist(om, repository_name) - url = typer.prompt("Origin url") + url = url or typer.prompt("Origin url") _validate_origin_url(url) - secret_needed = typer.confirm("Use secret for authentication?") + secret_needed = ( + False if no_secret else typer.confirm("Use secret for authentication?") + ) should_create_secret = False secret_name = None if secret_needed: @@ -157,7 +206,7 @@ def setup( om, object_type=ObjectType.SECRET, proposed_fqn=default_secret_name ), ) - secret_name = FQN.from_string( + secret_name = provided_secret_identifier or FQN.from_string( typer.prompt( "Secret identifier (will be created if not exists)", default=default_secret_name.name, @@ -175,13 +224,16 @@ def setup( else: should_create_secret = True cli_console.step(f"Secret '{secret_name}' will be created") - secret_username = typer.prompt("username") - secret_password = typer.prompt("password/token", hide_input=True) + secret_username = new_secret_user or typer.prompt("username") + secret_password = new_secret_password or typer.prompt( + "password/token", hide_input=True + ) # API integration is an account-level object api_integration = FQN.from_string(f"{repository_name.name}_api_integration") api_integration.set_name( - typer.prompt( + provided_api_integration_identifier + or typer.prompt( "API integration identifier (will be created if not exists)", default=_unique_new_object_name( om, @@ -220,6 +272,23 @@ def setup( ) +def _validate_exclusivity_of_git_setup_params( + no_secret: Optional[bool], + provided_secret_identifier: Optional[FQN], + new_secret_user: Optional[str], + new_secret_password: Optional[str], +): + def validate_incompability_with_no_secret( + param_name: str, param_value: Optional[Any] + ): + if no_secret and param_value: + raise IncompatibleParametersError([param_name, "--no-secret"]) + + validate_incompability_with_no_secret("--secret", provided_secret_identifier) + validate_incompability_with_no_secret("--new-secret-user", new_secret_user) + validate_incompability_with_no_secret("--new-secret-password", new_secret_password) + + @app.command( "list-branches", requires_connection=True, From 328660551a5d2e190defc9221086d98c8d94dfe8 Mon Sep 17 00:00:00 2001 From: Pawel Job Date: Tue, 29 Oct 2024 16:47:48 +0100 Subject: [PATCH 2/7] Fixes + integration test --- src/snowflake/cli/_plugins/git/commands.py | 26 ++++++++++++---------- tests_integration/test_git.py | 21 +++++++++++++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/snowflake/cli/_plugins/git/commands.py b/src/snowflake/cli/_plugins/git/commands.py index 3758bc7412..c89ead83d9 100644 --- a/src/snowflake/cli/_plugins/git/commands.py +++ b/src/snowflake/cli/_plugins/git/commands.py @@ -127,7 +127,6 @@ def setup( "--url", help="Origin URL.", show_default=False, - click_type=IdentifierType(), ), no_secret: Optional[bool] = typer.Option( False, @@ -230,18 +229,21 @@ def setup( ) # API integration is an account-level object - api_integration = FQN.from_string(f"{repository_name.name}_api_integration") - api_integration.set_name( - provided_api_integration_identifier - or typer.prompt( - "API integration identifier (will be created if not exists)", - default=_unique_new_object_name( - om, - object_type=ObjectType.INTEGRATION, - proposed_fqn=api_integration, - ), + if provided_api_integration_identifier: + api_integration = provided_api_integration_identifier + else: + api_integration = FQN.from_string(f"{repository_name.name}_api_integration") + api_integration.set_name( + provided_api_integration_identifier + or typer.prompt( + "API integration identifier (will be created if not exists)", + default=_unique_new_object_name( + om, + object_type=ObjectType.INTEGRATION, + proposed_fqn=api_integration, + ), + ) ) - ) if should_create_secret: manager.create_password_secret( diff --git a/tests_integration/test_git.py b/tests_integration/test_git.py index a41153ab5d..222ba42890 100644 --- a/tests_integration/test_git.py +++ b/tests_integration/test_git.py @@ -39,6 +39,27 @@ def sf_git_repository(runner, test_database): return repo_name +@pytest.mark.integration +def test_setup_repository_with_options(runner, test_database): + repo_name = "SNOWCLI_TESTING_REPO" + integration_name = "SNOWCLI_TESTING_REPO_API_INTEGRATION" + result = runner.invoke_with_connection( + [ + "git", + "setup", + repo_name, + "--url", + "https://github.com/snowflakedb/snowflake-cli.git", + "--no-secret", + "--api-integration", + integration_name, + ] + ) + assert result.exit_code == 0 + assert f"Git Repository {repo_name} was successfully created." in result.output + return repo_name + + @pytest.mark.integration def test_object_commands(runner, sf_git_repository): # object list From e155737dbf2f0337227f29309aa7b3db81e424c8 Mon Sep 17 00:00:00 2001 From: Pawel Job Date: Tue, 29 Oct 2024 17:19:11 +0100 Subject: [PATCH 3/7] First unit tests --- tests/git/test_git_commands.py | 70 ++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 16 deletions(-) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 6679736523..285397ea26 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -198,14 +198,24 @@ def test_setup_already_exists_error(mock_om_describe, mock_connector, runner, mo @mock.patch("snowflake.connector.connect") @mock.patch("snowflake.cli._plugins.snowpark.commands.ObjectManager.describe") -def test_setup_invalid_url_error(mock_om_describe, mock_connector, runner, mock_ctx): +@pytest.mark.parametrize( + "cli_options, stdin", + [ + ([], ["http://invalid_url.git", "s"]), + (["--url", "http://invalid_url.git"], []), + ], +) +def test_setup_invalid_url_error( + mock_om_describe, mock_connector, runner, mock_ctx, cli_options, stdin +): mock_om_describe.side_effect = ProgrammingError( errno=DOES_NOT_EXIST_OR_NOT_AUTHORIZED ) ctx = mock_ctx() mock_connector.return_value = ctx - communication = "http://invalid_url.git\ns" - result = runner.invoke(["git", "setup", "repo_name"], input=communication) + result = runner.invoke( + ["git", "setup", "repo_name"] + cli_options, input="\n".join(stdin) + ) assert result.exit_code == 1, result.output assert "Error" in result.output @@ -215,8 +225,44 @@ def test_setup_invalid_url_error(mock_om_describe, mock_connector, runner, mock_ @mock.patch("snowflake.connector.connect") @mock.patch("snowflake.cli._plugins.snowpark.commands.ObjectManager.describe") @mock.patch("snowflake.cli._plugins.snowpark.commands.ObjectManager.show") +@pytest.mark.parametrize( + "cli_options, stdin, expected_stdout", + [ + ( # Case 0 + [], # No CLI options + [EXAMPLE_URL, "n", "existing_api_integration", ""], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: n", + "API integration identifier (will be created if not exists) [repo_name_api_integration]: existing_api_integration", + "Using existing API integration 'existing_api_integration'.", + ], + ), + ( # Case 1 + [ # CLI options + "--url", + EXAMPLE_URL, + "--no-secret", + "--api-integration", + "existing_api_integration", + ], + [], # No STDIN + [ # Expected STDOUT + "Using existing API integration 'existing_api_integration'.", + ], + ), + ], +) def test_setup_no_secret_existing_api( - mock_om_show, mock_om_describe, mock_connector, runner, mock_ctx, mock_cursor + mock_om_show, + mock_om_describe, + mock_connector, + runner, + mock_ctx, + mock_cursor, + cli_options, + stdin, + expected_stdout, ): mock_om_show.return_value = mock_cursor([], []) mock_om_describe.side_effect = [ @@ -229,20 +275,12 @@ def test_setup_no_secret_existing_api( ctx = mock_ctx() mock_connector.return_value = ctx - communication = "\n".join([EXAMPLE_URL, "n", "existing_api_integration", ""]) - result = runner.invoke(["git", "setup", "repo_name"], input=communication) + result = runner.invoke( + ["git", "setup", "repo_name"] + cli_options, input="\n".join(stdin) + ) assert result.exit_code == 0, result.output - assert result.output.startswith( - "\n".join( - [ - "Origin url: https://github.com/an-example-repo.git", - "Use secret for authentication? [y/N]: n", - "API integration identifier (will be created if not exists) [repo_name_api_integration]: existing_api_integration", - "Using existing API integration 'existing_api_integration'.", - ] - ) - ) + assert result.output.startswith("\n".join(expected_stdout)) assert ctx.get_query() == dedent( """ create git repository IDENTIFIER('repo_name') From 307ec8e84eb5f024ecfcda7a3163f1499bdafcc6 Mon Sep 17 00:00:00 2001 From: Pawel Job Date: Wed, 30 Oct 2024 17:56:15 +0100 Subject: [PATCH 4/7] Next unit tests --- src/snowflake/cli/_plugins/git/commands.py | 349 +++++++++++++++------ tests/git/test_git_commands.py | 129 ++++++-- 2 files changed, 359 insertions(+), 119 deletions(-) diff --git a/src/snowflake/cli/_plugins/git/commands.py b/src/snowflake/cli/_plugins/git/commands.py index c89ead83d9..255267a738 100644 --- a/src/snowflake/cli/_plugins/git/commands.py +++ b/src/snowflake/cli/_plugins/git/commands.py @@ -16,9 +16,10 @@ import itertools import logging +from dataclasses import dataclass from os import path from pathlib import Path -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple import typer from click import ClickException @@ -44,6 +45,7 @@ from snowflake.cli.api.output.types import CollectionResult, CommandResult, QueryResult from snowflake.cli.api.utils.path_utils import is_stage_path from snowflake.connector import DictCursor +from typer.models import OptionInfo app = SnowTyperFactory( name="git", @@ -76,6 +78,61 @@ def _repo_path_argument_callback(path): callback=_repo_path_argument_callback, show_default=False, ) +OriginUrlOption: OptionInfo = typer.Option( + None, + "--url", + help="Origin URL.", + show_default=False, +) +NoSecretOption: OptionInfo = typer.Option( + False, + "--no-secret", + help="Mark that your repository does not require a secret.", + is_flag=True, + show_default=False, +) +SecretIdentifierOption: OptionInfo = typer.Option( + None, + "--secret", + help="The identifier of the secret (will be created if not exists).", + show_default=False, + click_type=IdentifierType(), +) +NewSecretDefaultNameOption: OptionInfo = typer.Option( + False, + "--new-secret-default-name", + help="Use a default name for a newly created secret.", + is_flag=True, + show_default=False, +) +NewSecretUserOption: OptionInfo = typer.Option( + None, + "--new-secret-user", + help="An user being a part of a new secret definition.", + show_default=False, +) +NewSecretPasswordOption: OptionInfo = typer.Option( + None, + "--new-secret-password", + "--new-secret-token", + help="A password or a token being a part of new a secret definition.", + show_default=False, +) +ApiIntegrationIdentifierOption: OptionInfo = typer.Option( + None, + "--api-integration", + help="The identifier of the API integration (will be created if not exists).", + show_default=False, + click_type=IdentifierType(), +) +NewApiIntegrationDefaultNameOption: OptionInfo = typer.Option( + False, + "--new-api-integration-default-name", + help="Use a default name for a newly created API integration.", + is_flag=True, + show_default=False, +) + add_object_command_aliases( app=app, object_type=ObjectType.GIT_REPOSITORY, @@ -119,81 +176,72 @@ def _unique_new_object_name( return result -@app.command("setup", requires_connection=True) -def setup( - repository_name: FQN = RepoNameArgument, - url: Optional[str] = typer.Option( - None, - "--url", - help="Origin URL.", - show_default=False, - ), - no_secret: Optional[bool] = typer.Option( - False, - "--no-secret", - help="Mark that your repository does not require a secret.", - is_flag=True, - show_default=False, - ), - provided_secret_identifier: Optional[FQN] = typer.Option( - None, - "--secret", - help="The identifier of the secret (will be created if not exists).", - show_default=False, - click_type=IdentifierType(), - ), - new_secret_user: Optional[str] = typer.Option( - None, - "--new-secret-user", - help="An user being a part of a new secret definition.", - show_default=False, - ), - new_secret_password: Optional[str] = typer.Option( - None, - "--new-secret-password", - "--new-secret-token", - help="A password or a token being a part of new a secret definition.", - show_default=False, - ), - provided_api_integration_identifier: Optional[FQN] = typer.Option( - None, - "--api-integration", - help="The identifier of the API integration (will be created if not exists).", - show_default=False, - click_type=IdentifierType(), - ), - **options, -) -> CommandResult: - """ - Sets up a git repository object. - - ## Usage notes - - You can use options to specify details, otherwise you will be prompted for: - - * url - address of repository to be used for git clone operation +def _validate_exclusivity_of_git_setup_params( + use_no_secret: Optional[bool], + provided_secret_identifier: Optional[FQN], + use_new_secret_default_name: Optional[bool], + new_secret_user: Optional[str], + new_secret_password: Optional[str], + provided_api_integration_identifier: Optional[FQN], + use_new_api_integration_default_name: Optional[bool], +): + def validate_params_incompatibility( + param1: Tuple[OptionInfo, Optional[Any]], + param2: Tuple[OptionInfo, Optional[Any]], + ): + def1: OptionInfo + def2: OptionInfo + (def1, value1) = param1 + (def2, value2) = param2 + if value1 and value2: + raise IncompatibleParametersError( + [def1.param_decls[0], def2.param_decls[0]] + ) - * secret - Snowflake secret containing authentication credentials. Not needed if origin repository does not require - authentication for RO operations (clone, fetch) + def validate_incompability_with_no_secret( + param_def: OptionInfo, param_value: Optional[Any] + ): + validate_params_incompatibility( + param1=(param_def, param_value), param2=(NoSecretOption, use_no_secret) + ) - * API integration - object allowing Snowflake to interact with git repository. - """ - _validate_exclusivity_of_git_setup_params( - no_secret, provided_secret_identifier, new_secret_user, new_secret_password + validate_incompability_with_no_secret( + SecretIdentifierOption, provided_secret_identifier + ) + validate_incompability_with_no_secret( + NewSecretDefaultNameOption, use_new_secret_default_name + ) + validate_incompability_with_no_secret(NewSecretUserOption, new_secret_user) + validate_incompability_with_no_secret(NewSecretPasswordOption, new_secret_password) + validate_params_incompatibility( + param1=(SecretIdentifierOption, provided_secret_identifier), + param2=(NewSecretDefaultNameOption, use_new_secret_default_name), + ) + validate_params_incompatibility( + param1=(ApiIntegrationIdentifierOption, provided_api_integration_identifier), + param2=( + NewApiIntegrationDefaultNameOption, + use_new_api_integration_default_name, + ), ) - manager = GitManager() - om = ObjectManager() - _assure_repository_does_not_exist(om, repository_name) - - url = url or typer.prompt("Origin url") - _validate_origin_url(url) +def _collect_git_setup_secret_details( + om: ObjectManager, + repository_name: FQN, + use_no_secret: Optional[bool], + provided_secret_identifier: Optional[FQN], + use_new_secret_default_name: Optional[bool], + new_secret_user: Optional[str], + new_secret_password: Optional[str], +) -> _GitSetupSecretDetails: secret_needed = ( - False if no_secret else typer.confirm("Use secret for authentication?") + False if use_no_secret else typer.confirm("Use secret for authentication?") ) should_create_secret = False secret_name = None + secret_username = None + secret_password = None if secret_needed: default_secret_name = ( FQN.from_string(f"{repository_name.name}_secret") @@ -205,10 +253,17 @@ def setup( om, object_type=ObjectType.SECRET, proposed_fqn=default_secret_name ), ) - secret_name = provided_secret_identifier or FQN.from_string( - typer.prompt( - "Secret identifier (will be created if not exists)", - default=default_secret_name.name, + forced_default_secret_name: Optional[FQN] = ( + default_secret_name if use_new_secret_default_name else None + ) + secret_name = ( + provided_secret_identifier + or forced_default_secret_name + or FQN.from_string( + typer.prompt( + "Secret identifier (will be created if not exists)", + default=default_secret_name.name, + ) ) ) if not secret_name.database: @@ -227,29 +282,63 @@ def setup( secret_password = new_secret_password or typer.prompt( "password/token", hide_input=True ) + return _GitSetupSecretDetails( + secret_needed=secret_needed, + should_create_secret=should_create_secret, + secret_name=secret_name, + secret_username=secret_username, + secret_password=secret_password, + ) + +def _collect_api_integration_details( + om: ObjectManager, + repository_name: FQN, + provided_api_integration_identifier: Optional[FQN], + use_new_api_integration_default_name: Optional[bool], +) -> FQN: # API integration is an account-level object if provided_api_integration_identifier: api_integration = provided_api_integration_identifier else: api_integration = FQN.from_string(f"{repository_name.name}_api_integration") + + def generate_api_integration_name(): + return _unique_new_object_name( + om, + object_type=ObjectType.INTEGRATION, + proposed_fqn=api_integration, + ) + + forced_default_api_integration_name: Optional[str] = ( + generate_api_integration_name() + if use_new_api_integration_default_name + else None + ) api_integration.set_name( - provided_api_integration_identifier + forced_default_api_integration_name or typer.prompt( "API integration identifier (will be created if not exists)", - default=_unique_new_object_name( - om, - object_type=ObjectType.INTEGRATION, - proposed_fqn=api_integration, - ), + default=generate_api_integration_name(), ) ) + return api_integration - if should_create_secret: + +def _create_secret_and_api_integration_objects_if_needed( + om: ObjectManager, + manager: GitManager, + url: str, + secret_details: _GitSetupSecretDetails, + api_integration: FQN, +): + if secret_details.should_create_secret: manager.create_password_secret( - name=secret_name, username=secret_username, password=secret_password + name=secret_details.secret_name, + username=secret_details.secret_username, + password=secret_details.secret_password, ) - cli_console.step(f"Secret '{secret_name}' successfully created.") + cli_console.step(f"Secret '{secret_details.secret_name}' successfully created.") if not om.object_exists( object_type=ObjectType.INTEGRATION.value.cli_name, fqn=api_integration @@ -258,39 +347,92 @@ def setup( name=api_integration, api_provider="git_https_api", allowed_prefix=url, - secret=secret_name, + secret=secret_details.secret_name, ) cli_console.step(f"API integration '{api_integration}' successfully created.") else: cli_console.step(f"Using existing API integration '{api_integration}'.") + +@app.command("setup", requires_connection=True) +def setup( + repository_name: FQN = RepoNameArgument, + url: Optional[str] = OriginUrlOption, + use_no_secret: Optional[bool] = NoSecretOption, + provided_secret_identifier: Optional[FQN] = SecretIdentifierOption, + use_new_secret_default_name: Optional[bool] = NewSecretDefaultNameOption, + new_secret_user: Optional[str] = NewSecretUserOption, + new_secret_password: Optional[str] = NewSecretPasswordOption, + provided_api_integration_identifier: Optional[FQN] = ApiIntegrationIdentifierOption, + use_new_api_integration_default_name: Optional[ + bool + ] = NewApiIntegrationDefaultNameOption, + **options, +) -> CommandResult: + """ + Sets up a git repository object. + + ## Usage notes + + You can use options to specify details, otherwise you will be prompted for: + + * url - address of repository to be used for git clone operation + + * secret - Snowflake secret containing authentication credentials. Not needed if origin repository does not require + authentication for RO operations (clone, fetch) + + * API integration - object allowing Snowflake to interact with git repository. + """ + _validate_exclusivity_of_git_setup_params( + use_no_secret=use_no_secret, + provided_secret_identifier=provided_secret_identifier, + use_new_secret_default_name=use_new_secret_default_name, + new_secret_user=new_secret_user, + new_secret_password=new_secret_password, + provided_api_integration_identifier=provided_api_integration_identifier, + use_new_api_integration_default_name=use_new_api_integration_default_name, + ) + + manager = GitManager() + om = ObjectManager() + _assure_repository_does_not_exist(om, repository_name) + + url = url or typer.prompt("Origin url") + _validate_origin_url(url) + + secret_details = _collect_git_setup_secret_details( + om=om, + repository_name=repository_name, + use_no_secret=use_no_secret, + provided_secret_identifier=provided_secret_identifier, + use_new_secret_default_name=use_new_secret_default_name, + new_secret_user=new_secret_user, + new_secret_password=new_secret_password, + ) + api_integration = _collect_api_integration_details( + om=om, + repository_name=repository_name, + provided_api_integration_identifier=provided_api_integration_identifier, + use_new_api_integration_default_name=use_new_api_integration_default_name, + ) + _create_secret_and_api_integration_objects_if_needed( + om=om, + manager=manager, + url=url, + secret_details=secret_details, + api_integration=api_integration, + ) + return QueryResult( manager.create( repo_name=repository_name, url=url, api_integration=api_integration, - secret=secret_name, + secret=secret_details.secret_name, ) ) -def _validate_exclusivity_of_git_setup_params( - no_secret: Optional[bool], - provided_secret_identifier: Optional[FQN], - new_secret_user: Optional[str], - new_secret_password: Optional[str], -): - def validate_incompability_with_no_secret( - param_name: str, param_value: Optional[Any] - ): - if no_secret and param_value: - raise IncompatibleParametersError([param_name, "--no-secret"]) - - validate_incompability_with_no_secret("--secret", provided_secret_identifier) - validate_incompability_with_no_secret("--new-secret-user", new_secret_user) - validate_incompability_with_no_secret("--new-secret-password", new_secret_password) - - @app.command( "list-branches", requires_connection=True, @@ -430,3 +572,12 @@ def get(source_path: str, destination_path: str, parallel: int): key=lambda e: (path.dirname(e["file"]), path.basename(e["file"])), ) return CollectionResult(sorted_results) + + +@dataclass +class _GitSetupSecretDetails: + secret_needed: bool = False + should_create_secret: bool = False + secret_name: Optional[FQN] = None + secret_username: Optional[str] = None + secret_password: Optional[str] = None diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 285397ea26..7c81528506 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -291,11 +291,107 @@ def test_setup_no_secret_existing_api( @pytest.mark.parametrize( - "repo_name, int_name, secret_name", + "repo_name, cli_options, stdin, expected_stdout", [ - ("db.schema.FooRepo", "FooRepo_api_integration", "db.schema.FooRepo_secret"), - ("schema.FooRepo", "FooRepo_api_integration", "schema.FooRepo_secret"), - ("FooRepo", "FooRepo_api_integration", "FooRepo_secret"), + ( # Case 0 + "db.schema.FooRepo", # Repo name + [], # No CLI options + [EXAMPLE_URL, "n", "", ""], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: n", + "API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 1 + "schema.FooRepo", # Repo name + [], # No CLI options + [EXAMPLE_URL, "n", "", ""], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: n", + "API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 2 + "FooRepo", # Repo name + [], # No CLI options + [EXAMPLE_URL, "n", "", ""], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: n", + "API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 3 + "db.schema.FooRepo", # Repo name + ["--url", EXAMPLE_URL, "--no-secret"], # CLI options + ["", ""], # STDIN + [ # Expected STDOUT + "API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 4 + "schema.FooRepo", # Repo name + ["--url", EXAMPLE_URL, "--no-secret"], # CLI options + ["", ""], # STDIN + [ # Expected STDOUT + "API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 5 + "FooRepo", # Repo name + ["--url", EXAMPLE_URL, "--no-secret"], # CLI options + ["", ""], # STDIN + [ # Expected STDOUT + "API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 6 + "db.schema.FooRepo", # Repo name + [ + "--url", + EXAMPLE_URL, + "--no-secret", + "--new-api-integration-default-name", + ], # CLI options + [], # No STDIN + [ # Expected STDOUT + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 7 + "schema.FooRepo", # Repo name + [ + "--url", + EXAMPLE_URL, + "--no-secret", + "--new-api-integration-default-name", + ], # CLI options + [], # No STDIN + [ # Expected STDOUT + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 8 + "FooRepo", # Repo name + [ + "--url", + EXAMPLE_URL, + "--no-secret", + "--new-api-integration-default-name", + ], # CLI options + [], # No STDIN + [ # Expected STDOUT + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), ], ) @mock.patch("snowflake.connector.connect") @@ -309,8 +405,9 @@ def test_setup_no_secret_create_api( mock_ctx, mock_cursor, repo_name, - int_name, - secret_name, + cli_options, + stdin, + expected_stdout, ): mock_om_show.return_value = mock_cursor([], []) mock_om_describe.side_effect = ProgrammingError( @@ -320,23 +417,15 @@ def test_setup_no_secret_create_api( ctx = mock_ctx() mock_connector.return_value = ctx - communication = "\n".join([EXAMPLE_URL, "n", "", ""]) - result = runner.invoke(["git", "setup", repo_name], input=communication) + result = runner.invoke( + ["git", "setup", repo_name] + cli_options, input="\n".join(stdin) + ) assert result.exit_code == 0, result.output - assert result.output.startswith( - "\n".join( - [ - "Origin url: https://github.com/an-example-repo.git", - "Use secret for authentication? [y/N]: n", - f"API integration identifier (will be created if not exists) [{int_name}]: ", - f"API integration '{int_name}' successfully created.", - ] - ) - ) + assert result.output.startswith("\n".join(expected_stdout)) assert ctx.get_query() == dedent( f""" - create api integration IDENTIFIER('{int_name}') + create api integration IDENTIFIER('FooRepo_api_integration') api_provider = git_https_api api_allowed_prefixes = ('https://github.com/an-example-repo.git') allowed_authentication_secrets = () @@ -344,7 +433,7 @@ def test_setup_no_secret_create_api( create git repository IDENTIFIER('{repo_name}') - api_integration = {int_name} + api_integration = FooRepo_api_integration origin = 'https://github.com/an-example-repo.git' """ ) From 6b1bf8bdd1d8a9f4ef4ff1d6e95afcc5783cd7a9 Mon Sep 17 00:00:00 2001 From: Pawel Job Date: Wed, 30 Oct 2024 18:21:18 +0100 Subject: [PATCH 5/7] Next unit tests --- src/snowflake/cli/_plugins/git/commands.py | 6 +- tests/git/test_git_commands.py | 66 ++++++++++++++++------ 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/snowflake/cli/_plugins/git/commands.py b/src/snowflake/cli/_plugins/git/commands.py index 255267a738..b30e2ee9a4 100644 --- a/src/snowflake/cli/_plugins/git/commands.py +++ b/src/snowflake/cli/_plugins/git/commands.py @@ -236,7 +236,11 @@ def _collect_git_setup_secret_details( new_secret_password: Optional[str], ) -> _GitSetupSecretDetails: secret_needed = ( - False if use_no_secret else typer.confirm("Use secret for authentication?") + False + if use_no_secret + else True + if provided_secret_identifier or use_new_secret_default_name + else typer.confirm("Use secret for authentication?") ) should_create_secret = False secret_name = None diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index 7c81528506..d6a305b912 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -442,8 +442,54 @@ def test_setup_no_secret_create_api( @mock.patch("snowflake.connector.connect") @mock.patch("snowflake.cli._plugins.snowpark.commands.ObjectManager.describe") @mock.patch("snowflake.cli._plugins.snowpark.commands.ObjectManager.show") +@pytest.mark.parametrize( + "cli_options, stdin, expected_stdout", + [ + ( # Case 0 + [], # No CLI options + [ + EXAMPLE_URL, + "y", + "existing_secret", + "existing_api_integration", + "", + ], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: y", + "Secret identifier (will be created if not exists) [repo_name_secret]: existing_secret", + "Using existing secret 'existing_secret'", + "API integration identifier (will be created if not exists) [repo_name_api_integration]: existing_api_integration", + "Using existing API integration 'existing_api_integration'.", + ], + ), + ( # Case 1 + [ # CLI options + "--url", + EXAMPLE_URL, + "--secret", + "existing_secret", + "--api-integration", + "existing_api_integration", + ], + [], # No STDIN + [ # Expected STDOUT + "Using existing secret 'existing_secret'", + "Using existing API integration 'existing_api_integration'.", + ], + ), + ], +) def test_setup_existing_secret_existing_api( - mock_om_show, mock_om_describe, mock_connector, runner, mock_ctx, mock_cursor + mock_om_show, + mock_om_describe, + mock_connector, + runner, + mock_ctx, + mock_cursor, + cli_options, + stdin, + expected_stdout, ): mock_om_show.return_value = mock_cursor([], []) mock_om_describe.side_effect = [ @@ -464,24 +510,12 @@ def test_setup_existing_secret_existing_api( ctx = mock_ctx() mock_connector.return_value = ctx - communication = "\n".join( - [EXAMPLE_URL, "y", "existing_secret", "existing_api_integration", ""] + result = runner.invoke( + ["git", "setup", "repo_name"] + cli_options, input="\n".join(stdin) ) - result = runner.invoke(["git", "setup", "repo_name"], input=communication) assert result.exit_code == 0, result.output - assert result.output.startswith( - "\n".join( - [ - "Origin url: https://github.com/an-example-repo.git", - "Use secret for authentication? [y/N]: y", - "Secret identifier (will be created if not exists) [repo_name_secret]: existing_secret", - "Using existing secret 'existing_secret'", - "API integration identifier (will be created if not exists) [repo_name_api_integration]: existing_api_integration", - "Using existing API integration 'existing_api_integration'.", - ] - ) - ) + assert result.output.startswith("\n".join(expected_stdout)) assert ctx.get_query() == dedent( """ create git repository IDENTIFIER('repo_name') From 853c1f044c1c7a51ded3ade9f01e6a449ebb7992 Mon Sep 17 00:00:00 2001 From: Pawel Job Date: Wed, 30 Oct 2024 18:37:24 +0100 Subject: [PATCH 6/7] Next unit tests --- tests/git/test_git_commands.py | 166 ++++++++++++++++++++++++++++----- 1 file changed, 142 insertions(+), 24 deletions(-) diff --git a/tests/git/test_git_commands.py b/tests/git/test_git_commands.py index d6a305b912..78ee8e5837 100644 --- a/tests/git/test_git_commands.py +++ b/tests/git/test_git_commands.py @@ -527,15 +527,141 @@ def test_setup_existing_secret_existing_api( @pytest.mark.parametrize( - "repo_name, int_name, existing_secret_name", + "repo_name, existing_secret_name, cli_options, stdin, expected_stdout", [ - ("db.schema.FooRepo", "FooRepo_api_integration", "db.schema.existing_secret"), - ( - "schema.FooRepo", - "FooRepo_api_integration", - "different_schema.existing_secret", + ( # Case 0 + "db.schema.FooRepo", # Repo name + "db.schema.existing_secret", # Existing secret name + [], # No CLI options + [EXAMPLE_URL, "y", "db.schema.existing_secret", "", ""], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: y", + f"Secret identifier (will be created if not exists) [FooRepo_secret]: db.schema.existing_secret", + f"Using existing secret 'db.schema.existing_secret'", + f"API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 1 + "schema.FooRepo", # Repo name + "different_schema.existing_secret", # Existing secret name + [], # No CLI options + [EXAMPLE_URL, "y", "different_schema.existing_secret", "", ""], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: y", + f"Secret identifier (will be created if not exists) [FooRepo_secret]: different_schema.existing_secret", + f"Using existing secret 'different_schema.existing_secret'", + f"API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 2 + "FooRepo", # Repo name + "existing_secret", # Existing secret name + [], # No CLI options + [EXAMPLE_URL, "y", "existing_secret", "", ""], # STDIN + [ # Expected STDOUT + "Origin url: https://github.com/an-example-repo.git", + "Use secret for authentication? [y/N]: y", + f"Secret identifier (will be created if not exists) [FooRepo_secret]: existing_secret", + f"Using existing secret 'existing_secret'", + f"API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 3 + "db.schema.FooRepo", # Repo name + "db.schema.existing_secret", # Existing secret name + [ + "--url", + EXAMPLE_URL, + "--secret", + "db.schema.existing_secret", + ], # CLI options + ["", ""], # STDIN + [ # Expected STDOUT + f"Using existing secret 'db.schema.existing_secret'", + f"API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 4 + "schema.FooRepo", # Repo name + "different_schema.existing_secret", # Existing secret name + [ + "--url", + EXAMPLE_URL, + "--secret", + "different_schema.existing_secret", + ], # CLI options + ["", ""], # STDIN + [ # Expected STDOUT + f"Using existing secret 'different_schema.existing_secret'", + f"API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 5 + "FooRepo", # Repo name + "existing_secret", # Existing secret name + ["--url", EXAMPLE_URL, "--secret", "existing_secret"], # CLI options + ["", ""], # STDIN + [ # Expected STDOUT + f"Using existing secret 'existing_secret'", + f"API integration identifier (will be created if not exists) [FooRepo_api_integration]: ", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 6 + "db.schema.FooRepo", # Repo name + "db.schema.existing_secret", # Existing secret name + [ + "--url", + EXAMPLE_URL, + "--secret", + "db.schema.existing_secret", + "--new-api-integration-default-name", + ], # CLI options + [], # No STDIN + [ # Expected STDOUT + f"Using existing secret 'db.schema.existing_secret'", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 7 + "schema.FooRepo", # Repo name + "different_schema.existing_secret", # Existing secret name + [ + "--url", + EXAMPLE_URL, + "--secret", + "different_schema.existing_secret", + "--new-api-integration-default-name", + ], # CLI options + [], # No STDIN + [ # Expected STDOUT + f"Using existing secret 'different_schema.existing_secret'", + f"API integration 'FooRepo_api_integration' successfully created.", + ], + ), + ( # Case 8 + "FooRepo", # Repo name + "existing_secret", # Existing secret name + [ + "--url", + EXAMPLE_URL, + "--secret", + "existing_secret", + "--new-api-integration-default-name", + ], # CLI options + [], # No STDIN + [ # Expected STDOUT + f"Using existing secret 'existing_secret'", + f"API integration 'FooRepo_api_integration' successfully created.", + ], ), - ("FooRepo", "FooRepo_api_integration", "existing_secret"), ], ) @mock.patch("snowflake.connector.connect") @@ -549,8 +675,10 @@ def test_setup_existing_secret_create_api( mock_ctx, mock_cursor, repo_name, - int_name, existing_secret_name, + cli_options, + stdin, + expected_stdout, ): mock_om_show.return_value = mock_cursor([], []) mock_om_describe.side_effect = [ @@ -565,25 +693,15 @@ def test_setup_existing_secret_create_api( ctx = mock_ctx() mock_connector.return_value = ctx - communication = "\n".join([EXAMPLE_URL, "y", existing_secret_name, "", ""]) - result = runner.invoke(["git", "setup", repo_name], input=communication) + result = runner.invoke( + ["git", "setup", repo_name] + cli_options, input="\n".join(stdin) + ) assert result.exit_code == 0, result.output - assert result.output.startswith( - "\n".join( - [ - "Origin url: https://github.com/an-example-repo.git", - "Use secret for authentication? [y/N]: y", - f"Secret identifier (will be created if not exists) [FooRepo_secret]: {existing_secret_name}", - f"Using existing secret '{existing_secret_name}'", - f"API integration identifier (will be created if not exists) [{int_name}]: ", - f"API integration '{int_name}' successfully created.", - ] - ) - ) + assert result.output.startswith("\n".join(expected_stdout)) assert ctx.get_query() == dedent( f""" - create api integration IDENTIFIER('{int_name}') + create api integration IDENTIFIER('FooRepo_api_integration') api_provider = git_https_api api_allowed_prefixes = ('https://github.com/an-example-repo.git') allowed_authentication_secrets = ({existing_secret_name}) @@ -591,7 +709,7 @@ def test_setup_existing_secret_create_api( create git repository IDENTIFIER('{repo_name}') - api_integration = {int_name} + api_integration = FooRepo_api_integration origin = 'https://github.com/an-example-repo.git' git_credentials = {existing_secret_name} """ From 1e70cad872828b7e21cfc6c0d6ab89e2854aa2a2 Mon Sep 17 00:00:00 2001 From: Pawel Job Date: Thu, 31 Oct 2024 15:27:45 +0100 Subject: [PATCH 7/7] Updated snapshots --- tests/__snapshots__/test_help_messages.ambr | 47 +++++++++------------ 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/tests/__snapshots__/test_help_messages.ambr b/tests/__snapshots__/test_help_messages.ambr index 883013654d..86c8035318 100644 --- a/tests/__snapshots__/test_help_messages.ambr +++ b/tests/__snapshots__/test_help_messages.ambr @@ -481,31 +481,6 @@ +------------------------------------------------------------------------------+ - ''' -# --- -# name: test_help_messages[app.init] - ''' - - Usage: default app init [OPTIONS] - - *** Deprecated. Use snow init instead *** - Initializes a Snowflake Native App project. - - +- Options --------------------------------------------------------------------+ - | --help -h Show this message and exit. | - +------------------------------------------------------------------------------+ - +- Global configuration -------------------------------------------------------+ - | --format [TABLE|JSON] Specifies the output format. | - | [default: TABLE] | - | --verbose -v Displays log entries for log levels info | - | and higher. | - | --debug Displays log entries for log levels debug | - | and higher; debug logs contains additional | - | information. | - | --silent Turns off intermediate output to console. | - +------------------------------------------------------------------------------+ - - ''' # --- # name: test_help_messages[app.open] @@ -2620,7 +2595,7 @@ Usage notes - You will be prompted for: + You can use options to specify details, otherwise you will be prompted for: • url - address of repository to be used for git clone operation • secret - Snowflake secret containing authentication credentials. Not needed @@ -2635,7 +2610,25 @@ | [required] | +------------------------------------------------------------------------------+ +- Options --------------------------------------------------------------------+ - | --help -h Show this message and exit. | + | --url TEXT Origin URL. | + | --no-secret Mark that your repository does | + | not require a secret. | + | --secret TEXT The identifier of the secret | + | (will be created if not | + | exists). | + | --new-secret-default-name Use a default name for a newly | + | created secret. | + | --new-secret-user TEXT An user being a part of a new | + | secret definition. | + | --new-secret-password,--new-s… TEXT A password or a token being a | + | part of new a secret | + | definition. | + | --api-integration TEXT The identifier of the API | + | integration (will be created | + | if not exists). | + | --new-api-integration-default… Use a default name for a newly | + | created API integration. | + | --help -h Show this message and exit. | +------------------------------------------------------------------------------+ +- Connection configuration ---------------------------------------------------+ | --connection,--environment -c TEXT Name of the connection, as |