From a94468cf79d803d7d185ff17655d2d10337bc6c9 Mon Sep 17 00:00:00 2001 From: guybartal Date: Sun, 6 Nov 2022 15:31:49 +0000 Subject: [PATCH 1/3] handle unsupported azure subscriptions in cost reporting --- api_app/_version.py | 2 +- api_app/api/routes/costs.py | 11 +++++-- api_app/resources/strings.py | 1 + api_app/services/cost_service.py | 20 +++++++++++-- .../test_services/test_cost_service.py | 30 ++++++++++++++++++- docs/azure-tre-overview/cost-reporting.md | 5 +++- ui/app/src/components/root/RootLayout.tsx | 12 ++++++-- ui/app/src/models/loadingState.ts | 3 +- 8 files changed, 73 insertions(+), 11 deletions(-) diff --git a/api_app/_version.py b/api_app/_version.py index db55ef1921..57f9f92e6f 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.5.10" +__version__ = "0.5.11" diff --git a/api_app/api/routes/costs.py b/api_app/api/routes/costs.py index 2076a8a42d..f8dfb8fa10 100644 --- a/api_app/api/routes/costs.py +++ b/api_app/api/routes/costs.py @@ -15,7 +15,7 @@ from models.domain.costs import CostReport, GranularityEnum, WorkspaceCostReport from resources import strings from services.authentication import get_current_admin_user, get_current_workspace_owner_or_tre_admin -from services.cost_service import CostService, WorkspaceDoesNotExist +from services.cost_service import CostService, SubscriptionNotSupported, WorkspaceDoesNotExist costs_core_router = APIRouter(dependencies=[Depends(get_current_admin_user)]) costs_workspace_router = APIRouter(dependencies=[Depends(get_current_workspace_owner_or_tre_admin)]) @@ -57,8 +57,11 @@ async def costs( shared_services_repo=Depends(get_repository(SharedServiceRepository))) -> CostReport: validate_report_period(params.from_date, params.to_date) - return cost_service.query_tre_costs( - config.TRE_ID, params.granularity, params.from_date, params.to_date, workspace_repo, shared_services_repo) + try: + return cost_service.query_tre_costs( + config.TRE_ID, params.granularity, params.from_date, params.to_date, workspace_repo, shared_services_repo) + except SubscriptionNotSupported: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=strings.API_GET_COSTS_SUBSCRIPTION_NOT_SUPPORTED) @costs_workspace_router.get("/workspaces/{workspace_id}/costs", response_model=WorkspaceCostReport, @@ -78,3 +81,5 @@ async def workspace_costs(workspace_id: UUID4, params: CostsQueryParams = Depend workspace_repo, workspace_services_repo, user_resource_repo) except WorkspaceDoesNotExist: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=strings.WORKSPACE_DOES_NOT_EXIST) + except SubscriptionNotSupported: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=strings.API_GET_COSTS_SUBSCRIPTION_NOT_SUPPORTED) diff --git a/api_app/resources/strings.py b/api_app/resources/strings.py index c93e179945..e169e9608b 100644 --- a/api_app/resources/strings.py +++ b/api_app/resources/strings.py @@ -71,6 +71,7 @@ API_GET_COSTS_MAX_TIME_PERIOD = "The time period for pulling the data cannot exceed 1 year" API_GET_COSTS_TO_DATE_NEED_TO_BE_LATER_THEN_FROM_DATE = "to_date needs to be later than from_date" API_GET_COSTS_FROM_DATE_NEED_TO_BE_BEFORE_TO_DATE = "from_date needs to be before to_date" +API_GET_COSTS_SUBSCRIPTION_NOT_SUPPORTED = "Azure subscription doesn't support cost management" # State store status OK = "OK" diff --git a/api_app/services/cost_service.py b/api_app/services/cost_service.py index 63be230eea..6fb3feb56c 100644 --- a/api_app/services/cost_service.py +++ b/api_app/services/cost_service.py @@ -2,10 +2,12 @@ from enum import Enum from typing import Dict, Optional import pandas as pd +import logging from azure.mgmt.costmanagement import CostManagementClient from azure.mgmt.costmanagement.models import QueryGrouping, QueryAggregation, QueryDataset, QueryDefinition, \ TimeframeType, ExportType, QueryTimePeriod, QueryFilter, QueryComparisonExpression, QueryResult +from azure.core.exceptions import ResourceNotFoundError from azure.mgmt.resource import ResourceManagementClient @@ -39,6 +41,10 @@ class WorkspaceDoesNotExist(Exception): """Raised when the workspace is not found by provided id""" +class SubscriptionNotSupported(Exception): + """Raised when subscription does not support cost management""" + + class CostService: scope: str client: CostManagementClient @@ -52,7 +58,7 @@ class CostService: def __init__(self): self.scope = "/subscriptions/{}".format(config.SUBSCRIPTION_ID) - self.client = CostManagementClient(credential=credentials.get_credential()) + self.client = CostManagementClient(credential=credentials.get_credential(), base_url="https://7b51abd8-dca0-4c5e-9f07-36ff74023532.mock.pstmn.io") self.resource_client = ResourceManagementClient(credentials.get_credential(), config.SUBSCRIPTION_ID) def query_tre_costs(self, tre_id, granularity: GranularityEnum, from_date: datetime, to_date: datetime, @@ -225,7 +231,17 @@ def query_costs(self, tag_name: str, tag_value: str, resource_groups: list) -> QueryResult: query_definition = self.build_query_definition(granularity, from_date, to_date, tag_name, tag_value, resource_groups) - return self.client.query.usage(self.scope, query_definition) + try: + return self.client.query.usage(self.scope, query_definition) + except ResourceNotFoundError as e: + # when cost management API returns 404 with an message: + # Given subscription {subscription_id} doesn't have valid WebDirect/AIRS offer type. + # it means that the Azure subscription deosn't support cost management + if "doesn't have valid WebDirect/AIRS" in e.message: + logging.error("Subscription doesn't support cost mangement", exc_info=e) + raise SubscriptionNotSupported(e) + else: + raise e def build_query_definition(self, granularity: GranularityEnum, from_date: Optional[datetime], to_date: Optional[datetime], tag_name: str, tag_value: str, resource_groups: list): diff --git a/api_app/tests_ma/test_services/test_cost_service.py b/api_app/tests_ma/test_services/test_cost_service.py index c2687651a9..513dabc325 100644 --- a/api_app/tests_ma/test_services/test_cost_service.py +++ b/api_app/tests_ma/test_services/test_cost_service.py @@ -5,9 +5,10 @@ from models.domain.user_resource import UserResource from models.domain.workspace import Workspace from models.domain.workspace_service import WorkspaceService -from services.cost_service import CostService +from services.cost_service import CostService, SubscriptionNotSupported from datetime import date, datetime, timedelta from azure.mgmt.costmanagement.models import QueryResult, TimeframeType, QueryDefinition, QueryColumn +from azure.core.exceptions import ResourceNotFoundError @patch('db.repositories.workspaces.WorkspaceRepository') @@ -187,6 +188,33 @@ def test_query_tre_costs_with_granularity_none_and_missing_costs_data_returns_em assert len(cost_report.workspaces[1].costs) == 0 +@patch('db.repositories.workspaces.WorkspaceRepository') +@patch('db.repositories.shared_services.SharedServiceRepository') +@patch('services.cost_service.CostManagementClient') +@patch('services.cost_service.CostService.get_resource_groups_by_tag') +def test_query_tre_costs_for_unsupported_subscription_raises_subscription_not_supported_exception(get_resource_groups_by_tag_mock, + client_mock, + shared_service_repo_mock, + workspace_repo_mock): + + client_mock.return_value.query.usage.side_effect = ResourceNotFoundError({ + "error": { + "code": "NotFound", + "message": "Given subscription xxx doesn't have valid WebDirect/AIRS offer type. (Request ID: 12daa3b6-8a53-4759-97ba-511ece1ac95b)" + } + }) + + __set_shared_service_repo_mock_return_value(shared_service_repo_mock) + __set_workspace_repo_mock_get_active_workspaces_return_value(workspace_repo_mock) + __set_resource_group_by_tag_return_value(get_resource_groups_by_tag_mock) + + cost_service = CostService() + + with pytest.raises(SubscriptionNotSupported): + cost_service.query_tre_costs( + "guy22", GranularityEnum.none, datetime.now(), datetime.now(), workspace_repo_mock, shared_service_repo_mock) + + @patch('db.repositories.workspaces.WorkspaceRepository') @patch('db.repositories.shared_services.SharedServiceRepository') @patch('services.cost_service.CostManagementClient') diff --git a/docs/azure-tre-overview/cost-reporting.md b/docs/azure-tre-overview/cost-reporting.md index 37a3abb503..21d9beff94 100644 --- a/docs/azure-tre-overview/cost-reporting.md +++ b/docs/azure-tre-overview/cost-reporting.md @@ -115,7 +115,10 @@ GET /api/workspaces/{workspace_id}/costs * Once cost data becomes available in Cost Management, it will be retained for at least seven years. Only the last 13 months is available from the TRE Cost API and Azure Portal. For historical data before 13 months, please use [Exports](https://docs.microsoft.com/en-us/azure/cost-management-billing/costs/tutorial-export-acm-data?tabs=azure-portal) or the [UsageDetails API](https://docs.microsoft.com/en-us/rest/api/consumption/usage-details/list?tabs=HTTP). -* For more information please refer to [Azure Cost Management](https://docs.microsoft.com/en-us/azure/cost-management-billing/costs/understand-cost-mgt-data) and Cost API swagger docs. +* There are several Azure Offers that currently are not supported yet, Azure Offers are types of Azure subscriptions, for full list of supported and unspported Azure Offers please refer to [Supported Microsoft Azure offers](https://learn.microsoft.com/en-us/azure/cost-management-billing/costs/understand-cost-mgt-data#supported-microsoft-azure-offers), +Azure TRE will not display costs for unsupported Azure subscriptions. + +* For more information please refer to [Understand Cost Management data](https://docs.microsoft.com/en-us/azure/cost-management-billing/costs/understand-cost-mgt-data) and Cost API swagger docs. ## Azure Resources Tagging diff --git a/ui/app/src/components/root/RootLayout.tsx b/ui/app/src/components/root/RootLayout.tsx index 6d1a7dfdd2..83c1e49e64 100644 --- a/ui/app/src/components/root/RootLayout.tsx +++ b/ui/app/src/components/root/RootLayout.tsx @@ -16,6 +16,7 @@ import { APIError } from '../../models/exceptions'; import { ExceptionLayout } from '../shared/ExceptionLayout'; import { AppRolesContext } from '../../contexts/AppRolesContext'; import { CostsContext } from '../../contexts/CostsContext'; +import config from "../../config.json"; export const RootLayout: React.FunctionComponent = () => { const [workspaces, setWorkspaces] = useState([] as Array); @@ -59,9 +60,16 @@ export const RootLayout: React.FunctionComponent = () => { setLoadingCostState(LoadingState.Ok); } catch (e:any) { - e.userMessage = 'Error retrieving costs'; + if (e instanceof APIError && e.status === 404) { + config.debug && console.warn(e.message); + setLoadingCostState(LoadingState.NotSupported); + } + else { + e.userMessage = 'Error retrieving costs'; + setLoadingCostState(LoadingState.Error); + } + setCostApiError(e); - setLoadingCostState(LoadingState.Error); } }; diff --git a/ui/app/src/models/loadingState.ts b/ui/app/src/models/loadingState.ts index a09fa7fd64..c5313a84a2 100644 --- a/ui/app/src/models/loadingState.ts +++ b/ui/app/src/models/loadingState.ts @@ -2,5 +2,6 @@ export enum LoadingState { Ok = 'ok', Error = 'error', Loading = 'loading', - AccessDenied = "access-denied" + AccessDenied = "access-denied", + NotSupported = "not-supported" } From 17d5ac737bcc81aec28760d624a2717baeb97cb8 Mon Sep 17 00:00:00 2001 From: guybartal Date: Sun, 6 Nov 2022 15:40:00 +0000 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44444f6051..515dcc391a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ ENHANCEMENTS: BUG FIXES: * Show the correct createdBy value for airlock requests in UI and in API queries ([#2779](https://github.com/microsoft/AzureTRE/pull/2779)) +* handle unsupported azure subscriptions in cost reporting ([#2823](https://github.com/microsoft/AzureTRE/pull/2823)) + COMPONENTS: From 0d89448d85ea262300bd3da1ee89dd362da3151c Mon Sep 17 00:00:00 2001 From: guybartal Date: Sun, 6 Nov 2022 15:43:24 +0000 Subject: [PATCH 3/3] bump api version --- api_app/_version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api_app/_version.py b/api_app/_version.py index 57f9f92e6f..e1e093c042 100644 --- a/api_app/_version.py +++ b/api_app/_version.py @@ -1 +1 @@ -__version__ = "0.5.11" +__version__ = "0.5.12"