From 1378fcfc15b2e7a4bc0fc28bf252259204fbbe95 Mon Sep 17 00:00:00 2001 From: Philip Kalinsky Date: Tue, 28 Jan 2020 15:06:48 -0500 Subject: [PATCH 01/12] merge conflict --- atst/domain/csp/cloud/azure_cloud_provider.py | 35 ++++++++++-------- atst/domain/csp/cloud/mock_cloud_provider.py | 37 +++++-------------- atst/domain/csp/cloud/models.py | 8 ++++ atst/domain/environments.py | 2 +- atst/jobs.py | 36 +++++++----------- tests/domain/cloud/test_azure_csp.py | 10 +++-- 6 files changed, 57 insertions(+), 71 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index d5ef5204..908474ab 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -24,6 +24,8 @@ from .models import ( BillingProfileTenantAccessCSPResult, BillingProfileVerificationCSPPayload, BillingProfileVerificationCSPResult, + EnvironmentCSPPayload, + EnvironmentCSPResult, KeyVaultCredentials, ManagementGroupCSPResponse, ProductPurchaseCSPPayload, @@ -135,24 +137,25 @@ class AzureCloudProvider(CloudProviderInterface): exc_info=1, ) - def create_environment(self, auth_credentials: Dict, user, environment): - # since this operation would only occur within a tenant, should we source the tenant - # via lookup from environment once we've created the portfolio csp data schema - # something like this: - # environment_tenant = environment.application.portfolio.csp_data.get('tenant_id', None) - # though we'd probably source the whole credentials for these calls from the portfolio csp - # data, as it would have to be where we store the creds for the at-at user within the portfolio tenant - # credentials = self._get_credential_obj(environment.application.portfolio.csp_data.get_creds()) - credentials = self._get_credential_obj(self._root_creds) - display_name = f"{environment.application.name}_{environment.name}_{environment.id}" # proposed format - management_group_id = "?" # management group id chained from environment - parent_id = "?" # from environment.application - - management_group = self._create_management_group( - credentials, management_group_id, display_name, parent_id, + def create_environment(self, payload: EnvironmentCSPPayload): + creds = self._source_creds(payload.tenant_id) + credentials = self._get_credential_obj( + { + "client_id": creds.root_sp_client_id, + "secret_key": creds.root_sp_key, + "tenant_id": creds.root_tenant_id, + }, + resource=AZURE_MANAGEMENT_API, ) - return ManagementGroupCSPResponse(**management_group) + response = self._create_management_group( + credentials, + payload.management_group_name, + payload.display_name, + payload.parent_id, + ) + + return EnvironmentCSPResult(**response) def create_atat_admin_user( self, auth_credentials: Dict, csp_environment_id: str diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index ec730a3b..5b9a8b34 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -35,6 +35,8 @@ from .models import ( SubscriptionCreationCSPResult, SubscriptionVerificationCSPPayload, SuscriptionVerificationCSPResult, + EnvironmentCSPPayload, + EnvironmentCSPResult, TaskOrderBillingCreationCSPPayload, TaskOrderBillingCreationCSPResult, TaskOrderBillingVerificationCSPPayload, @@ -91,34 +93,6 @@ class MockCloudProvider(CloudProviderInterface): def get_secret(self, secret_key: str, default=dict()): return default - def create_environment(self, auth_credentials, user, environment): - self._authorize(auth_credentials) - - self._delay(1, 5) - self._maybe_raise(self.NETWORK_FAILURE_PCT, self.NETWORK_EXCEPTION) - self._maybe_raise(self.SERVER_FAILURE_PCT, self.SERVER_EXCEPTION) - self._maybe_raise( - self.ENV_CREATE_FAILURE_PCT, - EnvironmentCreationException( - environment.id, "Could not create environment." - ), - ) - - csp_environment_id = self._id() - - self._delay(1, 5) - self._maybe_raise(self.NETWORK_FAILURE_PCT, self.NETWORK_EXCEPTION) - self._maybe_raise(self.SERVER_FAILURE_PCT, self.SERVER_EXCEPTION) - self._maybe_raise( - self.ATAT_ADMIN_CREATE_FAILURE_PCT, - BaselineProvisionException( - csp_environment_id, "Could not create environment baseline." - ), - ) - self._maybe_raise(self.UNAUTHORIZED_RATE, self.AUTHORIZATION_EXCEPTION) - - return csp_environment_id - def create_subscription(self, payload: SubscriptionCreationCSPPayload): return self.create_subscription_creation(payload) @@ -482,6 +456,13 @@ class MockCloudProvider(CloudProviderInterface): return UserCSPResult(id=str(uuid4())) + def create_environment(self, payload: EnvironmentCSPPayload): + self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) + + return EnvironmentCSPResult( + id=f"{AZURE_MGMNT_PATH}{payload.management_group_name}" + ) + def get_credentials(self, scope="portfolio", tenant_id=None): return self.root_creds() diff --git a/atst/domain/csp/cloud/models.py b/atst/domain/csp/cloud/models.py index 188c2cc7..05f3c866 100644 --- a/atst/domain/csp/cloud/models.py +++ b/atst/domain/csp/cloud/models.py @@ -358,6 +358,14 @@ class ApplicationCSPResult(ManagementGroupCSPResponse): pass +class EnvironmentCSPPayload(ManagementGroupCSPPayload): + pass + + +class EnvironmentCSPResult(ManagementGroupCSPResponse): + pass + + class KeyVaultCredentials(BaseModel): root_sp_client_id: Optional[str] root_sp_key: Optional[str] diff --git a/atst/domain/environments.py b/atst/domain/environments.py index b8a59485..78aa2af3 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -124,7 +124,7 @@ class Environments(object): Any environment with an active CLIN that doesn't yet have a `cloud_id`. """ results = ( - cls.base_provision_query(now).filter(Environment.cloud_id == None).all() + cls.base_provision_query(now).filter(Application.cloud_id != None).all() ) return [id_ for id_, in results] diff --git a/atst/jobs.py b/atst/jobs.py index 986b2004..c2e2d03e 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -12,7 +12,11 @@ from atst.domain.portfolios import Portfolios from atst.domain.application_roles import ApplicationRoles from atst.models.utils import claim_for_update, claim_many_for_update from atst.utils.localization import translate -from atst.domain.csp.cloud.models import ApplicationCSPPayload, UserCSPPayload +from atst.domain.csp.cloud.models import ( + ApplicationCSPPayload, + EnvironmentCSPPayload, + UserCSPPayload, +) class RecordFailure(celery.Task): @@ -109,34 +113,20 @@ def do_create_environment(csp: CloudProviderInterface, environment_id=None): with claim_for_update(environment) as environment: if environment.cloud_id is not None: - # TODO: Return value for this? return - user = environment.creator + csp_details = environment.application.portfolio.csp_data + parent_id = environment.application.cloud_id + tenant_id = csp_details.get("tenant_id") + payload = EnvironmentCSPPayload( + tenant_id=tenant_id, display_name=application.name, parent_id=parent_id + ) - # we'll need to do some checking in this job for cases where it's retrying - # when a failure occured after some successful steps - # (e.g. if environment.cloud_id is not None, then we can skip first step) - - # credentials either from a given user or pulled from config? - # if using global creds, do we need to log what user authorized action? - atat_root_creds = csp.root_creds() - - # user is needed because baseline root account in the environment will - # be assigned to the requesting user, open question how to handle duplicate - # email addresses across new environments - csp_environment_id = csp.create_environment(atat_root_creds, user, environment) - environment.cloud_id = csp_environment_id + env_result = csp.create_environment(payload) + environment.cloud_id = env_result.id db.session.add(environment) db.session.commit() - body = render_email( - "emails/application/environment_ready.txt", {"environment": environment} - ) - app.mailer.send( - [environment.creator.email], translate("email.environment_ready"), body - ) - def do_create_atat_admin_user(csp: CloudProviderInterface, environment_id=None): environment = Environments.get(environment_id) diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index eef5620e..639997b3 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -20,6 +20,8 @@ from atst.domain.csp.cloud.models import ( BillingProfileTenantAccessCSPResult, BillingProfileVerificationCSPPayload, BillingProfileVerificationCSPResult, + EnvironmentCSPPayload, + EnvironmentCSPResult, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, @@ -57,12 +59,14 @@ def mock_management_group_create(mock_azure, spec_dict): def test_create_environment_succeeds(mock_azure: AzureCloudProvider): environment = EnvironmentFactory.create() - mock_management_group_create(mock_azure, {"id": "Test Id"}) - result = mock_azure.create_environment( - AUTH_CREDENTIALS, environment.creator, environment + mock_azure = mock_get_secret(mock_azure, lambda *a, **k: json.dumps(MOCK_CREDS)) + + payload = EnvironmentCSPPayload( + tenant_id="1234", display_name=environment.name, parent_id=str(uuid4()) ) + result = mock_azure.create_environment(payload) assert result.id == "Test Id" From 21d48c55cf1467b3730582c3fd82232392bbfef2 Mon Sep 17 00:00:00 2001 From: tomdds Date: Fri, 31 Jan 2020 16:52:44 -0500 Subject: [PATCH 02/12] Use correct config value for create_environment resource --- atst/domain/csp/cloud/azure_cloud_provider.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 908474ab..ce6d45ee 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -145,7 +145,7 @@ class AzureCloudProvider(CloudProviderInterface): "secret_key": creds.root_sp_key, "tenant_id": creds.root_tenant_id, }, - resource=AZURE_MANAGEMENT_API, + resource=self.sdk.cloud.endpoints.resource_manager, ) response = self._create_management_group( @@ -155,6 +155,8 @@ class AzureCloudProvider(CloudProviderInterface): payload.parent_id, ) + if + return EnvironmentCSPResult(**response) def create_atat_admin_user( From bcb552134b48dc3e8bd5cb4ba043fa71d1f35e16 Mon Sep 17 00:00:00 2001 From: tomdds Date: Mon, 3 Feb 2020 15:01:42 -0500 Subject: [PATCH 03/12] Update tests for environment management group provisioning --- atst/jobs.py | 2 +- tests/domain/cloud/test_mock_csp.py | 15 ++++++++++++--- tests/test_jobs.py | 4 ++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/atst/jobs.py b/atst/jobs.py index c2e2d03e..6a91b03e 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -119,7 +119,7 @@ def do_create_environment(csp: CloudProviderInterface, environment_id=None): parent_id = environment.application.cloud_id tenant_id = csp_details.get("tenant_id") payload = EnvironmentCSPPayload( - tenant_id=tenant_id, display_name=application.name, parent_id=parent_id + tenant_id=tenant_id, display_name=environment.name, parent_id=parent_id ) env_result = csp.create_environment(payload) diff --git a/tests/domain/cloud/test_mock_csp.py b/tests/domain/cloud/test_mock_csp.py index b3a8d551..86727b38 100644 --- a/tests/domain/cloud/test_mock_csp.py +++ b/tests/domain/cloud/test_mock_csp.py @@ -1,6 +1,7 @@ import pytest from atst.domain.csp import MockCloudProvider +from atst.domain.csp.cloud.models import EnvironmentCSPPayload, EnvironmentCSPResult from tests.factories import EnvironmentFactory, EnvironmentRoleFactory, UserFactory @@ -14,9 +15,17 @@ def mock_csp(): def test_create_environment(mock_csp: MockCloudProvider): environment = EnvironmentFactory.create() - user = UserFactory.create() - environment_id = mock_csp.create_environment(CREDENTIALS, user, environment) - assert isinstance(environment_id, str) + environment.application.cloud_id = "parent_id" + environment.application.portfolio.csp_data = {"tenant_id": "fake"} + payload = EnvironmentCSPPayload( + **dict( + tenant_id=environment.application.portfolio.csp_data.get("tenant_id"), + display_name=environment.name, + parent_id=environment.application.cloud_id, + ) + ) + result = mock_csp.create_environment(payload) + assert isinstance(result, EnvironmentCSPResult) def test_create_admin_user(mock_csp: MockCloudProvider): diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 54f4c5dc..de17eb6a 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -94,6 +94,10 @@ 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"} + session.add(environment) + session.commit() do_create_environment(csp, environment.id) session.refresh(environment) From b364a1d2a6ed59b07716000e1502f2f096d0962c Mon Sep 17 00:00:00 2001 From: dandds Date: Sun, 2 Feb 2020 13:58:41 -0500 Subject: [PATCH 04/12] Celery wrapper for creating a user. --- atst/domain/csp/cloud/mock_cloud_provider.py | 5 +++++ atst/jobs.py | 21 ++++++++++---------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 5b9a8b34..96648b52 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -463,6 +463,11 @@ class MockCloudProvider(CloudProviderInterface): id=f"{AZURE_MGMNT_PATH}{payload.management_group_name}" ) + def create_user(self, payload: UserCSPPayload): + self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) + + return UserCSPResult(id=str(uuid4())) + def get_credentials(self, scope="portfolio", tenant_id=None): return self.root_creds() diff --git a/atst/jobs.py b/atst/jobs.py index 6a91b03e..0ce664da 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -1,22 +1,23 @@ -from flask import current_app as app import pendulum +from flask import current_app as app from atst.database import db -from atst.queue import celery -from atst.models import JobFailure -from atst.domain.csp.cloud.exceptions import GeneralCSPException -from atst.domain.csp.cloud import CloudProviderInterface -from atst.domain.applications import Applications -from atst.domain.environments import Environments -from atst.domain.portfolios import Portfolios from atst.domain.application_roles import ApplicationRoles -from atst.models.utils import claim_for_update, claim_many_for_update -from atst.utils.localization import translate +from atst.domain.applications import Applications +from atst.domain.csp.cloud import CloudProviderInterface +from atst.domain.csp.cloud.exceptions import GeneralCSPException from atst.domain.csp.cloud.models import ( ApplicationCSPPayload, EnvironmentCSPPayload, UserCSPPayload, ) +from atst.domain.environment_roles import EnvironmentRoles +from atst.domain.environments import Environments +from atst.domain.portfolios import Portfolios +from atst.models import JobFailure +from atst.models.utils import claim_for_update, claim_many_for_update +from atst.queue import celery +from atst.utils.localization import translate class RecordFailure(celery.Task): From 17ec944ad5abb71f8c2fb367f90b85ff3ca0ebf9 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 14:34:59 -0500 Subject: [PATCH 05/12] Update environment queries to look for app provisioning completion as well --- atst/domain/environments.py | 5 ++++- atst/jobs.py | 2 -- tests/domain/test_environments.py | 23 +++++++++++++++++------ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 78aa2af3..3abc4b18 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -124,7 +124,10 @@ class Environments(object): Any environment with an active CLIN that doesn't yet have a `cloud_id`. """ results = ( - cls.base_provision_query(now).filter(Application.cloud_id != None).all() + cls.base_provision_query(now) + .filter(Application.cloud_id != None) + .filter(Environment.cloud_id == None) + .all() ) return [id_ for id_, in results] diff --git a/atst/jobs.py b/atst/jobs.py index 0ce664da..ae0655fe 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -11,13 +11,11 @@ from atst.domain.csp.cloud.models import ( EnvironmentCSPPayload, UserCSPPayload, ) -from atst.domain.environment_roles import EnvironmentRoles from atst.domain.environments import Environments from atst.domain.portfolios import Portfolios from atst.models import JobFailure from atst.models.utils import claim_for_update, claim_many_for_update from atst.queue import celery -from atst.utils.localization import translate class RecordFailure(celery.Task): diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index ff4b8605..72f07651 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -132,15 +132,19 @@ class EnvQueryTest: def TOMORROW(self): return self.NOW.add(days=1) - def create_portfolio_with_clins(self, start_and_end_dates, env_data=None): + def create_portfolio_with_clins( + self, start_and_end_dates, env_data=None, app_data=None + ): env_data = env_data or {} + app_data = app_data or {} return PortfolioFactory.create( applications=[ { "name": "Mos Eisley", "description": "Where Han shot first", "environments": [{"name": "thebar", **env_data}], - } + **app_data, + }, ], task_orders=[ { @@ -168,9 +172,17 @@ class TestGetEnvironmentsPendingCreate(EnvQueryTest): self.create_portfolio_with_clins([(self.TOMORROW, self.TOMORROW)]) assert len(Environments.get_environments_pending_creation(self.NOW)) == 0 + def test_with_already_provisioned_app(self, session): + self.create_portfolio_with_clins( + [(self.YESTERDAY, self.TOMORROW)], app_data={"cloud_id": uuid4().hex} + ) + assert len(Environments.get_environments_pending_creation(self.NOW)) == 1 + def test_with_already_provisioned_env(self, session): self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], env_data={"cloud_id": uuid4().hex} + [(self.YESTERDAY, self.TOMORROW)], + env_data={"cloud_id": uuid4().hex}, + app_data={"cloud_id": uuid4().hex}, ) assert len(Environments.get_environments_pending_creation(self.NOW)) == 0 @@ -187,11 +199,10 @@ class TestGetEnvironmentsPendingAtatUserCreation(EnvQueryTest): def test_with_unprovisioned_environment(self): self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], - {"cloud_id": uuid4().hex, "root_user_info": None}, + [(self.YESTERDAY, self.TOMORROW)], app_data={"cloud_id": uuid4().hex}, ) assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 1 + len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 ) def test_with_unprovisioned_expired_clins_environment(self): From 1a7db62dac87ccc42126ab5ab86693c3f7eecbbb Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 15:16:18 -0500 Subject: [PATCH 06/12] Remove Environment level user provisioning --- ..._remove_root_user_info_from_environment.py | 28 +++++++++++++ atst/domain/environment_roles.py | 6 ++- atst/domain/environments.py | 12 ------ atst/jobs.py | 29 -------------- atst/models/environment.py | 19 +++------ atst/queue.py | 4 -- tests/domain/test_environment_roles.py | 6 +-- tests/domain/test_environments.py | 28 ------------- tests/models/test_environments.py | 14 +------ tests/test_jobs.py | 39 +------------------ 10 files changed, 42 insertions(+), 143 deletions(-) create mode 100644 alembic/versions/0039308c6351_remove_root_user_info_from_environment.py diff --git a/alembic/versions/0039308c6351_remove_root_user_info_from_environment.py b/alembic/versions/0039308c6351_remove_root_user_info_from_environment.py new file mode 100644 index 00000000..e9f3fa8a --- /dev/null +++ b/alembic/versions/0039308c6351_remove_root_user_info_from_environment.py @@ -0,0 +1,28 @@ +"""Remove root_user_info from Environment + +Revision ID: 0039308c6351 +Revises: 17da2a475429 +Create Date: 2020-02-04 14:37:06.814645 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '0039308c6351' # pragma: allowlist secret +down_revision = '17da2a475429' # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('environments', 'root_user_info') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('environments', sa.Column('root_user_info', postgresql.JSONB(astext_type=sa.Text()), autoincrement=False, nullable=True)) + # ### end Alembic commands ### diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index ef8a4b8e..a022b932 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -107,8 +107,10 @@ class EnvironmentRoles(object): environment_role = EnvironmentRoles.get_by_id(environment_role_id) if environment_role.csp_user_id and not environment_role.environment.is_pending: - credentials = environment_role.environment.csp_credentials - app.csp.cloud.disable_user(credentials, environment_role.csp_user_id) + tenant_id = environment_role.environment.application.portfolio.csp_data.get( + "tenant_id" + ) + app.csp.cloud.disable_user(tenant_id, environment_role.csp_user_id) environment_role.status = EnvironmentRole.Status.DISABLED db.session.add(environment_role) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 3abc4b18..f945f3f8 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -130,15 +130,3 @@ class Environments(object): .all() ) return [id_ for id_, in results] - - @classmethod - def get_environments_pending_atat_user_creation(cls, now) -> List[UUID]: - """ - Any environment with an active CLIN that has a cloud_id but no `root_user_info`. - """ - results = ( - cls.base_provision_query(now) - .filter(Environment.cloud_id != None) - .filter(Environment.root_user_info == None) - ).all() - return [id_ for id_, in results] diff --git a/atst/jobs.py b/atst/jobs.py index ae0655fe..3453f81b 100644 --- a/atst/jobs.py +++ b/atst/jobs.py @@ -127,20 +127,6 @@ def do_create_environment(csp: CloudProviderInterface, environment_id=None): db.session.commit() -def do_create_atat_admin_user(csp: CloudProviderInterface, environment_id=None): - environment = Environments.get(environment_id) - - with claim_for_update(environment) as environment: - atat_root_creds = csp.root_creds() - - atat_remote_root_user = csp.create_atat_admin_user( - atat_root_creds, environment.cloud_id - ) - environment.root_user_info = atat_remote_root_user - db.session.add(environment) - db.session.commit() - - def render_email(template_path, context): return app.jinja_env.get_template(template_path).render(context) @@ -180,13 +166,6 @@ def create_environment(self, environment_id=None): do_work(do_create_environment, self, app.csp.cloud, environment_id=environment_id) -@celery.task(bind=True, base=RecordFailure) -def create_atat_admin_user(self, environment_id=None): - do_work( - do_create_atat_admin_user, self, app.csp.cloud, environment_id=environment_id - ) - - @celery.task(bind=True) def dispatch_provision_portfolio(self): """ @@ -214,11 +193,3 @@ def dispatch_create_environment(self): pendulum.now() ): create_environment.delay(environment_id=environment_id) - - -@celery.task(bind=True) -def dispatch_create_atat_admin_user(self): - for environment_id in Environments.get_environments_pending_atat_user_creation( - pendulum.now() - ): - create_atat_admin_user.delay(environment_id=environment_id) diff --git a/atst/models/environment.py b/atst/models/environment.py index b9d16fe0..8408c874 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -1,11 +1,11 @@ -from sqlalchemy import Column, ForeignKey, String, UniqueConstraint -from sqlalchemy.orm import relationship -from sqlalchemy.dialects.postgresql import JSONB from enum import Enum -from atst.models.base import Base +from sqlalchemy import Column, ForeignKey, String, UniqueConstraint +from sqlalchemy.orm import relationship + import atst.models.mixins as mixins import atst.models.types as types +from atst.models.base import Base class Environment( @@ -30,7 +30,6 @@ class Environment( creator = relationship("User") cloud_id = Column(String) - root_user_info = Column(JSONB(none_as_null=True)) roles = relationship( "EnvironmentRole", @@ -70,7 +69,7 @@ class Environment( @property def provisioning_status(self) -> ProvisioningStatus: - if self.cloud_id is None or self.root_user_info is None: + if self.cloud_id is None: return self.ProvisioningStatus.PENDING else: return self.ProvisioningStatus.COMPLETED @@ -91,11 +90,3 @@ class Environment( @property def history(self): return self.get_changes() - - @property - def csp_credentials(self): - return ( - self.root_user_info.get("credentials") - if self.root_user_info is not None - else None - ) diff --git a/atst/queue.py b/atst/queue.py index 3f97f88c..0ea910fb 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -19,10 +19,6 @@ def update_celery(celery, app): "task": "atst.jobs.dispatch_create_environment", "schedule": 60, }, - "beat-dispatch_create_atat_admin_user": { - "task": "atst.jobs.dispatch_create_atat_admin_user", - "schedule": 60, - }, "beat-dispatch_create_user": { "task": "atst.jobs.dispatch_create_user", "schedule": 60, diff --git a/tests/domain/test_environment_roles.py b/tests/domain/test_environment_roles.py index 216f072e..77f6033a 100644 --- a/tests/domain/test_environment_roles.py +++ b/tests/domain/test_environment_roles.py @@ -99,7 +99,6 @@ def test_disable_checks_env_provisioning_status(session): assert env_role1.disabled environment.cloud_id = "cloud-id" - environment.root_user_info = {"credentials": "credentials"} session.add(environment) session.commit() session.refresh(environment) @@ -111,9 +110,8 @@ def test_disable_checks_env_provisioning_status(session): def test_disable_checks_env_role_provisioning_status(): - environment = EnvironmentFactory.create( - cloud_id="cloud-id", root_user_info={"credentials": "credentials"} - ) + environment = EnvironmentFactory.create(cloud_id="cloud-id") + environment.application.portfolio.csp_data = {"tenant_id": uuid4().hex} env_role1 = EnvironmentRoleFactory.create(environment=environment) assert not env_role1.csp_user_id env_role1 = EnvironmentRoles.disable(env_role1.id) diff --git a/tests/domain/test_environments.py b/tests/domain/test_environments.py index 72f07651..a86c6527 100644 --- a/tests/domain/test_environments.py +++ b/tests/domain/test_environments.py @@ -185,31 +185,3 @@ class TestGetEnvironmentsPendingCreate(EnvQueryTest): app_data={"cloud_id": uuid4().hex}, ) assert len(Environments.get_environments_pending_creation(self.NOW)) == 0 - - -class TestGetEnvironmentsPendingAtatUserCreation(EnvQueryTest): - def test_with_provisioned_environment(self): - self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], - {"cloud_id": uuid4().hex, "root_user_info": {}}, - ) - assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 - ) - - def test_with_unprovisioned_environment(self): - self.create_portfolio_with_clins( - [(self.YESTERDAY, self.TOMORROW)], app_data={"cloud_id": uuid4().hex}, - ) - assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 - ) - - def test_with_unprovisioned_expired_clins_environment(self): - self.create_portfolio_with_clins( - [(self.YESTERDAY, self.YESTERDAY)], - {"cloud_id": uuid4().hex, "root_user_info": None}, - ) - assert ( - len(Environments.get_environments_pending_atat_user_creation(self.NOW)) == 0 - ) diff --git a/tests/models/test_environments.py b/tests/models/test_environments.py index 4360cc93..8f8ac626 100644 --- a/tests/models/test_environments.py +++ b/tests/models/test_environments.py @@ -54,18 +54,8 @@ def test_audit_event_for_environment_deletion(session): @pytest.mark.parametrize( "env_data,expected_status", [ - [ - {"cloud_id": None, "root_user_info": None}, - Environment.ProvisioningStatus.PENDING, - ], - [ - {"cloud_id": 1, "root_user_info": None}, - Environment.ProvisioningStatus.PENDING, - ], - [ - {"cloud_id": 1, "root_user_info": {}}, - Environment.ProvisioningStatus.COMPLETED, - ], + [{"cloud_id": None,}, Environment.ProvisioningStatus.PENDING], + [{"cloud_id": 1}, Environment.ProvisioningStatus.COMPLETED], ], ) def test_environment_provisioning_status(env_data, expected_status): diff --git a/tests/test_jobs.py b/tests/test_jobs.py index de17eb6a..a1a821e6 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -12,14 +12,12 @@ from atst.jobs import ( dispatch_create_environment, dispatch_create_application, dispatch_create_user, - dispatch_create_atat_admin_user, dispatch_provision_portfolio, create_environment, do_create_user, do_provision_portfolio, do_create_environment, do_create_application, - do_create_atat_admin_user, ) from tests.factories import ( EnvironmentFactory, @@ -153,14 +151,6 @@ def test_create_user_job(session, csp): assert app_role.cloud_id -def test_create_atat_admin_user(csp, session): - environment = EnvironmentFactory.create(cloud_id="something") - do_create_atat_admin_user(csp, environment.id) - session.refresh(environment) - - assert environment.root_user_info - - def test_dispatch_create_environment(session, monkeypatch): # Given that I have a portfolio with an active CLIN and two environments, # one of which is deleted @@ -231,36 +221,9 @@ def test_dispatch_create_user(monkeypatch): mock.delay.assert_called_once_with(application_role_ids=[app_role.id]) -def test_dispatch_create_atat_admin_user(session, monkeypatch): - portfolio = PortfolioFactory.create( - applications=[ - {"environments": [{"cloud_id": uuid4().hex, "root_user_info": None}]} - ], - task_orders=[ - { - "create_clins": [ - { - "start_date": pendulum.now().subtract(days=1), - "end_date": pendulum.now().add(days=1), - } - ] - } - ], - ) - mock = Mock() - monkeypatch.setattr("atst.jobs.create_atat_admin_user", mock) - environment = portfolio.applications[0].environments[0] - - dispatch_create_atat_admin_user.run() - - mock.delay.assert_called_once_with(environment_id=environment.id) - - def test_create_environment_no_dupes(session, celery_app, celery_worker): portfolio = PortfolioFactory.create( - applications=[ - {"environments": [{"cloud_id": uuid4().hex, "root_user_info": {}}]} - ], + applications=[{"environments": [{"cloud_id": uuid4().hex}]}], task_orders=[ { "create_clins": [ From ca4feaa40375e1daa09b7f3e83a97af0e62c53ff Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 15:39:03 -0500 Subject: [PATCH 07/12] post-rebase fixes --- atst/domain/csp/cloud/azure_cloud_provider.py | 3 --- atst/domain/csp/cloud/mock_cloud_provider.py | 2 -- tests/domain/cloud/test_azure_csp.py | 2 +- tests/test_jobs.py | 2 +- 4 files changed, 2 insertions(+), 7 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index ce6d45ee..79f352d5 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -27,7 +27,6 @@ from .models import ( EnvironmentCSPPayload, EnvironmentCSPResult, KeyVaultCredentials, - ManagementGroupCSPResponse, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, @@ -155,8 +154,6 @@ class AzureCloudProvider(CloudProviderInterface): payload.parent_id, ) - if - return EnvironmentCSPResult(**response) def create_atat_admin_user( diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 96648b52..4955f327 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -4,9 +4,7 @@ from .cloud_provider_interface import CloudProviderInterface from .exceptions import ( AuthenticationException, AuthorizationException, - BaselineProvisionException, ConnectionException, - EnvironmentCreationException, GeneralCSPException, UnknownServerException, UserProvisioningException, diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 639997b3..2b55d1a5 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -61,7 +61,7 @@ def test_create_environment_succeeds(mock_azure: AzureCloudProvider): environment = EnvironmentFactory.create() mock_management_group_create(mock_azure, {"id": "Test Id"}) - mock_azure = mock_get_secret(mock_azure, lambda *a, **k: json.dumps(MOCK_CREDS)) + mock_azure = mock_get_secret(mock_azure) payload = EnvironmentCSPPayload( tenant_id="1234", display_name=environment.name, parent_id=str(uuid4()) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index a1a821e6..8e3d5b1d 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -155,7 +155,7 @@ def test_dispatch_create_environment(session, monkeypatch): # Given that I have a portfolio with an active CLIN and two environments, # one of which is deleted portfolio = PortfolioFactory.create( - applications=[{"environments": [{}, {}]}], + applications=[{"environments": [{}, {}], "cloud_id": uuid4().hex}], task_orders=[ { "create_clins": [ From 9d5918d6188744ff2e32a62c1fa998b55075818b Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 16:42:18 -0500 Subject: [PATCH 08/12] Add exception for problems with secrets --- atst/domain/csp/cloud/azure_cloud_provider.py | 27 ++++++++++++------- atst/domain/csp/cloud/exceptions.py | 14 ++++++++++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 79f352d5..6db3aba3 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -6,12 +6,8 @@ from uuid import uuid4 from atst.utils import sha256_hex from .cloud_provider_interface import CloudProviderInterface -from .exceptions import AuthenticationException, UserProvisioningException +from .exceptions import AuthenticationException, UserProvisioningException, SecretException from .models import ( - SubscriptionCreationCSPPayload, - SubscriptionCreationCSPResult, - SubscriptionVerificationCSPPayload, - SuscriptionVerificationCSPResult, AdminRoleDefinitionCSPPayload, AdminRoleDefinitionCSPResult, ApplicationCSPPayload, @@ -27,12 +23,16 @@ from .models import ( EnvironmentCSPPayload, EnvironmentCSPResult, KeyVaultCredentials, + PrincipalAdminRoleCSPPayload, + PrincipalAdminRoleCSPResult, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, ProductPurchaseVerificationCSPResult, - PrincipalAdminRoleCSPPayload, - PrincipalAdminRoleCSPResult, + SubscriptionCreationCSPPayload, + SubscriptionCreationCSPResult, + SubscriptionVerificationCSPPayload, + SuscriptionVerificationCSPResult, TaskOrderBillingCreationCSPPayload, TaskOrderBillingCreationCSPResult, TaskOrderBillingVerificationCSPPayload, @@ -54,7 +54,6 @@ from .models import ( ) from .policy import AzurePolicyManager - # This needs to be a fully pathed role definition identifier, not just a UUID # TODO: Extract these from sdk msrestazure.azure_cloud import AZURE_PUBLIC_CLOUD AZURE_SKU_ID = "0001" # probably a static sku specific to ATAT/JEDI @@ -117,11 +116,15 @@ class AzureCloudProvider(CloudProviderInterface): ) try: return secret_client.set_secret(secret_key, secret_value) - except self.exceptions.HttpResponseError: + except self.sdk.exceptions.HttpResponseError as exc: app.logger.error( f"Could not SET secret in Azure keyvault for key {secret_key}.", exc_info=1, ) + raise SecretException( + f"Could not SET secret in Azure keyvault for key {secret_key}.", + exc.message, + ) def get_secret(self, secret_key): credential = self._get_client_secret_credential_obj() @@ -130,11 +133,15 @@ class AzureCloudProvider(CloudProviderInterface): ) try: return secret_client.get_secret(secret_key).value - except self.exceptions.HttpResponseError: + except self.sdk.exceptions.HttpResponseError: app.logger.error( f"Could not GET secret in Azure keyvault for key {secret_key}.", exc_info=1, ) + raise SecretException( + f"Could not GET secret in Azure keyvault for key {secret_key}.", + exc.message, + ) def create_environment(self, payload: EnvironmentCSPPayload): creds = self._source_creds(payload.tenant_id) diff --git a/atst/domain/csp/cloud/exceptions.py b/atst/domain/csp/cloud/exceptions.py index 49b05fb4..3480180f 100644 --- a/atst/domain/csp/cloud/exceptions.py +++ b/atst/domain/csp/cloud/exceptions.py @@ -118,3 +118,17 @@ class BaselineProvisionException(GeneralCSPException): return "Could not complete baseline provisioning for environment ({}): {}".format( self.env_identifier, self.reason ) + + +class SecretException(GeneralCSPException): + """A problem occurred with setting or getting secrets""" + + def __init__(self, tenant_id, reason): + self.tenant_id = tenant_id + self.reason = reason + + @property + def message(self): + return "Could not get or set secret for ({}): {}".format( + self.tenant_id, self.reason + ) From 4d11f7217e1ea24dc951c7fcb14b1368e8014037 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 16:42:38 -0500 Subject: [PATCH 09/12] Add missing test for create_principal_admin_role --- atst/domain/csp/cloud/azure_cloud_provider.py | 6 ++- tests/domain/cloud/test_azure_csp.py | 39 +++++++++++++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 6db3aba3..dc5d92c7 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -6,7 +6,11 @@ from uuid import uuid4 from atst.utils import sha256_hex from .cloud_provider_interface import CloudProviderInterface -from .exceptions import AuthenticationException, UserProvisioningException, SecretException +from .exceptions import ( + AuthenticationException, + UserProvisioningException, + SecretException, +) from .models import ( AdminRoleDefinitionCSPPayload, AdminRoleDefinitionCSPResult, diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 2b55d1a5..2c946f2b 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -22,6 +22,8 @@ from atst.domain.csp.cloud.models import ( BillingProfileVerificationCSPResult, EnvironmentCSPPayload, EnvironmentCSPResult, + PrincipalAdminRoleCSPPayload, + PrincipalAdminRoleCSPResult, ProductPurchaseCSPPayload, ProductPurchaseCSPResult, ProductPurchaseVerificationCSPPayload, @@ -577,10 +579,10 @@ def test_create_tenant_principal_credential(mock_azure: AzureCloudProvider): def test_create_admin_role_definition(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, - "_get_elevated_management_token", - wraps=mock_azure._get_elevated_management_token, - ) as get_elevated_management_token: - get_elevated_management_token.return_value = "my fake token" + "_get_tenant_admin_token", + wraps=mock_azure._get_tenant_admin_token, + ) as get_tenant_admin_token: + get_tenant_admin_token.return_value = "my fake token" mock_result = Mock() mock_result.ok = True @@ -661,6 +663,35 @@ def test_create_tenant_principal_ownership(mock_azure: AzureCloudProvider): assert result.principal_owner_assignment_id == "id" +def test_create_principal_admin_role(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_tenant_admin_token", + wraps=mock_azure._get_tenant_admin_token, + ) as get_tenant_admin_token: + get_tenant_admin_token.return_value = "my fake token" + + mock_result = Mock() + mock_result.ok = True + mock_result.json.return_value = {"id": "id"} + + mock_azure.sdk.requests.post.return_value = mock_result + + payload = PrincipalAdminRoleCSPPayload( + **{ + "tenant_id": uuid4().hex, + "principal_id": "6d2d2d6c-a6d6-41e1-8bb1-73d11475f8f4", + "admin_role_def_id": uuid4().hex, + } + ) + + result: PrincipalAdminRoleCSPResult = mock_azure.create_principal_admin_role( + payload + ) + + assert result.principal_assignment_id == "id" + + def test_create_subscription_creation(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, From 350e648bebebbe869b9f55183ac71b6b9850edf5 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 17:39:31 -0500 Subject: [PATCH 10/12] Added tests for get and set secret --- atst/domain/csp/cloud/azure_cloud_provider.py | 11 ----- tests/domain/cloud/test_azure_csp.py | 45 +++++++++++++++++++ 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 22551ed2..0874e0d2 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -813,17 +813,6 @@ class AzureCloudProvider(CloudProviderInterface): return self._ok() - def create_billing_alerts(self, TBD): - # TODO: Add azure-mgmt-consumption for Budget and Notification entities/operations - # TODO: Determine how to auth against that API using the SDK, doesn't seeem possible at the moment - # TODO: billing alerts are registered as Notifications on Budget objects, which have start/end dates - # TODO: determine what the keys in the Notifications dict are supposed to be - # we may need to rotate budget objects when new TOs/CLINs are reported? - - # 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): # we really should be using graph.microsoft.com, but i'm getting # "expired token" errors for that diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index c58944b9..8744ca5c 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -172,6 +172,27 @@ def test_create_tenant(mock_azure: AzureCloudProvider): assert body.tenant_id == "60ff9d34-82bf-4f21-b565-308ef0533435" +def test_create_tenant_fails(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.json.return_value = {"error": "body"} + mock_result.status_code = 403 + mock_azure.sdk.requests.post.return_value = mock_result + payload = TenantCSPPayload( + **dict( + user_id="admin", + password="JediJan13$coot", # pragma: allowlist secret + domain_name="jediccpospawnedtenant2", + first_name="Tedry", + last_name="Tenet", + country_code="US", + password_recovery_email_address="thomas@promptworks.com", + ) + ) + mock_azure = mock_get_secret(mock_azure) + result = mock_azure.create_tenant(payload) + assert result.get("status") == "error" + + def test_create_billing_profile_creation(mock_azure: AzureCloudProvider): mock_azure.sdk.adal.AuthenticationContext.return_value.context.acquire_token_with_client_credentials.return_value = { "accessToken": "TOKEN" @@ -831,3 +852,27 @@ def test_get_reporting_data_malformed_payload(mock_azure: AzureCloudProvider): from_date="foo", to_date="bar", **malformed_payload, ) ) + +def test_get_secret(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_client_secret_credential_obj", + wraps=mock_azure._get_client_secret_credential_obj, + ) as _get_client_secret_credential_obj: + _get_client_secret_credential_obj.return_value = {} + + mock_azure.sdk.secrets.SecretClient.return_value.get_secret.return_value.value = "my secret" + + assert mock_azure.get_secret("secret key") == "my secret" + +def test_set_secret(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_client_secret_credential_obj", + wraps=mock_azure._get_client_secret_credential_obj, + ) as _get_client_secret_credential_obj: + _get_client_secret_credential_obj.return_value = {} + + mock_azure.sdk.secrets.SecretClient.return_value.set_secret.return_value = "my secret" + + assert mock_azure.set_secret("secret key", "secret_value") == "my secret" \ No newline at end of file From aaa4b401b57be998f2945a05ece634c43c820066 Mon Sep 17 00:00:00 2001 From: tomdds Date: Tue, 4 Feb 2020 17:42:08 -0500 Subject: [PATCH 11/12] Fix LGTM issues --- atst/domain/csp/cloud/cloud_provider_interface.py | 2 +- atst/domain/csp/cloud/mock_cloud_provider.py | 5 ----- tests/domain/cloud/test_azure_csp.py | 12 +++++++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/atst/domain/csp/cloud/cloud_provider_interface.py b/atst/domain/csp/cloud/cloud_provider_interface.py index d173396a..ac652b5c 100644 --- a/atst/domain/csp/cloud/cloud_provider_interface.py +++ b/atst/domain/csp/cloud/cloud_provider_interface.py @@ -11,7 +11,7 @@ class CloudProviderInterface: def root_creds(self) -> Dict: raise NotImplementedError() - def create_environment(self, auth_credentials: Dict, user, environment) -> str: + def create_environment(self, payload): """Create a new environment in the CSP. Arguments: diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index 4e4532d3..bb2f4941 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -452,11 +452,6 @@ class MockCloudProvider(CloudProviderInterface): id=f"{AZURE_MGMNT_PATH}{payload.management_group_name}" ) - def create_user(self, payload: UserCSPPayload): - self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) - - return UserCSPResult(id=str(uuid4())) - def create_environment(self, payload: EnvironmentCSPPayload): self._maybe_raise(self.UNAUTHORIZED_RATE, GeneralCSPException) diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 8744ca5c..7ef1e67b 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -853,6 +853,7 @@ def test_get_reporting_data_malformed_payload(mock_azure: AzureCloudProvider): ) ) + def test_get_secret(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, @@ -861,10 +862,13 @@ def test_get_secret(mock_azure: AzureCloudProvider): ) as _get_client_secret_credential_obj: _get_client_secret_credential_obj.return_value = {} - mock_azure.sdk.secrets.SecretClient.return_value.get_secret.return_value.value = "my secret" + mock_azure.sdk.secrets.SecretClient.return_value.get_secret.return_value.value = ( + "my secret" + ) assert mock_azure.get_secret("secret key") == "my secret" + def test_set_secret(mock_azure: AzureCloudProvider): with patch.object( AzureCloudProvider, @@ -873,6 +877,8 @@ def test_set_secret(mock_azure: AzureCloudProvider): ) as _get_client_secret_credential_obj: _get_client_secret_credential_obj.return_value = {} - mock_azure.sdk.secrets.SecretClient.return_value.set_secret.return_value = "my secret" + mock_azure.sdk.secrets.SecretClient.return_value.set_secret.return_value = ( + "my secret" + ) - assert mock_azure.set_secret("secret key", "secret_value") == "my secret" \ No newline at end of file + assert mock_azure.set_secret("secret key", "secret_value") == "my secret" From 13aca270ca378e25b50e6107116080648ef1fe62 Mon Sep 17 00:00:00 2001 From: tomdds Date: Wed, 5 Feb 2020 14:48:54 -0500 Subject: [PATCH 12/12] Remove unused code in both the cloud interfaces and environment models. Also add tests for some untested code in the cloud interface. --- atst/domain/csp/cloud/azure_cloud_provider.py | 43 +-------- .../csp/cloud/cloud_provider_interface.py | 27 ------ atst/domain/csp/cloud/mock_cloud_provider.py | 17 ---- atst/domain/environment_roles.py | 2 +- atst/domain/environments.py | 2 +- atst/models/environment.py | 17 ---- atst/routes/applications/settings.py | 1 - tests/domain/cloud/test_azure_csp.py | 88 +++++++++++++++---- tests/domain/cloud/test_mock_csp.py | 11 --- tests/domain/test_environment_roles.py | 4 +- tests/models/test_environments.py | 12 --- 11 files changed, 79 insertions(+), 145 deletions(-) diff --git a/atst/domain/csp/cloud/azure_cloud_provider.py b/atst/domain/csp/cloud/azure_cloud_provider.py index 0874e0d2..1d921fde 100644 --- a/atst/domain/csp/cloud/azure_cloud_provider.py +++ b/atst/domain/csp/cloud/azure_cloud_provider.py @@ -153,9 +153,9 @@ class AzureCloudProvider(CloudProviderInterface): creds = self._source_creds(payload.tenant_id) credentials = self._get_credential_obj( { - "client_id": creds.root_sp_client_id, - "secret_key": creds.root_sp_key, - "tenant_id": creds.root_tenant_id, + "client_id": creds.tenant_sp_client_id, + "secret_key": creds.tenant_sp_key, + "tenant_id": creds.tenant_id, }, resource=self.sdk.cloud.endpoints.resource_manager, ) @@ -169,43 +169,6 @@ class AzureCloudProvider(CloudProviderInterface): return EnvironmentCSPResult(**response) - def create_atat_admin_user( - self, auth_credentials: Dict, csp_environment_id: str - ) -> Dict: - root_creds = self._root_creds - credentials = self._get_credential_obj(root_creds) - - sub_client = self.sdk.subscription.SubscriptionClient(credentials) - subscription = sub_client.subscriptions.get(csp_environment_id) - - managment_principal = self._get_management_service_principal() - - auth_client = self.sdk.authorization.AuthorizationManagementClient( - credentials, - # TODO: Determine which subscription this needs to point at - # Once we're in a multi-sub environment - subscription.id, - ) - - # Create role assignment for - role_assignment_id = str(uuid4()) - role_assignment_create_params = auth_client.role_assignments.models.RoleAssignmentCreateParameters( - role_definition_id=REMOTE_ROOT_ROLE_DEF_ID, - principal_id=managment_principal.id, - ) - - auth_client.role_assignments.create( - scope=f"/subscriptions/{subscription.id}/", - role_assignment_name=role_assignment_id, - parameters=role_assignment_create_params, - ) - - return { - "csp_user_id": managment_principal.object_id, - "credentials": managment_principal.password_credentials, - "role_name": role_assignment_id, - } - def create_application(self, payload: ApplicationCSPPayload): creds = self._source_creds(payload.tenant_id) credentials = self._get_credential_obj( diff --git a/atst/domain/csp/cloud/cloud_provider_interface.py b/atst/domain/csp/cloud/cloud_provider_interface.py index ac652b5c..88b55f96 100644 --- a/atst/domain/csp/cloud/cloud_provider_interface.py +++ b/atst/domain/csp/cloud/cloud_provider_interface.py @@ -31,33 +31,6 @@ class CloudProviderInterface: """ raise NotImplementedError() - def create_atat_admin_user( - self, auth_credentials: Dict, csp_environment_id: str - ) -> Dict: - """Creates a new, programmatic user in the CSP. Grants this user full permissions to administer - the CSP. - - Arguments: - auth_credentials -- Object containing CSP account credentials - csp_environment_id -- ID of the CSP Environment the admin user should be created in - - Returns: - object: Object representing new remote admin user, including credentials - Something like: - { - "user_id": string, - "credentials": dict, # structure TBD based on csp - } - - Raises: - AuthenticationException: Problem with the credentials - AuthorizationException: Credentials not authorized for current action(s) - ConnectionException: Issue with the CSP API connection - UnknownServerException: Unknown issue on the CSP side - UserProvisioningException: Problem creating the root user - """ - raise NotImplementedError() - def create_or_update_user( self, auth_credentials: Dict, user_info, csp_role_id: str ) -> str: diff --git a/atst/domain/csp/cloud/mock_cloud_provider.py b/atst/domain/csp/cloud/mock_cloud_provider.py index bb2f4941..7ec0636f 100644 --- a/atst/domain/csp/cloud/mock_cloud_provider.py +++ b/atst/domain/csp/cloud/mock_cloud_provider.py @@ -117,23 +117,6 @@ class MockCloudProvider(CloudProviderInterface): subscription_id="subscriptions/60fbbb72-0516-4253-ab18-c92432ba3230" ) - def create_atat_admin_user(self, auth_credentials, csp_environment_id): - self._authorize(auth_credentials) - - self._delay(1, 5) - self._maybe_raise(self.NETWORK_FAILURE_PCT, self.NETWORK_EXCEPTION) - self._maybe_raise(self.SERVER_FAILURE_PCT, self.SERVER_EXCEPTION) - self._maybe_raise( - self.ATAT_ADMIN_CREATE_FAILURE_PCT, - UserProvisioningException( - csp_environment_id, "atat_admin", "Could not create admin user." - ), - ) - - self._maybe_raise(self.UNAUTHORIZED_RATE, self.AUTHORIZATION_EXCEPTION) - - return {"id": self._id(), "credentials": self._auth_credentials} - def create_tenant(self, payload: TenantCSPPayload): """ payload is an instance of TenantCSPPayload data class diff --git a/atst/domain/environment_roles.py b/atst/domain/environment_roles.py index a022b932..f0b600c6 100644 --- a/atst/domain/environment_roles.py +++ b/atst/domain/environment_roles.py @@ -106,7 +106,7 @@ class EnvironmentRoles(object): def disable(cls, environment_role_id): environment_role = EnvironmentRoles.get_by_id(environment_role_id) - if environment_role.csp_user_id and not environment_role.environment.is_pending: + if environment_role.csp_user_id and not environment_role.environment.cloud_id: tenant_id = environment_role.environment.application.portfolio.csp_data.get( "tenant_id" ) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index f945f3f8..b43ae495 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -126,7 +126,7 @@ class Environments(object): results = ( cls.base_provision_query(now) .filter(Application.cloud_id != None) - .filter(Environment.cloud_id == None) + .filter(Environment.cloud_id.is_(None)) .all() ) return [id_ for id_, in results] diff --git a/atst/models/environment.py b/atst/models/environment.py index 8408c874..cd202fd0 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -1,5 +1,3 @@ -from enum import Enum - from sqlalchemy import Column, ForeignKey, String, UniqueConstraint from sqlalchemy.orm import relationship @@ -43,10 +41,6 @@ class Environment( ), ) - class ProvisioningStatus(Enum): - PENDING = "pending" - COMPLETED = "completed" - @property def users(self): return {r.application_role.user for r in self.roles} @@ -67,17 +61,6 @@ class Environment( def portfolio_id(self): return self.application.portfolio_id - @property - def provisioning_status(self) -> ProvisioningStatus: - if self.cloud_id is None: - return self.ProvisioningStatus.PENDING - else: - return self.ProvisioningStatus.COMPLETED - - @property - def is_pending(self): - return self.provisioning_status == self.ProvisioningStatus.PENDING - def __repr__(self): return "".format( self.name, diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 8b744b04..d1ca1d44 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -39,7 +39,6 @@ def get_environments_obj_for_app(application): { "id": env.id, "name": env.name, - "pending": env.is_pending, "edit_form": EditEnvironmentForm(obj=env), "member_count": len(env.roles), "members": sorted( diff --git a/tests/domain/cloud/test_azure_csp.py b/tests/domain/cloud/test_azure_csp.py index 7ef1e67b..3a25f849 100644 --- a/tests/domain/cloud/test_azure_csp.py +++ b/tests/domain/cloud/test_azure_csp.py @@ -2,11 +2,11 @@ import json from unittest.mock import Mock, patch from uuid import uuid4 +import pendulum +import pydantic import pytest from tests.factories import ApplicationFactory, EnvironmentFactory from tests.mock_azure import AUTH_CREDENTIALS, mock_azure -import pendulum -import pydantic from atst.domain.csp.cloud import AzureCloudProvider from atst.domain.csp.cloud.models import ( @@ -52,6 +52,7 @@ from atst.domain.csp.cloud.models import ( TenantPrincipalCSPResult, TenantPrincipalOwnershipCSPPayload, TenantPrincipalOwnershipCSPResult, + UserCSPPayload, ) BILLING_ACCOUNT_NAME = "52865e4c-52e8-5a6c-da6b-c58f0814f06f:7ea5de9d-b8ce-4901-b1c5-d864320c7b03_2019-05-31" @@ -107,20 +108,6 @@ def test_create_application_succeeds(mock_azure: AzureCloudProvider): assert result.id == "Test Id" -def test_create_atat_admin_user_succeeds(mock_azure: AzureCloudProvider): - environment_id = str(uuid4()) - - csp_user_id = str(uuid4) - - mock_azure.sdk.graphrbac.GraphRbacManagementClient.return_value.service_principals.create.return_value.object_id = ( - csp_user_id - ) - - result = mock_azure.create_atat_admin_user(AUTH_CREDENTIALS, environment_id) - - assert result.get("csp_user_id") == csp_user_id - - def test_create_policy_definition_succeeds(mock_azure: AzureCloudProvider): subscription_id = str(uuid4()) management_group_id = str(uuid4()) @@ -882,3 +869,72 @@ def test_set_secret(mock_azure: AzureCloudProvider): ) assert mock_azure.set_secret("secret key", "secret_value") == "my secret" + + +def test_create_active_directory_user(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.ok = True + mock_result.json.return_value = {"id": "id"} + mock_azure.sdk.requests.post.return_value = mock_result + + payload = UserCSPPayload( + tenant_id=uuid4().hex, + display_name="Test Testerson", + tenant_host_name="testtenant", + email="test@testerson.test", + password="asdfghjkl", # pragma: allowlist secret + ) + + result = mock_azure._create_active_directory_user("token", payload) + + assert result.id == "id" + + +def test_update_active_directory_user_email(mock_azure: AzureCloudProvider): + mock_result = Mock() + mock_result.ok = True + mock_azure.sdk.requests.patch.return_value = mock_result + + payload = UserCSPPayload( + tenant_id=uuid4().hex, + display_name="Test Testerson", + tenant_host_name="testtenant", + email="test@testerson.test", + password="asdfghjkl", # pragma: allowlist secret + ) + + result = mock_azure._update_active_directory_user_email( + "token", uuid4().hex, payload + ) + + assert result + + +def test_create_user(mock_azure: AzureCloudProvider): + with patch.object( + AzureCloudProvider, + "_get_tenant_principal_token", + wraps=mock_azure._get_tenant_principal_token, + ) as _get_tenant_principal_token: + _get_tenant_principal_token.return_value = "token" + + mock_result_create = Mock() + mock_result_create.ok = True + mock_result_create.json.return_value = {"id": "id"} + mock_azure.sdk.requests.post.return_value = mock_result_create + + mock_result_update = Mock() + mock_result_update.ok = True + mock_azure.sdk.requests.patch.return_value = mock_result_update + + payload = UserCSPPayload( + tenant_id=uuid4().hex, + display_name="Test Testerson", + tenant_host_name="testtenant", + email="test@testerson.test", + password="asdfghjkl", # pragma: allowlist secret + ) + + result = mock_azure.create_user(payload) + + assert result.id == "id" diff --git a/tests/domain/cloud/test_mock_csp.py b/tests/domain/cloud/test_mock_csp.py index 86727b38..c65e11e1 100644 --- a/tests/domain/cloud/test_mock_csp.py +++ b/tests/domain/cloud/test_mock_csp.py @@ -28,17 +28,6 @@ def test_create_environment(mock_csp: MockCloudProvider): assert isinstance(result, EnvironmentCSPResult) -def test_create_admin_user(mock_csp: MockCloudProvider): - admin_user = mock_csp.create_atat_admin_user(CREDENTIALS, "env_id") - assert isinstance(admin_user["id"], str) - assert isinstance(admin_user["credentials"], dict) - - -def test_create_environment_baseline(mock_csp: MockCloudProvider): - baseline = mock_csp.create_atat_admin_user(CREDENTIALS, "env_id") - assert isinstance(baseline, dict) - - def test_create_or_update_user(mock_csp: MockCloudProvider): env_role = EnvironmentRoleFactory.create() csp_user_id = mock_csp.create_or_update_user(CREDENTIALS, env_role, "csp_role_id") diff --git a/tests/domain/test_environment_roles.py b/tests/domain/test_environment_roles.py index 77f6033a..e91a837d 100644 --- a/tests/domain/test_environment_roles.py +++ b/tests/domain/test_environment_roles.py @@ -93,7 +93,7 @@ def test_disable_completed(application_role, environment): def test_disable_checks_env_provisioning_status(session): environment = EnvironmentFactory.create() - assert environment.is_pending + assert not environment.cloud_id env_role1 = EnvironmentRoleFactory.create(environment=environment) env_role1 = EnvironmentRoles.disable(env_role1.id) assert env_role1.disabled @@ -103,7 +103,7 @@ def test_disable_checks_env_provisioning_status(session): session.commit() session.refresh(environment) - assert not environment.is_pending + assert environment.cloud_id env_role2 = EnvironmentRoleFactory.create(environment=environment) env_role2 = EnvironmentRoles.disable(env_role2.id) assert env_role2.disabled diff --git a/tests/models/test_environments.py b/tests/models/test_environments.py index 8f8ac626..ed55d737 100644 --- a/tests/models/test_environments.py +++ b/tests/models/test_environments.py @@ -51,18 +51,6 @@ def test_audit_event_for_environment_deletion(session): assert after -@pytest.mark.parametrize( - "env_data,expected_status", - [ - [{"cloud_id": None,}, Environment.ProvisioningStatus.PENDING], - [{"cloud_id": 1}, Environment.ProvisioningStatus.COMPLETED], - ], -) -def test_environment_provisioning_status(env_data, expected_status): - environment = EnvironmentFactory.create(**env_data) - assert environment.provisioning_status == expected_status - - def test_environment_roles_do_not_include_deleted(): member_list = [ {"role_name": CSPRole.ADMIN},