From 0899a76949f8e6c3f7d65830e343033295ff2fdc Mon Sep 17 00:00:00 2001 From: S0okJu Date: Tue, 12 Aug 2025 20:59:11 +0900 Subject: [PATCH 1/9] feat(identity): Add get-domain, get-domains(#31) - Change response/keystone.py -> response/identity - Add get-domain, get-domains tool --- .../tools/identity_tools.py | 32 ++++++- .../response/{keystone.py => identity.py} | 6 ++ tests/tools/test_identity_tools.py | 96 ++++++++++++++++++- 3 files changed, 132 insertions(+), 2 deletions(-) rename src/openstack_mcp_server/tools/response/{keystone.py => identity.py} (67%) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 918fe65..75aa8d2 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -1,7 +1,7 @@ from fastmcp import FastMCP from .base import get_openstack_conn -from .response.keystone import Region +from .response.identity import Region, Domain class IdentityTools: @@ -100,3 +100,33 @@ def update_region(self, id: str, description: str = "") -> Region: id=updated_region.id, description=updated_region.description, ) + + def get_domains(self) -> list[Domain]: + """ + Get the list of Identity domains. + + :return: A list of Domain objects representing the domains. + """ + conn = get_openstack_conn() + + domain_list = [] + for domain in conn.identity.domains(): + domain_list.append( + Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled), + ) + return domain_list + + def get_domain(self, id: str) -> Domain: + """ + Get a domain. + + :param id: The ID of the domain. (required) + + :return: The Domain object. + """ + conn = get_openstack_conn() + + domain = conn.identity.get_domain(domain=id) + + return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) + diff --git a/src/openstack_mcp_server/tools/response/keystone.py b/src/openstack_mcp_server/tools/response/identity.py similarity index 67% rename from src/openstack_mcp_server/tools/response/keystone.py rename to src/openstack_mcp_server/tools/response/identity.py index 0d54bcb..44620ae 100644 --- a/src/openstack_mcp_server/tools/response/keystone.py +++ b/src/openstack_mcp_server/tools/response/identity.py @@ -6,3 +6,9 @@ class Region(BaseModel): id: str description: str = "" + +class Domain(BaseModel): + id: str + name: str + description: str = "" + is_enabled: bool = False diff --git a/tests/tools/test_identity_tools.py b/tests/tools/test_identity_tools.py index 5a6ad9c..3b81e1e 100644 --- a/tests/tools/test_identity_tools.py +++ b/tests/tools/test_identity_tools.py @@ -4,7 +4,8 @@ from openstack import exceptions -from openstack_mcp_server.tools.identity_tools import IdentityTools, Region +from openstack_mcp_server.tools.identity_tools import IdentityTools +from openstack_mcp_server.tools.response.identity import Region, Domain class TestIdentityTools: @@ -334,3 +335,96 @@ def test_get_region_not_found(self, mock_get_openstack_conn_identity): mock_conn.identity.get_region.assert_called_once_with( region="RegionOne", ) + + def test_get_domains_success(self, mock_get_openstack_conn_identity): + """Test getting identity domains successfully.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain objects + mock_domain1 = Mock() + mock_domain1.id = "domainone" + mock_domain1.name = "DomainOne" + mock_domain1.description = "Domain One description" + mock_domain1.is_enabled = True + + mock_domain2 = Mock() + mock_domain2.id = "domaintwo" + mock_domain2.name = "DomainTwo" + mock_domain2.description = "Domain Two description" + mock_domain2.is_enabled = False + + # Configure mock domain.domains() + mock_conn.identity.domains.return_value = [mock_domain1, mock_domain2] + + # Test get_domains() + identity_tools = self.get_identity_tools() + result = identity_tools.get_domains() + + # Verify results + assert result == [ + Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=True), + Domain(id="domaintwo", name="DomainTwo", description="Domain Two description", is_enabled=False), + ] + + # Verify mock calls + mock_conn.identity.domains.assert_called_once() + + def test_get_domains_empty_list(self, mock_get_openstack_conn_identity): + """Test getting identity domains when there are no domains.""" + mock_conn = mock_get_openstack_conn_identity + + # Empty domain list + mock_conn.identity.domains.return_value = [] + + # Test get_domains() + identity_tools = self.get_identity_tools() + result = identity_tools.get_domains() + + # Verify results + assert result == [] + + # Verify mock calls + mock_conn.identity.domains.assert_called_once() + + def test_get_domain_success(self, mock_get_openstack_conn_identity): + """Test getting a identity domain successfully.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "domainone" + mock_domain.name = "DomainOne" + mock_domain.description = "Domain One description" + mock_domain.is_enabled = True + + # Configure mock domain.get_domain() + mock_conn.identity.get_domain.return_value = mock_domain + + # Test get_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.get_domain(id="domainone") + + # Verify results + assert result == Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=True) + + # Verify mock calls + mock_conn.identity.get_domain.assert_called_once_with(domain="domainone") + + def test_get_domain_not_found(self, mock_get_openstack_conn_identity): + """Test getting a identity domain that does not exist.""" + mock_conn = mock_get_openstack_conn_identity + + # Configure mock to raise NotFoundException + mock_conn.identity.get_domain.side_effect = exceptions.NotFoundException( + "Domain 'domainone' not found", + ) + + # Test get_domain() + identity_tools = self.get_identity_tools() + + # Verify exception is raised + with pytest.raises(exceptions.NotFoundException, match="Domain 'domainone' not found"): + identity_tools.get_domain(id="domainone") + + # Verify mock calls + mock_conn.identity.get_domain.assert_called_once_with(domain="domainone") From e396261edd9319b7ea3729c792545c294649685d Mon Sep 17 00:00:00 2001 From: S0okJu Date: Tue, 12 Aug 2025 21:02:25 +0900 Subject: [PATCH 2/9] feat(identity): Register tool(#31) --- src/openstack_mcp_server/tools/identity_tools.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 75aa8d2..977f17b 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -19,6 +19,9 @@ def register_tools(self, mcp: FastMCP): mcp.tool()(self.create_region) mcp.tool()(self.delete_region) mcp.tool()(self.update_region) + + mcp.tool()(self.get_domains) + mcp.tool()(self.get_domain) def get_regions(self) -> list[Region]: """ From d7d60da163653af86da9ffa64fb394cbb1ea8c40 Mon Sep 17 00:00:00 2001 From: S0okJu Date: Thu, 14 Aug 2025 17:49:15 +0900 Subject: [PATCH 3/9] feat(identity): Add create domain tool(#31) - Add create_domain tool - Tests are updated and passing --- .../tools/identity_tools.py | 25 +++- tests/tools/test_identity_tools.py | 120 +++++++++++++++++- 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 977f17b..b964ab2 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -1,7 +1,7 @@ from fastmcp import FastMCP from .base import get_openstack_conn -from .response.identity import Region, Domain +from .response.identity import Domain, Region class IdentityTools: @@ -22,6 +22,7 @@ def register_tools(self, mcp: FastMCP): mcp.tool()(self.get_domains) mcp.tool()(self.get_domain) + mcp.tool()(self.create_domain) def get_regions(self) -> list[Region]: """ @@ -133,3 +134,25 @@ def get_domain(self, id: str) -> Domain: return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) + def create_domain(self, + id: str, + name: str, + description: str = "", + is_enabled: bool = False, + ) -> Domain: + """ + Create a new domain. + + :param id: The ID of the domain. (required) + :param name: The name of the domain. (required) + :param description: The description of the domain. (optional) + :param is_enabled: Whether the domain is enabled. (optional) + + :return: The created Domain object. + """ + + conn = get_openstack_conn() + + domain = conn.identity.create_domain(id=id, name=name, description=description, is_enabled=is_enabled) + + return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) \ No newline at end of file diff --git a/tests/tools/test_identity_tools.py b/tests/tools/test_identity_tools.py index 3b81e1e..8d06080 100644 --- a/tests/tools/test_identity_tools.py +++ b/tests/tools/test_identity_tools.py @@ -5,7 +5,7 @@ from openstack import exceptions from openstack_mcp_server.tools.identity_tools import IdentityTools -from openstack_mcp_server.tools.response.identity import Region, Domain +from openstack_mcp_server.tools.response.identity import Domain, Region class TestIdentityTools: @@ -428,3 +428,121 @@ def test_get_domain_not_found(self, mock_get_openstack_conn_identity): # Verify mock calls mock_conn.identity.get_domain.assert_called_once_with(domain="domainone") + + def test_create_domain_success(self, mock_get_openstack_conn_identity): + """Test creating a identity domain successfully.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "domainone" + mock_domain.name = "DomainOne" + mock_domain.description = "Domain One description" + mock_domain.is_enabled = False + + # Configure mock domain.create_domain() + mock_conn.identity.create_domain.return_value = mock_domain + + # Test create_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.create_domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=False) + + # Verify results + assert result == Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=False) + + # Verify mock calls + mock_conn.identity.create_domain.assert_called_once_with(id="domainone", name="DomainOne", description="Domain One description", is_enabled=False) + + + def test_create_domain_with_empty_id(self, mock_get_openstack_conn_identity): + """Test creating a identity domain with an empty ID.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "" + mock_domain.name = "DomainOne" + mock_domain.description = "Domain One description" + mock_domain.is_enabled = False + + # Configure mock domain.create_domain() + mock_conn.identity.create_domain.return_value = mock_domain + + # If id is empty, the id will be generated by the server + temp_id = "abcdefghijklmnopqrs12345" + mock_domain.id = temp_id + + # Test create_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.create_domain(id="", name="DomainOne", description="Domain One description", is_enabled=False) + + # Verify results + assert result == Domain(id=temp_id, name="DomainOne", description="Domain One description", is_enabled=False) + + def test_create_domain_with_empty_name(self, mock_get_openstack_conn_identity): + """Test creating a identity domain with an empty name.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "domainone" + mock_domain.name = "" + mock_domain.description = "Domain One description" + mock_domain.is_enabled = False + + # Configure mock domain.create_domain() + mock_conn.identity.create_domain.return_value = mock_domain + + # Test create_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.create_domain(id="domainone", name="", description="Domain One description", is_enabled=False) + + # Verify results + assert result == Domain(id="domainone", name="", description="Domain One description", is_enabled=False) + + def test_create_domain_with_empty_description(self, mock_get_openstack_conn_identity): + """Test creating a identity domain with an empty description.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "domainone" + mock_domain.name = "DomainOne" + mock_domain.description = "" + mock_domain.is_enabled = False + + # Configure mock domain.create_domain() + mock_conn.identity.create_domain.return_value = mock_domain + + # Test create_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.create_domain(id="domainone", name="DomainOne", description="", is_enabled=False) + + # Verify results + assert result == Domain(id="domainone", name="DomainOne", description="", is_enabled=False) + + def test_create_domain_with_empty_is_enabled(self, mock_get_openstack_conn_identity): + """Test creating a identity domain with an empty is_enabled.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "domainone" + mock_domain.name = "DomainOne" + mock_domain.description = "Domain One description" + mock_domain.is_enabled = False + + # Configure mock domain.create_domain() + mock_conn.identity.create_domain.return_value = mock_domain + + # Test create_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.create_domain(id="domainone", name="DomainOne", description="Domain One description") + + # Verify results + assert result == Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=False) + + # Verify mock calls + mock_conn.identity.create_domain.assert_called_once_with(id="domainone", name="DomainOne", description="Domain One description", is_enabled=False) + + From 7b3a56e638aaf8b278fa66ffd5684ac66b2643b5 Mon Sep 17 00:00:00 2001 From: S0okJu Date: Sun, 17 Aug 2025 12:16:55 +0900 Subject: [PATCH 4/9] feat(identity): Add create, delete, update domain(#31) - Add create, delete, update domain tool - Test codes are updated and passing --- .../tools/identity_tools.py | 60 ++++- tests/tools/test_identity_tools.py | 208 +++++++++++++++++- 2 files changed, 255 insertions(+), 13 deletions(-) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 977f17b..985404c 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -22,6 +22,9 @@ def register_tools(self, mcp: FastMCP): mcp.tool()(self.get_domains) mcp.tool()(self.get_domain) + mcp.tool()(self.create_domain) + mcp.tool()(self.delete_domain) + mcp.tool()(self.update_domain) def get_regions(self) -> list[Region]: """ @@ -119,17 +122,68 @@ def get_domains(self) -> list[Domain]: ) return domain_list - def get_domain(self, id: str) -> Domain: + def get_domain(self, name: str) -> Domain: """ Get a domain. - :param id: The ID of the domain. (required) + :param name: The name of the domain. (required) :return: The Domain object. """ conn = get_openstack_conn() - domain = conn.identity.get_domain(domain=id) + domain = conn.identity.find_domain(name_or_id=name) return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) + def create_domain(self, name: str, description: str = "", is_enabled: bool = False) -> Domain: + """ + Create a new domain. + + :param name: The name of the domain. (required) + :param description: The description of the domain. (optional) + :param is_enabled: Whether the domain is enabled. (optional) + """ + conn = get_openstack_conn() + + domain = conn.identity.create_domain(name=name, description=description, enabled=is_enabled) + + return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) + + def delete_domain(self, name: str) -> None: + """ + Delete a domain. + + :param name: The name of the domain. (required) + """ + conn = get_openstack_conn() + + domain = conn.identity.find_domain(name_or_id=name) + conn.identity.delete_domain(domain=domain, ignore_missing=False) + + return None + + def update_domain(self, name: str, description: str | None = None, is_enabled: bool | None = None) -> Domain: + """ + Update a domain. + + :param name: The name of the domain. (required) + :param description: The description of the domain. (optional) + :param is_enabled: Whether the domain is enabled. (optional) + """ + conn = get_openstack_conn() + + args = {} + if name is not None: + args["name"] = name + if description is not None: + args["description"] = description + if is_enabled is not None: + args["is_enabled"] = is_enabled + + domain = conn.identity.find_domain(name_or_id=name) + updated_domain = conn.identity.update_domain(domain=domain, **args) + + return Domain(id=updated_domain.id, name=updated_domain.name, description=updated_domain.description, is_enabled=updated_domain.is_enabled) + + \ No newline at end of file diff --git a/tests/tools/test_identity_tools.py b/tests/tools/test_identity_tools.py index 3b81e1e..abdba28 100644 --- a/tests/tools/test_identity_tools.py +++ b/tests/tools/test_identity_tools.py @@ -1,6 +1,7 @@ from unittest.mock import Mock import pytest +import pydantic from openstack import exceptions @@ -392,30 +393,30 @@ def test_get_domain_success(self, mock_get_openstack_conn_identity): # Create mock domain object mock_domain = Mock() - mock_domain.id = "domainone" - mock_domain.name = "DomainOne" - mock_domain.description = "Domain One description" + mock_domain.id = "d01a81393377480cbd75c0210442e687" + mock_domain.name = "domainone" + mock_domain.description = "domainone description" mock_domain.is_enabled = True # Configure mock domain.get_domain() - mock_conn.identity.get_domain.return_value = mock_domain + mock_conn.identity.find_domain.return_value = mock_domain # Test get_domain() identity_tools = self.get_identity_tools() - result = identity_tools.get_domain(id="domainone") + result = identity_tools.get_domain(name="domainone") # Verify results - assert result == Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=True) + assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) # Verify mock calls - mock_conn.identity.get_domain.assert_called_once_with(domain="domainone") + mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") def test_get_domain_not_found(self, mock_get_openstack_conn_identity): """Test getting a identity domain that does not exist.""" mock_conn = mock_get_openstack_conn_identity # Configure mock to raise NotFoundException - mock_conn.identity.get_domain.side_effect = exceptions.NotFoundException( + mock_conn.identity.find_domain.side_effect = exceptions.NotFoundException( "Domain 'domainone' not found", ) @@ -424,7 +425,194 @@ def test_get_domain_not_found(self, mock_get_openstack_conn_identity): # Verify exception is raised with pytest.raises(exceptions.NotFoundException, match="Domain 'domainone' not found"): - identity_tools.get_domain(id="domainone") + identity_tools.get_domain(name="domainone") # Verify mock calls - mock_conn.identity.get_domain.assert_called_once_with(domain="domainone") + mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") + + def test_create_domain_success(self, mock_get_openstack_conn_identity): + """Test creating a identity domain successfully.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "d01a81393377480cbd75c0210442e687" + mock_domain.name = "domainone" + mock_domain.description = "domainone description" + mock_domain.is_enabled = True + + # Configure mock domain.create_domain() + mock_conn.identity.create_domain.return_value = mock_domain + + # Test create_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.create_domain(name="domainone", description="domainone description", is_enabled=True) + + # Verify results + assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) + + # Verify mock calls + mock_conn.identity.create_domain.assert_called_once_with(name="domainone", description="domainone description", enabled=True) + + def test_create_domain_without_name(self, mock_get_openstack_conn_identity): + """Test creating a identity domain without a name.""" + mock_conn = mock_get_openstack_conn_identity + + # Test create_domain() + identity_tools = self.get_identity_tools() + + # Verify pydantic validation exception is raised + with pytest.raises(pydantic.ValidationError): + identity_tools.create_domain(name="", description="domainone description", is_enabled=False) + + def test_create_domain_without_description(self, mock_get_openstack_conn_identity): + """Test creating a identity domain without a description.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "d01a81393377480cbd75c0210442e687" + mock_domain.name = "domainone" + mock_domain.description = "" + mock_domain.is_enabled = False + + # Configure mock domain.create_domain() + mock_conn.identity.create_domain.return_value = mock_domain + + # Test create_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.create_domain(name="domainone") + + # Verify results + assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="", is_enabled=False) + + # Verify mock calls + mock_conn.identity.create_domain.assert_called_once_with(name="domainone", description="", enabled=False) + + + def test_delete_domain_success(self, mock_get_openstack_conn_identity): + """Test deleting a identity domain successfully.""" + mock_conn = mock_get_openstack_conn_identity + + # mock + mock_domain = Mock() + mock_domain.id = "d01a81393377480cbd75c0210442e687" + mock_domain.name = "domainone" + mock_domain.description = "domainone description" + mock_domain.is_enabled = True + + mock_conn.identity.find_domain.return_value = mock_domain + + # Test delete_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.delete_domain(name="domainone") + + # Verify results + assert result is None + + # Verify mock calls + mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") + mock_conn.identity.delete_domain.assert_called_once_with(domain=mock_domain, ignore_missing=False) + + + def test_delete_domain_not_found(self, mock_get_openstack_conn_identity): + """Test deleting a identity domain that does not exist.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "d01a81393377480cbd75c0210442e687" + mock_domain.name = "domainone" + mock_domain.description = "domainone description" + mock_domain.is_enabled = True + + mock_conn.identity.find_domain.return_value = mock_domain + + # Configure mock to raise NotFoundException + mock_conn.identity.delete_domain.side_effect = exceptions.NotFoundException( + "Domain 'domainone' not found", + ) + + # Test delete_domain() + identity_tools = self.get_identity_tools() + + # Verify exception is raised + with pytest.raises(exceptions.NotFoundException, match="Domain 'domainone' not found"): + identity_tools.delete_domain(name="domainone") + + # Verify mock calls + mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") + mock_conn.identity.delete_domain.assert_called_once_with(domain=mock_domain, ignore_missing=False) + + def test_update_domain_with_all_fields_success(self, mock_get_openstack_conn_identity): + """Test updating a identity domain successfully.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "d01a81393377480cbd75c0210442e687" + mock_domain.name = "domainone" + mock_domain.description = "domainone description" + mock_domain.is_enabled = True + + # Configure mock domain.find_domain() and update_domain() + mock_conn.identity.find_domain.return_value = mock_domain + mock_conn.identity.update_domain.return_value = mock_domain + + # Test update_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.update_domain(name="domainone", description="domainone description", is_enabled=True) + + # Verify results + assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) + + # Verify mock calls + mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") + mock_conn.identity.update_domain.assert_called_once_with(domain=mock_domain, name="domainone", description="domainone description", is_enabled=True) + + + def test_update_domain_with_empty_args(self, mock_get_openstack_conn_identity): + """Test updating a identity domain with empty arguments.""" + mock_conn = mock_get_openstack_conn_identity + + # Create mock domain object + mock_domain = Mock() + mock_domain.id = "d01a81393377480cbd75c0210442e687" + mock_domain.name = "domainone" + mock_domain.description = "domainone description" + mock_domain.is_enabled = True + + # Configure mock domain.find_domain() and update_domain() + mock_conn.identity.find_domain.return_value = mock_domain + mock_conn.identity.update_domain.return_value = mock_domain + + # Test update_domain() + identity_tools = self.get_identity_tools() + result = identity_tools.update_domain(name="domainone") + + # Verify results + assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) + + # Verify mock calls + mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") + mock_conn.identity.update_domain.assert_called_once_with(domain=mock_domain, name="domainone") + + def test_update_domain_with_empty_name(self, mock_get_openstack_conn_identity): + """Test updating a identity domain with an empty name.""" + mock_conn = mock_get_openstack_conn_identity + + mock_conn.identity.find_domain.side_effect = exceptions.BadRequestException( + "Field required", + ) + + # Test update_domain() + identity_tools = self.get_identity_tools() + + # Verify exception is raised + with pytest.raises(exceptions.BadRequestException, match="Field required"): + identity_tools.update_domain(name="") + + # Verify mock calls + mock_conn.identity.find_domain.assert_called_once_with(name_or_id="") + + \ No newline at end of file From 98e34288065c9f336fa7b30d98e301660ad137ce Mon Sep 17 00:00:00 2001 From: S0okJu Date: Sun, 17 Aug 2025 12:23:49 +0900 Subject: [PATCH 5/9] feat(identity): Add id field in update domain tool(#31) - Add id field in update_domain tool - Tests are updated and passing --- .../tools/identity_tools.py | 8 +++---- tests/tools/test_identity_tools.py | 24 ++++++++----------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 985404c..293afc1 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -163,11 +163,12 @@ def delete_domain(self, name: str) -> None: return None - def update_domain(self, name: str, description: str | None = None, is_enabled: bool | None = None) -> Domain: + def update_domain(self, id: str, name: str | None = None, description: str | None = None, is_enabled: bool | None = None) -> Domain: """ Update a domain. - :param name: The name of the domain. (required) + :param id: The ID of the domain. (required) + :param name: The name of the domain. (optional) :param description: The description of the domain. (optional) :param is_enabled: Whether the domain is enabled. (optional) """ @@ -181,8 +182,7 @@ def update_domain(self, name: str, description: str | None = None, is_enabled: b if is_enabled is not None: args["is_enabled"] = is_enabled - domain = conn.identity.find_domain(name_or_id=name) - updated_domain = conn.identity.update_domain(domain=domain, **args) + updated_domain = conn.identity.update_domain(domain=id, **args) return Domain(id=updated_domain.id, name=updated_domain.name, description=updated_domain.description, is_enabled=updated_domain.is_enabled) diff --git a/tests/tools/test_identity_tools.py b/tests/tools/test_identity_tools.py index abdba28..65f9ffc 100644 --- a/tests/tools/test_identity_tools.py +++ b/tests/tools/test_identity_tools.py @@ -555,20 +555,18 @@ def test_update_domain_with_all_fields_success(self, mock_get_openstack_conn_ide mock_domain.description = "domainone description" mock_domain.is_enabled = True - # Configure mock domain.find_domain() and update_domain() - mock_conn.identity.find_domain.return_value = mock_domain + # Configure mock domain.update_domain() mock_conn.identity.update_domain.return_value = mock_domain # Test update_domain() identity_tools = self.get_identity_tools() - result = identity_tools.update_domain(name="domainone", description="domainone description", is_enabled=True) + result = identity_tools.update_domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) # Verify results assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) # Verify mock calls - mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") - mock_conn.identity.update_domain.assert_called_once_with(domain=mock_domain, name="domainone", description="domainone description", is_enabled=True) + mock_conn.identity.update_domain.assert_called_once_with(domain="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) def test_update_domain_with_empty_args(self, mock_get_openstack_conn_identity): @@ -582,26 +580,24 @@ def test_update_domain_with_empty_args(self, mock_get_openstack_conn_identity): mock_domain.description = "domainone description" mock_domain.is_enabled = True - # Configure mock domain.find_domain() and update_domain() - mock_conn.identity.find_domain.return_value = mock_domain + # Configure mock domain.update_domain() mock_conn.identity.update_domain.return_value = mock_domain # Test update_domain() identity_tools = self.get_identity_tools() - result = identity_tools.update_domain(name="domainone") + result = identity_tools.update_domain(id="d01a81393377480cbd75c0210442e687") # Verify results assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) # Verify mock calls - mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") - mock_conn.identity.update_domain.assert_called_once_with(domain=mock_domain, name="domainone") + mock_conn.identity.update_domain.assert_called_once_with(domain="d01a81393377480cbd75c0210442e687") - def test_update_domain_with_empty_name(self, mock_get_openstack_conn_identity): + def test_update_domain_with_empty_id(self, mock_get_openstack_conn_identity): """Test updating a identity domain with an empty name.""" mock_conn = mock_get_openstack_conn_identity - mock_conn.identity.find_domain.side_effect = exceptions.BadRequestException( + mock_conn.identity.update_domain.side_effect = exceptions.BadRequestException( "Field required", ) @@ -610,9 +606,9 @@ def test_update_domain_with_empty_name(self, mock_get_openstack_conn_identity): # Verify exception is raised with pytest.raises(exceptions.BadRequestException, match="Field required"): - identity_tools.update_domain(name="") + identity_tools.update_domain(id="") # Verify mock calls - mock_conn.identity.find_domain.assert_called_once_with(name_or_id="") + mock_conn.identity.update_domain.assert_called_once_with(domain="") \ No newline at end of file From 8a5cf10ed5b7d55bb58ae4cd15169cbc2a7fc2b3 Mon Sep 17 00:00:00 2001 From: S0okJu Date: Sun, 17 Aug 2025 12:30:49 +0900 Subject: [PATCH 6/9] feat(identity): Add id field in update domain tool(#31) - Resolve conflict --- src/openstack_mcp_server/tools/identity_tools.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 7881c71..62a74c5 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -23,11 +23,8 @@ def register_tools(self, mcp: FastMCP): mcp.tool()(self.get_domains) mcp.tool()(self.get_domain) mcp.tool()(self.create_domain) -<<<<<<< HEAD mcp.tool()(self.delete_domain) mcp.tool()(self.update_domain) -======= ->>>>>>> d7d60da163653af86da9ffa64fb394cbb1ea8c40 def get_regions(self) -> list[Region]: """ From b593684878952bfd60f2c80348c86721cee07063 Mon Sep 17 00:00:00 2001 From: S0okJu Date: Sun, 17 Aug 2025 14:32:19 +0900 Subject: [PATCH 7/9] fix(identity): Fix linter problem(#31) --- .../tools/identity_tools.py | 62 +++- .../tools/response/identity.py | 1 + tests/tools/test_identity_tools.py | 289 ++++++++++++------ 3 files changed, 245 insertions(+), 107 deletions(-) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 5d85516..bcdc5c3 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -19,16 +19,13 @@ def register_tools(self, mcp: FastMCP): mcp.tool()(self.create_region) mcp.tool()(self.delete_region) mcp.tool()(self.update_region) - + mcp.tool()(self.get_domains) mcp.tool()(self.get_domain) mcp.tool()(self.create_domain) mcp.tool()(self.delete_domain) mcp.tool()(self.update_domain) - mcp.tool()(self.get_domains) - mcp.tool()(self.get_domain) - def get_regions(self) -> list[Region]: """ Get the list of Identity regions. @@ -121,7 +118,12 @@ def get_domains(self) -> list[Domain]: domain_list = [] for domain in conn.identity.domains(): domain_list.append( - Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled), + Domain( + id=domain.id, + name=domain.name, + description=domain.description, + is_enabled=domain.is_enabled, + ), ) return domain_list @@ -134,12 +136,22 @@ def get_domain(self, name: str) -> Domain: :return: The Domain object. """ conn = get_openstack_conn() - + domain = conn.identity.find_domain(name_or_id=name) - return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) - - def create_domain(self, name: str, description: str = "", is_enabled: bool = False) -> Domain: + return Domain( + id=domain.id, + name=domain.name, + description=domain.description, + is_enabled=domain.is_enabled, + ) + + def create_domain( + self, + name: str, + description: str = "", + is_enabled: bool = False, + ) -> Domain: """ Create a new domain. @@ -149,10 +161,19 @@ def create_domain(self, name: str, description: str = "", is_enabled: bool = Fal """ conn = get_openstack_conn() - domain = conn.identity.create_domain(name=name, description=description, enabled=is_enabled) + domain = conn.identity.create_domain( + name=name, + description=description, + enabled=is_enabled, + ) + + return Domain( + id=domain.id, + name=domain.name, + description=domain.description, + is_enabled=domain.is_enabled, + ) - return Domain(id=domain.id, name=domain.name, description=domain.description, is_enabled=domain.is_enabled) - def delete_domain(self, name: str) -> None: """ Delete a domain. @@ -165,8 +186,14 @@ def delete_domain(self, name: str) -> None: conn.identity.delete_domain(domain=domain, ignore_missing=False) return None - - def update_domain(self, id: str, name: str | None = None, description: str | None = None, is_enabled: bool | None = None) -> Domain: + + def update_domain( + self, + id: str, + name: str | None = None, + description: str | None = None, + is_enabled: bool | None = None, + ) -> Domain: """ Update a domain. @@ -187,4 +214,9 @@ def update_domain(self, id: str, name: str | None = None, description: str | Non updated_domain = conn.identity.update_domain(domain=id, **args) - return Domain(id=updated_domain.id, name=updated_domain.name, description=updated_domain.description, is_enabled=updated_domain.is_enabled) + return Domain( + id=updated_domain.id, + name=updated_domain.name, + description=updated_domain.description, + is_enabled=updated_domain.is_enabled, + ) diff --git a/src/openstack_mcp_server/tools/response/identity.py b/src/openstack_mcp_server/tools/response/identity.py index 44620ae..2e6bd08 100644 --- a/src/openstack_mcp_server/tools/response/identity.py +++ b/src/openstack_mcp_server/tools/response/identity.py @@ -7,6 +7,7 @@ class Region(BaseModel): id: str description: str = "" + class Domain(BaseModel): id: str name: str diff --git a/tests/tools/test_identity_tools.py b/tests/tools/test_identity_tools.py index d27ad56..fe29e29 100644 --- a/tests/tools/test_identity_tools.py +++ b/tests/tools/test_identity_tools.py @@ -360,11 +360,21 @@ def test_get_domains_success(self, mock_get_openstack_conn_identity): # Test get_domains() identity_tools = self.get_identity_tools() result = identity_tools.get_domains() - + # Verify results assert result == [ - Domain(id="domainone", name="DomainOne", description="Domain One description", is_enabled=True), - Domain(id="domaintwo", name="DomainTwo", description="Domain Two description", is_enabled=False), + Domain( + id="domainone", + name="DomainOne", + description="Domain One description", + is_enabled=True, + ), + Domain( + id="domaintwo", + name="DomainTwo", + description="Domain Two description", + is_enabled=False, + ), ] # Verify mock calls @@ -383,7 +393,7 @@ def test_get_domains_empty_list(self, mock_get_openstack_conn_identity): # Verify results assert result == [] - + # Verify mock calls mock_conn.identity.domains.assert_called_once() @@ -397,38 +407,52 @@ def test_get_domain_success(self, mock_get_openstack_conn_identity): mock_domain.name = "domainone" mock_domain.description = "domainone description" mock_domain.is_enabled = True - + # Configure mock domain.get_domain() mock_conn.identity.find_domain.return_value = mock_domain - + # Test get_domain() identity_tools = self.get_identity_tools() result = identity_tools.get_domain(name="domainone") - + # Verify results - assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) - + assert result == Domain( + id="d01a81393377480cbd75c0210442e687", + name="domainone", + description="domainone description", + is_enabled=True, + ) + # Verify mock calls - mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") - + mock_conn.identity.find_domain.assert_called_once_with( + name_or_id="domainone", + ) + def test_get_domain_not_found(self, mock_get_openstack_conn_identity): """Test getting a identity domain that does not exist.""" mock_conn = mock_get_openstack_conn_identity # Configure mock to raise NotFoundException - mock_conn.identity.find_domain.side_effect = exceptions.NotFoundException( - "Domain 'domainone' not found", + mock_conn.identity.find_domain.side_effect = ( + exceptions.NotFoundException( + "Domain 'domainone' not found", + ) ) - + # Test get_domain() identity_tools = self.get_identity_tools() - + # Verify exception is raised - with pytest.raises(exceptions.NotFoundException, match="Domain 'domainone' not found"): + with pytest.raises( + exceptions.NotFoundException, + match="Domain 'domainone' not found", + ): identity_tools.get_domain(name="domainone") - + # Verify mock calls - mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") + mock_conn.identity.find_domain.assert_called_once_with( + name_or_id="domainone", + ) def test_create_domain_success(self, mock_get_openstack_conn_identity): """Test creating a identity domain successfully.""" @@ -440,173 +464,254 @@ def test_create_domain_success(self, mock_get_openstack_conn_identity): mock_domain.name = "domainone" mock_domain.description = "domainone description" mock_domain.is_enabled = True - + # Configure mock domain.create_domain() mock_conn.identity.create_domain.return_value = mock_domain - + # Test create_domain() identity_tools = self.get_identity_tools() - result = identity_tools.create_domain(name="domainone", description="domainone description", is_enabled=True) - + result = identity_tools.create_domain( + name="domainone", + description="domainone description", + is_enabled=True, + ) + # Verify results - assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) - + assert result == Domain( + id="d01a81393377480cbd75c0210442e687", + name="domainone", + description="domainone description", + is_enabled=True, + ) + # Verify mock calls - mock_conn.identity.create_domain.assert_called_once_with(name="domainone", description="domainone description", enabled=True) - - def test_create_domain_without_name(self, mock_get_openstack_conn_identity): + mock_conn.identity.create_domain.assert_called_once_with( + name="domainone", + description="domainone description", + enabled=True, + ) + + def test_create_domain_without_name( + self, + mock_get_openstack_conn_identity, + ): """Test creating a identity domain without a name.""" - + # Test create_domain() identity_tools = self.get_identity_tools() - + # Verify pydantic validation exception is raised with pytest.raises(pydantic.ValidationError): - identity_tools.create_domain(name="", description="domainone description", is_enabled=False) - - def test_create_domain_without_description(self, mock_get_openstack_conn_identity): + identity_tools.create_domain( + name="", + description="domainone description", + is_enabled=False, + ) + + def test_create_domain_without_description( + self, + mock_get_openstack_conn_identity, + ): """Test creating a identity domain without a description.""" mock_conn = mock_get_openstack_conn_identity - + # Create mock domain object mock_domain = Mock() mock_domain.id = "d01a81393377480cbd75c0210442e687" mock_domain.name = "domainone" mock_domain.description = "" mock_domain.is_enabled = False - + # Configure mock domain.create_domain() mock_conn.identity.create_domain.return_value = mock_domain - + # Test create_domain() identity_tools = self.get_identity_tools() result = identity_tools.create_domain(name="domainone") - + # Verify results - assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="", is_enabled=False) - + assert result == Domain( + id="d01a81393377480cbd75c0210442e687", + name="domainone", + description="", + is_enabled=False, + ) + # Verify mock calls - mock_conn.identity.create_domain.assert_called_once_with(name="domainone", description="", enabled=False) - - + mock_conn.identity.create_domain.assert_called_once_with( + name="domainone", + description="", + enabled=False, + ) + def test_delete_domain_success(self, mock_get_openstack_conn_identity): """Test deleting a identity domain successfully.""" mock_conn = mock_get_openstack_conn_identity - - # mock + + # mock mock_domain = Mock() mock_domain.id = "d01a81393377480cbd75c0210442e687" mock_domain.name = "domainone" mock_domain.description = "domainone description" mock_domain.is_enabled = True - + mock_conn.identity.find_domain.return_value = mock_domain - + # Test delete_domain() identity_tools = self.get_identity_tools() result = identity_tools.delete_domain(name="domainone") # Verify results assert result is None - + # Verify mock calls - mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") - mock_conn.identity.delete_domain.assert_called_once_with(domain=mock_domain, ignore_missing=False) - - + mock_conn.identity.find_domain.assert_called_once_with( + name_or_id="domainone", + ) + mock_conn.identity.delete_domain.assert_called_once_with( + domain=mock_domain, + ignore_missing=False, + ) + def test_delete_domain_not_found(self, mock_get_openstack_conn_identity): """Test deleting a identity domain that does not exist.""" mock_conn = mock_get_openstack_conn_identity - + # Create mock domain object mock_domain = Mock() mock_domain.id = "d01a81393377480cbd75c0210442e687" mock_domain.name = "domainone" mock_domain.description = "domainone description" mock_domain.is_enabled = True - + mock_conn.identity.find_domain.return_value = mock_domain - + # Configure mock to raise NotFoundException - mock_conn.identity.delete_domain.side_effect = exceptions.NotFoundException( - "Domain 'domainone' not found", + mock_conn.identity.delete_domain.side_effect = ( + exceptions.NotFoundException( + "Domain 'domainone' not found", + ) ) - + # Test delete_domain() identity_tools = self.get_identity_tools() - + # Verify exception is raised - with pytest.raises(exceptions.NotFoundException, match="Domain 'domainone' not found"): + with pytest.raises( + exceptions.NotFoundException, + match="Domain 'domainone' not found", + ): identity_tools.delete_domain(name="domainone") - + # Verify mock calls - mock_conn.identity.find_domain.assert_called_once_with(name_or_id="domainone") - mock_conn.identity.delete_domain.assert_called_once_with(domain=mock_domain, ignore_missing=False) - - def test_update_domain_with_all_fields_success(self, mock_get_openstack_conn_identity): + mock_conn.identity.find_domain.assert_called_once_with( + name_or_id="domainone", + ) + mock_conn.identity.delete_domain.assert_called_once_with( + domain=mock_domain, + ignore_missing=False, + ) + + def test_update_domain_with_all_fields_success( + self, + mock_get_openstack_conn_identity, + ): """Test updating a identity domain successfully.""" mock_conn = mock_get_openstack_conn_identity - + # Create mock domain object mock_domain = Mock() mock_domain.id = "d01a81393377480cbd75c0210442e687" mock_domain.name = "domainone" mock_domain.description = "domainone description" mock_domain.is_enabled = True - + # Configure mock domain.update_domain() mock_conn.identity.update_domain.return_value = mock_domain - + # Test update_domain() identity_tools = self.get_identity_tools() - result = identity_tools.update_domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) - + result = identity_tools.update_domain( + id="d01a81393377480cbd75c0210442e687", + name="domainone", + description="domainone description", + is_enabled=True, + ) + # Verify results - assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) - + assert result == Domain( + id="d01a81393377480cbd75c0210442e687", + name="domainone", + description="domainone description", + is_enabled=True, + ) + # Verify mock calls - mock_conn.identity.update_domain.assert_called_once_with(domain="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) - - - def test_update_domain_with_empty_args(self, mock_get_openstack_conn_identity): + mock_conn.identity.update_domain.assert_called_once_with( + domain="d01a81393377480cbd75c0210442e687", + name="domainone", + description="domainone description", + is_enabled=True, + ) + + def test_update_domain_with_empty_args( + self, + mock_get_openstack_conn_identity, + ): """Test updating a identity domain with empty arguments.""" mock_conn = mock_get_openstack_conn_identity - + # Create mock domain object mock_domain = Mock() mock_domain.id = "d01a81393377480cbd75c0210442e687" mock_domain.name = "domainone" mock_domain.description = "domainone description" mock_domain.is_enabled = True - + # Configure mock domain.update_domain() mock_conn.identity.update_domain.return_value = mock_domain - + # Test update_domain() identity_tools = self.get_identity_tools() - result = identity_tools.update_domain(id="d01a81393377480cbd75c0210442e687") - + result = identity_tools.update_domain( + id="d01a81393377480cbd75c0210442e687", + ) + # Verify results - assert result == Domain(id="d01a81393377480cbd75c0210442e687", name="domainone", description="domainone description", is_enabled=True) - + assert result == Domain( + id="d01a81393377480cbd75c0210442e687", + name="domainone", + description="domainone description", + is_enabled=True, + ) + # Verify mock calls - mock_conn.identity.update_domain.assert_called_once_with(domain="d01a81393377480cbd75c0210442e687") - - def test_update_domain_with_empty_id(self, mock_get_openstack_conn_identity): + mock_conn.identity.update_domain.assert_called_once_with( + domain="d01a81393377480cbd75c0210442e687", + ) + + def test_update_domain_with_empty_id( + self, + mock_get_openstack_conn_identity, + ): """Test updating a identity domain with an empty name.""" mock_conn = mock_get_openstack_conn_identity - - mock_conn.identity.update_domain.side_effect = exceptions.BadRequestException( - "Field required", + + mock_conn.identity.update_domain.side_effect = ( + exceptions.BadRequestException( + "Field required", + ) ) - + # Test update_domain() identity_tools = self.get_identity_tools() - + # Verify exception is raised - with pytest.raises(exceptions.BadRequestException, match="Field required"): + with pytest.raises( + exceptions.BadRequestException, + match="Field required", + ): identity_tools.update_domain(id="") - + # Verify mock calls mock_conn.identity.update_domain.assert_called_once_with(domain="") - From 438c0788651c1bc3c54caceae5fb3eac37fe4a89 Mon Sep 17 00:00:00 2001 From: S0okJu Date: Sun, 17 Aug 2025 15:48:35 +0900 Subject: [PATCH 8/9] fix(identity): Remove docstring required, optional keywords(#31) --- .../tools/identity_tools.py | 38 +++++++++---------- .../tools/response/identity.py | 6 +-- tests/tools/test_identity_tools.py | 12 +++--- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index bcdc5c3..4e64f48 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -46,7 +46,7 @@ def get_region(self, id: str) -> Region: """ Get a region. - :param id: The ID of the region. (required) + :param id: The ID of the region. :return: The Region object. """ @@ -56,12 +56,12 @@ def get_region(self, id: str) -> Region: return Region(id=region.id, description=region.description) - def create_region(self, id: str, description: str = "") -> Region: + def create_region(self, id: str, description: str | None= None) -> Region: """ Create a new region. - :param id: The ID of the region. (required) - :param description: The description of the region. (optional) + :param id: The ID of the region. + :param description: The description of the region. :return: The created Region object. """ @@ -75,7 +75,7 @@ def delete_region(self, id: str) -> None: """ Delete a region. - :param id: The ID of the region. (required) + :param id: The ID of the region. :return: None """ @@ -86,12 +86,12 @@ def delete_region(self, id: str) -> None: return None - def update_region(self, id: str, description: str = "") -> Region: + def update_region(self, id: str, description: str | None = None) -> Region: """ Update a region. - :param id: The ID of the region. (required) - :param description: The string description of the region. (optional) + :param id: The ID of the region. + :param description: The string description of the region. :return: The updated Region object. """ @@ -131,7 +131,7 @@ def get_domain(self, name: str) -> Domain: """ Get a domain. - :param name: The name of the domain. (required) + :param name: The name of the domain. :return: The Domain object. """ @@ -149,15 +149,15 @@ def get_domain(self, name: str) -> Domain: def create_domain( self, name: str, - description: str = "", - is_enabled: bool = False, + description: str | None = None, + is_enabled: bool | None = False, ) -> Domain: """ Create a new domain. - :param name: The name of the domain. (required) - :param description: The description of the domain. (optional) - :param is_enabled: Whether the domain is enabled. (optional) + :param name: The name of the domain. + :param description: The description of the domain. + :param is_enabled: Whether the domain is enabled. """ conn = get_openstack_conn() @@ -178,7 +178,7 @@ def delete_domain(self, name: str) -> None: """ Delete a domain. - :param name: The name of the domain. (required) + :param name: The name of the domain. """ conn = get_openstack_conn() @@ -197,10 +197,10 @@ def update_domain( """ Update a domain. - :param id: The ID of the domain. (required) - :param name: The name of the domain. (optional) - :param description: The description of the domain. (optional) - :param is_enabled: Whether the domain is enabled. (optional) + :param id: The ID of the domain. + :param name: The name of the domain. + :param description: The description of the domain. + :param is_enabled: Whether the domain is enabled. """ conn = get_openstack_conn() diff --git a/src/openstack_mcp_server/tools/response/identity.py b/src/openstack_mcp_server/tools/response/identity.py index 2e6bd08..527ff4d 100644 --- a/src/openstack_mcp_server/tools/response/identity.py +++ b/src/openstack_mcp_server/tools/response/identity.py @@ -5,11 +5,11 @@ # In this case, we are only using description field as optional. class Region(BaseModel): id: str - description: str = "" + description: str | None = None class Domain(BaseModel): id: str name: str - description: str = "" - is_enabled: bool = False + description: str | None = None + is_enabled: bool | None = None diff --git a/tests/tools/test_identity_tools.py b/tests/tools/test_identity_tools.py index fe29e29..47965bc 100644 --- a/tests/tools/test_identity_tools.py +++ b/tests/tools/test_identity_tools.py @@ -103,7 +103,7 @@ def test_create_region_without_description( # Create mock region object mock_region = Mock() mock_region.id = "RegionOne" - mock_region.description = "" + mock_region.description = None # Configure mock region.create_region() mock_conn.identity.create_region.return_value = mock_region @@ -233,7 +233,7 @@ def test_update_region_without_description( # Create mock region object mock_region = Mock() mock_region.id = "RegionOne" - mock_region.description = "" + mock_region.description = None # Configure mock region.update_region() mock_conn.identity.update_region.return_value = mock_region @@ -248,7 +248,7 @@ def test_update_region_without_description( # Verify mock calls mock_conn.identity.update_region.assert_called_once_with( region="RegionOne", - description="", + description=None, ) def test_update_region_invalid_id_format( @@ -519,7 +519,7 @@ def test_create_domain_without_description( mock_domain = Mock() mock_domain.id = "d01a81393377480cbd75c0210442e687" mock_domain.name = "domainone" - mock_domain.description = "" + mock_domain.description = None mock_domain.is_enabled = False # Configure mock domain.create_domain() @@ -533,14 +533,14 @@ def test_create_domain_without_description( assert result == Domain( id="d01a81393377480cbd75c0210442e687", name="domainone", - description="", + description=None, is_enabled=False, ) # Verify mock calls mock_conn.identity.create_domain.assert_called_once_with( name="domainone", - description="", + description=None, enabled=False, ) From a7213c447e7277207b579285a05092d57cc4fef5 Mon Sep 17 00:00:00 2001 From: S0okJu Date: Mon, 18 Aug 2025 12:02:09 +0900 Subject: [PATCH 9/9] fix(identity): Fix linter problem(#31) --- src/openstack_mcp_server/tools/identity_tools.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/openstack_mcp_server/tools/identity_tools.py b/src/openstack_mcp_server/tools/identity_tools.py index 4e64f48..f3fe85d 100644 --- a/src/openstack_mcp_server/tools/identity_tools.py +++ b/src/openstack_mcp_server/tools/identity_tools.py @@ -56,12 +56,12 @@ def get_region(self, id: str) -> Region: return Region(id=region.id, description=region.description) - def create_region(self, id: str, description: str | None= None) -> Region: + def create_region(self, id: str, description: str | None = None) -> Region: """ Create a new region. :param id: The ID of the region. - :param description: The description of the region. + :param description: The description of the region. :return: The created Region object. """ @@ -150,7 +150,7 @@ def create_domain( self, name: str, description: str | None = None, - is_enabled: bool | None = False, + is_enabled: bool | None = False, ) -> Domain: """ Create a new domain.