Merge pull request #1437 from dod-ccpo/unique-subscription-names

Unique subscription names

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 = environment_role.environment.portfolio.csp_data.get("tenant_id")
-                "tenant_id"
-            )
             app.csp.cloud.disable_user(tenant_id, environment_role.csp_user_id)
 
         environment_role.status = EnvironmentRole.Status.DISABLED

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

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,
         )
 

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": self.environment.portfolio.name,
-            "portfolio_id": str(self.environment.application.portfolio.id),
+            "portfolio_id": str(self.environment.portfolio.id),
         }
 
 

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"),

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,
         )

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)

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()

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)