From 9012b62106745801f5e68c1d364bcc3fb6a9e04d Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Sat, 18 Jan 2025 15:27:12 -0700 Subject: [PATCH 1/3] Refactor repository to not create blank credentials. Remove commit option in favor of ref option to match API and allow setting the ref on CTL. --- infrahub_sdk/ctl/repository.py | 28 +++++++--- tests/unit/ctl/test_repository_app.py | 78 ++++++++++++++++++++++----- 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/infrahub_sdk/ctl/repository.py b/infrahub_sdk/ctl/repository.py index bd16e57b..53b7f82f 100644 --- a/infrahub_sdk/ctl/repository.py +++ b/infrahub_sdk/ctl/repository.py @@ -34,7 +34,9 @@ def get_repository_config(repo_config_file: Path) -> InfrahubRepositoryConfig: try: data = InfrahubRepositoryConfig(**config_file_data) except ValidationError as exc: - console.print(f"[red]Repository config file not valid, found {len(exc.errors())} error(s)") + console.print( + f"[red]Repository config file not valid, found {len(exc.errors())} error(s)" + ) for error in exc.errors(): loc_str = [str(item) for item in error["loc"]] console.print(f" {'/'.join(loc_str)} | {error['msg']} ({error['type']})") @@ -70,7 +72,7 @@ async def add( description: str = "", username: str | None = None, password: str = "", - commit: str = "", + ref: str = "", read_only: bool = False, debug: bool = False, branch: str = typer.Option("main", help="Branch on which to add the repository."), @@ -85,20 +87,30 @@ async def add( "name": {"value": name}, "location": {"value": location}, "description": {"value": description}, - "commit": {"value": commit}, + "ref": {"value": ref}, }, } client = initialize_client() - credential = await client.create(kind="CorePasswordCredential", name=name, username=username, password=password) - await credential.save(allow_upsert=True) - input_data["data"]["credential"] = {"id": credential.id} + if username or password: + credential = await client.create( + kind="CorePasswordCredential", + name=name, + username=username, + password=password, + ) + await credential.save(allow_upsert=True) + input_data["data"]["credential"] = {"id": credential.id} query = Mutation( - mutation="CoreReadOnlyRepositoryCreate" if read_only else "CoreRepositoryCreate", + mutation="CoreReadOnlyRepositoryCreate" + if read_only + else "CoreRepositoryCreate", input_data=input_data, query={"ok": None}, ) - await client.execute_graphql(query=query.render(), branch_name=branch, tracker="mutation-repository-create") + await client.execute_graphql( + query=query.render(), branch_name=branch, tracker="mutation-repository-create" + ) diff --git a/tests/unit/ctl/test_repository_app.py b/tests/unit/ctl/test_repository_app.py index 6b14d866..741d9a3d 100644 --- a/tests/unit/ctl/test_repository_app.py +++ b/tests/unit/ctl/test_repository_app.py @@ -11,7 +11,9 @@ runner = CliRunner() -requires_python_310 = pytest.mark.skipif(sys.version_info < (3, 10), reason="Requires Python 3.10 or higher") +requires_python_310 = pytest.mark.skipif( + sys.version_info < (3, 10), reason="Requires Python 3.10 or higher" +) @pytest.fixture @@ -28,6 +30,54 @@ def mock_client() -> mock.Mock: class TestInfrahubctlRepository: """Groups the 'infrahubctl repository' test cases.""" + @requires_python_310 + def test_repo_no_username_or_password(self, mock_init_client, mock_client) -> None: + """Case allow no username to be passed in and set it as None rather than blank string that fails.""" + mock_cred = mock.AsyncMock() + mock_cred.id = "1234" + mock_client.create.return_value = mock_cred + + mock_init_client.return_value = mock_client + output = runner.invoke( + app, + [ + "repository", + "add", + "Gitlab", + "https://gitlab.com/opsmill/example-repo.git", + ], + ) + assert output.exit_code == 0 + mock_client.create.assert_not_called() + mock_cred.save.assert_not_called() + mock_client.execute_graphql.assert_called_once() + mock_client.execute_graphql.assert_called_with( + query=""" +mutation { + CoreRepositoryCreate( + data: { + name: { + value: "Gitlab" + } + location: { + value: "https://gitlab.com/opsmill/example-repo.git" + } + description: { + value: "" + } + ref: { + value: "" + } + } + ){ + ok + } +} +""", + branch_name="main", + tracker="mutation-repository-create", + ) + @requires_python_310 def test_repo_no_username(self, mock_init_client, mock_client) -> None: """Case allow no username to be passed in and set it as None rather than blank string that fails.""" @@ -72,7 +122,7 @@ def test_repo_no_username(self, mock_init_client, mock_client) -> None: description: { value: "" } - commit: { + ref: { value: "" } credential: { @@ -134,7 +184,7 @@ def test_repo_username(self, mock_init_client, mock_client) -> None: description: { value: "" } - commit: { + ref: { value: "" } credential: { @@ -164,7 +214,7 @@ def test_repo_readonly_true(self, mock_init_client, mock_client) -> None: "repository", "add", "Gitlab", - "https://gitlab.com/FragmentedPacket/nautobot-plugin-ansible-filters.git", + "https://gitlab.com/opsmill/example-repo.git", "--password", "mySup3rSecureP@ssw0rd", "--read-only", @@ -190,12 +240,12 @@ def test_repo_readonly_true(self, mock_init_client, mock_client) -> None: value: "Gitlab" } location: { - value: "https://gitlab.com/FragmentedPacket/nautobot-plugin-ansible-filters.git" + value: "https://gitlab.com/opsmill/example-repo.git" } description: { value: "" } - commit: { + ref: { value: "" } credential: { @@ -212,7 +262,9 @@ def test_repo_readonly_true(self, mock_init_client, mock_client) -> None: ) @requires_python_310 - def test_repo_description_commit_branch(self, mock_init_client, mock_client) -> None: + def test_repo_description_commit_branch( + self, mock_init_client, mock_client + ) -> None: """Case allow no username to be passed in and set it as None rather than blank string that fails.""" mock_cred = mock.AsyncMock() mock_cred.id = "1234" @@ -225,15 +277,15 @@ def test_repo_description_commit_branch(self, mock_init_client, mock_client) -> "repository", "add", "Gitlab", - "https://gitlab.com/FragmentedPacket/nautobot-plugin-ansible-filters.git", + "https://gitlab.com/opsmill/example-repo.git", "--password", "mySup3rSecureP@ssw0rd", "--username", "opsmill", "--description", "This is a test description", - "--commit", - "myHashCommit", + "--ref", + "my-custom-branch", "--branch", "develop", ], @@ -258,13 +310,13 @@ def test_repo_description_commit_branch(self, mock_init_client, mock_client) -> value: "Gitlab" } location: { - value: "https://gitlab.com/FragmentedPacket/nautobot-plugin-ansible-filters.git" + value: "https://gitlab.com/opsmill/example-repo.git" } description: { value: "This is a test description" } - commit: { - value: "myHashCommit" + ref: { + value: "my-custom-branch" } credential: { id: "1234" From 9826af2e2abb0888b26038e645f86fceb564728b Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Sat, 18 Jan 2025 15:31:49 -0700 Subject: [PATCH 2/3] Format/Lint. --- infrahub_sdk/ctl/repository.py | 12 +++--------- tests/unit/ctl/test_repository_app.py | 8 ++------ 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/infrahub_sdk/ctl/repository.py b/infrahub_sdk/ctl/repository.py index 53b7f82f..cafa0dd9 100644 --- a/infrahub_sdk/ctl/repository.py +++ b/infrahub_sdk/ctl/repository.py @@ -34,9 +34,7 @@ def get_repository_config(repo_config_file: Path) -> InfrahubRepositoryConfig: try: data = InfrahubRepositoryConfig(**config_file_data) except ValidationError as exc: - console.print( - f"[red]Repository config file not valid, found {len(exc.errors())} error(s)" - ) + console.print(f"[red]Repository config file not valid, found {len(exc.errors())} error(s)") for error in exc.errors(): loc_str = [str(item) for item in error["loc"]] console.print(f" {'/'.join(loc_str)} | {error['msg']} ({error['type']})") @@ -104,13 +102,9 @@ async def add( input_data["data"]["credential"] = {"id": credential.id} query = Mutation( - mutation="CoreReadOnlyRepositoryCreate" - if read_only - else "CoreRepositoryCreate", + mutation="CoreReadOnlyRepositoryCreate" if read_only else "CoreRepositoryCreate", input_data=input_data, query={"ok": None}, ) - await client.execute_graphql( - query=query.render(), branch_name=branch, tracker="mutation-repository-create" - ) + await client.execute_graphql(query=query.render(), branch_name=branch, tracker="mutation-repository-create") diff --git a/tests/unit/ctl/test_repository_app.py b/tests/unit/ctl/test_repository_app.py index 741d9a3d..28831173 100644 --- a/tests/unit/ctl/test_repository_app.py +++ b/tests/unit/ctl/test_repository_app.py @@ -11,9 +11,7 @@ runner = CliRunner() -requires_python_310 = pytest.mark.skipif( - sys.version_info < (3, 10), reason="Requires Python 3.10 or higher" -) +requires_python_310 = pytest.mark.skipif(sys.version_info < (3, 10), reason="Requires Python 3.10 or higher") @pytest.fixture @@ -262,9 +260,7 @@ def test_repo_readonly_true(self, mock_init_client, mock_client) -> None: ) @requires_python_310 - def test_repo_description_commit_branch( - self, mock_init_client, mock_client - ) -> None: + def test_repo_description_commit_branch(self, mock_init_client, mock_client) -> None: """Case allow no username to be passed in and set it as None rather than blank string that fails.""" mock_cred = mock.AsyncMock() mock_cred.id = "1234" From 1b2397ee68d9d727e5a64949f864b287f4f2475e Mon Sep 17 00:00:00 2001 From: Mikhail Yohman Date: Mon, 10 Feb 2025 21:14:21 -0700 Subject: [PATCH 3/3] Fixed payload between CoreRepositoryCreate and CoreReadOnlyRepositoryCreate mutations. --- infrahub_sdk/ctl/repository.py | 5 ++++- tests/unit/ctl/test_repository_app.py | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/infrahub_sdk/ctl/repository.py b/infrahub_sdk/ctl/repository.py index cafa0dd9..78094cc2 100644 --- a/infrahub_sdk/ctl/repository.py +++ b/infrahub_sdk/ctl/repository.py @@ -85,9 +85,12 @@ async def add( "name": {"value": name}, "location": {"value": location}, "description": {"value": description}, - "ref": {"value": ref}, }, } + if read_only: + input_data["data"]["ref"] = {"value": ref} + else: + input_data["data"]["default_branch"] = {"value": ref} client = initialize_client() diff --git a/tests/unit/ctl/test_repository_app.py b/tests/unit/ctl/test_repository_app.py index 28831173..bcd80b77 100644 --- a/tests/unit/ctl/test_repository_app.py +++ b/tests/unit/ctl/test_repository_app.py @@ -63,7 +63,7 @@ def test_repo_no_username_or_password(self, mock_init_client, mock_client) -> No description: { value: "" } - ref: { + default_branch: { value: "" } } @@ -120,7 +120,7 @@ def test_repo_no_username(self, mock_init_client, mock_client) -> None: description: { value: "" } - ref: { + default_branch: { value: "" } credential: { @@ -182,7 +182,7 @@ def test_repo_username(self, mock_init_client, mock_client) -> None: description: { value: "" } - ref: { + default_branch: { value: "" } credential: { @@ -311,7 +311,7 @@ def test_repo_description_commit_branch(self, mock_init_client, mock_client) -> description: { value: "This is a test description" } - ref: { + default_branch: { value: "my-custom-branch" } credential: {