From a6377b8d8698057b9eb317ffad03056966c57d2b Mon Sep 17 00:00:00 2001 From: leigh-mil Date: Mon, 10 Feb 2020 11:40:14 -0500 Subject: [PATCH 01/11] Update TO number text to remove references to 13-digits --- translations.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/translations.yaml b/translations.yaml index c16f89c6..b80f5dca 100644 --- a/translations.yaml +++ b/translations.yaml @@ -313,7 +313,7 @@ forms: upload_error: There was an error uploading your file. Please try again. If you encounter repeated problems uploading this file, please contact CCPO. size_error: "The file you have selected is too large. Please choose a file no larger than {file_size_limit}MB." filename_error: File names can only contain the characters A-Z, 0-9, space, hyphen, underscore, and period. - number_description: 13-Digit Task Order Number + number_description: Task Order Number pop_errors: date_order: PoP start date must be before end date. range: Date must be between {start} and {end}. @@ -530,7 +530,7 @@ task_orders: form: add_clin: Add Another CLIN add_to_header: Enter the Task Order number - add_to_description: Please input your 13-digit Task Order number. This number may be listed under "Order Number" if your Contracting Officer used form 1149, or "Delivery Order/Call No." if form 1155 was used. Moving forward, this portion of funding will be referenced by the recorded Task Order number. + add_to_description: Please input your Task Order number. This number may be listed under "Order Number" if your Contracting Officer used form 1149, or "Delivery Order/Call No." if form 1155 was used. Moving forward, this portion of funding will be referenced by the recorded Task Order number. builder_base: cancel_modal: Do you want to save this draft? delete_draft: No, delete it From 5716e20b9e69e3a6e4081968df7060e621339f5a Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 5 Feb 2020 11:06:50 -0500 Subject: [PATCH 02/11] Edit query body to aggregate by month --- atst/domain/csp/cloud/azure_cloud_provider.py | 2 +- atst/domain/csp/cloud/mock_cloud_provider.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 1d921fde..2d00dcf4 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -1060,7 +1060,7 @@ class AzureCloudProvider(CloudProviderInterface): "timeframe": "Custom", "timePeriod": {"from": payload.from_date, "to": payload.to_date,}, "dataset": { - "granularity": "Daily", + "granularity": "Monthly", "aggregation": {"totalCost": {"name": "PreTaxCost", "function": "Sum"}}, "grouping": [{"type": "Dimension", "name": "InvoiceId"}], }, diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 7ec0636f..8213ac70 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -463,7 +463,7 @@ class MockCloudProvider(CloudProviderInterface): **dict( columns=[ {"name": "PreTaxCost", "type": "Number"}, - {"name": "UsageDate", "type": "Number"}, + {"name": "BillingMonth", "type": "Datetime"}, {"name": "InvoiceId", "type": "String"}, {"name": "Currency", "type": "String"}, ], From 411d8a877cf8b22c2068bb1a726689179295a425 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 5 Feb 2020 11:10:51 -0500 Subject: [PATCH 03/11] Add sample reporting data to mock cloud provider --- atst/domain/csp/cloud/mock_cloud_provider.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 8213ac70..da1cccd8 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -1,4 +1,5 @@ from uuid import uuid4 +import pendulum from .cloud_provider_interface import CloudProviderInterface from .exceptions import ( @@ -459,6 +460,11 @@ class MockCloudProvider(CloudProviderInterface): self._maybe_raise(self.UNAUTHORIZED_RATE, self.AUTHORIZATION_EXCEPTION) object_id = str(uuid4()) + start_of_month = pendulum.today(tz="utc").start_of("month").replace(tzinfo=None) + this_month = start_of_month.to_atom_string() + last_month = start_of_month.subtract(months=1).to_atom_string() + two_months_ago = start_of_month.subtract(months=2).to_atom_string() + properties = CostManagementQueryProperties( **dict( columns=[ @@ -467,7 +473,13 @@ class MockCloudProvider(CloudProviderInterface): {"name": "InvoiceId", "type": "String"}, {"name": "Currency", "type": "String"}, ], - rows=[], + rows=[ + [1.0, two_months_ago, "", "USD"], + [500.0, two_months_ago, "e05009w9sf", "USD"], + [50.0, last_month, "", "USD"], + [1000.0, last_month, "e0500a4qhw", "USD"], + [500.0, this_month, "", "USD"], + ], ) ) From 2f1c57aef4554f1a139c151ee6a90795220bc429 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 5 Feb 2020 14:20:02 -0500 Subject: [PATCH 04/11] Add fn to prepare Azure reporting data for views --- atst/domain/csp/reports.py | 24 +++++++++++ tests/domain/cloud/reports/test_reports.py | 47 +++++++++++++++++++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/atst/domain/csp/reports.py b/atst/domain/csp/reports.py index 3f9ccbf8..0e29c5ff 100644 --- a/atst/domain/csp/reports.py +++ b/atst/domain/csp/reports.py @@ -1,6 +1,7 @@ from collections import defaultdict import json from decimal import Decimal +import pendulum def load_fixture_data(): @@ -136,3 +137,26 @@ class MockReportingProvider: CLIN_spend_dict[clin]["invoiced"] += Decimal(spend) return CLIN_spend_dict return {} + + +def prepare_azure_reporting_data(rows: list): + """ + Returns a dict representing invoiced and estimated funds for a portfolio given + a list of rows from CostManagementQueryCSPResult.properties.rows + { + invoiced: Decimal, + estimated: Decimal + } + """ + + estimated = [] + while rows: + if pendulum.parse(rows[-1][1]) >= pendulum.now(tz="utc").start_of("month"): + estimated.append(rows.pop()) + else: + break + + return dict( + invoiced=Decimal(sum([row[0] for row in rows])), + estimated=Decimal(sum([row[0] for row in estimated])), + ) diff --git a/tests/domain/cloud/reports/test_reports.py b/tests/domain/cloud/reports/test_reports.py index 1e74e9b9..4c85a9bc 100644 --- a/tests/domain/cloud/reports/test_reports.py +++ b/tests/domain/cloud/reports/test_reports.py @@ -1,5 +1,7 @@ -from atst.domain.csp.reports import MockReportingProvider +from atst.domain.csp.reports import MockReportingProvider, prepare_azure_reporting_data from tests.factories import PortfolioFactory +from decimal import Decimal +import pendulum def test_get_environment_monthly_totals(): @@ -56,3 +58,46 @@ def test_get_application_monthly_totals(): assert totals["last_month"] == 700 assert totals["total"] == 2500 assert [env["name"] for env in totals["environments"]] == ["A", "Z"] + + +class TestPrepareAzureData: + start_of_month = pendulum.today(tz="utc").start_of("month").replace(tzinfo=None) + next_month = start_of_month.add(months=1).to_atom_string() + this_month = start_of_month.to_atom_string() + last_month = start_of_month.subtract(months=1).to_atom_string() + two_months_ago = last_month = start_of_month.subtract(months=2).to_atom_string() + + def test_estimated_and_invoiced(self): + rows = [ + [150.0, self.two_months_ago, "", "USD"], + [100.0, self.last_month, "e0500a4qhw", "USD"], + [50.0, self.this_month, "", "USD"], + [50.0, self.next_month, "", "USD"], + ] + output = prepare_azure_reporting_data(rows) + + assert output.get("invoiced") == Decimal(250.0) + assert output.get("estimated") == Decimal(100.0) + + def test_just_estimated(self): + rows = [ + [100.0, self.this_month, "", "USD"], + ] + output = prepare_azure_reporting_data(rows) + + assert output.get("invoiced") == Decimal(0.0) + assert output.get("estimated") == Decimal(100.0) + + def test_just_invoiced(self): + rows = [ + [100.0, self.last_month, "", "USD"], + ] + output = prepare_azure_reporting_data(rows) + + assert output.get("invoiced") == Decimal(100.0) + assert output.get("estimated") == Decimal(0.0) + + def test_no_rows(self): + output = prepare_azure_reporting_data([]) + assert output.get("invoiced") == Decimal(0.0) + assert output.get("estimated") == Decimal(0.0) From 55736b723eb221d1d8bfa789d154780d62a27fbf Mon Sep 17 00:00:00 2001 From: graham-dds Date: Wed, 5 Feb 2020 15:04:28 -0500 Subject: [PATCH 05/11] Move calc of a portfolio's obligated funds to prop Add a property on the portfolio model to calculate the total obligated funds for a portfolio. This replaces a one-off calculation in a view function, and sets up functionality for future access --- atst/models/portfolio.py | 6 ++++++ atst/routes/portfolios/index.py | 10 ++-------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/atst/models/portfolio.py b/atst/models/portfolio.py index 5a8f0f1e..2ddcaa41 100644 --- a/atst/models/portfolio.py +++ b/atst/models/portfolio.py @@ -89,6 +89,12 @@ class Portfolio( def active_task_orders(self): return [task_order for task_order in self.task_orders if task_order.is_active] + @property + def total_obligated_funds(self): + return sum( + (task_order.total_obligated_funds for task_order in self.active_task_orders) + ) + @property def funding_duration(self): """ diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index f9e7d5cf..c0fea654 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -40,17 +40,11 @@ def reports(portfolio_id): if any(map(lambda clin: clin["remaining"] < 0, current_obligated_funds)): flash("insufficient_funds") - # wrapped in str() because the sum of obligated funds returns a Decimal object - total_portfolio_value = str( - sum( - task_order.total_obligated_funds - for task_order in portfolio.active_task_orders - ) - ) return render_template( "portfolios/reports/index.html", portfolio=portfolio, - total_portfolio_value=total_portfolio_value, + # wrapped in str() because the sum of obligated funds returns a Decimal object + total_portfolio_value=str(portfolio.total_obligated_funds), current_obligated_funds=current_obligated_funds, expired_task_orders=Reports.expired_task_orders(portfolio), monthly_spending=Reports.monthly_spending(portfolio), From baa3f390d278584f8828dcc94a9d2e9127ded077 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Thu, 6 Feb 2020 14:05:52 -0500 Subject: [PATCH 06/11] Extend graph width filter to handle 0/0 --- atst/filters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atst/filters.py b/atst/filters.py index 3508f1e9..84191017 100644 --- a/atst/filters.py +++ b/atst/filters.py @@ -5,7 +5,7 @@ from flask import render_template from jinja2 import contextfilter from jinja2.exceptions import TemplateNotFound from urllib.parse import urlparse, urlunparse, parse_qs, urlencode -from decimal import DivisionByZero as DivisionByZeroException +from decimal import DivisionByZero as DivisionByZeroException, InvalidOperation def iconSvg(name): @@ -43,7 +43,7 @@ def obligatedFundingGraphWidth(values): numerator, denominator = values try: return (numerator / denominator) * 100 - except DivisionByZeroException: + except (DivisionByZeroException, InvalidOperation): return 0 From 63cb05249f576bf641b38de3260f827cd213708b Mon Sep 17 00:00:00 2001 From: graham-dds Date: Thu, 6 Feb 2020 14:45:05 -0500 Subject: [PATCH 07/11] Add Reports domain method to get portfolio spend --- atst/domain/reports.py | 24 ++++++++++++++++++++++++ tests/domain/test_reports.py | 35 +++++++++++++++++++++++++++++------ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/atst/domain/reports.py b/atst/domain/reports.py index 99b229e3..30996290 100644 --- a/atst/domain/reports.py +++ b/atst/domain/reports.py @@ -1,5 +1,11 @@ from flask import current_app from itertools import groupby +from atst.domain.csp.cloud.models import ( + ReportingCSPPayload, + CostManagementQueryCSPResult, +) +from atst.domain.csp.reports import prepare_azure_reporting_data +import pendulum class Reports: @@ -42,3 +48,21 @@ class Reports: } ) return output + + @classmethod + def get_portfolio_spending(cls, portfolio): + # TODO: Extend this function to make from_date and to_date configurable + from_date = pendulum.now().subtract(years=1).add(days=1).format("YYYY-MM-DD") + to_date = pendulum.now().format("YYYY-MM-DD") + rows = [] + + if portfolio.csp_data: + payload = ReportingCSPPayload( + from_date=from_date, to_date=to_date, **portfolio.csp_data + ) + response: CostManagementQueryCSPResult = current_app.csp.cloud.get_reporting_data( + payload + ) + rows = response.properties.rows + + return prepare_azure_reporting_data(rows) diff --git a/tests/domain/test_reports.py b/tests/domain/test_reports.py index 33ac926e..cdd5de5e 100644 --- a/tests/domain/test_reports.py +++ b/tests/domain/test_reports.py @@ -1,8 +1,31 @@ -# TODO: Implement when we get real reporting data -def test_expired_task_orders(): - pass +import pytest + +from atst.domain.reports import Reports +from tests.factories import PortfolioFactory +from decimal import Decimal -# TODO: Implement when we get real reporting data -def test_obligated_funds_by_JEDI_clin(): - pass +@pytest.fixture(scope="function") +def portfolio(): + portfolio = PortfolioFactory.create() + return portfolio + + +class TestGetPortfolioSpending: + csp_data = { + "tenant_id": "", + "billing_profile_properties": { + "invoice_sections": [{"invoice_section_id": "",}] + }, + } + + def test_with_csp_data(self, portfolio): + portfolio.csp_data = self.csp_data + data = Reports.get_portfolio_spending(portfolio) + assert data["invoiced"] == Decimal(1551.0) + assert data["estimated"] == Decimal(500.0) + + def test_without_csp_data(self, portfolio): + data = Reports.get_portfolio_spending(portfolio) + assert data["invoiced"] == Decimal(0) + assert data["estimated"] == Decimal(0) From 4a78aa07c9d39455a923e8fa3ed9975f60e05113 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Thu, 6 Feb 2020 14:45:44 -0500 Subject: [PATCH 08/11] Remove non-MVP reporting elements - Edits views to only show spending at the portfolio level -- no longer broken out by JEDI CLIN - Removes Monthly Spending table -- keeps the template, just no longer uses it. - Removes amount unspent from the expired funding table --- atst/routes/portfolios/index.py | 12 ++- .../reports/expired_task_orders.html | 8 +- templates/portfolios/reports/index.html | 2 - .../portfolios/reports/obligated_funds.html | 93 +++++++++---------- 4 files changed, 56 insertions(+), 59 deletions(-) diff --git a/atst/routes/portfolios/index.py b/atst/routes/portfolios/index.py index c0fea654..795d4b70 100644 --- a/atst/routes/portfolios/index.py +++ b/atst/routes/portfolios/index.py @@ -34,10 +34,17 @@ def create_portfolio(): @user_can(Permissions.VIEW_PORTFOLIO_REPORTS, message="view portfolio reports") def reports(portfolio_id): portfolio = Portfolios.get(g.current_user, portfolio_id) + spending = Reports.get_portfolio_spending(portfolio) + obligated = portfolio.total_obligated_funds + remaining = obligated - (spending["invoiced"] + spending["estimated"]) - current_obligated_funds = Reports.obligated_funds_by_JEDI_clin(portfolio) + current_obligated_funds = { + **spending, + "obligated": obligated, + "remaining": remaining, + } - if any(map(lambda clin: clin["remaining"] < 0, current_obligated_funds)): + if current_obligated_funds["remaining"] < 0: flash("insufficient_funds") return render_template( @@ -47,6 +54,5 @@ def reports(portfolio_id): total_portfolio_value=str(portfolio.total_obligated_funds), current_obligated_funds=current_obligated_funds, expired_task_orders=Reports.expired_task_orders(portfolio), - monthly_spending=Reports.monthly_spending(portfolio), retrieved=datetime.now(), # mocked datetime of reporting data retrival ) diff --git a/templates/portfolios/reports/expired_task_orders.html b/templates/portfolios/reports/expired_task_orders.html index dcde6683..f55bb57e 100644 --- a/templates/portfolios/reports/expired_task_orders.html +++ b/templates/portfolios/reports/expired_task_orders.html @@ -16,13 +16,12 @@ PoP CLIN Value Amount Obligated - Amount Unspent {% for task_order in expired_task_orders %} - + Task Order {{ task_order.number }} {{ Icon("caret_right", classes="icon--tiny icon--blue" ) }} @@ -39,9 +38,8 @@ - {{ clin.end_date | formattedDate(formatter="%b %d, %Y") }} - {{ clin.total_amount | dollars }} - {{ clin.obligated_amount | dollars }} - {{ 0 | dollars }} + {{ clin.total_amount | dollars }} + {{ clin.obligated_amount | dollars }} {% endfor %} {% endfor %} diff --git a/templates/portfolios/reports/index.html b/templates/portfolios/reports/index.html index 747610d9..51364bf7 100644 --- a/templates/portfolios/reports/index.html +++ b/templates/portfolios/reports/index.html @@ -13,7 +13,5 @@
{% include "portfolios/reports/obligated_funds.html" %} {% include "portfolios/reports/expired_task_orders.html" %} -
- {% include "portfolios/reports/application_and_env_spending.html" %} {% endblock %} diff --git a/templates/portfolios/reports/obligated_funds.html b/templates/portfolios/reports/obligated_funds.html index e0f11792..0b41b16f 100644 --- a/templates/portfolios/reports/obligated_funds.html +++ b/templates/portfolios/reports/obligated_funds.html @@ -7,61 +7,56 @@
- {% for JEDI_clin in current_obligated_funds | sort(attribute='name')%} -
-

- {{ "JEDICLINType.{}".format(JEDI_clin.name) | translate }} -

-

Total obligated amount: {{ JEDI_clin.obligated | dollars }}

-
- {% if JEDI_clin.remaining < 0 %} - - {% else %} - {% set invoiced_width = (JEDI_clin.invoiced, JEDI_clin.obligated) | obligatedFundingGraphWidth %} - {% if invoiced_width %} - - - {% endif %} - - {% set estimated_width = (JEDI_clin.estimated, JEDI_clin.obligated) | obligatedFundingGraphWidth %} - {% if estimated_width %} - - - {% endif %} - +
+

+ Total obligated amount: {{ current_obligated_funds.obligated | dollars }} +

+
+ {% if current_obligated_funds.remaining < 0 %} + + {% else %} + {% set invoiced_width = (current_obligated_funds.invoiced, current_obligated_funds.obligated) | obligatedFundingGraphWidth %} + {% if invoiced_width %} + {% endif %} - + {% set estimated_width = (current_obligated_funds.estimated, current_obligated_funds.obligated) | obligatedFundingGraphWidth %} + {% if estimated_width %} + + + {% endif %} + + + {% endif %} +
+
+
+

+ + Invoiced expended funds: +

+

{{ current_obligated_funds.invoiced | dollars }}

-
-
-

- - Invoiced expended funds: -

-

{{ JEDI_clin.invoiced | dollars }}

-
-
-

- - Estimated expended funds: -

-

{{ JEDI_clin.estimated | dollars }}

-
-
-

- 0 else "insufficient"}}"> - Remaining funds: -

