search

terraform-aws-modules / terraform-aws-datadog-forwarders

Terraform module to create resources on AWS to forward logs/metrics to Datadog 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/datadog-forwarders/aws
Apache License 2.0
55 stars 41 forks source link

fix: Append Object Modifier for "s3:GetObject" #2

Closed kylegoch closed 3 years ago

kylegoch commented 3 years ago

Description

Change the IAM Resource to apply to the objects in the bucket. (/* required)

Motivation and Context

The DD Lambda will throw an access denied error as is. Adding /* in the IAM policy resolves this issue, as s3:GetObject is an object level action, and thus the lambda needs access to the objects.

How Has This Been Tested?

Tested locally, by applying to my DD Log Forwarder setup to confirm the IAM policy appeared correctly.

Screenshots (if appropriate):

kylegoch commented 3 years ago

Sorry for the extemporaneous commits. Thats what I get for trying to do this in the GH UI.

bryantbiggs commented 3 years ago

hi @kylegoch - thanks for the PR! I think the way the module is written allows for the most flexibility and avoids making assumptions. meaning, if we accept this change, then all users are forced into a convention. I think to solve your situation, the best route would be to pass in the * where you see fit for your use case. for example:

    s3_log_bucket_arns = ["arn:aws:s3:::my_bucket/*", "arn:aws:s3:::sensitive_bucket/some/prefix/*"]

you can pass in the bucket ARN scoped with the level of access you wish to permit for the log forwarder. some users will be ok with giving access to the whole bucket, but its quite common for users to want to give only limited access to a specific prefix. I hope this helps clarify things

kylegoch commented 3 years ago

Interesting. I would maybe add a line in the docs/Readme on that. I assumed if gave the module the ARN it would take care of the rest. Its a super easy fix on my end to add /* to the ARN list. Just wasnt aware I needed to until I went digging.

bryantbiggs commented 3 years ago

yes you're right @kylegoch - I can add something in to demonstrate/highlight this

github-actions[bot] commented 1 year ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.