Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: Account for cases when we are using an existing cloudwatch log group for flow logs #1118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: Account for cases when we are using an existing cloudwatch log group for flow logs #1118

wants to merge 1 commit into from

Conversation

danielmklein
Copy link

@danielmklein danielmklein commented Sep 4, 2024 •
edited
Loading

Description

When generating the flow log group ARNs to include in the policy, if we are using a pre-existing log group, take that into account and use the destination ARN passed in as a variable, rather than assuming that we created a log group ourselves.

Motivation and Context

Since 5.12.0, if you are using an existing CloudWatch Log Group for your flow logs destination ARN, this module tries to update the relevant IAM policy with an invalid policy document, which fails. See PR #1088.

This fixes #1117

Breaking Changes

None

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
    I have tested this in our own workspaces using a fork of this module -- I am happy to go further with the examples here if needed/desired.
  • I have executed pre-commit run -a on my pull request

@danielmklein danielmklein changed the title account for cases when we are using an existing cloudwatch log group for flow logs fix: account for cases when we are using an existing cloudwatch log group for flow logs Sep 4, 2024
@danielmklein danielmklein marked this pull request as ready for review September 4, 2024 18:32
@danielmklein danielmklein changed the title fix: account for cases when we are using an existing cloudwatch log group for flow logs fix: Account for cases when we are using an existing cloudwatch log group for flow logs Sep 5, 2024
@danielmklein
Copy link
Author

Hiya @antonbabenko, apologies for poking, but is there anything I can do to help ferry this along? Am I missing anything here?

@danielmklein
Copy link
Author

@antonbabenko / @bryantbiggs -- sorry to re-bump, but I'd love to get this upstream. Please let me know if I'm missing anything critical here!

@github-actions GitHub Actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Oct 27, 2024
@staticaland
Copy link

This is still relevant ✌️

@github-actions github-actions bot removed the stale label Oct 29, 2024
@github-actions GitHub Actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Nov 28, 2024
@danielmklein
Copy link
Author

✨ still valid! ✨

@github-actions github-actions bot removed the stale label Nov 29, 2024
@github-actions GitHub Actions
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Dec 29, 2024
@danielmklein
Copy link
Author

Still relevant.

@github-actions github-actions bot removed the stale label Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

when using an existing cloudwatch log group for flow logs, vpc_flow_log_cloudwatch policy breaks
2 participants
@danielmklein @staticaland