search

nozaq / terraform-aws-secure-baseline

Terraform module to set up your AWS account with the secure baseline configuration based on CIS Amazon Web Services Foundations and AWS Foundational Security Best Practices.
MIT License
1.14k stars 372 forks source link

Bucket.tf code is dependent on aws_organization #99

Closed Prophecy67 closed 3 years ago

Prophecy67 commented 4 years ago

The creation of an audit log bucket is currently dependent on an AWS organization.

Right now, if an account is not the start of the OU, it is impossible to create a master account and adding members to them, since said master account needs access to the aws_organization, which is impossible, if it's not at the top of the OU.

This is an issue for accounts already created, and in an already established OU. In addition, the master account would be the one that collects the logs instead of a dedicated 'logs' account, separating it with other access restrictions and permissions such as how AWS Landing Zone does things.

A better set-up would be that the bucket module doesn't make use of the aws_organization_organizations datasource, and using the member_accounts input, leaving free choice as to what accounts to grant access.

Or optionalizing this, since this is the only part of the code that makes use of reading out all the account-IDs related to the organization.

nozaq commented 4 years ago

@Prophecy67 Thank you for the suggestion! It's originally designed to rely on AWS Organization to use the organization trail feature, but AWS seems to recommend having a separated account for log aggregation then enabling CloudTrail in each account which send events to the log aggregation account. I agree to let users to choose which account to aggregate logs rather than forcing to use the master account for this.

It seems you're working on the PR already, I'd be happy to review when it becomes ready 👍

omerfsen commented 4 years ago

I think @Prophecy67 has made a contribution on #100 for this already.

nozaq commented 4 years ago

@omerfsen yes #100 is already open, but still in draft status 👀

Prophecy67 commented 4 years ago

Hey @nozaq , I've been mucking around with the code and eventually ran in to issues that still led to the organization being necessary. Because of that, I didn't really want to press this matter yet and moved the PR in to a draft.

All the more however, is it nicer to iterate over a list of accounts rather than read out the AWS Organization specs altogether! It is ready to be merged, but might require more work!

github-actions[bot] commented 4 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

github-actions[bot] commented 4 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

github-actions[bot] commented 4 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

MaksimKlepikov commented 1 year ago

Can it be reopened? This is a really useful feature when using AWS Control Tower with a dedicated OU for shared accounts and the log archive account in it.

nozaq commented 1 year ago

@kevrat Thanks, I've created #309 to think how this module can support the organization which is managed by AWS Control Tower.

Please add your comment there if you have any suggestion or proposal!