Skip to content

add permission needed by datadog for cache bucket#98

Open
bwmetcalf wants to merge 3 commits intocloudposse:mainfrom
bwmetcalf:add-s3-action-for-cache-bucket
Open

add permission needed by datadog for cache bucket#98
bwmetcalf wants to merge 3 commits intocloudposse:mainfrom
bwmetcalf:add-s3-action-for-cache-bucket

Conversation

@bwmetcalf
Copy link
Contributor

@bwmetcalf bwmetcalf commented Jun 1, 2025

what

We are seeing security signals from Datadog indicating s3:ListBucket is needed by the role accessing the cache bucket created by module "tags_cache_s3_bucket" {}.

User: arn:aws:sts::*********:assumed-role/dev-global-datadog-logs/dev-global-datadog-logs is not authorized to perform: s3:ListBucket on resource: "arn:aws:s3:::dev-global-datadog-logs-cache" because no identity-based policy allows the s3:ListBucket action

why

Ensure Datadog has permissions needed.

@bwmetcalf bwmetcalf requested review from a team as code owners June 1, 2025 14:59
@bwmetcalf bwmetcalf requested review from Gowiem and nitrocode June 1, 2025 14:59
@mergify mergify bot added the triage Needs triage label Jun 1, 2025
@bwmetcalf
Copy link
Contributor Author

@Gowiem @nitrocode Could one of you take a look at this MR? Thanks!

Gowiem
Gowiem previously approved these changes Jun 3, 2025
@Gowiem Gowiem enabled auto-merge (squash) June 3, 2025 18:07
@Gowiem Gowiem added bugfix Change that restores intended behavior patch A minor, backward compatible change and removed triage Needs triage labels Jun 3, 2025
@Gowiem
Copy link
Member

Gowiem commented Jun 3, 2025

/terratest

@Gowiem Gowiem disabled auto-merge June 3, 2025 18:08
@Gowiem Gowiem enabled auto-merge (squash) June 3, 2025 18:08
auto-merge was automatically disabled June 4, 2025 17:03

Head branch was pushed to by a user without write access

@bwmetcalf
Copy link
Contributor Author

hi @Gowiem, thanks for thereview. It turns out datadog is now complaining about not being able to tag:GetResources so I've added this as well.

@bwmetcalf
Copy link
Contributor Author

@Gowiem Any chance you could review this again. I added a new permission that datadog requires.

"s3:PutObject",
"s3:ListObject",
"s3:DeleteObject",
"s3:ListBucket",
Copy link
Member

Choose a reason for hiding this comment

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

Is this the correct statement block to add this? From the problem statement, it doesn't seem like it's related to a forwarder, is it ?

Copy link
Member

Choose a reason for hiding this comment

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

It's interesting that this is also included in a statement block above if s3_logs_enabled is true. I wonder if this would be better to add to the new statement block for tag:GetResources ?

cc @Gowiem

Copy link
Contributor Author

@bwmetcalf bwmetcalf Jul 2, 2025

Choose a reason for hiding this comment

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

The s3:ListBucket error we were getting was reported against the cache bucket, but given that the tag:GetResources applies to all resources it would work in either place, I think.

My fork with these changes seems to have addressed the issue, so I believe the resulting permissions are what we want. The updated permissions where applied to the IAM role in question.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll go through our use case again with this additional context and provide an update. Thanks for the feedback.

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

Labels

bugfix Change that restores intended behavior patch A minor, backward compatible change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants