Skip to content

Conversation

@raymondeah
Copy link
Contributor

Note: Please remember to review the contribution guidelines
if you have not yet done so.

What does this PR do?

Changes the naming of the iam policies created by the aws_attach_integration_permissions template to match the naming in our quickstart and organizations templates

  • Before: datadog-aws-integration-iam-permissions-{hash}-part1, part2, etc.
  • After: {RoleName}-ManagedPolicy-1, {RoleName}-ManagedPolicy-2, etc.

Motivation

A customer noticed that we are creating policies with duplicate permissions since the naming is not aligned

Testing Guidelines

How did you test this pull request?

  • Integrated a new account using the quickstart template
  • Ran this template, verified that the existing managed policies were successfully deleted and recreated

Additional Notes

Anything else we should know when reviewing?

@raymondeah raymondeah requested a review from a team as a code owner November 21, 2025 18:28
Copy link

@ksirrah13 ksirrah13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the legacy clean up won't work since the old policy won't match right now.

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"]

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?

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}"

Choose a reason for hiding this comment

The 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?

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}"

Choose a reason for hiding this comment

The 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

"""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):

Choose a reason for hiding this comment

The 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants