aws-ia / terraform-aws-control_tower_account_factory

AWS Control Tower Account Factory
Apache License 2.0
624 stars 413 forks source link

S3 buckets for aft_feature_cloudtrail_data_events are re-created, even if aft_feature_cloudtrail_data_events is disabled #304

Open matthewbarreiro opened 1 year ago

matthewbarreiro commented 1 year ago

Terraform Version & Prov:

AFT Version: 1.7.0

Terraform Version & Provider Versions

terraform version: v1.3.6

Click to expand for full version details ``` Terraform v1.3.6 on darwin_amd64 + provider registry.terraform.io/hashicorp/archive v2.2.0 + provider registry.terraform.io/hashicorp/aws v4.48.0 + provider registry.terraform.io/hashicorp/local v2.3.0 + provider registry.terraform.io/hashicorp/random v3.4.3 + provider registry.terraform.io/hashicorp/time v0.9.1 ```

terraform providers

hashicorp/time v0.9.1
hashicorp/aws v4.48.0
hashicorp/local v2.3.0
hashicorp/random v3.4.3
hashicorp/archive v2.2.0
Click to expand for full providers details ``` Providers required by configuration: . ├── provider[registry.terraform.io/hashicorp/aws] 4.48.0 └── module.control_tower_account_factory ├── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0, < 5.0.0 ├── provider[registry.terraform.io/hashicorp/local] ├── module.aft_feature_options │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 ├── module.aft_iam_roles │ ├── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── module.ct_management_service_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── module.log_archive_exec_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── module.log_archive_service_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── module.aft_exec_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── module.aft_service_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── module.audit_exec_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── module.audit_service_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ └── module.ct_management_exec_role │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 ├── module.aft_code_repositories │ ├── provider[registry.terraform.io/hashicorp/local] │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 ├── module.aft_lambda_layer │ ├── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ ├── provider[registry.terraform.io/hashicorp/random] │ └── provider[registry.terraform.io/hashicorp/local] ├── module.aft_customizations │ ├── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ └── provider[registry.terraform.io/hashicorp/local] ├── module.aft_backend │ └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 ├── module.aft_account_request_framework │ ├── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ └── provider[registry.terraform.io/hashicorp/time] ├── module.aft_ssm_parameters │ ├── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 │ └── provider[registry.terraform.io/hashicorp/random] ├── module.packaging │ └── provider[registry.terraform.io/hashicorp/archive] └── module.aft_account_provisioning_framework └── provider[registry.terraform.io/hashicorp/aws] >= 4.9.0 Providers required by state: provider[registry.terraform.io/hashicorp/time] provider[registry.terraform.io/hashicorp/random] provider[registry.terraform.io/hashicorp/aws] provider[registry.terraform.io/hashicorp/archive] provider[registry.terraform.io/hashicorp/local] ```

Bug Description Switching the value of aft_feature_cloudtrail_data_events from true to false does not appear to do anything.

To Reproduce Steps to reproduce the behavior:

  1. Deploy AFT to a new account with aft_feature_cloudtrail_data_events = true
  2. Modify value from true to false
  3. Run terraform plan

Expected behavior Either TF should attempt to delete the cloudtrail trail, buckets, etc OR at least not try to re-create them if manually removed.

Additional context I initially enabled aft_feature_cloudtrail_data_events on first deployment of AFT. I then realized this was duplicating the cloudtrail logs already collected by the organization trail enabled through Control Tower.

I swapped the feature flag to false and ran a terraform apply, but noting happened. I ended up manually deleting all the resources, as I had a loop between data events in the TF-created logs/buckets and my CT org trail.

Now a plan wants to re-create the resources which I manually deleted, even though the feature flag is set to false.

Related Logs