-

{{ JEDI_clin.remaining | dollars }}

-
+
+

+ + Estimated expended funds: +

+

{{ current_obligated_funds.estimated | dollars }}

+
+
+

+ 0 else "insufficient"}}"> + Remaining funds: +

+

{{ current_obligated_funds.remaining | dollars }}

- {% endfor %} +

Active Task Orders From 5b60a54dbc39c738033848b7097f2a8b83d377e0 Mon Sep 17 00:00:00 2001 From: graham-dds Date: Thu, 6 Feb 2020 15:42:08 -0500 Subject: [PATCH 09/11] Remove fixture-based reporting methods These methods probably can be reused to handle real Azure reporting data --- atst/domain/csp/reports.py | 127 --------------------- atst/domain/reports.py | 35 ------ tests/domain/cloud/reports/test_reports.py | 58 +--------- 3 files changed, 1 insertion(+), 219 deletions(-) diff --git a/atst/domain/csp/reports.py b/atst/domain/csp/reports.py index 0e29c5ff..700947f7 100644 --- a/atst/domain/csp/reports.py +++ b/atst/domain/csp/reports.py @@ -1,4 +1,3 @@ -from collections import defaultdict import json from decimal import Decimal import pendulum @@ -12,132 +11,6 @@ def load_fixture_data(): class MockReportingProvider: FIXTURE_SPEND_DATA = load_fixture_data() - @classmethod - def get_portfolio_monthly_spending(cls, portfolio): - """ - returns an array of application and environment spending for the - portfolio. Applications and their nested environments are sorted in - alphabetical order by name. - [ - { - name - this_month - last_month - total - environments [ - { - name - this_month - last_month - total - } - ] - } - ] - """ - - fixture_apps = cls.FIXTURE_SPEND_DATA.get(portfolio.name, {}).get( - "applications", [] - ) - - for application in portfolio.applications: - if application.name not in [app["name"] for app in fixture_apps]: - fixture_apps.append({"name": application.name, "environments": []}) - - return sorted( - [ - cls._get_application_monthly_totals(portfolio, fixture_app) - for fixture_app in fixture_apps - if fixture_app["name"] - in [application.name for application in portfolio.applications] - ], - key=lambda app: app["name"], - ) - - @classmethod - def _get_environment_monthly_totals(cls, environment): - """ - returns a dictionary that represents spending totals for an environment e.g. - { - name - this_month - last_month - total - } - """ - return { - "name": environment["name"], - "this_month": sum(environment["spending"]["this_month"].values()), - "last_month": sum(environment["spending"]["last_month"].values()), - "total": sum(environment["spending"]["total"].values()), - } - - @classmethod - def _get_application_monthly_totals(cls, portfolio, fixture_app): - """ - returns a dictionary that represents spending totals for an application - and its environments e.g. - { - name - this_month - last_month - total - environments: [ - { - name - this_month - last_month - total - } - ] - } - """ - application_envs = [ - env - for env in portfolio.all_environments - if env.application.name == fixture_app["name"] - ] - - environments = [ - cls._get_environment_monthly_totals(env) - for env in fixture_app["environments"] - if env["name"] in [e.name for e in application_envs] - ] - - for env in application_envs: - if env.name not in [env["name"] for env in environments]: - environments.append({"name": env.name}) - - return { - "name": fixture_app["name"], - "this_month": sum(env.get("this_month", 0) for env in environments), - "last_month": sum(env.get("last_month", 0) for env in environments), - "total": sum(env.get("total", 0) for env in environments), - "environments": sorted(environments, key=lambda env: env["name"]), - } - - @classmethod - def get_spending_by_JEDI_clin(cls, portfolio): - """ - returns an dictionary of spending per JEDI CLIN for a portfolio - { - jedi_clin: { - invoiced - estimated - }, - } - """ - if portfolio.name in cls.FIXTURE_SPEND_DATA: - CLIN_spend_dict = defaultdict(lambda: defaultdict(Decimal)) - for application in cls.FIXTURE_SPEND_DATA[portfolio.name]["applications"]: - for environment in application["environments"]: - for clin, spend in environment["spending"]["this_month"].items(): - CLIN_spend_dict[clin]["estimated"] += Decimal(spend) - for clin, spend in environment["spending"]["total"].items(): - CLIN_spend_dict[clin]["invoiced"] += Decimal(spend) - return CLIN_spend_dict - return {} - def prepare_azure_reporting_data(rows: list): """ diff --git a/atst/domain/reports.py b/atst/domain/reports.py index 30996290..fc619649 100644 --- a/atst/domain/reports.py +++ b/atst/domain/reports.py @@ -1,5 +1,4 @@ from flask import current_app -from itertools import groupby from atst.domain.csp.cloud.models import ( ReportingCSPPayload, CostManagementQueryCSPResult, @@ -9,46 +8,12 @@ import pendulum class Reports: - @classmethod - def monthly_spending(cls, portfolio): - return current_app.csp.reports.get_portfolio_monthly_spending(portfolio) - @classmethod def expired_task_orders(cls, portfolio): return [ task_order for task_order in portfolio.task_orders if task_order.is_expired ] - @classmethod - def obligated_funds_by_JEDI_clin(cls, portfolio): - clin_spending = current_app.csp.reports.get_spending_by_JEDI_clin(portfolio) - active_clins = portfolio.active_clins - for jedi_clin, clins in groupby( - active_clins, key=lambda clin: clin.jedi_clin_type - ): - if not clin_spending.get(jedi_clin.name): - clin_spending[jedi_clin.name] = {} - clin_spending[jedi_clin.name]["obligated"] = sum( - clin.obligated_amount for clin in clins - ) - - output = [] - for clin in clin_spending.keys(): - invoiced = clin_spending[clin].get("invoiced", 0) - estimated = clin_spending[clin].get("estimated", 0) - obligated = clin_spending[clin].get("obligated", 0) - remaining = obligated - (invoiced + estimated) - output.append( - { - "name": clin, - "invoiced": invoiced, - "estimated": estimated, - "obligated": obligated, - "remaining": remaining, - } - ) - return output - @classmethod def get_portfolio_spending(cls, portfolio): # TODO: Extend this function to make from_date and to_date configurable diff --git a/tests/domain/cloud/reports/test_reports.py b/tests/domain/cloud/reports/test_reports.py index 4c85a9bc..91079d44 100644 --- a/tests/domain/cloud/reports/test_reports.py +++ b/tests/domain/cloud/reports/test_reports.py @@ -1,65 +1,9 @@ -from atst.domain.csp.reports import MockReportingProvider, prepare_azure_reporting_data +from atst.domain.csp.reports import prepare_azure_reporting_data from tests.factories import PortfolioFactory from decimal import Decimal import pendulum -def test_get_environment_monthly_totals(): - environment = { - "name": "Test Environment", - "spending": { - "this_month": {"JEDI_CLIN_1": 100, "JEDI_CLIN_2": 100}, - "last_month": {"JEDI_CLIN_1": 200, "JEDI_CLIN_2": 200}, - "total": {"JEDI_CLIN_1": 1000, "JEDI_CLIN_2": 1000}, - }, - } - totals = MockReportingProvider._get_environment_monthly_totals(environment) - assert totals == { - "name": "Test Environment", - "this_month": 200, - "last_month": 400, - "total": 2000, - } - - -def test_get_application_monthly_totals(): - portfolio = PortfolioFactory.create( - applications=[ - {"name": "Test Application", "environments": [{"name": "Z"}, {"name": "A"}]} - ], - ) - application = { - "name": "Test Application", - "environments": [ - { - "name": "Z", - "spending": { - "this_month": {"JEDI_CLIN_1": 50, "JEDI_CLIN_2": 50}, - "last_month": {"JEDI_CLIN_1": 150, "JEDI_CLIN_2": 150}, - "total": {"JEDI_CLIN_1": 250, "JEDI_CLIN_2": 250}, - }, - }, - { - "name": "A", - "spending": { - "this_month": {"JEDI_CLIN_1": 100, "JEDI_CLIN_2": 100}, - "last_month": {"JEDI_CLIN_1": 200, "JEDI_CLIN_2": 200}, - "total": {"JEDI_CLIN_1": 1000, "JEDI_CLIN_2": 1000}, - }, - }, - ], - } - - totals = MockReportingProvider._get_application_monthly_totals( - portfolio, application - ) - assert totals["name"] == "Test Application" - assert totals["this_month"] == 300 - assert totals["last_month"] == 700 - assert totals["total"] == 2500 - assert [env["name"] for env in totals["environments"]] == ["A", "Z"] - - class TestPrepareAzureData: start_of_month = pendulum.today(tz="utc").start_of("month").replace(tzinfo=None) next_month = start_of_month.add(months=1).to_atom_string() From e6d5369cb0ca7383a95545f0c2df4d7d2c901c9f Mon Sep 17 00:00:00 2001 From: tomdds Date: Mon, 10 Feb 2020 16:14:42 -0500 Subject: [PATCH 10/11] Ensure credential updates properly merge values. Previously updating the credentials would delete values from the existing crednetials if they weren't also present in the update. This adds a method for merging credentials to the KeyVaultCredentials model and adds tests to both the cloud provider and model. --- atst/domain/csp/cloud/azure_cloud_provider.py | 9 +++----- atst/domain/csp/cloud/models.py | 9 ++++++++ tests/domain/cloud/test_azure_csp.py | 21 +++++++++++++++++++ tests/domain/cloud/test_models.py | 20 ++++++++++++++++++ 4 files changed, 53 insertions(+), 6 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 1d921fde..06d80280 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -1,6 +1,5 @@ import json from secrets import token_urlsafe -from typing import Any, Dict from uuid import uuid4 from atst.utils import sha256_hex @@ -1026,12 +1025,10 @@ class AzureCloudProvider(CloudProviderInterface): def update_tenant_creds(self, tenant_id, secret: KeyVaultCredentials): hashed = sha256_hex(tenant_id) - new_secrets = secret.dict() curr_secrets = self._source_tenant_creds(tenant_id) - updated_secrets: Dict[str, Any] = {**curr_secrets.dict(), **new_secrets} - us = KeyVaultCredentials(**updated_secrets) - self.set_secret(hashed, json.dumps(us.dict())) - return us + updated_secrets = curr_secrets.merge_credentials(secret) + self.set_secret(hashed, json.dumps(updated_secrets.dict())) + return updated_secrets def _source_tenant_creds(self, tenant_id) -> KeyVaultCredentials: hashed = sha256_hex(tenant_id) diff --git a/atst/domain/csp/cloud/models.py b/atst/domain/csp/cloud/models.py index 358f7934..25521bb9 100644 --- a/atst/domain/csp/cloud/models.py +++ b/atst/domain/csp/cloud/models.py @@ -417,6 +417,15 @@ class KeyVaultCredentials(BaseModel): return values + def merge_credentials( + self, new_creds: "KeyVaultCredentials" + ) -> "KeyVaultCredentials": + updated_creds = {k: v for k, v in new_creds.dict().items() if v} + old_creds = self.dict() + old_creds.update(updated_creds) + + return KeyVaultCredentials(**old_creds) + class SubscriptionCreationCSPPayload(BaseCSPPayload): display_name: str diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 3a25f849..c89e6a77 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -25,6 +25,7 @@ from atst.domain.csp.cloud.models import ( CostManagementQueryCSPResult, EnvironmentCSPPayload, EnvironmentCSPResult, + KeyVaultCredentials, PrincipalAdminRoleCSPPayload, PrincipalAdminRoleCSPResult, ProductPurchaseCSPPayload, @@ -938,3 +939,23 @@ def test_create_user(mock_azure: AzureCloudProvider): result = mock_azure.create_user(payload) assert result.id == "id" + + +def test_update_tenant_creds(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, "set_secret", wraps=mock_azure.set_secret, + ) as set_secret: + set_secret.return_value = None + existing_secrets = { + "tenant_id": "mytenant", + "tenant_admin_username": "admin", + "tenant_admin_password": "foo", # pragma: allowlist secret + } + mock_azure = mock_get_secret(mock_azure, json.dumps(existing_secrets)) + + mock_new_secrets = KeyVaultCredentials(**MOCK_CREDS) + updated_secret = mock_azure.update_tenant_creds("mytenant", mock_new_secrets) + + assert updated_secret == KeyVaultCredentials( + **{**existing_secrets, **MOCK_CREDS} + ) diff --git a/tests/domain/cloud/test_models.py b/tests/domain/cloud/test_models.py index 10c81293..29bc60cb 100644 --- a/tests/domain/cloud/test_models.py +++ b/tests/domain/cloud/test_models.py @@ -100,6 +100,26 @@ def test_KeyVaultCredentials_enforce_root_creds(): ) +def test_KeyVaultCredentials_merge_credentials(): + old_secret = KeyVaultCredentials( + tenant_id="foo", + tenant_admin_username="bar", + tenant_admin_password="baz", # pragma: allowlist secret + ) + new_secret = KeyVaultCredentials( + tenant_id="foo", tenant_sp_client_id="bip", tenant_sp_key="bop" + ) + + expected_update = KeyVaultCredentials( + tenant_id="foo", + tenant_admin_username="bar", + tenant_admin_password="baz", # pragma: allowlist secret + tenant_sp_client_id="bip", + tenant_sp_key="bop", + ) + assert old_secret.merge_credentials(new_secret) == expected_update + + user_payload = { "tenant_id": "123", "display_name": "Han Solo", From f975249f07e1efd2c98d773f1ca296c97938e4f4 Mon Sep 17 00:00:00 2001 From: dandds Date: Mon, 10 Feb 2020 16:58:07 -0500 Subject: [PATCH 11/11] Set Redis verification mode for TLS connections. If the app is making a TLS connection to Redis, the new config setting REDIS_SSLMODE determines whether CA verification should be performed. Acceptable values are Python `None` or strings "none", "optional", and "required". --- .secrets.baseline | 4 ++-- atst/app.py | 8 +++++++- config/base.ini | 1 + tests/test_app.py | 16 ++++++++++++++++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index a233e4cf..f7145df8 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -3,7 +3,7 @@ "files": "^.secrets.baseline$|^.*pgsslrootcert.yml$", "lines": null }, - "generated_at": "2020-01-27T19:24:43Z", + "generated_at": "2020-02-10T21:40:38Z", "plugins_used": [ { "base64_limit": 4.5, @@ -82,7 +82,7 @@ "hashed_secret": "afc848c316af1a89d49826c5ae9d00ed769415f3", "is_secret": false, "is_verified": false, - "line_number": 32, + "line_number": 33, "type": "Secret Keyword" } ], diff --git a/atst/app.py b/atst/app.py index 05578827..db6a09c7 100644 --- a/atst/app.py +++ b/atst/app.py @@ -233,12 +233,18 @@ def make_config(direct_config=None): config.set("default", "DATABASE_URI", database_uri) # Assemble REDIS_URI value + redis_use_tls = config["default"].getboolean("REDIS_TLS") redis_uri = "redis{}://{}:{}@{}".format( # pragma: allowlist secret - ("s" if config["default"].getboolean("REDIS_TLS") else ""), + ("s" if redis_use_tls else ""), (config.get("default", "REDIS_USER") or ""), (config.get("default", "REDIS_PASSWORD") or ""), config.get("default", "REDIS_HOST"), ) + if redis_use_tls: + tls_mode = config.get("default", "REDIS_SSLMODE") + tls_mode_str = tls_mode.lower() if tls_mode else "none" + redis_uri = f"{redis_uri}/?ssl_cert_reqs={tls_mode_str}" + config.set("default", "REDIS_URI", redis_uri) return map_config(config) diff --git a/config/base.ini b/config/base.ini index 1f4c732a..55482741 100644 --- a/config/base.ini +++ b/config/base.ini @@ -38,6 +38,7 @@ PGUSER = postgres PORT=8000 REDIS_HOST=localhost:6379 REDIS_PASSWORD +REDIS_SSLMODE REDIS_TLS=False REDIS_USER SECRET_KEY = change_me_into_something_secret diff --git a/tests/test_app.py b/tests/test_app.py index 937a15e2..21fd8284 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -7,6 +7,7 @@ from atst.app import ( make_crl_validator, apply_config_from_directory, apply_config_from_environment, + make_config, ) @@ -67,3 +68,18 @@ def test_apply_config_from_environment_skips_unknown_settings( monkeypatch.setenv("FLARF", "MAYO") apply_config_from_environment(config_object) assert "FLARF" not in config_object.options("default") + + +class TestMakeConfig: + def test_redis_ssl_connection(self): + config = make_config({"REDIS_TLS": True}) + uri = config.get("REDIS_URI") + assert "rediss" in uri + assert "ssl_cert_reqs" in uri + + def test_non_redis_ssl_connection(self): + config = make_config({"REDIS_TLS": False}) + uri = config.get("REDIS_URI") + assert "rediss" not in uri + assert "redis" in uri + assert "ssl_cert_reqs" not in uri