-
Notifications
You must be signed in to change notification settings - Fork 48
[aws-ints] Align IAM policy naming to match onboarding templates #260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| import json | ||
| import logging | ||
| import hashlib | ||
| from urllib.request import Request | ||
| import urllib | ||
| import cfnresponse | ||
|
|
@@ -10,16 +9,10 @@ | |
| LOGGER.setLevel(logging.INFO) | ||
| API_CALL_SOURCE_HEADER_VALUE = "cfn-iam-permissions" | ||
| CHUNK_SIZE = 150 # Maximum number of IAM permissions per customer managed policy | ||
| BASE_POLICY_PREFIX = "datadog-aws-integration-iam-permissions" | ||
|
|
||
| class DatadogAPIError(Exception): | ||
| pass | ||
|
|
||
| def generate_policy_hash(role_name, account_id): | ||
| """Generate a unique hash for policy naming.""" | ||
| unique_string = f"{role_name}-{account_id}" | ||
| return hashlib.sha256(unique_string.encode()).hexdigest()[:8] | ||
|
|
||
| def get_policy_arn(account_id, policy_name): | ||
| """Generate a policy ARN.""" | ||
| return f"arn:aws:iam::{account_id}:policy/{policy_name}" | ||
|
|
@@ -41,16 +34,18 @@ def fetch_permissions_from_datadog(): | |
|
|
||
| return json_response["data"]["attributes"]["permissions"] | ||
|
|
||
| def cleanup_existing_policies(iam_client, role_name, account_id, base_policy_name, max_policies=10): | ||
| """Clean up existing policies with the base_policy_name prefix""" | ||
| for i in range(max_policies): | ||
| policy_name = f"{base_policy_name}-part{i+1}" | ||
| def cleanup_existing_policies(iam_client, role_name, account_id, max_policies=20): | ||
| """Clean up existing managed policies (both new and legacy naming)""" | ||
| # Clean up policies with ManagedPolicy naming (e.g., DatadogIntegrationRole-ManagedPolicy-1) | ||
| for i in range(1, max_policies + 1): | ||
| policy_name = f"{role_name}-ManagedPolicy-{i}" | ||
| policy_arn = get_policy_arn(account_id, policy_name) | ||
| try: | ||
| iam_client.detach_role_policy( | ||
| RoleName=role_name, | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Detached policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| # Policy to detach doesn't exist | ||
| pass | ||
|
|
@@ -61,23 +56,51 @@ def cleanup_existing_policies(iam_client, role_name, account_id, base_policy_nam | |
| iam_client.delete_policy( | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Deleted policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| # Policy to delete doesn't exist | ||
| pass | ||
| except Exception as e: | ||
| LOGGER.error(f"Error deleting policy {policy_name}: {str(e)}") | ||
|
|
||
| # Clean up legacy hash-based policies if they exist (from old versions of this template) | ||
| legacy_prefixes = ["datadog-aws-integration-iam-permissions"] | ||
| for prefix in legacy_prefixes: | ||
| for i in range(1, max_policies + 1): | ||
| policy_name = f"{prefix}-part{i}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't this missing the hash so it won't match the previous naming convention? |
||
| policy_arn = get_policy_arn(account_id, policy_name) | ||
| try: | ||
| iam_client.detach_role_policy( | ||
| RoleName=role_name, | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Detached legacy policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| pass | ||
| except Exception as e: | ||
| LOGGER.error(f"Error detaching legacy policy {policy_name}: {str(e)}") | ||
|
|
||
| try: | ||
| iam_client.delete_policy( | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Deleted legacy policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| pass | ||
| except Exception as e: | ||
| LOGGER.error(f"Error deleting legacy policy {policy_name}: {str(e)}") | ||
|
|
||
| def handle_delete(event, context, role_name, account_id, base_policy_name): | ||
| def handle_delete(event, context, role_name, account_id): | ||
| """Handle stack deletion.""" | ||
| iam_client = boto3.client('iam') | ||
| try: | ||
| cleanup_existing_policies(iam_client, role_name, account_id, base_policy_name) | ||
| cleanup_existing_policies(iam_client, role_name, account_id) | ||
| cfnresponse.send(event, context, cfnresponse.SUCCESS, responseData={}) | ||
| except Exception as e: | ||
| LOGGER.error(f"Error deleting policy: {str(e)}") | ||
| cfnresponse.send(event, context, cfnresponse.FAILED, responseData={"Message": str(e)}) | ||
|
|
||
| def handle_create_update(event, context, role_name, account_id, base_policy_name): | ||
| def handle_create_update(event, context, role_name, account_id): | ||
| """Handle stack creation or update.""" | ||
| try: | ||
| # Fetch and chunk permissions | ||
|
|
@@ -86,12 +109,11 @@ def handle_create_update(event, context, role_name, account_id, base_policy_name | |
|
|
||
| # Clean up existing policies | ||
| iam_client = boto3.client('iam') | ||
| cleanup_existing_policies(iam_client, role_name, account_id, base_policy_name) | ||
| cleanup_existing_policies(iam_client, role_name, account_id) | ||
|
|
||
| # Create and attach new policies | ||
| for i, chunk in enumerate(permission_chunks): | ||
| # Create policy | ||
| policy_name = f"{base_policy_name}-part{i+1}" | ||
| policy_name = f"{role_name}-ManagedPolicy-{i+1}" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe move this to a get policy method that you can share so we don't get out of sync again with the format for creation and deletion |
||
| policy_document = { | ||
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
|
|
@@ -128,10 +150,8 @@ def handler(event, context): | |
|
|
||
| role_name = event['ResourceProperties']['DatadogIntegrationRole'] | ||
| account_id = event['ResourceProperties']['AccountId'] | ||
| unique_hash = generate_policy_hash(role_name, account_id) | ||
| base_policy_name = f"{BASE_POLICY_PREFIX}-{unique_hash}" | ||
|
|
||
| if event['RequestType'] == 'Delete': | ||
| handle_delete(event, context, role_name, account_id, base_policy_name) | ||
| handle_delete(event, context, role_name, account_id) | ||
| else: | ||
| handle_create_update(event, context, role_name, account_id, base_policy_name) | ||
| handle_create_update(event, context, role_name, account_id) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ Resources: | |
| - iam:DetachRolePolicy | ||
| Resource: | ||
| - !Sub arn:aws:iam::${AWS::AccountId}:role/${DatadogIntegrationRole} | ||
| - !Sub arn:aws:iam::${AWS::AccountId}:policy/${DatadogIntegrationRole}-ManagedPolicy-* | ||
| - !Sub arn:aws:iam::${AWS::AccountId}:policy/datadog-aws-integration-iam-permissions-* | ||
| - !Sub "arn:${AWS::Partition}:iam::aws:policy/SecurityAudit" | ||
| DatadogAttachIntegrationPermissionsFunction: | ||
|
|
@@ -54,12 +55,11 @@ Resources: | |
| ApplicationLogLevel: "INFO" | ||
| LogFormat: "JSON" | ||
| Runtime: "python3.11" | ||
| Timeout: 5 | ||
| Timeout: 120 | ||
| Code: | ||
| ZipFile: | | ||
| import json | ||
| import logging | ||
| import hashlib | ||
| from urllib.request import Request | ||
| import urllib | ||
| import cfnresponse | ||
|
|
@@ -69,16 +69,10 @@ Resources: | |
| LOGGER.setLevel(logging.INFO) | ||
| API_CALL_SOURCE_HEADER_VALUE = "cfn-iam-permissions" | ||
| CHUNK_SIZE = 150 # Maximum number of IAM permissions per customer managed policy | ||
| BASE_POLICY_PREFIX = "datadog-aws-integration-iam-permissions" | ||
|
|
||
| class DatadogAPIError(Exception): | ||
| pass | ||
|
|
||
| def generate_policy_hash(role_name, account_id): | ||
| """Generate a unique hash for policy naming.""" | ||
| unique_string = f"{role_name}-{account_id}" | ||
| return hashlib.sha256(unique_string.encode()).hexdigest()[:8] | ||
|
|
||
| def get_policy_arn(account_id, policy_name): | ||
| """Generate a policy ARN.""" | ||
| return f"arn:aws:iam::{account_id}:policy/{policy_name}" | ||
|
|
@@ -100,16 +94,18 @@ Resources: | |
|
|
||
| return json_response["data"]["attributes"]["permissions"] | ||
|
|
||
| def cleanup_existing_policies(iam_client, role_name, account_id, base_policy_name, max_policies=10): | ||
| """Clean up existing policies with the base_policy_name prefix""" | ||
| for i in range(max_policies): | ||
| policy_name = f"{base_policy_name}-part{i+1}" | ||
| def cleanup_existing_policies(iam_client, role_name, account_id, max_policies=20): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this just a copy paste from above or does this get generated or something? |
||
| """Clean up existing managed policies (both new and legacy naming)""" | ||
| # Clean up policies with ManagedPolicy naming (e.g., DatadogIntegrationRole-ManagedPolicy-1) | ||
| for i in range(1, max_policies + 1): | ||
| policy_name = f"{role_name}-ManagedPolicy-{i}" | ||
| policy_arn = get_policy_arn(account_id, policy_name) | ||
| try: | ||
| iam_client.detach_role_policy( | ||
| RoleName=role_name, | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Detached policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| # Policy to detach doesn't exist | ||
| pass | ||
|
|
@@ -120,23 +116,51 @@ Resources: | |
| iam_client.delete_policy( | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Deleted policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| # Policy to delete doesn't exist | ||
| pass | ||
| except Exception as e: | ||
| LOGGER.error(f"Error deleting policy {policy_name}: {str(e)}") | ||
|
|
||
| # Clean up legacy hash-based policies if they exist (from old versions of this template) | ||
| legacy_prefixes = ["datadog-aws-integration-iam-permissions"] | ||
| for prefix in legacy_prefixes: | ||
| for i in range(1, max_policies + 1): | ||
| policy_name = f"{prefix}-part{i}" | ||
| policy_arn = get_policy_arn(account_id, policy_name) | ||
| try: | ||
| iam_client.detach_role_policy( | ||
| RoleName=role_name, | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Detached legacy policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| pass | ||
| except Exception as e: | ||
| LOGGER.error(f"Error detaching legacy policy {policy_name}: {str(e)}") | ||
|
|
||
| def handle_delete(event, context, role_name, account_id, base_policy_name): | ||
| try: | ||
| iam_client.delete_policy( | ||
| PolicyArn=policy_arn | ||
| ) | ||
| LOGGER.info(f"Deleted legacy policy {policy_name}") | ||
| except iam_client.exceptions.NoSuchEntityException: | ||
| pass | ||
| except Exception as e: | ||
| LOGGER.error(f"Error deleting legacy policy {policy_name}: {str(e)}") | ||
|
|
||
| def handle_delete(event, context, role_name, account_id): | ||
| """Handle stack deletion.""" | ||
| iam_client = boto3.client('iam') | ||
| try: | ||
| cleanup_existing_policies(iam_client, role_name, account_id, base_policy_name) | ||
| cleanup_existing_policies(iam_client, role_name, account_id) | ||
| cfnresponse.send(event, context, cfnresponse.SUCCESS, responseData={}) | ||
| except Exception as e: | ||
| LOGGER.error(f"Error deleting policy: {str(e)}") | ||
| cfnresponse.send(event, context, cfnresponse.FAILED, responseData={"Message": str(e)}) | ||
|
|
||
| def handle_create_update(event, context, role_name, account_id, base_policy_name): | ||
| def handle_create_update(event, context, role_name, account_id): | ||
| """Handle stack creation or update.""" | ||
| try: | ||
| # Fetch and chunk permissions | ||
|
|
@@ -145,12 +169,11 @@ Resources: | |
|
|
||
| # Clean up existing policies | ||
| iam_client = boto3.client('iam') | ||
| cleanup_existing_policies(iam_client, role_name, account_id, base_policy_name) | ||
| cleanup_existing_policies(iam_client, role_name, account_id) | ||
|
|
||
| # Create and attach new policies | ||
| for i, chunk in enumerate(permission_chunks): | ||
| # Create policy | ||
| policy_name = f"{base_policy_name}-part{i+1}" | ||
| policy_name = f"{role_name}-ManagedPolicy-{i+1}" | ||
| policy_document = { | ||
| "Version": "2012-10-17", | ||
| "Statement": [ | ||
|
|
@@ -171,6 +194,7 @@ Resources: | |
| RoleName=role_name, | ||
| PolicyArn=policy['Policy']['Arn'] | ||
| ) | ||
|
|
||
| # Attach the SecurityAudit policy | ||
| iam_client.attach_role_policy( | ||
| RoleName=role_name, | ||
|
|
@@ -186,13 +210,11 @@ Resources: | |
|
|
||
| role_name = event['ResourceProperties']['DatadogIntegrationRole'] | ||
| account_id = event['ResourceProperties']['AccountId'] | ||
| unique_hash = generate_policy_hash(role_name, account_id) | ||
| base_policy_name = f"{BASE_POLICY_PREFIX}-{unique_hash}" | ||
|
|
||
| if event['RequestType'] == 'Delete': | ||
| handle_delete(event, context, role_name, account_id, base_policy_name) | ||
| handle_delete(event, context, role_name, account_id) | ||
| else: | ||
| handle_create_update(event, context, role_name, account_id, base_policy_name) | ||
| handle_create_update(event, context, role_name, account_id) | ||
| DatadogAttachIntegrationPermissionsFunctionTrigger: | ||
| Type: Custom::DatadogAttachIntegrationPermissionsFunctionTrigger | ||
| Properties: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| v1.1.0 | ||
| v1.2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this expected to have more prefixes to deal with or can we eliminate the loop to reduce nesting?