From 590ff8babec1dbe5819addb503fdfdc450b83167 Mon Sep 17 00:00:00 2001 From: Simon Kok Date: Thu, 28 Mar 2024 14:21:20 +0100 Subject: [PATCH 1/3] Feat change to static bootstrap stack names **Why?** Initially, ADF would generate bootstrap stack names that included the name of the OU at the end. For example, for an OU named banking, it would generate the global `adf-global-base-banking` stack. This, however, makes it harder to harden ADF. As it would need access rights to deploy and manage CloudFormation stacks with a wildcard at the end. Instead of listing a limited number of stack names. Additionally, it makes it harder to write an SCP to limit who can update these stacks as well. **What?** * Instead of using the OU name, the bootstrap stacks will be named: `adf-(global|regional)-base-bootstrap`. * Exception being the `adf-(global|regional)-base-deployment` stack, as this stack contains the resources that ADF needs to operate. As well as the `adf-global-base-adf-build` stack that gets deployed to the management account. Renaming these stacks would require uninstalling ADF and reinstalling it from scratch. Hence these are kept as-is. * Tightened the IAM policies that grant access to manage the bootstrap stacks. * Added a functionality to delete deprecated stacks automatically and upgrade to the new stack name via the `aws-deployment-framework-bootstrap` pipeline. * When a deprecated bootstrap stack is deleted, it will first delete the global-iam stack if required. As the global-iam stack adds policies to the roles that are created in the bootstrap stack. Therefore, the global-iam stack should be removed before the bootstrap stack can be deleted in the global region. * Fix CloudFormation Stack/ChangeSet waiter error capture, to report back the account, region, and stack name that ran into a failure when needed. --- .../bootstrap_repository/adf-build/global.yml | 25 +- .../bootstrap_repository/adf-build/main.py | 3 + .../adf-build/shared/python/cloudformation.py | 224 +++++++++- .../python/tests/stubs/stub_cloudformation.py | 108 +++++ .../python/tests/test_cloudformation.py | 407 +++++++++++++++++- src/template.yml | 33 +- 6 files changed, 758 insertions(+), 42 deletions(-) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/global.yml b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/global.yml index 426e6a473..d1637fcc0 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/global.yml +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/global.yml @@ -86,11 +86,12 @@ Resources: - cloudformation:CreateChangeSet - cloudformation:CreateStack - cloudformation:CreateUploadBucket - - cloudformation:DeleteStack - cloudformation:DeleteChangeSet - - cloudformation:DescribeStacks + - cloudformation:DeleteStack - cloudformation:DescribeChangeSet + - cloudformation:DescribeStacks - cloudformation:ExecuteChangeSet + - cloudformation:ListStacks - cloudformation:SetStackPolicy - cloudformation:SignalResource - cloudformation:UpdateStack @@ -125,15 +126,16 @@ Resources: - !Sub "arn:${AWS::Partition}:ssm:*:${AWS::AccountId}:parameter/adf/*" - Effect: Allow Action: - - iam:CreateRole - iam:CreatePolicy - - iam:UpdateAssumeRolePolicy + - iam:CreateRole + - iam:DeleteRole + - iam:DeleteRolePolicy - iam:GetRole - iam:GetRolePolicy - - iam:DeleteRole - - iam:TagRole - iam:PutRolePolicy - - iam:DeleteRolePolicy + - iam:TagRole + - iam:UntagRole + - iam:UpdateAssumeRolePolicy Resource: - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-cloudformation-role" - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-cloudformation-deployment-role" @@ -141,7 +143,14 @@ Resources: - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-automation-role" - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-readonly-automation-role" - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-update-cross-account-access-role" - - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/${CrossAccountAccessRole}" - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-terraform-role" + - Effect: "Allow" + Action: + - iam:DeleteRole + - iam:DeleteRolePolicy + - iam:UntagRole + Resource: + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/${CrossAccountAccessRole}" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/${CrossAccountAccessRole}-readonly" Roles: - !Ref OrganizationsRole diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/main.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/main.py index c4ce3777f..8bdc26282 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/main.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/main.py @@ -310,6 +310,7 @@ def worker_thread( account_id=account_id ) try: + cloudformation.delete_deprecated_base_stacks() cloudformation.create_stack() if region == config.deployment_account_region: cloudformation.create_iam_stack() @@ -498,6 +499,7 @@ def main(): # pylint: disable=R0915 s3_key_path="adf-bootstrap/" + account_path, account_id=deployment_account_id ) + cloudformation.delete_deprecated_base_stacks() cloudformation.create_stack() update_deployment_account_output_parameters( deployment_account_region=config.deployment_account_region, @@ -520,6 +522,7 @@ def main(): # pylint: disable=R0915 s3_key_path='adf-build', account_id=ACCOUNT_ID ) + cloudformation.delete_deprecated_base_stacks() cloudformation.create_stack() threads = [] account_ids = [ diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py index 39d466838..dbff7e07b 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py @@ -8,7 +8,7 @@ import re import os -from botocore.exceptions import WaiterError, ClientError +from botocore.exceptions import WaiterError, ClientError, ValidationError from botocore.config import Config import tenacity @@ -27,6 +27,8 @@ # A stack name can contain only alphanumeric characters (case sensitive) # and hyphens. CFN_UNACCEPTED_CHARS = re.compile(r"[^-a-zA-Z0-9]") +ADF_GLOBAL_IAM_STACK_NAME = 'adf-global-base-iam' +ADF_GLOBAL_ADF_BUILD_STACK_NAME = 'adf-global-base-adf-build' class StackProperties: @@ -62,6 +64,30 @@ class StackProperties: 'DELETE_IN_PROGRESS': 'stack_delete_complete', 'REVIEW_IN_PROGRESS': 'change_set_create_complete', } + all_except_deleted_states = [ + 'CREATE_IN_PROGRESS', + 'CREATE_FAILED', + 'CREATE_COMPLETE', + 'ROLLBACK_IN_PROGRESS', + 'ROLLBACK_FAILED', + 'ROLLBACK_COMPLETE', + 'DELETE_IN_PROGRESS', + 'DELETE_FAILED', + 'UPDATE_IN_PROGRESS', + 'UPDATE_COMPLETE_CLEANUP_IN_PROGRESS', + 'UPDATE_COMPLETE', + 'UPDATE_FAILED', + 'UPDATE_ROLLBACK_IN_PROGRESS', + 'UPDATE_ROLLBACK_FAILED', + 'UPDATE_ROLLBACK_COMPLETE_CLEANUP_IN_PROGRESS', + 'UPDATE_ROLLBACK_COMPLETE', + 'REVIEW_IN_PROGRESS', + 'IMPORT_IN_PROGRESS', + 'IMPORT_COMPLETE', + 'IMPORT_ROLLBACK_IN_PROGRESS', + 'IMPORT_ROLLBACK_FAILED', + 'IMPORT_ROLLBACK_COMPLETE', + ] def __init__( self, @@ -109,9 +135,21 @@ def get_parameters(self): return [] def _get_stack_name(self): - raw_stack_name = f'adf-{self._get_geo_prefix()}-base-{self.ou_name}' + stack_suffix = ( + self.ou_name if self.ou_name in ['deployment', 'adf-build'] + else 'bootstrap' + ) + raw_stack_name = f'adf-{self._get_geo_prefix()}-base-{stack_suffix}' return CFN_UNACCEPTED_CHARS.sub("-", raw_stack_name) + def _get_valid_stack_names(self): + valid_stack_names = [self._get_stack_name()] + if self.region == self.deployment_account_region: + valid_stack_names.append(ADF_GLOBAL_IAM_STACK_NAME) + valid_stack_names.append(ADF_GLOBAL_ADF_BUILD_STACK_NAME) + + return valid_stack_names + class WaitException(Exception): pass @@ -196,7 +234,7 @@ def _wait_stack(self, waiter_type, stack_name): 'MaxAttempts': 45 } ) - except ClientError as client_error: + except (WaiterError, ClientError) as client_error: LOGGER.error( "%s in %s - Failed to wait for stack %s error %s", self.account_id, @@ -226,14 +264,18 @@ def _wait_change_set(self): 'MaxAttempts': 20 } ) - except ClientError as client_error: - LOGGER.error( - "%s in %s - Failed to wait for change set of %s error %s", - self.account_id, - self.region, - self.stack_name, - client_error, - ) + except (WaiterError, ClientError) as error: + if not CloudFormation._change_set_failed_due_to_empty( + error.last_response["Status"], + error.last_response["StatusReason"], + ): + LOGGER.error( + "%s in %s - Failed to wait for change set of %s error %s", + self.account_id, + self.region, + self.stack_name, + error, + ) raise def _get_waiter_type(self): @@ -450,7 +492,7 @@ def create_iam_stack(self): self.template_url = self.s3.fetch_s3_url( self._create_template_path(self.s3_key_path, 'global-iam') ) - self.stack_name = 'adf-global-base-iam' + self.stack_name = ADF_GLOBAL_IAM_STACK_NAME self._wait_if_in_progress() waiter = self._get_waiter_type() create_change_set = self._create_change_set() @@ -496,17 +538,153 @@ def get_stack_regional_outputs(self): } def delete_all_base_stacks(self, wait_override=False): - for stack in paginator(self.client.list_stacks): - if bool( - re.search( - 'adf-(global|regional)-base', - stack.get('StackName'))): - if stack.get( - 'StackStatus') in StackProperties.clean_stack_status: - LOGGER.warning( - 'Removing Stack: %s', - stack.get('StackName')) - self.delete_stack(stack.get('StackName'), wait_override) + self._delete_base_stacks( + wait_override=wait_override, + ) + + def delete_deprecated_base_stacks(self): + self._delete_base_stacks( + wait_override=True, + deprecated_only=True, + ) + + def _delete_base_stacks( + self, + wait_override=False, + deprecated_only=False, + ): + deleted_any = False + bootstrap_stack_found = False + for stack in paginator( + self.client.list_stacks, + StackStatusFilter=StackProperties.all_except_deleted_states, + ): + matches_search = bool( + re.search( + 'adf-(global|regional)-base', + stack.get('StackName'), + ) + ) + if not matches_search: + continue + if len(stack.get('ParentId', '')) > 0: + # Skip nested stacks + continue + + if deleted_any and stack.get('StackName') == ADF_GLOBAL_IAM_STACK_NAME: + # We deleted the IAM stack already + continue + + should_be_deleted = ( + not deprecated_only + or stack.get('StackName') not in self._get_valid_stack_names() + ) + if not should_be_deleted: + if stack.get('StackName') != ADF_GLOBAL_IAM_STACK_NAME: + bootstrap_stack_found = True + continue + + if stack.get('StackStatus') == 'DELETE_COMPLETE': + # Nothing to do here + continue + + LOGGER.debug( + 'Base stack should be deleted: %s', + stack.get('StackName'), + ) + + should_delete_iam_stack = ( + not deleted_any + and self.region == self.deployment_account_region + and stack.get('StackName') != ADF_GLOBAL_IAM_STACK_NAME + ) + if should_delete_iam_stack: + # Remove the IAM stack before deleting an ADF global stack + # If we are deleting a bootstrap stack, we need to assume this + # might hosts the roles that get policies attached by the + # global-iam stack. Since the policies need to be deleted + # before one can delete the role, we need to delete the global + # IAM stack first. + self._delete_iam_stack_if_exists() + + self._delete_stack_or_instruct_user( + stack_name=stack.get('StackName'), + stack_status=stack.get('StackStatus'), + wait_override=wait_override, + ) + deleted_any = True + + if deprecated_only and not bootstrap_stack_found and not deleted_any: + # If we did not find any bootstrap stack but we did run into the + # global IAM stack, then we should delete the global IAM stack. + # As the policies that the CloudFormation stack manages would + # need to be recreated and applied to new IAM Roles as created + # by a upcoming bootstrap stack. + self._delete_iam_stack_if_exists() + + def _get_stack_status(self, name): + try: + LOGGER.debug( + "%s in %s - Retrieve stack status of: %s", + self.account_id, + self.region, + name, + ) + response = self.client.describe_stacks( + StackName=name, + ) + if response and len(response.get('Stacks', [])) > 0: + return response['Stacks'][0]['StackStatus'] + return None + except (ClientError, ValidationError) as error: + if error.response['Error']['Code'] == 'ValidationError': + LOGGER.debug( + "%s in %s - Stack does not exist: %s", + self.account_id, + self.region, + name, + ) + # If the stack does not exist, a ValidationError is raised. + return None # None implies missing + LOGGER.error( + "%s in %s - Retrieve stack status of: %s failed (%s): %s", + self.account_id, + self.region, + name, + error.response['Error']['Code'], + error.response['Error']['Message'], + ) + raise + + def _delete_iam_stack_if_exists(self): + iam_stack_status = self._get_stack_status(ADF_GLOBAL_IAM_STACK_NAME) + if iam_stack_status: + self._delete_stack_or_instruct_user( + stack_name=ADF_GLOBAL_IAM_STACK_NAME, + stack_status=iam_stack_status, + wait_override=True, + ) + + def _delete_stack_or_instruct_user( + self, + stack_name, + stack_status, + wait_override, + ): + clean_stack_status = ( + stack_status in StackProperties.clean_stack_status + ) + if clean_stack_status: + LOGGER.warning('Removing stack: %s', stack_name) + self.delete_stack(stack_name, wait_override) + return + + LOGGER.warning( + 'Please remove stack %s manually, state %s implies that it ' + 'cannot be deleted automatically', + stack_name, + stack_status, + ) def get_stack_output(self, value): try: diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py index d2d26db99..6f26f7565 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py @@ -19,3 +19,111 @@ 'StackStatus': 'CREATE_IN_PROGRESS' }] } + +list_stacks = { + 'StackSummaries': [ + { + # Should be filtered out, not a ADF base stack + 'StackName': 'adf-different-stack', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Should be filtered out when deleting deprecated base stacks + # This is current, not deprecated + 'StackName': 'adf-global-base-bootstrap', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Should be filtered out when deleting deprecated base stacks + # This is current, but should only exist in non global regions. + 'StackName': 'adf-regional-base-bootstrap', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Should be filtered out when deleting deprecated base stacks + # This is current, but should only exist in the global deployment + # account. + 'StackName': 'adf-global-base-deployment', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Should be filtered out when deleting deprecated base stacks + # This is current, but should only exist in the global deployment + # account. + 'StackName': ( + 'adf-global-base-deployment-PipelineManagementApplication-156BTR33REBR' + ), + 'StackStatus': 'CREATE_COMPLETE', + 'ParentId': 'Unique-Stack-Id', + }, + { + # Should be deprecated when deleting deprecated base stacks + 'StackName': 'adf-global-base-deployment-SomeOtherStack', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Should be deprecated when deleting deprecated base stacks + 'StackName': 'adf-global-base-bootstrap-SomeNestedStack', + 'StackStatus': 'CREATE_COMPLETE', + 'ParentId': 'Unique-Stack-Id', + }, + { + # Should be filtered out when deleting deprecated base stacks + # This is current, but should only exist in the global management + # account. + 'StackName': 'adf-global-base-adf-build', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Should be filtered out when deleting deprecated base stacks + # This is current, not deprecated + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Using a deprecated OU name in the base stack name, should be + # deleted when deleting deprecated base stacks. + 'StackName': 'adf-global-base-dev', + 'StackStatus': 'CREATE_COMPLETE', + }, + { + # Using a deprecated OU name in the base stack name, should be + # deleted when deleting deprecated base stacks. + # Note the stack status, this should print a warning instead + # of deleting it. + 'StackName': 'adf-global-base-test', + 'StackStatus': 'CREATE_FAILED', + }, + { + # Using a deprecated OU name in the base stack name, should be + # deleted when deleting deprecated base stacks. + # Note the stack status, this should print a warning instead + # of deleting it. + 'StackName': 'adf-global-base-acceptance', + 'StackStatus': 'ROLLBACK_FAILED', + }, + { + # Using a deprecated OU name in the base stack name, should be + # deleted when deleting deprecated base stacks. + 'StackName': 'adf-global-base-prod', + 'StackStatus': 'UPDATE_COMPLETE', + }, + { + # Using a deprecated OU name in the base stack name, should be + # deleted when deleting deprecated base stacks. + # Note the stack status, this should print a warning instead + # of deleting it. + 'StackName': 'adf-global-base-some-ou', + 'StackStatus': 'CREATE_IN_PROGRESS', + }, + { + # Using a deprecated OU name in the base stack name, should be + # deleted when deleting deprecated base stacks. + # Note the stack status, this should print a warning instead + # of deleting it. + 'StackName': 'adf-global-base-some-old-ou', + 'StackStatus': 'DELETE_COMPLETE', + }, + ], + 'NextToken': 'string', +} diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py index 3a767c846..c3118079c 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py @@ -5,9 +5,10 @@ import os import boto3 -from pytest import fixture +from botocore.stub import Stubber +from pytest import fixture, raises from stubs import stub_cloudformation -from mock import Mock +from mock import Mock, call, patch from cloudformation import CloudFormation, StackProperties from s3 import S3 @@ -40,7 +41,7 @@ def global_cls(): stack_name=None, template_url='https://some/path/global.yml', s3=None, - s3_key_path='/some/location', + s3_key_path='adf-bootstrap/some-ou', account_id=123 ) @@ -58,7 +59,67 @@ def test_global_get_geo_prefix(global_cls): def test_global_get_stack_name(global_cls): - assert global_cls.stack_name == 'adf-global-base-location' + assert global_cls.stack_name == 'adf-global-base-bootstrap' + + +def test_global_build_get_stack_name(): + cfn = CloudFormation( + region='us-east-1', + deployment_account_region='us-east-1', + role=boto3, + wait=False, + stack_name=None, + template_url='https://some/path/global.yml', + s3=None, + s3_key_path='adf-build', + account_id=123 + ) + assert cfn.stack_name == 'adf-global-base-adf-build' + + +def test_global_deployment_get_stack_name(): + cfn = CloudFormation( + region='us-east-1', + deployment_account_region='us-east-1', + role=boto3, + wait=False, + stack_name=None, + template_url='https://some/path/global.yml', + s3=None, + s3_key_path='adf-bootstrap/deployment', + account_id=123 + ) + assert cfn.stack_name == 'adf-global-base-deployment' + + +def test_regional_deployment_get_stack_name(): + cfn = CloudFormation( + region='eu-west-1', + deployment_account_region='us-east-1', + role=boto3, + wait=False, + stack_name=None, + template_url='https://some/path/global.yml', + s3=None, + s3_key_path='adf-bootstrap/deployment', + account_id=123 + ) + assert cfn.stack_name == 'adf-regional-base-deployment' + + +def test_regional_target_get_stack_name(): + cfn = CloudFormation( + region='eu-west-1', + deployment_account_region='us-east-1', + role=boto3, + wait=False, + stack_name=None, + template_url='https://some/path/global.yml', + s3=None, + s3_key_path='adf-bootstrap/some/ou/path', + account_id=123 + ) + assert cfn.stack_name == 'adf-regional-base-bootstrap' def test_get_stack_regional_outputs(global_cls): @@ -103,10 +164,344 @@ def test_get_waiter_type_create_complete(global_cls): def test_get_stack_name_remove_unaccepted_chars(): for unaccepted_char in [' ', '%', '$', '*']: props = StackProperties( - region='eu-west-1', + region='eu-central-1', deployment_account_region='eu-west-1', stack_name=None, s3=None, s3_key_path='/some/weird{}location'.format(unaccepted_char), ) - assert props._get_stack_name() == 'adf-global-base-weird-location' + assert props._get_stack_name() == 'adf-regional-base-bootstrap' + + +@patch('cloudformation.LOGGER') +def test_describe_stack_status_success(logger, global_cls): + global_cls.client = Mock() + global_cls.client.describe_stacks.return_value = { + "Stacks": [ + { + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }, + ], + } + response = global_cls._get_stack_status('adf-global-base-iam') + assert response == 'CREATE_COMPLETE' + global_cls.client.describe_stacks.assert_has_calls([ + call(StackName='adf-global-base-iam'), + ]) + assert global_cls.client.describe_stacks.call_count == 1 + logger.error.assert_not_called() + + +@patch('cloudformation.LOGGER') +def test_describe_stack_status_empty_stack_list(logger, global_cls): + global_cls.client = Mock() + global_cls.client.describe_stacks.return_value = { + "Stacks": [] + } + response = global_cls._get_stack_status('adf-global-base-iam') + assert response is None + global_cls.client.describe_stacks.assert_has_calls([ + call(StackName='adf-global-base-iam'), + ]) + assert global_cls.client.describe_stacks.call_count == 1 + logger.error.assert_not_called() + + +@patch('cloudformation.LOGGER') +def test_describe_stack_status_empty_response(logger, global_cls): + global_cls.client = Mock() + global_cls.client.describe_stacks.return_value = None + response = global_cls._get_stack_status('adf-global-base-iam') + assert response is None + global_cls.client.describe_stacks.assert_has_calls([ + call(StackName='adf-global-base-iam'), + ]) + assert global_cls.client.describe_stacks.call_count == 1 + logger.error.assert_not_called() + + +@patch('cloudformation.LOGGER') +def test_describe_stack_status_raises_validation_error(logger, global_cls): + client = boto3.client('cloudformation') + stubber = Stubber(client) + stubber.add_client_error('describe_stacks', service_error_code='ValidationError') + stubber.activate() + global_cls.client = client + response = global_cls._get_stack_status('adf-global-base-iam') + assert response is None + logger.error.assert_not_called() + + +@patch('cloudformation.LOGGER') +def test_describe_stack_status_raises_other_error(logger, global_cls): + client = boto3.client('cloudformation') + stubber = Stubber(client) + stubber.add_client_error('describe_stacks', service_error_code='ClientError') + stubber.activate() + global_cls.client = client + with raises(Exception): + global_cls._get_stack_status('adf-global-base-iam') + logger.error.assert_has_calls([ + call( + "%s in %s - Retrieve stack status of: %s failed (%s): %s", + global_cls.account_id, + global_cls.region, + 'adf-global-base-iam', + 'ClientError', + '', + ) + ]) + + +@patch('cloudformation.LOGGER') +@patch("cloudformation.paginator") +def test_delete_all_base_stacks(paginator_mock, logger, global_cls): + global_cls.client = Mock() + paginator_mock.return_value = stub_cloudformation.list_stacks.get('StackSummaries') + global_cls.client.describe_stacks.return_value = { + "Stacks": [ + { + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }, + ], + } + global_cls.delete_all_base_stacks() + global_cls.client.delete_stack.assert_has_calls([ + call(StackName='adf-global-base-iam'), + call(StackName='adf-global-base-bootstrap'), + call(StackName='adf-regional-base-bootstrap'), + call(StackName='adf-global-base-deployment'), + call(StackName='adf-global-base-deployment-SomeOtherStack'), + call(StackName='adf-global-base-adf-build'), + call(StackName='adf-global-base-dev'), + call(StackName='adf-global-base-test'), + call(StackName='adf-global-base-acceptance'), + call(StackName='adf-global-base-prod'), + ]) + assert global_cls.client.delete_stack.call_count == 10 + logger.warning.assert_has_calls([ + call('Removing stack: %s', 'adf-global-base-iam'), + # ^ We are deploying in a global region, not regional + call('Removing stack: %s', 'adf-global-base-bootstrap'), + call('Removing stack: %s', 'adf-regional-base-bootstrap'), + call('Removing stack: %s', 'adf-global-base-deployment'), + call('Removing stack: %s', 'adf-global-base-deployment-SomeOtherStack'), + call('Removing stack: %s', 'adf-global-base-adf-build'), + call('Removing stack: %s', 'adf-global-base-dev'), + call('Removing stack: %s', 'adf-global-base-test'), + call('Removing stack: %s', 'adf-global-base-acceptance'), + call('Removing stack: %s', 'adf-global-base-prod'), + call( + 'Please remove stack %s manually, state %s implies that it ' + 'cannot be deleted automatically', + 'adf-global-base-some-ou', + 'CREATE_IN_PROGRESS', + ), + ]) + + +@patch('cloudformation.LOGGER') +@patch("cloudformation.paginator") +def test_delete_deprecated_base_stacks_some_deletions(paginator_mock, logger, global_cls): + global_cls.client = Mock() + paginator_mock.return_value = stub_cloudformation.list_stacks.get('StackSummaries') + global_cls.client.describe_stacks.return_value = { + "Stacks": [ + { + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }, + ], + } + global_cls.delete_deprecated_base_stacks() + global_cls.client.delete_stack.assert_has_calls([ + call(StackName='adf-global-base-iam'), + call(StackName='adf-regional-base-bootstrap'), + # ^ We are deploying in a global region, not regional + call(StackName='adf-global-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call(StackName='adf-global-base-deployment-SomeOtherStack'), + call(StackName='adf-global-base-dev'), + call(StackName='adf-global-base-test'), + call(StackName='adf-global-base-acceptance'), + call(StackName='adf-global-base-prod'), + ]) + assert global_cls.client.delete_stack.call_count == 8 + logger.warning.assert_has_calls([ + call('Removing stack: %s', 'adf-global-base-iam'), + # ^ As we delete a bootstrap stack we need to recreate the IAM stack, + # hence deleting it. + call('Removing stack: %s', 'adf-regional-base-bootstrap'), + # ^ We are deploying in a global region, not regional + call('Removing stack: %s', 'adf-global-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call('Removing stack: %s', 'adf-global-base-deployment-SomeOtherStack'), + call('Removing stack: %s', 'adf-global-base-dev'), + call('Removing stack: %s', 'adf-global-base-test'), + call('Removing stack: %s', 'adf-global-base-acceptance'), + call('Removing stack: %s', 'adf-global-base-prod'), + call( + 'Please remove stack %s manually, state %s implies that it ' + 'cannot be deleted automatically', + 'adf-global-base-some-ou', + 'CREATE_IN_PROGRESS', + ), + ]) + + +@patch('cloudformation.LOGGER') +@patch("cloudformation.paginator") +def test_delete_deprecated_base_stacks_no_iam(paginator_mock, logger, global_cls): + global_cls.client = Mock() + paginator_mock.return_value = list(filter( + lambda stack: stack.get('StackName') != 'adf-global-base-iam', + stub_cloudformation.list_stacks.get('StackSummaries'), + )) + global_cls.client.describe_stacks.return_value = { + "Stacks": [], + } + global_cls.delete_deprecated_base_stacks() + global_cls.client.delete_stack.assert_has_calls([ + call(StackName='adf-regional-base-bootstrap'), + # ^ We are deploying in a global region, not regional + call(StackName='adf-global-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call(StackName='adf-global-base-deployment-SomeOtherStack'), + call(StackName='adf-global-base-dev'), + call(StackName='adf-global-base-test'), + call(StackName='adf-global-base-acceptance'), + call(StackName='adf-global-base-prod'), + ]) + assert global_cls.client.delete_stack.call_count == 7 + logger.warning.assert_has_calls([ + call('Removing stack: %s', 'adf-regional-base-bootstrap'), + # ^ We are deploying in a global region, not regional + call('Removing stack: %s', 'adf-global-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call('Removing stack: %s', 'adf-global-base-deployment-SomeOtherStack'), + call('Removing stack: %s', 'adf-global-base-dev'), + call('Removing stack: %s', 'adf-global-base-test'), + call('Removing stack: %s', 'adf-global-base-acceptance'), + call('Removing stack: %s', 'adf-global-base-prod'), + call( + 'Please remove stack %s manually, state %s implies that it ' + 'cannot be deleted automatically', + 'adf-global-base-some-ou', + 'CREATE_IN_PROGRESS', + ), + ]) + + +@patch('cloudformation.LOGGER') +@patch("cloudformation.paginator") +def test_delete_deprecated_base_stacks_all_valid(paginator_mock, logger, global_cls): + global_cls.client = Mock() + paginator_mock.return_value = list(filter( + lambda stack: stack.get('StackName') in [ + 'adf-global-base-bootstrap', + 'adf-global-base-iam', + ], + stub_cloudformation.list_stacks.get('StackSummaries'), + )) + global_cls.client.describe_stacks.return_value = { + "Stacks": [ + { + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }, + ], + } + global_cls.delete_deprecated_base_stacks() + global_cls.client.delete_stack.assert_not_called() + logger.warning.assert_not_called() + + +@patch('cloudformation.LOGGER') +@patch("cloudformation.paginator") +def test_delete_deprecated_base_stacks_only_iam(paginator_mock, logger, global_cls): + global_cls.client = Mock() + paginator_mock.return_value = list(filter( + lambda stack: stack.get('StackName') in [ + 'adf-global-base-iam', + ], + stub_cloudformation.list_stacks.get('StackSummaries'), + )) + global_cls.client.describe_stacks.return_value = { + "Stacks": [ + { + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }, + ], + } + global_cls.delete_deprecated_base_stacks() + global_cls.client.delete_stack.assert_has_calls([ + call(StackName='adf-global-base-iam'), + ]) + assert global_cls.client.delete_stack.call_count == 1 + logger.warning.assert_has_calls([ + call('Removing stack: %s', 'adf-global-base-iam'), + # ^ As the IAM stack cannot live on its own, it should be deleted + ]) + + +@patch('cloudformation.LOGGER') +@patch("cloudformation.paginator") +def test_delete_deprecated_base_stacks_regional(paginator_mock, logger, regional_cls): + regional_cls.client = Mock() + regional_list_stacks = list(map( + lambda stack: { + **stack, + "StackName": ( + stack.get("StackName") + .replace("regional", "tmp") + .replace("global", "regional") + .replace("tmp", "global") + ), + }, + stub_cloudformation.list_stacks.get('StackSummaries'), + )) + regional_list_stacks.append({ + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }) + paginator_mock.return_value = regional_list_stacks + regional_cls.client.describe_stacks.return_value = { + "Stacks": [], + } + regional_cls.delete_deprecated_base_stacks() + regional_cls.client.delete_stack.assert_has_calls([ + call(StackName='adf-global-base-bootstrap'), + # ^ We are deploying in a non-global + call(StackName='adf-regional-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call(StackName='adf-regional-base-deployment-SomeOtherStack'), + call(StackName='adf-regional-base-adf-build'), + call(StackName='adf-regional-base-iam'), + call(StackName='adf-regional-base-dev'), + call(StackName='adf-regional-base-test'), + call(StackName='adf-regional-base-acceptance'), + call(StackName='adf-regional-base-prod'), + ]) + assert regional_cls.client.delete_stack.call_count == 9 + logger.warning.assert_has_calls([ + call('Removing stack: %s', 'adf-global-base-bootstrap'), + # ^ We are deploying in a non-global + call('Removing stack: %s', 'adf-regional-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call('Removing stack: %s', 'adf-regional-base-deployment-SomeOtherStack'), + call('Removing stack: %s', 'adf-regional-base-adf-build'), + call('Removing stack: %s', 'adf-regional-base-iam'), + call('Removing stack: %s', 'adf-regional-base-dev'), + call('Removing stack: %s', 'adf-regional-base-test'), + call('Removing stack: %s', 'adf-regional-base-acceptance'), + call('Removing stack: %s', 'adf-regional-base-prod'), + call( + 'Please remove stack %s manually, state %s implies that it ' + 'cannot be deleted automatically', + 'adf-regional-base-some-ou', + 'CREATE_IN_PROGRESS', + ), + ]) diff --git a/src/template.yml b/src/template.yml index fba734adf..41157a7c5 100644 --- a/src/template.yml +++ b/src/template.yml @@ -1250,6 +1250,7 @@ Resources: Statement: - Effect: "Allow" Action: + - "cloudformation:ListStacks" - "logs:CreateLogGroup" - "logs:CreateLogStream" - "logs:PutLogEvents" @@ -1294,17 +1295,12 @@ Resources: - !Ref AccountBootstrappingStateMachine - Effect: "Allow" Action: - - "cloudformation:CreateStack" - "cloudformation:DescribeChangeSet" - "cloudformation:DeleteStack" - - "cloudformation:UpdateStack" - "cloudformation:CancelUpdateStack" - "cloudformation:ContinueUpdateRollback" - - "cloudformation:CreateChangeSet" - "cloudformation:DeleteChangeSet" - "cloudformation:DescribeStacks" - - "cloudformation:CreateUploadBucket" - - "cloudformation:ExecuteChangeSet" - "cloudformation:SetStackPolicy" - "cloudformation:SignalResource" - "cloudformation:UpdateTerminationProtection" @@ -1312,6 +1308,20 @@ Resources: - !Sub "arn:${AWS::Partition}:cloudformation:*:*:stack/adf-global-base-*/*" - !Sub "arn:${AWS::Partition}:cloudformation:*:*:stack/adf-regional-base-*/*" - !Sub "arn:${AWS::Partition}:cloudformation:*:${AWS::AccountId}:stack/adf-global-base-adf-build/*" + - Effect: "Allow" + Action: + - "cloudformation:CreateStack" + - "cloudformation:UpdateStack" + - "cloudformation:CreateChangeSet" + - "cloudformation:CreateUploadBucket" + - "cloudformation:ExecuteChangeSet" + Resource: + - !Sub "arn:${AWS::Partition}:cloudformation:${DeploymentAccountMainRegion}:*:stack/adf-global-base-bootstrap/*" + - !Sub "arn:${AWS::Partition}:cloudformation:${DeploymentAccountMainRegion}:*:stack/adf-global-base-iam/*" + - !Sub "arn:${AWS::Partition}:cloudformation:${DeploymentAccountMainRegion}:${DeploymentAccount.AccountId}:stack/adf-global-base-deployment/*" + - !Sub "arn:${AWS::Partition}:cloudformation:${DeploymentAccountMainRegion}:${AWS::AccountId}:stack/adf-global-base-adf-build/*" + - !Sub "arn:${AWS::Partition}:cloudformation:*:*:stack/adf-regional-base-bootstrap/*" + - !Sub "arn:${AWS::Partition}:cloudformation:*:${DeploymentAccount.AccountId}:stack/adf-regional-base-deployment/*" - Effect: "Allow" Action: - "s3:DeleteObject" @@ -1349,6 +1359,19 @@ Resources: Resource: - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/${CrossAccountAccessRoleName}" - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/${CrossAccountAccessRoleName}-readonly" + - Effect: "Allow" + Action: + - "iam:DeleteRole" + - "iam:DeleteRolePolicy" + - "iam:UntagRole" + Resource: + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-automation-role" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-cloudformation-deployment-role" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-cloudformation-role" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-codecommit-role" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-readonly-automation-role" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-terraform-role" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/adf-update-cross-account-access-role" CodeCommitRepository: Type: AWS::CodeCommit::Repository From ae86ac51fdfc055b05adfa109d69d991b1ef7a71 Mon Sep 17 00:00:00 2001 From: Simon Kok Date: Fri, 5 Apr 2024 18:24:29 +0200 Subject: [PATCH 2/3] Fix /adf_version param lookup to /adf/adf_version --- .../adf-build/shared/python/cloudformation.py | 2 + .../shared/python/parameter_store.py | 2 +- .../python/tests/test_cloudformation.py | 60 +++++++++++++++++++ .../python/tests/test_parameter_store.py | 2 + 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py index dbff7e07b..fcaf84ef9 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/cloudformation.py @@ -28,6 +28,7 @@ # and hyphens. CFN_UNACCEPTED_CHARS = re.compile(r"[^-a-zA-Z0-9]") ADF_GLOBAL_IAM_STACK_NAME = 'adf-global-base-iam' +ADF_GLOBAL_BOOTSTRAP_STACK_NAME = 'adf-global-base-bootstrap' ADF_GLOBAL_ADF_BUILD_STACK_NAME = 'adf-global-base-adf-build' @@ -146,6 +147,7 @@ def _get_valid_stack_names(self): valid_stack_names = [self._get_stack_name()] if self.region == self.deployment_account_region: valid_stack_names.append(ADF_GLOBAL_IAM_STACK_NAME) + valid_stack_names.append(ADF_GLOBAL_BOOTSTRAP_STACK_NAME) valid_stack_names.append(ADF_GLOBAL_ADF_BUILD_STACK_NAME) return valid_stack_names diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/parameter_store.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/parameter_store.py index 690656dbf..7709fb8f4 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/parameter_store.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/parameter_store.py @@ -94,7 +94,7 @@ def _build_param_name(name, adf_only=True): slash_name = name if name.startswith('/') else f"/{name}" add_prefix = ( adf_only - and not slash_name.startswith(PARAMETER_PREFIX) + and not slash_name.startswith(f"{PARAMETER_PREFIX}/") ) param_prefix = PARAMETER_PREFIX if add_prefix else '' return f"{param_prefix}{slash_name}" diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py index c3118079c..173c68e45 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py @@ -351,6 +351,66 @@ def test_delete_deprecated_base_stacks_some_deletions(paginator_mock, logger, gl ]) +@patch('cloudformation.LOGGER') +@patch("cloudformation.paginator") +def test_delete_deprecated_base_stacks_mgmt_account_adf_build(paginator_mock, logger): + global_cls = CloudFormation( + region='us-east-1', + deployment_account_region='us-east-1', + role=boto3, + wait=False, + stack_name=None, + template_url='https://some/path/global.yml', + s3=None, + s3_key_path='adf-build', + account_id=123 + ) + global_cls.client = Mock() + paginator_mock.return_value = stub_cloudformation.list_stacks.get('StackSummaries') + global_cls.client.describe_stacks.return_value = { + "Stacks": [ + { + 'StackName': 'adf-global-base-iam', + 'StackStatus': 'CREATE_COMPLETE', + }, + ], + } + global_cls.delete_deprecated_base_stacks() + global_cls.client.delete_stack.assert_has_calls([ + call(StackName='adf-global-base-iam'), + call(StackName='adf-regional-base-bootstrap'), + # ^ We are deploying in a global region, not regional + call(StackName='adf-global-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call(StackName='adf-global-base-deployment-SomeOtherStack'), + call(StackName='adf-global-base-dev'), + call(StackName='adf-global-base-test'), + call(StackName='adf-global-base-acceptance'), + call(StackName='adf-global-base-prod'), + ]) + assert global_cls.client.delete_stack.call_count == 8 + logger.warning.assert_has_calls([ + call('Removing stack: %s', 'adf-global-base-iam'), + # ^ As we delete a bootstrap stack we need to recreate the IAM stack, + # hence deleting it. + call('Removing stack: %s', 'adf-regional-base-bootstrap'), + # ^ We are deploying in a global region, not regional + call('Removing stack: %s', 'adf-global-base-deployment'), + # ^ We are not in the deployment OU with this CloudFormation instance + call('Removing stack: %s', 'adf-global-base-deployment-SomeOtherStack'), + call('Removing stack: %s', 'adf-global-base-dev'), + call('Removing stack: %s', 'adf-global-base-test'), + call('Removing stack: %s', 'adf-global-base-acceptance'), + call('Removing stack: %s', 'adf-global-base-prod'), + call( + 'Please remove stack %s manually, state %s implies that it ' + 'cannot be deleted automatically', + 'adf-global-base-some-ou', + 'CREATE_IN_PROGRESS', + ), + ]) + + @patch('cloudformation.LOGGER') @patch("cloudformation.paginator") def test_delete_deprecated_base_stacks_no_iam(paginator_mock, logger, global_cls): diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_parameter_store.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_parameter_store.py index f8e3d277a..aaf0c4e0f 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_parameter_store.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_parameter_store.py @@ -26,6 +26,7 @@ def cls(): [ ('/adf/test', '/adf/test'), ('adf/test', '/adf/test'), + ('/adf_version', '/adf_version'), ('/test', '/test'), ('test', '/test'), ('/other/test', '/other/test'), @@ -44,6 +45,7 @@ def test_build_param_name_not_adf_only(input_name, output_path): [ ('/adf/test', '/adf/test'), ('adf/test', '/adf/test'), + ('/adf_version', '/adf/adf_version'), ('/test', '/adf/test'), ('test', '/adf/test'), ('/other/test', '/adf/other/test'), From 0e5ae1bee8a9ff589090a59c947e0ebea8dcfd2e Mon Sep 17 00:00:00 2001 From: Simon Kok Date: Mon, 8 Apr 2024 13:47:43 +0200 Subject: [PATCH 3/3] Fixup --- .../adf-build/shared/python/tests/stubs/stub_cloudformation.py | 2 +- .../adf-build/shared/python/tests/test_cloudformation.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py index 6f26f7565..1906fbc68 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/stubs/stub_cloudformation.py @@ -51,7 +51,7 @@ # This is current, but should only exist in the global deployment # account. 'StackName': ( - 'adf-global-base-deployment-PipelineManagementApplication-156BTR33REBR' + 'adf-global-base-deployment-PipelineManagementApplication-156BTR33REGR' ), 'StackStatus': 'CREATE_COMPLETE', 'ParentId': 'Unique-Stack-Id', diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py index 173c68e45..650ac06c7 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_cloudformation.py @@ -353,7 +353,7 @@ def test_delete_deprecated_base_stacks_some_deletions(paginator_mock, logger, gl @patch('cloudformation.LOGGER') @patch("cloudformation.paginator") -def test_delete_deprecated_base_stacks_mgmt_account_adf_build(paginator_mock, logger): +def test_delete_deprecated_base_stacks_management_account_adf_build(paginator_mock, logger): global_cls = CloudFormation( region='us-east-1', deployment_account_region='us-east-1',