From f2fbd5a1e402256995a298aab2a3ff059ed53c36 Mon Sep 17 00:00:00 2001 From: Sagar Yadav Date: Wed, 22 May 2024 12:09:40 +0530 Subject: [PATCH 1/3] initial commit to enable multiple SCPs --- .../adf-build/organization_policy.py | 149 ++++++++++-------- .../adf-build/shared/python/organizations.py | 21 +-- 2 files changed, 89 insertions(+), 81 deletions(-) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py index bab013907..b10aacbae 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py @@ -22,10 +22,10 @@ def __init__(self): pass @staticmethod - def _find_all(policy): + def _find_all_paths(policy): _files = list( glob.iglob( - f"./adf-bootstrap/**/{policy}.json", + f"./adf-bootstrap/**/{policy}*.json", recursive=True, ) ) @@ -40,12 +40,16 @@ def _compare_ordered_policy(self, obj): return obj @staticmethod - def _trim_scp_file_name(policy): - return policy[1:][:-8] if policy[1:][:-8] == "/" else policy[2:][:-9] - - @staticmethod - def _trim_tagging_policy_file_name(policy): - return policy[1:][:-19] if policy[1:][:-19] == "/" else policy[2:][:-20] + def _trim_policy_file_name(policy): + # Returns policy target and filename as tuple + policy_filename = policy.split('/')[-1] + # `policy` should be formatted "./{target/path/here}/{policy_filename.json}" + # Target is defined by {target/path/here} + policy_target = '/'.join(policy.split('/')[1:-1]) + if policy_target == '': + # if {target/path/here} is empty, target is root + policy_target = '/' + return policy_target, policy_filename @staticmethod def _is_govcloud(region: str) -> bool: @@ -56,6 +60,17 @@ def _is_govcloud(region: str) -> bool: :return: Returns True if the region is GovCloud, False otherwise. """ return region.startswith("us-gov") + + @staticmethod + def return_policy_name(policy_type, target_path, policy_filename): + _type = 'scp' if policy_type == "SERVICE_CONTROL_POLICY" else 'tagging-policy' + if policy_filename != f'{_type}.json': + #filter the policy name to remove the .json extension + policy_filename = policy_filename.split('.')[0] + return 'adf-{0}-{1}-{2}'.format(_type, target_path, policy_filename) + else: + # Added for backwards-compatibility with previous versions of ADF + return 'adf-{0}-{1}'.format(_type, target_path) @staticmethod def set_scp_attachment(access_identifier, organization_mapping, path, organizations): @@ -85,10 +100,12 @@ def set_scp_attachment(access_identifier, organization_mapping, path, organizati @staticmethod def clean_and_remove_policy_attachment( - organization_mapping, path, organizations, policy_type + organization_mapping, path, policy_filename, organizations, policy_type ): + policy_name = OrganizationPolicy.return_policy_name(policy_type, path, policy_filename) policy_id = organizations.describe_policy_id_for_target( organization_mapping[path], + policy_name, policy_type, ) if policy_type == "SERVICE_CONTROL_POLICY": @@ -99,14 +116,11 @@ def clean_and_remove_policy_attachment( ) except organizations.client.exceptions.DuplicatePolicyAttachmentException: pass - organizations.detach_policy(policy_id, organization_mapping[path]) - organizations.delete_policy(policy_id) - LOGGER.info( - "Policy (%s) %s will be deleted. Path is: %s", - policy_type, - organization_mapping[path], - path, - ) + if policy_id: + organizations.detach_policy(policy_id, organization_mapping[path]) + organizations.delete_policy(policy_id) + LOGGER.info('Policy (%s) %s will be deleted. Name is %s', + policy_type, organization_mapping[path], policy_name) def apply( self, organizations, parameter_store, config @@ -130,73 +144,75 @@ def apply( } ) - supported_policies = ["scp", "tagging-policy"] + supported_policies = ["SERVICE_CONTROL_POLICY", "TAG_POLICY"] if self._is_govcloud(REGION_DEFAULT): - supported_policies = ["scp"] + supported_policies = ["SERVICE_CONTROL_POLICY"] - for policy in supported_policies: - _type = "SERVICE_CONTROL_POLICY" if policy == "scp" else "TAG_POLICY" - organizations.enable_organization_policies(_type) - _policies = OrganizationPolicy._find_all(policy) + for policy_type in supported_policies: + _type = 'scp' if policy_type == "SERVICE_CONTROL_POLICY" else 'tagging-policy' + organizations.enable_organization_policies(policy_type) + policy_paths = OrganizationPolicy._find_all_paths(_type) + LOGGER.info( + "Policy_paths are %s", + policy_paths + ) try: current_stored_policy = ast.literal_eval( - parameter_store.fetch_parameter(policy.replace('-', '_')) + parameter_store.fetch_parameter(_type) + ) + LOGGER.info( + "current_stored_policy for type %s is: %s", + _type, + current_stored_policy, ) + for stored_policy in current_stored_policy: - path = ( - OrganizationPolicy._trim_scp_file_name(stored_policy) - if policy == "scp" - else OrganizationPolicy._trim_tagging_policy_file_name( - stored_policy - ) - ) + path, policy_filename = OrganizationPolicy._trim_policy_file_name(stored_policy) OrganizationPolicy.set_scp_attachment( config.get("scp"), organization_mapping, path, organizations ) - if stored_policy not in _policies: + if stored_policy not in policy_paths: + path, policy_filename = OrganizationPolicy._trim_policy_file_name(stored_policy) OrganizationPolicy.clean_and_remove_policy_attachment( organization_mapping, path, + policy_filename, organizations, - _type, - ) + policy_type + ) except ParameterNotFoundError: LOGGER.debug( "Parameter %s was not found in Parameter Store, continuing.", - policy, + _type, ) - for _policy in _policies: - path = ( - OrganizationPolicy._trim_scp_file_name(_policy) - if policy == "scp" - else OrganizationPolicy._trim_tagging_policy_file_name( - _policy, - ) - ) + for policy_path in policy_paths: + path, policy_filename = OrganizationPolicy._trim_policy_file_name(policy_path) + policy_name = OrganizationPolicy.return_policy_name(policy_type, path, policy_filename) policy_id = organizations.describe_policy_id_for_target( organization_mapping[path], - _type, + policy_name, + policy_type ) - proposed_policy = Organizations.get_policy_body(_policy) + proposed_policy = Organizations.get_policy_body(policy_path) if policy_id: current_policy = organizations.describe_policy(policy_id) if self._compare_ordered_policy( current_policy.get("Content") ) == self._compare_ordered_policy(proposed_policy): LOGGER.info( - "Policy (%s) %s does not require updating. Path is: %s", - policy, + "Policy (%s) %s does not require updating. Name is: %s", + _type, organization_mapping[path], - path, + policy_name, ) continue LOGGER.info( - "Policy (%s) will be updated for %s. Path is: %s", - policy, + "Policy (%s) will be updated for %s. Name is: %s", + _type, organization_mapping[path], - path, + policy_name, ) organizations.update_policy( proposed_policy, @@ -206,14 +222,14 @@ def apply( try: policy_id = organizations.create_policy( proposed_policy, - path, - _type, + policy_name, + policy_type ) LOGGER.info( - "Policy (%s) has been created for %s. Path is: %s", - policy, + "Policy (%s) has been created for %s. Name is: %s", + _type, organization_mapping[path], - path, + policy_name ) organizations.attach_policy( policy_id, @@ -221,24 +237,21 @@ def apply( ) except organizations.client.exceptions.DuplicatePolicyAttachmentException: LOGGER.info( - "Policy (%s) for %s exists and is attached already.", - policy, + "Policy (%s) for %s with name %s exists and is attached already.", + _type, organization_mapping[path], + policy_name ) except organizations.client.exceptions.DuplicatePolicyException: LOGGER.info( - "Policy (%s) for %s exists ensuring attached.", - policy, + "Policy (%s) for %s with name %s exists ensuring attached.", + _type, organization_mapping[path], + policy_name ) policy_id = organizations.list_policies( - f"adf-{policy}-{path}", - _type, + policy_name, + _type ) organizations.attach_policy(policy_id, organization_mapping[path]) - parameter_store.put_parameter( - # Make the key consistently use underscores instead of dashes. - # So tagging-policy gets changed into tagging_policy. - policy.replace('-', '_'), - str(_policies), - ) + parameter_store.put_parameter(_type, str(policy_paths)) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py index afdaa72a3..c35d583e6 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py @@ -177,16 +177,14 @@ def update_policy(self, content, policy_id): def create_policy( self, content, - ou_path, + policy_name, policy_type="SERVICE_CONTROL_POLICY", ): - policy_type_name = ( - "scp" if policy_type == "SERVICE_CONTROL_POLICY" else "tagging-policy" - ) + response = self.client.create_policy( Content=content, Description=f"ADF Managed {policy_type}", - Name=f"adf-{policy_type_name}-{ou_path}", + Name=policy_name, Type=policy_type, ) return response["Policy"]["PolicySummary"]["Id"] @@ -207,19 +205,16 @@ def list_policies(self, name, policy_type="SERVICE_CONTROL_POLICY"): def describe_policy_id_for_target( self, target_id, + policy_name, policy_type="SERVICE_CONTROL_POLICY", ): response = self.client.list_policies_for_target( TargetId=target_id, Filter=policy_type ) - adf_managed_policies = [ - policy - for policy in response["Policies"] - if f"ADF Managed {policy_type}" in policy["Description"] - ] - if adf_managed_policies: - return adf_managed_policies[0]["Id"] - return [] + try: + return [p for p in response['Policies'] if 'ADF Managed {0}'.format(policy_type) in p['Description'] and p['Name'] == policy_name][0]['Id'] + except IndexError: + return [] def describe_policy(self, policy_id): response = self.client.describe_policy( From dcc0d5fc3f9757bdf7ff8c9d76adb01d2c9bd5c5 Mon Sep 17 00:00:00 2001 From: itsnotsagar Date: Fri, 24 May 2024 19:27:40 +0530 Subject: [PATCH 2/3] fixed linting issue --- .../adf-build/organization_policy.py | 21 ++++++++++++------- .../adf-build/shared/python/organizations.py | 7 +++++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py index b10aacbae..8fd3cd53e 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py @@ -60,17 +60,16 @@ def _is_govcloud(region: str) -> bool: :return: Returns True if the region is GovCloud, False otherwise. """ return region.startswith("us-gov") - + @staticmethod def return_policy_name(policy_type, target_path, policy_filename): _type = 'scp' if policy_type == "SERVICE_CONTROL_POLICY" else 'tagging-policy' if policy_filename != f'{_type}.json': #filter the policy name to remove the .json extension policy_filename = policy_filename.split('.')[0] - return 'adf-{0}-{1}-{2}'.format(_type, target_path, policy_filename) - else: - # Added for backwards-compatibility with previous versions of ADF - return 'adf-{0}-{1}'.format(_type, target_path) + return f'adf-{_type}-{target_path}-{policy_filename}' + # Added for backwards-compatibility with previous versions of ADF + return f'adf-{_type}-{target_path}' @staticmethod def set_scp_attachment(access_identifier, organization_mapping, path, organizations): @@ -173,14 +172,16 @@ def apply( config.get("scp"), organization_mapping, path, organizations ) if stored_policy not in policy_paths: - path, policy_filename = OrganizationPolicy._trim_policy_file_name(stored_policy) + path, policy_filename = OrganizationPolicy._trim_policy_file_name( + stored_policy + ) OrganizationPolicy.clean_and_remove_policy_attachment( organization_mapping, path, policy_filename, organizations, policy_type - ) + ) except ParameterNotFoundError: LOGGER.debug( "Parameter %s was not found in Parameter Store, continuing.", @@ -189,7 +190,11 @@ def apply( for policy_path in policy_paths: path, policy_filename = OrganizationPolicy._trim_policy_file_name(policy_path) - policy_name = OrganizationPolicy.return_policy_name(policy_type, path, policy_filename) + policy_name = OrganizationPolicy.return_policy_name( + policy_type, + path, + policy_filename + ) policy_id = organizations.describe_policy_id_for_target( organization_mapping[path], policy_name, diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py index c35d583e6..3ef837998 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py @@ -180,7 +180,7 @@ def create_policy( policy_name, policy_type="SERVICE_CONTROL_POLICY", ): - + response = self.client.create_policy( Content=content, Description=f"ADF Managed {policy_type}", @@ -212,7 +212,10 @@ def describe_policy_id_for_target( TargetId=target_id, Filter=policy_type ) try: - return [p for p in response['Policies'] if 'ADF Managed {0}'.format(policy_type) in p['Description'] and p['Name'] == policy_name][0]['Id'] + return [ + p for p in response['Policies'] + if f'ADF Managed {policy_type}' in p['Description'] and p['Name'] == policy_name + ][0]['Id'] except IndexError: return [] From 45c2cf2b25aeeb492e7cbe0e214f074b62843c9f Mon Sep 17 00:00:00 2001 From: itsnotsagar Date: Fri, 24 May 2024 19:40:45 +0530 Subject: [PATCH 3/3] fixed linting --- .../bootstrap_repository/adf-build/organization_policy.py | 4 ++-- .../adf-build/shared/python/organizations.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py index 8fd3cd53e..0f9c28efe 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/organization_policy.py @@ -191,8 +191,8 @@ def apply( for policy_path in policy_paths: path, policy_filename = OrganizationPolicy._trim_policy_file_name(policy_path) policy_name = OrganizationPolicy.return_policy_name( - policy_type, - path, + policy_type, + path, policy_filename ) policy_id = organizations.describe_policy_id_for_target( diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py index 3ef837998..b0f6dfe75 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organizations.py @@ -213,7 +213,7 @@ def describe_policy_id_for_target( ) try: return [ - p for p in response['Policies'] + p for p in response['Policies'] if f'ADF Managed {policy_type}' in p['Description'] and p['Name'] == policy_name ][0]['Id'] except IndexError: