Skip to content

Commit 3dea6e0

Browse files
fix(billing-cost-management-mcp-server): send filter for getTagsOrValues and getCostCategories
cost-explorer accepted `filter` for getTagsOrValues and getCostCategories but never sent it, so a filtered call returned the unfiltered list as a success. Both APIs take Filter and apply it: the lists keep only the keys, names or values found on matching costs. - Forward filter to GetTags and GetCostCategories (keys and values). - Describe filter for both operations and their limit: an And of leaves only; Or, Not and a nested And are rejected by the service. - Extend the Tags leaves section and the response hint for a user: key or a group_by label to both operations: they now apply the filter, and there too a user: key with Values matches nothing (with ABSENT, every cost). Both also reject an empty Tags key, like getDimensionValues. - Treat filter '""' (or a blank JSON string) as empty, like ''. - cost-comparison: call a blank filter empty, before creating the client. - group_by '[]' means no grouping on operations without GroupBy, as it does in getCostAndUsage. A group_by that is not JSON is still rejected there as not supported.
1 parent c25c287 commit 3dea6e0

8 files changed

Lines changed: 224 additions & 50 deletions

File tree

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_comparison_tools.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ async def cost_comparison(
405405
metric_for_comparison,
406406
today,
407407
)
408-
validate_comparison_filter(parse_json(filter, 'filter'))
408+
validate_comparison_filter(parse_filter(filter))
409409

