From 8810a59e0adc62736777986592063570287d834a Mon Sep 17 00:00:00 2001 From: dandds Date: Sat, 25 Jan 2020 17:29:17 -0500 Subject: [PATCH] Orchestration for creating app management groups. This adds: - A Celery beat task for enqueuing application creation tasks - A Celery task for creating the application - Payload and Response dataclasses for creating management groups It also does some incidental cleanup. --- atst/domain/csp/cloud.py | 113 +++++++++++++++++-- atst/jobs.py | 35 ++++++ atst/queue.py | 4 + tests/domain/cloud/test_azure_csp.py | 12 +- tests/domain/cloud/test_payloads.py | 72 ++++++++++++ tests/domain/test_portfolio_state_machine.py | 1 - tests/test_jobs.py | 36 ++++++ 7 files changed, 258 insertions(+), 15 deletions(-) create mode 100644 tests/domain/cloud/test_payloads.py diff --git a/atst/domain/csp/cloud.py b/atst/domain/csp/cloud.py index d22f9475..eff5a6d8 100644 --- a/atst/domain/csp/cloud.py +++ b/atst/domain/csp/cloud.py @@ -7,7 +7,6 @@ from pydantic import BaseModel, validator from flask import current_app as app from atst.models.user import User -from atst.models.application import Application from atst.models.environment import Environment from atst.models.environment_role import EnvironmentRole from atst.utils import snake_to_camel @@ -376,6 +375,60 @@ class BillingInstructionCSPResult(AliasModel): } +AZURE_MGMNT_PATH = "/providers/Microsoft.Management/managementGroups/" + +MANAGEMENT_GROUP_NAME_REGEX = "^[a-zA-Z0-9\-_\(\)\.]+$" + + +class ManagementGroupCSPPayload(BaseCSPPayload): + """ + :param: management_group_name: Just pass a UUID for this. + :param: display_name: This can contain any character and + spaces, but should be 90 characters or fewer long. + :param: parent_id: This should be the fully qualified Azure ID, + i.e. /providers/Microsoft.Management/managementGroups/[management group ID] + """ + + management_group_name: Optional[str] + display_name: str + parent_id: str + + @validator("management_group_name", pre=True, always=True) + def supply_management_group_name_default(cls, name): + if name: + if re.match(MANAGEMENT_GROUP_NAME_REGEX, name) is None: + raise ValueError( + f"Management group name must match {MANAGEMENT_GROUP_NAME_REGEX}" + ) + + return name[0:90] + else: + return str(uuid4()) + + @validator("display_name", pre=True, always=True) + def enforce_display_name_length(cls, name): + return name[0:90] + + @validator("parent_id", pre=True, always=True) + def enforce_parent_id_pattern(cls, id_): + if AZURE_MGMNT_PATH not in id_: + return f"{AZURE_MGMNT_PATH}{id_}" + else: + return id_ + + +class ManagementGroupCSPResponse(AliasModel): + id: str + + +class ApplicationCSPPayload(ManagementGroupCSPPayload): + pass + + +class ApplicationCSPResult(ManagementGroupCSPResponse): + pass + + class CloudProviderInterface: def set_secret(self, secret_key: str, secret_value: str): raise NotImplementedError() @@ -806,6 +859,15 @@ class MockCloudProvider(CloudProviderInterface): if self._with_authorization and credentials != self._auth_credentials: raise self.AUTHENTICATION_EXCEPTION + def create_application(self, payload: ApplicationCSPPayload): + self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) + + id_ = f"{AZURE_MGMNT_PATH}{payload.management_group_name}" + return ApplicationCSPResult(id=id_) + + def get_credentials(self, scope="portfolio", tenant_id=None): + return self.root_creds() + AZURE_ENVIRONMENT = "AZURE_PUBLIC_CLOUD" # TBD AZURE_SKU_ID = "?" # probably a static sku specific to ATAT/JEDI @@ -840,7 +902,7 @@ class AzureSDKProvider(object): self.graphrbac = graphrbac self.credentials = credentials self.identity = identity - self.exceptions = exceptions + # self.exceptions = exceptions self.secrets = secrets self.requests = requests # may change to a JEDI cloud @@ -908,7 +970,7 @@ class AzureCloudProvider(CloudProviderInterface): credentials, management_group_id, display_name, parent_id, ) - return management_group + return ManagementGroupCSPResponse(**management_group) def create_atat_admin_user( self, auth_credentials: Dict, csp_environment_id: str @@ -947,16 +1009,19 @@ class AzureCloudProvider(CloudProviderInterface): "role_name": role_assignment_id, } - def _create_application(self, auth_credentials: Dict, application: Application): - management_group_name = str(uuid4()) # can be anything, not just uuid - display_name = application.name # Does this need to be unique? - credentials = self._get_credential_obj(auth_credentials) - parent_id = "?" # application.portfolio.csp_details.management_group_id + def create_application(self, payload: ApplicationCSPPayload): + creds = payload.creds + credentials = self._get_credential_obj(creds, resource=AZURE_MANAGEMENT_API) - return self._create_management_group( - credentials, management_group_name, display_name, parent_id, + response = self._create_management_group( + credentials, + payload.management_group_name, + payload.display_name, + payload.parent_id, ) + return ApplicationCSPResult(**response) + def _create_management_group( self, credentials, management_group_id, display_name, parent_id=None, ): @@ -978,6 +1043,9 @@ class AzureCloudProvider(CloudProviderInterface): # result is a synchronous wait, might need to do a poll instead to handle first mgmt group create # since we were told it could take 10+ minutes to complete, unless this handles that polling internally + # TODO: what to do is status is not 'Succeeded' on the + # response object? Will it always raise its own error + # instead? return create_request.result() def _create_subscription( @@ -1289,6 +1357,7 @@ class AzureCloudProvider(CloudProviderInterface): # we likely only want the budget ID, can be updated or replaced? response = {"id": "id"} + return self._ok({"budget_id": response["id"]}) def _get_management_service_principal(self): @@ -1406,3 +1475,27 @@ class AzureCloudProvider(CloudProviderInterface): "secret_key": self.secret_key, "tenant_id": self.tenant_id, } + + def get_credentials(self, scope="portfolio", tenant_id=None): + """ + This could be implemented to determine, based on type, whether to return creds for: + - scope="atat": the ATAT main app registration in ATAT's home tenant + - scope="tenantadmin": the tenant administrator credentials + - scope="portfolio": the credentials for the ATAT SP in the portfolio tenant + """ + if scope == "atat": + return self._root_creds + elif scope == "tenantadmin": + # magic with key vault happens + return { + "client_id": "some id", + "secret_key": "very secret", + "tenant_id": tenant_id, + } + elif scope == "portfolio": + # magic with key vault happens + return { + "client_id": "some id", + "secret_key": "very secret", + "tenant_id": tenant_id, + } diff --git a/atst/jobs.py b/atst/jobs.py index 47fefb71..37f73450 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -5,11 +5,13 @@ from atst.database import db from atst.queue import celery from atst.models import EnvironmentRole, JobFailure from atst.domain.csp.cloud import CloudProviderInterface, GeneralCSPException +from atst.domain.applications import Applications from atst.domain.environments import Environments from atst.domain.portfolios import Portfolios from atst.domain.environment_roles import EnvironmentRoles from atst.models.utils import claim_for_update from atst.utils.localization import translate +from atst.domain.csp.cloud import ApplicationCSPPayload class RecordFailure(celery.Task): @@ -51,6 +53,28 @@ def send_notification_mail(recipients, subject, body): app.mailer.send(recipients, subject, body) +def do_create_application(csp: CloudProviderInterface, application_id=None): + application = Applications.get(application_id) + + with claim_for_update(application) as application: + + if application.cloud_id is not None: + return + + csp_details = application.portfolio.csp_data + parent_id = csp_details.get("root_management_group_id") + tenant_id = csp_details.get("tenant_id") + creds = csp.get_credentials(tenant_id) + payload = ApplicationCSPPayload( + creds=creds, display_name=application.name, parent_id=parent_id + ) + + app_result = csp.create_application(payload) + application.cloud_id = app_result.id + db.session.add(application) + db.session.commit() + + def do_create_environment(csp: CloudProviderInterface, environment_id=None): environment = Environments.get(environment_id) @@ -137,6 +161,11 @@ def provision_portfolio(self, portfolio_id=None): do_work(do_provision_portfolio, self, app.csp.cloud, portfolio_id=portfolio_id) +@celery.task(bind=True, base=RecordFailure) +def create_application(self, application_id=None): + do_work(do_create_application, self, app.csp.cloud, application_id=application_id) + + @celery.task(bind=True, base=RecordFailure) def create_environment(self, environment_id=None): do_work(do_create_environment, self, app.csp.cloud, environment_id=environment_id) @@ -165,6 +194,12 @@ def dispatch_provision_portfolio(self): provision_portfolio.delay(portfolio_id=portfolio_id) +@celery.task(bind=True) +def dispatch_create_application(self): + for application_id in Applications.get_applications_pending_creation(): + create_application.delay(application_id=application_id) + + @celery.task(bind=True) def dispatch_create_environment(self): for environment_id in Environments.get_environments_pending_creation( diff --git a/atst/queue.py b/atst/queue.py index 1dce690c..70718150 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -11,6 +11,10 @@ def update_celery(celery, app): "task": "atst.jobs.dispatch_provision_portfolio", "schedule": 60, }, + "beat-dispatch_create_application": { + "task": "atst.jobs.dispatch_create_application", + "schedule": 60, + }, "beat-dispatch_create_environment": { "task": "atst.jobs.dispatch_create_environment", "schedule": 60, diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 9e87e7a7..c98a43ee 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -18,6 +18,7 @@ from atst.domain.csp.cloud import ( TaskOrderBillingVerificationCSPResult, TenantCSPPayload, TenantCSPResult, + ApplicationCSPPayload, ) from tests.mock_azure import mock_azure, AUTH_CREDENTIALS @@ -67,8 +68,8 @@ def test_create_subscription_succeeds(mock_azure: AzureCloudProvider): def mock_management_group_create(mock_azure, spec_dict): - mock_azure.sdk.managementgroups.ManagementGroupsAPI.return_value.management_groups.create_or_update.return_value.result.return_value = Mock( - **spec_dict + mock_azure.sdk.managementgroups.ManagementGroupsAPI.return_value.management_groups.create_or_update.return_value.result.return_value = ( + spec_dict ) @@ -89,7 +90,10 @@ def test_create_application_succeeds(mock_azure: AzureCloudProvider): mock_management_group_create(mock_azure, {"id": "Test Id"}) - result = mock_azure._create_application(AUTH_CREDENTIALS, application) + payload = ApplicationCSPPayload( + creds={}, display_name=application.name, parent_id=str(uuid4()) + ) + result = mock_azure.create_application(payload) assert result.id == "Test Id" @@ -150,7 +154,7 @@ def test_create_tenant(mock_azure: AzureCloudProvider): **dict( creds=creds, user_id="admin", - password="JediJan13$coot", + password="JediJan13$coot", # pragma: allowlist secret domain_name="jediccpospawnedtenant2", first_name="Tedry", last_name="Tenet", diff --git a/tests/domain/cloud/test_payloads.py b/tests/domain/cloud/test_payloads.py new file mode 100644 index 00000000..08ca147c --- /dev/null +++ b/tests/domain/cloud/test_payloads.py @@ -0,0 +1,72 @@ +import pytest + +from pydantic import ValidationError + +from atst.domain.csp.cloud import ( + AZURE_MGMNT_PATH, + ManagementGroupCSPPayload, + ManagementGroupCSPResponse, +) + + +def test_ManagementGroupCSPPayload_management_group_name(): + # supplies management_group_name when absent + payload = ManagementGroupCSPPayload( + creds={}, display_name="Council of Naboo", parent_id="Galactic_Senate" + ) + assert payload.management_group_name + # validates management_group_name + with pytest.raises(ValidationError): + payload = ManagementGroupCSPPayload( + creds={}, + management_group_name="council of Naboo 1%^&", + display_name="Council of Naboo", + parent_id="Galactic_Senate", + ) + # shortens management_group_name to fit + name = "council_of_naboo" + for _ in range(90): + name = f"{name}1" + + assert len(name) > 90 + payload = ManagementGroupCSPPayload( + creds={}, + management_group_name=name, + display_name="Council of Naboo", + parent_id="Galactic_Senate", + ) + assert len(payload.management_group_name) == 90 + + +def test_ManagementGroupCSPPayload_display_name(): + # shortens display_name to fit + name = "Council of Naboo" + for _ in range(90): + name = f"{name}1" + assert len(name) > 90 + payload = ManagementGroupCSPPayload( + creds={}, display_name=name, parent_id="Galactic_Senate" + ) + assert len(payload.display_name) == 90 + + +def test_ManagementGroupCSPPayload_parent_id(): + full_path = f"{AZURE_MGMNT_PATH}Galactic_Senate" + # adds full path + payload = ManagementGroupCSPPayload( + creds={}, display_name="Council of Naboo", parent_id="Galactic_Senate" + ) + assert payload.parent_id == full_path + # keeps full path + payload = ManagementGroupCSPPayload( + creds={}, display_name="Council of Naboo", parent_id=full_path + ) + assert payload.parent_id == full_path + + +def test_ManagementGroupCSPResponse_id(): + full_id = "/path/to/naboo-123" + response = ManagementGroupCSPResponse( + **{"id": "/path/to/naboo-123", "other": "stuff"} + ) + assert response.id == full_id diff --git a/tests/domain/test_portfolio_state_machine.py b/tests/domain/test_portfolio_state_machine.py index 0d37c9c7..330d5195 100644 --- a/tests/domain/test_portfolio_state_machine.py +++ b/tests/domain/test_portfolio_state_machine.py @@ -141,7 +141,6 @@ def test_fsm_transition_start(portfolio: Portfolio): config = {"billing_account_name": "billing_account_name"} for expected_state in expected_states: - print(expected_state) collected_data = dict( list(csp_data.items()) + list(portfolio_data.items()) + list(config.items()) ) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 9734bd75..2ac5f408 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -10,6 +10,7 @@ from atst.domain.portfolios import Portfolios from atst.jobs import ( RecordFailure, dispatch_create_environment, + dispatch_create_application, dispatch_create_atat_admin_user, dispatch_provision_portfolio, dispatch_provision_user, @@ -17,6 +18,7 @@ from atst.jobs import ( do_provision_user, do_provision_portfolio, do_create_environment, + do_create_application, do_create_atat_admin_user, ) from atst.models.utils import claim_for_update @@ -26,6 +28,7 @@ from tests.factories import ( EnvironmentRoleFactory, PortfolioFactory, PortfolioStateMachineFactory, + ApplicationFactory, ApplicationRoleFactory, ) from atst.models import CSPRole, EnvironmentRole, ApplicationRoleStatus, JobFailure @@ -105,6 +108,24 @@ def test_create_environment_job_is_idempotent(csp, session): csp.create_environment.assert_not_called() +def test_create_application_job(session, csp): + portfolio = PortfolioFactory.create( + csp_data={"tenant_id": str(uuid4()), "root_management_group_id": str(uuid4())} + ) + application = ApplicationFactory.create(portfolio=portfolio, cloud_id=None) + do_create_application(csp, application.id) + session.refresh(application) + + assert application.cloud_id + + +def test_create_application_job_is_idempotent(csp): + application = ApplicationFactory.create(cloud_id=uuid4()) + do_create_application(csp, application.id) + + csp.create_application.assert_not_called() + + def test_create_atat_admin_user(csp, session): environment = EnvironmentFactory.create(cloud_id="something") do_create_atat_admin_user(csp, environment.id) @@ -145,6 +166,21 @@ def test_dispatch_create_environment(session, monkeypatch): mock.delay.assert_called_once_with(environment_id=e1.id) +def test_dispatch_create_application(monkeypatch): + portfolio = PortfolioFactory.create(state="COMPLETED") + app = ApplicationFactory.create(portfolio=portfolio) + + mock = Mock() + monkeypatch.setattr("atst.jobs.create_application", mock) + + # When dispatch_create_application is called + dispatch_create_application.run() + + # It should cause the create_application task to be called once + # with the application id + mock.delay.assert_called_once_with(application_id=app.id) + + def test_dispatch_create_atat_admin_user(session, monkeypatch): portfolio = PortfolioFactory.create( applications=[