From b95d3b7824f337e496c55b8ee545f250adc5c511 Mon Sep 17 00:00:00 2001 From: JiAnJeong Date: Tue, 19 Aug 2025 10:59:16 +0900 Subject: [PATCH 1/3] feat: add get images (#20) --- src/openstack_mcp_server/prompts/__init__.py | 14 ++ .../prompts/image_tools_prompt.md | 49 ++++++ src/openstack_mcp_server/tools/image_tools.py | 41 +++-- tests/tools/test_image_tools.py | 153 ++++++++++++------ 4 files changed, 198 insertions(+), 59 deletions(-) create mode 100644 src/openstack_mcp_server/prompts/image_tools_prompt.md diff --git a/src/openstack_mcp_server/prompts/__init__.py b/src/openstack_mcp_server/prompts/__init__.py index e69de29..b70ecb2 100644 --- a/src/openstack_mcp_server/prompts/__init__.py +++ b/src/openstack_mcp_server/prompts/__init__.py @@ -0,0 +1,14 @@ +from fastmcp import FastMCP + + +def register_prompt(mcp: FastMCP): + """ + Register OpenStack MCP prompts. + """ + + # Add image tools prompt + mcp.add_prompt( + "image_tools", + "ImageTools Usage Guide - Only use parameters that the user explicitly requests", + "prompts/image_tools_prompt.md", + ) diff --git a/src/openstack_mcp_server/prompts/image_tools_prompt.md b/src/openstack_mcp_server/prompts/image_tools_prompt.md new file mode 100644 index 0000000..26cb4ad --- /dev/null +++ b/src/openstack_mcp_server/prompts/image_tools_prompt.md @@ -0,0 +1,49 @@ +# ImageTools Usage Guide + +## get_images Method + +### Core Principles +- **Only use parameters that the user explicitly requests** +- **Do NOT add default values or assume values for parameters not mentioned** +- **Leave parameters as null if the user doesn't specify them** + +### Usage Examples + +#### 1. "Show me public images" +```json +{ + "visibility": "public" +} +``` +- `status` and `name` are not set (null) + +#### 2. "Show me active status images" +```json +{ + "status": "active" +} +``` +- `visibility` and `name` are not set (null) + +#### 3. "Show me ubuntu images" +```json +{ + "name": "ubuntu" +} +``` +- `status` and `visibility` are not set (null) + +#### 4. "Show me public active images" +```json +{ + "visibility": "public", + "status": "active" +} +``` +- `name` is not set (null) + +### Important Notes +- If user asks for "public images only", do NOT automatically set `status` to "active" +- If user asks for "list of images", leave all parameters as null +- When in doubt, ask the user for clarification rather than making assumptions +- Always respect the user's explicit requests and avoid adding unrequested filters diff --git a/src/openstack_mcp_server/tools/image_tools.py b/src/openstack_mcp_server/tools/image_tools.py index 953f603..c1e8827 100644 --- a/src/openstack_mcp_server/tools/image_tools.py +++ b/src/openstack_mcp_server/tools/image_tools.py @@ -16,26 +16,45 @@ def register_tools(self, mcp: FastMCP): Register Image-related tools with the FastMCP instance. """ - mcp.tool()(self.get_image_images) + mcp.tool()(self.get_images) mcp.tool()(self.create_image) - def get_image_images(self) -> str: + def get_images( + self, + name: str | None = None, + status: str | None = None, + visibility: str | None = None, + ) -> list[Image]: """ - Get the list of Image images by invoking the registered tool. + Get the list of OpenStack images with optional filtering. - :return: A string containing the names, IDs, and statuses of the images. + Prompt: + See prompts/image_tools_prompt.md for detailed usage examples. + + Args: + name: Filter by image name + status: Filter by status (e.g., 'active', 'queued', 'killed') + visibility: Filter by visibility (e.g., 'public', 'private', 'shared') + + Returns: + A list of Image objects. """ - # Initialize connection conn = get_openstack_conn() - # List the servers + # Build filters for the image query + filters = {} + if name and name.strip(): + filters["name"] = name.strip() + if status and status.strip(): + filters["status"] = status.strip() + if visibility and visibility.strip(): + filters["visibility"] = visibility.strip() + image_list = [] - for image in conn.image.images(): - image_list.append( - f"{image.name} ({image.id}) - Status: {image.status}", - ) + for image in conn.image.images(**filters): + image_list.append(Image(**image)) - return "\n".join(image_list) + return image_list def create_image(self, image_data: CreateImage) -> Image: """Create a new Openstack image. diff --git a/tests/tools/test_image_tools.py b/tests/tools/test_image_tools.py index f29dac8..be8ec4a 100644 --- a/tests/tools/test_image_tools.py +++ b/tests/tools/test_image_tools.py @@ -2,6 +2,8 @@ from unittest.mock import Mock +import pytest + from openstack_mcp_server.tools.image_tools import ImageTools from openstack_mcp_server.tools.request.image import CreateImage from openstack_mcp_server.tools.response.image import Image @@ -46,80 +48,135 @@ def image_factory(**overrides): return defaults - def test_get_image_images_success(self, mock_get_openstack_conn_image): + def test_get_images_success(self, mock_get_openstack_conn_image): """Test getting image images successfully.""" mock_conn = mock_get_openstack_conn_image - # Create mock image objects - mock_image1 = Mock() - mock_image1.name = "ubuntu-20.04-server" - mock_image1.id = "img-123-abc-def" - mock_image1.status = "active" + mock_image1 = self.image_factory( + id="img-123-abc-def", + name="ubuntu-20.04-server", + status="active", + visibility="public", + checksum="abc123", + size=1073741824, + ) - mock_image2 = Mock() - mock_image2.name = "centos-8-stream" - mock_image2.id = "img-456-ghi-jkl" - mock_image2.status = "active" + mock_image2 = self.image_factory( + id="img-456-ghi-jkl", + name="centos-8-stream", + status="active", + visibility="public", + checksum="def456", + size=2147483648, + ) - # Configure mock image.images() mock_conn.image.images.return_value = [mock_image1, mock_image2] - # Test ImageTools - image_tools = ImageTools() - result = image_tools.get_image_images() + result = ImageTools().get_images() - # Verify results - expected_output = ( - "ubuntu-20.04-server (img-123-abc-def) - Status: active\n" - "centos-8-stream (img-456-ghi-jkl) - Status: active" - ) + expected_output = [ + Image(**mock_image1), + Image(**mock_image2), + ] assert result == expected_output - # Verify mock calls mock_conn.image.images.assert_called_once() - def test_get_image_images_empty_list(self, mock_get_openstack_conn_image): + def test_get_images_empty_list(self, mock_get_openstack_conn_image): """Test getting image images when no images exist.""" mock_conn = mock_get_openstack_conn_image - - # Empty image list mock_conn.image.images.return_value = [] - image_tools = ImageTools() - result = image_tools.get_image_images() - - # Verify empty string - assert result == "" + result = ImageTools().get_images() + assert result == [] mock_conn.image.images.assert_called_once() - def test_get_image_images_with_empty_name( + @pytest.mark.parametrize( + "filter_name,filter_value,expected_count", + [ + ("name", "ubuntu-20.04-server", 1), # exact name match + ("name", "ubuntu", 0), # partial match not supported + ("name", "nonexistent", 0), # non-existent name + ("name", "", 2), # empty filter value + ("name", " ", 2), # whitespace only + ("status", "active", 2), + ("visibility", "public", 2), + ("status", "deleted", 0), + ("visibility", "private", 0), + ], + ) + def test_get_images_with_filters( self, mock_get_openstack_conn_image, + filter_name, + filter_value, + expected_count, ): - """Test images with empty or None names.""" + """Test getting images with various filters.""" mock_conn = mock_get_openstack_conn_image - # Images with empty name (edge case) - mock_image1 = Mock() - mock_image1.name = "normal-image" - mock_image1.id = "img-normal" - mock_image1.status = "active" - - mock_image2 = Mock() - mock_image2.name = "" # Empty name - mock_image2.id = "img-empty-name" - mock_image2.status = "active" - - mock_conn.image.images.return_value = [mock_image1, mock_image2] - - image_tools = ImageTools() - result = image_tools.get_image_images() + mock_image1 = self.image_factory( + id="img-123-abc-def", + name="ubuntu-20.04-server", + status="active", + visibility="public", + checksum="abc123", + size=1073741824, + ) - assert "normal-image (img-normal) - Status: active" in result - assert " (img-empty-name) - Status: active" in result # Empty name + mock_image2 = self.image_factory( + id="img-456-ghi-jkl", + name="centos-8-stream", + status="active", + visibility="public", + checksum="def456", + size=2147483648, + ) - mock_conn.image.images.assert_called_once() + if filter_name == "name": + if filter_value == "ubuntu-20.04-server": + mock_conn.image.images.return_value = [mock_image1] + elif filter_value in ["", " "]: + mock_conn.image.images.return_value = [ + mock_image1, + mock_image2, + ] + else: + mock_conn.image.images.return_value = [] + elif filter_name == "status": + if filter_value == "active": + mock_conn.image.images.return_value = [ + mock_image1, + mock_image2, + ] + else: + mock_conn.image.images.return_value = [] + elif filter_name == "visibility": + if filter_value == "public": + mock_conn.image.images.return_value = [ + mock_image1, + mock_image2, + ] + else: + mock_conn.image.images.return_value = [] + + result = ImageTools().get_images(**{filter_name: filter_value}) + + if expected_count == 0: + assert result == [] + elif expected_count == 1: + assert result == [Image(**mock_image1)] + else: + assert result == [Image(**mock_image1), Image(**mock_image2)] + + # For empty/whitespace filters, no filter should be applied + if filter_value in ["", " "]: + mock_conn.image.images.assert_called_once_with() + else: + mock_conn.image.images.assert_called_once_with( + **{filter_name: filter_value} + ) def test_create_image_success_with_volume_id( self, From 6056460388f0ab346e8354424a2e14869f7a0b19 Mon Sep 17 00:00:00 2001 From: jja6312 <127708710+jja6312@users.noreply.github.com> Date: Sun, 24 Aug 2025 22:56:14 +0900 Subject: [PATCH 2/3] docs: change get_images docstring to sphinx style --- src/openstack_mcp_server/prompts/__init__.py | 14 ------ .../prompts/image_tools_prompt.md | 49 ------------------- src/openstack_mcp_server/tools/image_tools.py | 18 +++---- 3 files changed, 8 insertions(+), 73 deletions(-) delete mode 100644 src/openstack_mcp_server/prompts/image_tools_prompt.md diff --git a/src/openstack_mcp_server/prompts/__init__.py b/src/openstack_mcp_server/prompts/__init__.py index b70ecb2..e69de29 100644 --- a/src/openstack_mcp_server/prompts/__init__.py +++ b/src/openstack_mcp_server/prompts/__init__.py @@ -1,14 +0,0 @@ -from fastmcp import FastMCP - - -def register_prompt(mcp: FastMCP): - """ - Register OpenStack MCP prompts. - """ - - # Add image tools prompt - mcp.add_prompt( - "image_tools", - "ImageTools Usage Guide - Only use parameters that the user explicitly requests", - "prompts/image_tools_prompt.md", - ) diff --git a/src/openstack_mcp_server/prompts/image_tools_prompt.md b/src/openstack_mcp_server/prompts/image_tools_prompt.md deleted file mode 100644 index 26cb4ad..0000000 --- a/src/openstack_mcp_server/prompts/image_tools_prompt.md +++ /dev/null @@ -1,49 +0,0 @@ -# ImageTools Usage Guide - -## get_images Method - -### Core Principles -- **Only use parameters that the user explicitly requests** -- **Do NOT add default values or assume values for parameters not mentioned** -- **Leave parameters as null if the user doesn't specify them** - -### Usage Examples - -#### 1. "Show me public images" -```json -{ - "visibility": "public" -} -``` -- `status` and `name` are not set (null) - -#### 2. "Show me active status images" -```json -{ - "status": "active" -} -``` -- `visibility` and `name` are not set (null) - -#### 3. "Show me ubuntu images" -```json -{ - "name": "ubuntu" -} -``` -- `status` and `visibility` are not set (null) - -#### 4. "Show me public active images" -```json -{ - "visibility": "public", - "status": "active" -} -``` -- `name` is not set (null) - -### Important Notes -- If user asks for "public images only", do NOT automatically set `status` to "active" -- If user asks for "list of images", leave all parameters as null -- When in doubt, ask the user for clarification rather than making assumptions -- Always respect the user's explicit requests and avoid adding unrequested filters diff --git a/src/openstack_mcp_server/tools/image_tools.py b/src/openstack_mcp_server/tools/image_tools.py index c1e8827..ef6a230 100644 --- a/src/openstack_mcp_server/tools/image_tools.py +++ b/src/openstack_mcp_server/tools/image_tools.py @@ -28,16 +28,14 @@ def get_images( """ Get the list of OpenStack images with optional filtering. - Prompt: - See prompts/image_tools_prompt.md for detailed usage examples. - - Args: - name: Filter by image name - status: Filter by status (e.g., 'active', 'queued', 'killed') - visibility: Filter by visibility (e.g., 'public', 'private', 'shared') - - Returns: - A list of Image objects. + The filtering behavior is as follows: + - By default, all available images are returned without any filtering applied. + - Filters are only applied when specific values are provided by the user. + + :param name: Filter by image name + :param status: Filter by status + :param visibility: Filter by visibility + :return: A list of Image objects. """ conn = get_openstack_conn() From ec290775b62b1dd0b40feaae3872366c35fcf4e2 Mon Sep 17 00:00:00 2001 From: jja6312 <127708710+jja6312@users.noreply.github.com> Date: Mon, 25 Aug 2025 22:33:19 +0900 Subject: [PATCH 3/3] refactor: restructure get images testing for better maintainability --- src/openstack_mcp_server/tools/image_tools.py | 2 +- tests/tools/test_image_tools.py | 148 +++++++++--------- 2 files changed, 72 insertions(+), 78 deletions(-) diff --git a/src/openstack_mcp_server/tools/image_tools.py b/src/openstack_mcp_server/tools/image_tools.py index ef6a230..d3544b3 100644 --- a/src/openstack_mcp_server/tools/image_tools.py +++ b/src/openstack_mcp_server/tools/image_tools.py @@ -29,7 +29,7 @@ def get_images( Get the list of OpenStack images with optional filtering. The filtering behavior is as follows: - - By default, all available images are returned without any filtering applied. + - By default, all available images are returned without any filtering applied. - Filters are only applied when specific values are provided by the user. :param name: Filter by image name diff --git a/tests/tools/test_image_tools.py b/tests/tools/test_image_tools.py index be8ec4a..0544ef5 100644 --- a/tests/tools/test_image_tools.py +++ b/tests/tools/test_image_tools.py @@ -2,8 +2,6 @@ from unittest.mock import Mock -import pytest - from openstack_mcp_server.tools.image_tools import ImageTools from openstack_mcp_server.tools.request.image import CreateImage from openstack_mcp_server.tools.response.image import Image @@ -51,7 +49,6 @@ def image_factory(**overrides): def test_get_images_success(self, mock_get_openstack_conn_image): """Test getting image images successfully.""" mock_conn = mock_get_openstack_conn_image - mock_image1 = self.image_factory( id="img-123-abc-def", name="ubuntu-20.04-server", @@ -60,7 +57,6 @@ def test_get_images_success(self, mock_get_openstack_conn_image): checksum="abc123", size=1073741824, ) - mock_image2 = self.image_factory( id="img-456-ghi-jkl", name="centos-8-stream", @@ -69,19 +65,17 @@ def test_get_images_success(self, mock_get_openstack_conn_image): checksum="def456", size=2147483648, ) - mock_conn.image.images.return_value = [mock_image1, mock_image2] result = ImageTools().get_images() + mock_conn.image.images.assert_called_once() expected_output = [ Image(**mock_image1), Image(**mock_image2), ] assert result == expected_output - mock_conn.image.images.assert_called_once() - def test_get_images_empty_list(self, mock_get_openstack_conn_image): """Test getting image images when no images exist.""" mock_conn = mock_get_openstack_conn_image @@ -89,34 +83,15 @@ def test_get_images_empty_list(self, mock_get_openstack_conn_image): result = ImageTools().get_images() - assert result == [] mock_conn.image.images.assert_called_once() + assert result == [] - @pytest.mark.parametrize( - "filter_name,filter_value,expected_count", - [ - ("name", "ubuntu-20.04-server", 1), # exact name match - ("name", "ubuntu", 0), # partial match not supported - ("name", "nonexistent", 0), # non-existent name - ("name", "", 2), # empty filter value - ("name", " ", 2), # whitespace only - ("status", "active", 2), - ("visibility", "public", 2), - ("status", "deleted", 0), - ("visibility", "private", 0), - ], - ) - def test_get_images_with_filters( - self, - mock_get_openstack_conn_image, - filter_name, - filter_value, - expected_count, + def test_get_images_with_status_filter( + self, mock_get_openstack_conn_image ): - """Test getting images with various filters.""" + """Test getting images with status filter.""" mock_conn = mock_get_openstack_conn_image - - mock_image1 = self.image_factory( + mock_image = self.image_factory( id="img-123-abc-def", name="ubuntu-20.04-server", status="active", @@ -124,59 +99,78 @@ def test_get_images_with_filters( checksum="abc123", size=1073741824, ) + mock_conn.image.images.return_value = [mock_image] - mock_image2 = self.image_factory( + result = ImageTools().get_images(status="active") + + mock_conn.image.images.assert_called_once_with(status="active") + expected_output = [Image(**mock_image)] + assert result == expected_output + + def test_get_images_with_visibility_filter( + self, mock_get_openstack_conn_image + ): + """Test getting images with visibility filter.""" + mock_conn = mock_get_openstack_conn_image + mock_image = self.image_factory( id="img-456-ghi-jkl", name="centos-8-stream", - status="active", - visibility="public", + status="queued", + visibility="private", checksum="def456", size=2147483648, ) + mock_conn.image.images.return_value = [mock_image] - if filter_name == "name": - if filter_value == "ubuntu-20.04-server": - mock_conn.image.images.return_value = [mock_image1] - elif filter_value in ["", " "]: - mock_conn.image.images.return_value = [ - mock_image1, - mock_image2, - ] - else: - mock_conn.image.images.return_value = [] - elif filter_name == "status": - if filter_value == "active": - mock_conn.image.images.return_value = [ - mock_image1, - mock_image2, - ] - else: - mock_conn.image.images.return_value = [] - elif filter_name == "visibility": - if filter_value == "public": - mock_conn.image.images.return_value = [ - mock_image1, - mock_image2, - ] - else: - mock_conn.image.images.return_value = [] - - result = ImageTools().get_images(**{filter_name: filter_value}) - - if expected_count == 0: - assert result == [] - elif expected_count == 1: - assert result == [Image(**mock_image1)] - else: - assert result == [Image(**mock_image1), Image(**mock_image2)] - - # For empty/whitespace filters, no filter should be applied - if filter_value in ["", " "]: - mock_conn.image.images.assert_called_once_with() - else: - mock_conn.image.images.assert_called_once_with( - **{filter_name: filter_value} - ) + result = ImageTools().get_images(visibility="private") + + mock_conn.image.images.assert_called_once_with(visibility="private") + expected_output = [Image(**mock_image)] + assert result == expected_output + + def test_get_images_with_name_filter(self, mock_get_openstack_conn_image): + """Test getting images with name filter.""" + mock_conn = mock_get_openstack_conn_image + mock_image = self.image_factory( + id="img-789-mno-pqr", + name="centos-8-stream", + status="active", + visibility="public", + checksum="ghi789", + size=3221225472, + ) + mock_conn.image.images.return_value = [mock_image] + + result = ImageTools().get_images(name="centos-8-stream") + + mock_conn.image.images.assert_called_once_with(name="centos-8-stream") + expected_output = [Image(**mock_image)] + assert result == expected_output + + def test_get_images_with_multiple_filters( + self, mock_get_openstack_conn_image + ): + """Test getting images with multiple filters.""" + mock_conn = mock_get_openstack_conn_image + mock_image = self.image_factory( + id="img-multi-filter", + name="ubuntu-20.04-server", + status="active", + visibility="public", + checksum="multi123", + size=1073741824, + ) + mock_conn.image.images.return_value = [mock_image] + + result = ImageTools().get_images( + name="ubuntu-20.04-server", status="active", visibility="public" + ) + + mock_conn.image.images.assert_called_once_with( + name="ubuntu-20.04-server", status="active", visibility="public" + ) + expected_output = [Image(**mock_image)] + assert result == expected_output def test_create_image_success_with_volume_id( self,