Output of `terraform plan` ```text ... module.control_tower_account_factory.module.aft_customizations.aws_sfn_state_machine.aft_invoke_customizations_sfn: Refreshing state... [id=arn:aws:states:us-east-1:12345EXAMPLE:stateMachine:aft-invoke-customizations] Note: Objects have changed outside of Terraform Terraform detected the following changes made outside of Terraform since the last "terraform apply" which may have affected this plan: # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket.aft_access_logs has been deleted - resource "aws_s3_bucket" "aft_access_logs" { - arn = "arn:aws:s3:::aws-aft-s3-access-logs-12345EXAMPLE-us-east-1" -> null - id = "aws-aft-s3-access-logs-12345EXAMPLE-us-east-1" -> null - region = "us-east-1" -> null tags = {} # (8 unchanged attributes hidden) # (5 unchanged blocks hidden) } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket.aft_logging_bucket has been deleted - resource "aws_s3_bucket" "aft_logging_bucket" { - arn = "arn:aws:s3:::aws-aft-logs-12345EXAMPLE-us-east-1" -> null - id = "aws-aft-logs-12345EXAMPLE-us-east-1" -> null - region = "us-east-1" -> null tags = {} # (9 unchanged attributes hidden) # (4 unchanged blocks hidden) } Unless you have made equivalent changes to your configuration, or ignored the relevant attributes using ignore_changes, the following plan may include actions to undo or respond to these changes. ──────────────────────────────────────────────────────────────────────────────────────────── Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols: + create ~ update in-place Terraform will perform the following actions: # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket.aft_access_logs will be created + resource "aws_s3_bucket" "aft_access_logs" { + acceleration_status = (known after apply) + acl = (known after apply) + arn = (known after apply) + bucket = "aws-aft-s3-access-logs-12345EXAMPLE-us-east-1" + bucket_domain_name = (known after apply) + bucket_regional_domain_name = (known after apply) + force_destroy = false + hosted_zone_id = (known after apply) + id = (known after apply) + object_lock_enabled = (known after apply) + policy = (known after apply) + region = (known after apply) + request_payer = (known after apply) + tags_all = { + "managed_by" = "AFT" } + website_domain = (known after apply) + website_endpoint = (known after apply) + cors_rule { + allowed_headers = (known after apply) + allowed_methods = (known after apply) + allowed_origins = (known after apply) + expose_headers = (known after apply) + max_age_seconds = (known after apply) } + grant { + id = (known after apply) + permissions = (known after apply) + type = (known after apply) + uri = (known after apply) } + lifecycle_rule { + abort_incomplete_multipart_upload_days = (known after apply) + enabled = (known after apply) + id = (known after apply) + prefix = (known after apply) + tags = (known after apply) + expiration { + date = (known after apply) + days = (known after apply) + expired_object_delete_marker = (known after apply) } + noncurrent_version_expiration { + days = (known after apply) } + noncurrent_version_transition { + days = (known after apply) + storage_class = (known after apply) } + transition { + date = (known after apply) + days = (known after apply) + storage_class = (known after apply) } } + logging { + target_bucket = (known after apply) + target_prefix = (known after apply) } + object_lock_configuration { + object_lock_enabled = (known after apply) + rule { + default_retention { + days = (known after apply) + mode = (known after apply) + years = (known after apply) } } } + replication_configuration { + role = (known after apply) + rules { + delete_marker_replication_status = (known after apply) + id = (known after apply) + prefix = (known after apply) + priority = (known after apply) + status = (known after apply) + destination { + account_id = (known after apply) + bucket = (known after apply) + replica_kms_key_id = (known after apply) + storage_class = (known after apply) + access_control_translation { + owner = (known after apply) } + metrics { + minutes = (known after apply) + status = (known after apply) } + replication_time { + minutes = (known after apply) + status = (known after apply) } } + filter { + prefix = (known after apply) + tags = (known after apply) } + source_selection_criteria { + sse_kms_encrypted_objects { + enabled = (known after apply) } } } } + server_side_encryption_configuration { + rule { + bucket_key_enabled = (known after apply) + apply_server_side_encryption_by_default { + kms_master_key_id = (known after apply) + sse_algorithm = (known after apply) } } } + versioning { + enabled = (known after apply) + mfa_delete = (known after apply) } + website { + error_document = (known after apply) + index_document = (known after apply) + redirect_all_requests_to = (known after apply) + routing_rules = (known after apply) } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket.aft_logging_bucket will be created + resource "aws_s3_bucket" "aft_logging_bucket" { + acceleration_status = (known after apply) + acl = (known after apply) + arn = (known after apply) + bucket = "aws-aft-logs-12345EXAMPLE-us-east-1" + bucket_domain_name = (known after apply) + bucket_regional_domain_name = (known after apply) + force_destroy = false + hosted_zone_id = (known after apply) + id = (known after apply) + object_lock_enabled = (known after apply) + policy = (known after apply) + region = (known after apply) + request_payer = (known after apply) + tags_all = { + "managed_by" = "AFT" } + website_domain = (known after apply) + website_endpoint = (known after apply) + cors_rule { + allowed_headers = (known after apply) + allowed_methods = (known after apply) + allowed_origins = (known after apply) + expose_headers = (known after apply) + max_age_seconds = (known after apply) } + grant { + id = (known after apply) + permissions = (known after apply) + type = (known after apply) + uri = (known after apply) } + lifecycle_rule { + abort_incomplete_multipart_upload_days = (known after apply) + enabled = (known after apply) + id = (known after apply) + prefix = (known after apply) + tags = (known after apply) + expiration { + date = (known after apply) + days = (known after apply) + expired_object_delete_marker = (known after apply) } + noncurrent_version_expiration { + days = (known after apply) } + noncurrent_version_transition { + days = (known after apply) + storage_class = (known after apply) } + transition { + date = (known after apply) + days = (known after apply) + storage_class = (known after apply) } } + logging { + target_bucket = (known after apply) + target_prefix = (known after apply) } + object_lock_configuration { + object_lock_enabled = (known after apply) + rule { + default_retention { + days = (known after apply) + mode = (known after apply) + years = (known after apply) } } } + replication_configuration { + role = (known after apply) + rules { + delete_marker_replication_status = (known after apply) + id = (known after apply) + prefix = (known after apply) + priority = (known after apply) + status = (known after apply) + destination { + account_id = (known after apply) + bucket = (known after apply) + replica_kms_key_id = (known after apply) + storage_class = (known after apply) + access_control_translation { + owner = (known after apply) } + metrics { + minutes = (known after apply) + status = (known after apply) } + replication_time { + minutes = (known after apply) + status = (known after apply) } } + filter { + prefix = (known after apply) + tags = (known after apply) } + source_selection_criteria { + sse_kms_encrypted_objects { + enabled = (known after apply) } } } } + server_side_encryption_configuration { + rule { + bucket_key_enabled = (known after apply) + apply_server_side_encryption_by_default { + kms_master_key_id = (known after apply) + sse_algorithm = (known after apply) } } } + versioning { + enabled = (known after apply) + mfa_delete = (known after apply) } + website { + error_document = (known after apply) + index_document = (known after apply) + redirect_all_requests_to = (known after apply) + routing_rules = (known after apply) } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_acl.aft_access_logs_acl will be created + resource "aws_s3_bucket_acl" "aft_access_logs_acl" { + acl = "log-delivery-write" + bucket = (known after apply) + id = (known after apply) + access_control_policy { + grant { + permission = (known after apply) + grantee { + display_name = (known after apply) + email_address = (known after apply) + id = (known after apply) + type = (known after apply) + uri = (known after apply) } } + owner { + display_name = (known after apply) + id = (known after apply) } } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_lifecycle_configuration.aft_access_logs_lifecycle_configuration will be created + resource "aws_s3_bucket_lifecycle_configuration" "aft_access_logs_lifecycle_configuration" { + bucket = (known after apply) + id = (known after apply) + rule { + id = "aft_access_logs_lifecycle_configuration_rule" + status = "Enabled" + filter { + prefix = "log/" } + noncurrent_version_expiration { + noncurrent_days = 365 } } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_lifecycle_configuration.aft_logging_bucket_lifecycle_configuration will be created + resource "aws_s3_bucket_lifecycle_configuration" "aft_logging_bucket_lifecycle_configuration" { + bucket = (known after apply) + id = (known after apply) + rule { + id = "aft_logging_bucket_lifecycle_configuration_rule" + status = "Enabled" + noncurrent_version_expiration { + noncurrent_days = 365 } } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_logging.aft_logging_bucket_logging will be created + resource "aws_s3_bucket_logging" "aft_logging_bucket_logging" { + bucket = (known after apply) + id = (known after apply) + target_bucket = (known after apply) + target_prefix = "log/" } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_policy.aft_logging_bucket will be created + resource "aws_s3_bucket_policy" "aft_logging_bucket" { + bucket = (known after apply) + id = (known after apply) + policy = (known after apply) } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_public_access_block.aft_access_logs will be created + resource "aws_s3_bucket_public_access_block" "aft_access_logs" { + block_public_acls = true + block_public_policy = true + bucket = (known after apply) + id = (known after apply) + ignore_public_acls = true + restrict_public_buckets = true } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_public_access_block.aft_logging_bucket will be created + resource "aws_s3_bucket_public_access_block" "aft_logging_bucket" { + block_public_acls = true + block_public_policy = true + bucket = (known after apply) + id = (known after apply) + ignore_public_acls = true + restrict_public_buckets = true } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_server_side_encryption_configuration.aft_access_logs_encryption will be created + resource "aws_s3_bucket_server_side_encryption_configuration" "aft_access_logs_encryption" { + bucket = (known after apply) + id = (known after apply) + rule { + apply_server_side_encryption_by_default { + sse_algorithm = "AES256" } } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_server_side_encryption_configuration.aft_logging_bucket_encryption will be created + resource "aws_s3_bucket_server_side_encryption_configuration" "aft_logging_bucket_encryption" { + bucket = (known after apply) + id = (known after apply) + rule { + apply_server_side_encryption_by_default { + kms_master_key_id = "arn:aws:kms:us-east-1:12345EXAMPLE:key/00000000-0000-0000-0000-000000000000" + sse_algorithm = "aws:kms" } } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_versioning.aft_access_logs_versioning will be created + resource "aws_s3_bucket_versioning" "aft_access_logs_versioning" { + bucket = (known after apply) + id = (known after apply) + versioning_configuration { + mfa_delete = (known after apply) + status = "Enabled" } } # module.control_tower_account_factory.module.aft_feature_options.aws_s3_bucket_versioning.aft_logging_bucket_versioning will be created + resource "aws_s3_bucket_versioning" "aft_logging_bucket_versioning" { + bucket = (known after apply) + id = (known after apply) + versioning_configuration { + mfa_delete = (known after apply) + status = "Enabled" } } # module.control_tower_account_factory.module.aft_ssm_parameters.aws_ssm_parameter.aft_logging_bucket_arn will be updated in-place ~ resource "aws_ssm_parameter" "aft_logging_bucket_arn" { id = "/aft/account/log-archive/log_bucket_arn" + insecure_value = (known after apply) name = "/aft/account/log-archive/log_bucket_arn" tags = {} ~ value = (sensitive value) ~ version = 1 -> (known after apply) # (5 unchanged attributes hidden) } Plan: 13 to add, 1 to change, 0 to destroy. ──────────────────────────────────────────────────────────────────────────────────────────── Note: You didn't use the -out option to save this plan, so Terraform can't guarantee to take exactly these actions if you run "terraform apply" now. ```

Thank you in advance!

matthewbarreiro commented 1 year ago

On further investigation, I discovered the buckets are re-created but the actual cloudtrail trail is not. I assume this is expected behavior?

It would be nice to be able to remove the buckets, as mine are currently empty. BUT I can see this as being complicated though, particularly to avoid deleting logs on someone who has been using the integration but is now disabling it.

Does anyone have an idea on how to enable this? Perhaps check if the bucket is empty?