410410
# Initialize Cost Explorer client using shared utility
411411
ce_client = create_aws_client('ce', region_name='us-east-1')

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_explorer_operations.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,7 @@ async def get_tags(
660660
end_date: Optional[str] = None,
661661
search_string: Optional[str] = None,
662662
tag_key: Optional[str] = None,
663+
filter_expr: Optional[Union[str, Dict[str, Any]]] = None,
663664
next_token: Optional[str] = None,
664665
max_items: Optional[int] = None,
665666
billing_view_arn: Optional[str] = None,
@@ -673,6 +674,7 @@ async def get_tags(
673674
end_date: End date in YYYY-MM-DD format (exclusive)
674675
search_string: Optional string to filter results
675676
tag_key: Optional specific tag key to get values for
677+
filter_expr: Optional filters, as an object or JSON string
676678
next_token: Continuation token from a previous response
677679
max_items: Most items to return; None fetches every page
678680
billing_view_arn: Optional ARN of a billing view to scope the query.
@@ -699,6 +701,10 @@ async def get_tags(
699701
if tag_key:
700702
request_params['TagKey'] = str(tag_key)
701703

704+
filters = parse_filter(filter_expr)
705+
if filters is not None:
706+
request_params['Filter'] = filters
707+
702708
if billing_view_arn:
703709
request_params['BillingViewArn'] = billing_view_arn
704710

@@ -731,6 +737,7 @@ async def get_cost_categories(
731737
end_date: Optional[str] = None,
732738
search_string: Optional[str] = None,
733739
cost_category_name: Optional[str] = None,
740+
filter_expr: Optional[Union[str, Dict[str, Any]]] = None,
734741
next_token: Optional[str] = None,
735742
max_items: Optional[int] = None,
736743
billing_view_arn: Optional[str] = None,
@@ -744,6 +751,7 @@ async def get_cost_categories(
744751
end_date: End date in YYYY-MM-DD format (exclusive)
745752
search_string: Optional string to filter results
746753
cost_category_name: Optional specific cost category to get values for
754+
filter_expr: Optional filters, as an object or JSON string
747755
next_token: Continuation token from a previous response
748756
max_items: Most items to return; None fetches every page
749757
billing_view_arn: Optional ARN of a billing view to scope the query.
@@ -773,6 +781,10 @@ async def get_cost_categories(
773781
if cost_category_name:
774782
request_params['CostCategoryName'] = str(cost_category_name)
775783

784+
filters = parse_filter(filter_expr)
785+
if filters is not None:
786+
request_params['Filter'] = filters
787+
776788
if billing_view_arn:
777789
request_params['BillingViewArn'] = billing_view_arn
778790

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/tools/cost_explorer_tools.py‎

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@
1818
"""
1919

2020
import json
21-
from ..utilities.aws_service_base import create_aws_client, format_response, handle_aws_error
21+
from ..utilities.aws_service_base import (
22+
create_aws_client,
23+
format_response,
24+
handle_aws_error,
25+
parse_json,
26+
)
2227
from ..utilities.constants import PAGINATION_LIMIT_TEXT
2328
from .cost_explorer_operations import (
2429
get_cost_and_usage,
@@ -183,7 +188,8 @@ def _with_example_dates(description: str) -> str:
183188
where the tag or cost category key is not present, so there is no value to compare against:
184189
omit `Values` entirely (an empty `Values` list is rejected too).
185190
186-
Tags leaves in getCostAndUsage, getCostAndUsageWithResources and getDimensionValues filters:
191+
Tags leaves in getCostAndUsage, getCostAndUsageWithResources, getDimensionValues,
192+
getTagsOrValues and getCostCategories filters:
187193
- `Key` is the tag key as getTagsOrValues lists it, with no `user:` prefix even where billing
188194
exports show one: `Environment`, not `user:Environment` (which matches nothing, with no
189195
error). The forecasts accept either form. Tags that AWS applies itself keep
@@ -196,10 +202,11 @@ def _with_example_dates(description: str) -> str:
196202
- getCostAndUsage only: `{"Tags": {"Key": "", "Values": []}}` (empty key, empty list) matches
197203
costs that carry no activated cost allocation tag (user-defined or AWS-generated), as does
198204
`{"Tags": {"MatchOptions": ["ABSENT"]}}` with no Key. An empty key with any value, or with
199-
Values omitted, is rejected, and getDimensionValues and getCostAndUsageWithResources reject
200-
an empty key.
201-
- A wrong key is not an error. With `Values` it matches nothing ($0); with `ABSENT` it matches
202-
every cost, since no cost carries that key. Copy the key from getTagsOrValues.
205+
Values omitted, is rejected, and getCostAndUsageWithResources, getDimensionValues,
206+
getTagsOrValues and getCostCategories reject an empty key.
207+
- A wrong key is not an error. With `Values` it matches nothing ($0, or an empty list); with
208+
`ABSENT` it matches every cost, since no cost carries that key. Copy the key from
209+
getTagsOrValues.
203210
204211
Correct filter examples:
205212
- `{"Dimensions": {"Key": "SERVICE", "Values": ["Amazon Relational Database Service"]}}`
@@ -211,7 +218,8 @@ def _with_example_dates(description: str) -> str:
211218
- Every operation: at most 6,000 values in total, counted across all leaves of the filter.
212219
- getCostAndUsage, getCostAndUsageWithResources: `And`, `Or` and `Not` nest up to 4 levels
213220
deep; a 5th is rejected.
214-
- getDimensionValues (default COST_AND_USAGE context): `And` only; `Or` and `Not` are rejected.
221+
- getDimensionValues (default COST_AND_USAGE context), getTagsOrValues and getCostCategories:
222+
`And` of leaves only (an `And` inside it is rejected); `Or` and `Not` are rejected.
215223
- getSavingsPlansUtilization: `And` and `Not` only; `Or` is rejected.
216224
217225
## OPERATIONS
@@ -260,19 +268,19 @@ def _with_example_dates(description: str) -> str:
260268
261269
6. getTagsOrValues - Available cost allocation tags or values
262270
Required: operation="getTagsOrValues"
263-
Optional: start_date, end_date, tag_key, search_string, max_items, next_token, billing_view_arn
271+
Optional: start_date, end_date, tag_key, search_string, filter, max_items, next_token, billing_view_arn
264272
Defaults: If start_date is omitted, the tool uses 30 days before today; if end_date is omitted, it uses today (exclusive, so today is not included).
265273
Example 1: {"operation": "getTagsOrValues"}
266274
Example 2: {"operation": "getTagsOrValues", "tag_key": "Environment"}
267-
Returns: List of available cost allocation tags (see PAGINATION). If tag values for a particular key are needed, pass the tag key as a parameter. The tag key list holds at most 5,000 keys in total.
275+
Returns: List of available cost allocation tags (see PAGINATION). If tag values for a particular key are needed, pass the tag key as a parameter. The tag key list holds at most 5,000 keys in total. A filter keeps only the keys (or values) found on costs that match it, e.g. the tags used in one Region.
268276
269277
7. getCostCategories - Available cost categories or values inside a category
270278
Required: operation="getCostCategories"
271-
Optional: start_date, end_date, cost_category_name, search_string, max_items, next_token, billing_view_arn
279+
Optional: start_date, end_date, cost_category_name, search_string, filter, max_items, next_token, billing_view_arn
272280
Defaults: If start_date is omitted, the tool uses 30 days before today; if end_date is omitted, it uses today (exclusive, so today is not included).
273281
Example 1 (list category names): {"operation": "getCostCategories", "start_date": "__SIX_MONTHS_AGO_START__", "end_date": "__MONTH_START__"}
274282
Example 2 (list values inside a category): {"operation": "getCostCategories", "cost_category_name": "CostCenters", "start_date": "__SIX_MONTHS_AGO_START__", "end_date": "__MONTH_START__"}
275-
Notes: Without cost_category_name the response contains CostCategoryNames. With cost_category_name set the response contains CostCategoryValues for that category. The category name list holds at most 5,000 names in total. See PAGINATION.
283+
Notes: Without cost_category_name the response contains CostCategoryNames. With cost_category_name set the response contains CostCategoryValues for that category. The category name list holds at most 5,000 names in total. A filter keeps only the names (or values) found on costs that match it. See PAGINATION.
276284
277285
8. getSavingsPlansUtilization - Savings Plans utilization data
278286
Required: operation="getSavingsPlansUtilization"
@@ -423,10 +431,10 @@ def _tag_leaf_hints(key: str, values: List[Any], absent: bool) -> List[str]:
423431
def _tag_filter_hint(filter: Optional[Union[str, Dict[str, Any]]]) -> Optional[str]:
424432
"""Return a hint for a Tags filter that Cost Explorer accepts but that cannot match.
425433
426-
getCostAndUsage, getCostAndUsageWithResources and getDimensionValues add the `user:`
427-
prefix to user-defined tag keys themselves, so a `user:` key matches no tagged cost, and a TAG group_by label
428-
(`key$value`) is not a key or value. With no error, Values then match nothing and
429-
ABSENT matches every cost.
434+
getCostAndUsage, getCostAndUsageWithResources, getDimensionValues, getTagsOrValues and
435+
getCostCategories add the `user:` prefix to user-defined tag keys themselves, so a
436+
`user:` key matches no tagged cost, and a TAG group_by label (`key$value`) is not a key
437+
or value. With no error, Values then match nothing and ABSENT matches every cost.
430438
"""
431439
try:
432440
if isinstance(filter, str):
@@ -447,6 +455,19 @@ def _tag_filter_hint(filter: Optional[Union[str, Dict[str, Any]]]) -> Optional[s
447455
return ' '.join(hints) or None
448456

449457

458+
def _asks_for_grouping(group_by: Any) -> bool:
459+
"""Return whether group_by asks for grouping.
460+
461+
An empty or falsy JSON value ('[]', 'null', '{}') does not, as getCostAndUsage reads
462+
it. A group_by that is not JSON still does, so it gets the not-supported error rather
463+
than a JSON one.
464+
"""
465+
try:
466+
return bool(parse_json(group_by, 'group_by'))
467+
except ValueError:
468+
return True
469+
470+
450471
def _with_hint(result: Dict[str, Any], hint: Optional[str]) -> Dict[str, Any]:
451472
"""Add the hint to a successful response.
452473
@@ -543,7 +564,7 @@ async def cost_explorer(
543564
tag_hint = _tag_filter_hint(filter)
544565
await ctx.info(f'Routing to operation: {operation}')
545566

546-
if group_by and operation in OPERATIONS_WITHOUT_GROUP_BY:
567+
if operation in OPERATIONS_WITHOUT_GROUP_BY and _asks_for_grouping(group_by):
547568
return format_response(
548569
'error',
549570
{
@@ -722,32 +743,40 @@ async def cost_explorer(
722743
)
723744

724745
elif operation == 'getTagsOrValues':
725-
return await get_tags(
726-
ctx,
727-
ce_client,
728-
start_date,
729-
end_date,
730-
search_string,
731-
tag_key,
732-
next_token,
733-
max_items,
734-
billing_view_arn,
746+
return _with_hint(
747+
await get_tags(
748+
ctx,
749+
ce_client,
750+
start_date,
751+
end_date,
752+
search_string,
753+
tag_key,
754+
filter,
755+
next_token,
756+
max_items,
757+
billing_view_arn,
758+
),
759+
tag_hint,
735760
)
736761

737762
elif operation == 'getCostCategories':
738763
# When `cost_category_name` is provided, the same AWS API
739764
# (GetCostCategories) returns the values inside that category
740765
# instead of the list of category names.
741-
return await get_cost_categories(
742-
ctx,
743-
ce_client,
744-
start_date,
745-
end_date,
746-
search_string,
747-
cost_category_name,
748-
next_token,
749-
max_items,
750-
billing_view_arn,
766+
return _with_hint(
767+
await get_cost_categories(
768+
ctx,
769+
ce_client,
770+
start_date,
771+
end_date,
772+
search_string,
773+
cost_category_name,
774+
filter,
775+
next_token,
776+
max_items,
777+
billing_view_arn,
778+
),
779+
tag_hint,
751780
)
752781

753782
elif operation == 'getSavingsPlansUtilization':

‎src/billing-cost-management-mcp-server/awslabs/billing_cost_management_mcp_server/utilities/aws_service_base.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,9 @@ def parse_filter(
233233
) -> Any:
234234
"""Parse a filter expression, rejecting an empty one.
235235
236-
An empty filter (``''``, ``{}``, ``[]``, ``null``) is an error rather than "no filter":
237-
dropping it would run the query unfiltered, and the result would read as a filtered one.
236+
An empty filter (``''``, ``""`` or another blank JSON string, ``{}``, ``[]``, ``null``)
237+
is an error rather than "no filter": dropping it would run the query unfiltered, and the
238+
result would read as a filtered one.
238239
239240
Args:
240241
filter_expr: The filter, as an object or a JSON string. None means no filter.
@@ -252,7 +253,7 @@ def parse_filter(
252253
if isinstance(filter_expr, str):
253254
filter_expr = filter_expr.strip()
254255
parsed = parse_json(filter_expr, parameter_name)
255-
if parsed is None or parsed == {} or parsed == []:
256+
if parsed in (None, {}, []) or (isinstance(parsed, str) and not parsed.strip()):
256257
raise ValueError(
257258
f'{parameter_name} is empty. Omit {parameter_name} to query without one, or pass '
258259
'a filter expression.'

‎src/billing-cost-management-mcp-server/tests/tools/test_cost_comparison_tools.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,6 +1528,19 @@ async def test_invalid_filter_json_is_a_validation_error(
15281528
)
15291529
_assert_local_validation_error(result, dispatch_ce_client, 'Invalid JSON format')
15301530

1531+
@pytest.mark.parametrize('flt', [' ', '{}', '""'], ids=repr)
1532+
async def test_empty_filter_is_called_empty(self, mock_context, dispatch_ce_client, flt):
1533+
"""A blank or empty filter gets the "filter is empty" error, not a JSON error."""
1534+
result = await _call(
1535+
mock_context,
1536+
dispatch_ce_client,
1537+
'getCostAndUsageComparisons',
1538+
month(2),
1539+
month(1),
1540+
filter=flt,
1541+
)
1542+
_assert_local_validation_error(result, dispatch_ce_client, 'filter is empty')
1543+
15311544

15321545
class TestValidateComparisonRequest:
15331546
"""Boundaries of the rules, at a fixed date."""

‎src/billing-cost-management-mcp-server/tests/tools/test_cost_explorer_operations.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
from unittest.mock import AsyncMock, MagicMock, patch
3232

3333

34+
REGION_FILTER = {'Dimensions': {'Key': 'REGION', 'Values': ['us-east-1']}}
35+
36+
3437
@pytest.fixture
3538
def mock_context():
3639
"""Create a mock MCP context."""
@@ -1559,6 +1562,34 @@ async def test_with_search_string(self, mock_context, mock_ce_client):
15591562
assert 'SearchString' in call_kwargs
15601563
assert call_kwargs['SearchString'] == 'Env'
15611564

1565+
@pytest.mark.parametrize('filter_expr', [REGION_FILTER, json.dumps(REGION_FILTER)])
1566+
@pytest.mark.parametrize('tag_key', [None, 'Environment'], ids=['keys', 'values'])
1567+
async def test_filter_is_sent(self, mock_context, mock_ce_client, filter_expr, tag_key):
1568+
"""The filter reaches GetTags as Filter, for tag keys and for one key's values.
1569+
1570+
Dropping it would return the unfiltered list as if it were filtered.
1571+
"""
1572+
result = await get_tags(
1573+
mock_context, mock_ce_client, tag_key=tag_key, filter_expr=filter_expr
1574+
)
1575+
1576+
assert result['status'] == 'success'
1577+
assert mock_ce_client.get_tags.call_args[1]['Filter'] == REGION_FILTER
1578+
1579+
async def test_no_filter_sends_none(self, mock_context, mock_ce_client):
1580+
"""Without a filter, no Filter key is sent."""
1581+
await get_tags(mock_context, mock_ce_client)
1582+
1583+
assert 'Filter' not in mock_ce_client.get_tags.call_args[1]
1584+
1585+
async def test_empty_filter_is_rejected_before_aws(self, mock_context, mock_ce_client):
1586+
"""An empty filter is an error, not an unfiltered call."""
1587+
result = await get_tags(mock_context, mock_ce_client, filter_expr='{}')
1588+
1589+
assert result['status'] == 'error'
1590+
assert 'filter is empty' in result['message']
1591+
mock_ce_client.get_tags.assert_not_called()
1592+
15621593
async def test_error_handling(self, mock_context, mock_ce_client):
15631594
"""Test error handling in get_tags."""
15641595
# Setup error
@@ -1672,6 +1703,34 @@ async def test_with_search_string(self, mock_context, mock_ce_client):
16721703
assert 'SearchString' in call_kwargs
16731704
assert call_kwargs['SearchString'] == 'Dep'
16741705

1706+
@pytest.mark.parametrize('filter_expr', [REGION_FILTER, json.dumps(REGION_FILTER)])
1707+
@pytest.mark.parametrize('name', [None, 'Department'], ids=['names', 'values'])
1708+
async def test_filter_is_sent(self, mock_context, mock_ce_client, filter_expr, name):
1709+
"""The filter reaches GetCostCategories as Filter, for names and for one category's values.
1710+
1711+
Dropping it would return the unfiltered list as if it were filtered.
1712+
"""
1713+
result = await get_cost_categories(
1714+
mock_context, mock_ce_client, cost_category_name=name, filter_expr=filter_expr
1715+
)
1716+
1717+
assert result['status'] == 'success'
1718+
assert mock_ce_client.get_cost_categories.call_args[1]['Filter'] == REGION_FILTER
1719+
1720+
async def test_no_filter_sends_none(self, mock_context, mock_ce_client):
1721+
"""Without a filter, no Filter key is sent."""
1722+
await get_cost_categories(mock_context, mock_ce_client)
1723+
1724+
assert 'Filter' not in mock_ce_client.get_cost_categories.call_args[1]
1725+
1726+
async def test_empty_filter_is_rejected_before_aws(self, mock_context, mock_ce_client):
1727+
"""An empty filter is an error, not an unfiltered call."""
1728+
result = await get_cost_categories(mock_context, mock_ce_client, filter_expr='{}')
1729+
1730+
assert result['status'] == 'error'
1731+
assert 'filter is empty' in result['message']
1732+
mock_ce_client.get_cost_categories.assert_not_called()
1733+
16751734
async def test_error_handling(self, mock_context, mock_ce_client):
16761735
"""Test error handling in get_cost_categories."""
16771736
# Setup error

0 commit comments

Comments
 (0)