diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index cc942f2d..c0bd8e7b 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -111,9 +111,7 @@ class EnvironmentRoles(object): environment_role = EnvironmentRoles.get_by_id(environment_role_id) if environment_role.cloud_id and not environment_role.environment.cloud_id: - tenant_id = environment_role.environment.application.portfolio.csp_data.get( - "tenant_id" - ) + tenant_id = environment_role.environment.portfolio.csp_data.get("tenant_id") app.csp.cloud.disable_user(tenant_id, environment_role.csp_user_id) environment_role.status = EnvironmentRole.Status.DISABLED diff --git a/atst/jobs.py b/atst/jobs.py index b3e5643b..8be4f7e4 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -135,7 +135,7 @@ def do_create_environment(csp: CloudProviderInterface, environment_id=None): if environment.cloud_id is not None: return - csp_details = environment.application.portfolio.csp_data + csp_details = environment.portfolio.csp_data parent_id = environment.application.cloud_id tenant_id = csp_details.get("tenant_id") payload = EnvironmentCSPPayload( @@ -156,7 +156,7 @@ def do_create_environment_role(csp: CloudProviderInterface, environment_role_id= return env = env_role.environment - csp_details = env.application.portfolio.csp_data + csp_details = env.portfolio.csp_data app_role = env_role.application_role role = None diff --git a/atst/models/environment.py b/atst/models/environment.py index 6eb0c02d..cc2d235f 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -70,7 +70,7 @@ class Environment( self.name, self.num_users, self.application.name, - self.application.portfolio.name, + self.portfolio.name, self.id, ) diff --git a/atst/models/environment_role.py b/atst/models/environment_role.py index 871f39a1..61338286 100644 --- a/atst/models/environment_role.py +++ b/atst/models/environment_role.py @@ -58,7 +58,7 @@ class EnvironmentRole( @property def portfolio_id(self): - return self.environment.application.portfolio_id + return self.environment.portfolio_id @property def application_id(self): @@ -86,8 +86,8 @@ class EnvironmentRole( "environment_id": str(self.environment_id), "application": self.environment.application.name, "application_id": str(self.environment.application_id), - "portfolio": self.environment.application.portfolio.name, - "portfolio_id": str(self.environment.application.portfolio.id), + "portfolio": self.environment.portfolio.name, + "portfolio_id": str(self.environment.portfolio.id), } diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 8807c7f1..524fa93a 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -6,6 +6,7 @@ from flask import ( request as http_request, url_for, ) +from secrets import token_urlsafe from .blueprint import applications_bp from atst.domain.exceptions import AlreadyExistsError @@ -529,13 +530,15 @@ def resend_invite(application_id, application_role_id): def build_subscription_payload(environment) -> SubscriptionCreationCSPPayload: - csp_data = environment.application.portfolio.csp_data + csp_data = environment.portfolio.csp_data parent_group_id = environment.cloud_id invoice_section_name = csp_data["billing_profile_properties"]["invoice_sections"][ 0 ]["invoice_section_name"] - display_name = f"{environment.application.name}-{environment.name}" + display_name = ( + f"{environment.application.name}-{environment.name}-{token_urlsafe(6)}" + ) return SubscriptionCreationCSPPayload( tenant_id=csp_data.get("tenant_id"), diff --git a/tests/domain/cloud/test_mock_csp.py b/tests/domain/cloud/test_mock_csp.py index 098e91b1..50320da3 100644 --- a/tests/domain/cloud/test_mock_csp.py +++ b/tests/domain/cloud/test_mock_csp.py @@ -23,10 +23,10 @@ def mock_csp(): def test_create_environment(mock_csp: MockCloudProvider): environment = EnvironmentFactory.create() environment.application.cloud_id = "parent_id" - environment.application.portfolio.csp_data = {"tenant_id": "fake"} + environment.portfolio.csp_data = {"tenant_id": "fake"} payload = EnvironmentCSPPayload( **dict( - tenant_id=environment.application.portfolio.csp_data.get("tenant_id"), + tenant_id=environment.portfolio.csp_data.get("tenant_id"), display_name=environment.name, parent_id=environment.application.cloud_id, ) diff --git a/tests/domain/test_environment_roles.py b/tests/domain/test_environment_roles.py index b4e40b66..1fa4dc28 100644 --- a/tests/domain/test_environment_roles.py +++ b/tests/domain/test_environment_roles.py @@ -111,7 +111,7 @@ def test_disable_checks_env_provisioning_status(session): def test_disable_checks_env_role_provisioning_status(): environment = EnvironmentFactory.create(cloud_id="cloud-id") - environment.application.portfolio.csp_data = {"tenant_id": uuid4().hex} + environment.portfolio.csp_data = {"tenant_id": uuid4().hex} env_role1 = EnvironmentRoleFactory.create(environment=environment) assert not env_role1.cloud_id env_role1 = EnvironmentRoles.disable(env_role1.id) diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index 415dab4c..63d21d21 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -15,7 +15,10 @@ from atst.domain.applications import Applications from atst.domain.common import Paginator from atst.domain.csp.cloud.azure_cloud_provider import AzureCloudProvider from atst.domain.csp.cloud.exceptions import GeneralCSPException -from atst.domain.csp.cloud.models import SubscriptionCreationCSPResult +from atst.domain.csp.cloud.models import ( + SubscriptionCreationCSPResult, + SubscriptionCreationCSPPayload, +) from atst.domain.environment_roles import EnvironmentRoles from atst.domain.invitations import ApplicationInvitations from atst.domain.permission_sets import PermissionSets @@ -26,6 +29,7 @@ from atst.models.application_role import Status as ApplicationRoleStatus from atst.models.environment_role import CSPRole, EnvironmentRole from atst.models.permissions import Permissions from atst.routes.applications.settings import ( + build_subscription_payload, filter_env_roles_data, filter_env_roles_form_data, get_environments_obj_for_app, @@ -786,10 +790,9 @@ def test_create_subscription_success( client, user_session, mock_azure: AzureCloudProvider ): environment = EnvironmentFactory.create() - user_session(environment.portfolio.owner) environment.cloud_id = "management/group/id" - environment.application.portfolio.csp_data = { + environment.portfolio.csp_data = { "billing_account_name": "xxxx-xxxx-xxx-xxx", "billing_profile_name": "xxxxxxxxxxx:xxxxxxxxxxxxx_xxxxxx", "tenant_id": "xxxxxxxxxxx-xxxxxxxxxx-xxxxxxx-xxxxx", @@ -832,7 +835,7 @@ def test_create_subscription_failure(client, user_session, monkeypatch): user_session(environment.portfolio.owner) environment.cloud_id = "management/group/id" - environment.application.portfolio.csp_data = { + environment.portfolio.csp_data = { "billing_account_name": "xxxx-xxxx-xxx-xxx", "billing_profile_name": "xxxxxxxxxxx:xxxxxxxxxxxxx_xxxxxx", "tenant_id": "xxxxxxxxxxx-xxxxxxxxxx-xxxxxxx-xxxxx", @@ -846,3 +849,66 @@ def test_create_subscription_failure(client, user_session, monkeypatch): ) assert response.status_code == 400 + + +class TestBuildSubscriptionPayload: + def test_unique_display_name(self): + # Create 2 Applications with the same name that both have an environment named 'Environment' + app_1 = ApplicationFactory.create( + name="Application", environments=[{"name": "Environment", "cloud_id": 123}] + ) + env_1 = app_1.environments[0] + app_2 = ApplicationFactory.create( + name="Application", environments=[{"name": "Environment", "cloud_id": 456}] + ) + env_2 = app_2.environments[0] + env_1.portfolio.csp_data = { + "billing_account_name": "xxxx-xxxx-xxx-xxx", + "billing_profile_name": "xxxxxxxxxxx:xxxxxxxxxxxxx_xxxxxx", + "tenant_id": "xxxxxxxxxxx-xxxxxxxxxx-xxxxxxx-xxxxx", + "billing_profile_properties": { + "invoice_sections": [{"invoice_section_name": "xxxx-xxxx-xxx-xxx"}] + }, + } + env_2.portfolio.csp_data = { + "billing_account_name": "xxxx-xxxx-xxx-xxx", + "billing_profile_name": "xxxxxxxxxxx:xxxxxxxxxxxxx_xxxxxx", + "tenant_id": "xxxxxxxxxxx-xxxxxxxxxx-xxxxxxx-xxxxx", + "billing_profile_properties": { + "invoice_sections": [{"invoice_section_name": "xxxx-xxxx-xxx-xxx"}] + }, + } + + # Create subscription payload for each environment + payload_1 = build_subscription_payload(env_1) + payload_2 = build_subscription_payload(env_2) + + assert payload_1.display_name != payload_2.display_name + + def test_populates_payload_correctly(self): + app = ApplicationFactory.create( + name="Application", environments=[{"name": "Environment", "cloud_id": 123}] + ) + environment = app.environments[0] + account_name = "fake-account-name" + profile_name = "fake-profile-name" + tenant_id = "123" + section_name = "fake-section-name" + environment.portfolio.csp_data = { + "billing_account_name": account_name, + "billing_profile_name": profile_name, + "tenant_id": tenant_id, + "billing_profile_properties": { + "invoice_sections": [{"invoice_section_name": section_name}] + }, + } + payload = build_subscription_payload(environment) + assert type(payload) == SubscriptionCreationCSPPayload + # Check all key/value pairs except for display_name because of the appended random string + assert { + "tenant_id": tenant_id, + "parent_group_id": environment.cloud_id, + "billing_account_name": account_name, + "billing_profile_name": profile_name, + "invoice_section_name": section_name, + }.items() <= payload.dict().items() diff --git a/tests/test_jobs.py b/tests/test_jobs.py index ea781c1b..d4a61c02 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -104,7 +104,7 @@ tomorrow = now.add(days=1) def test_create_environment_job(session, csp): environment = EnvironmentFactory.create() environment.application.cloud_id = "parentId" - environment.application.portfolio.csp_data = {"tenant_id": "fake"} + environment.portfolio.csp_data = {"tenant_id": "fake"} session.add(environment) session.commit() do_create_environment(csp, environment.id)