S3 buckets created with versioning

[imunhatep]

Describe the Bug

It's related to aws_s3_bucket resource bug or feature: hashicorp/terraform-provider-aws#4494

In short setting versioning to false, actually creates versioned bucket with suspended versioning:

resource "aws_s3_bucket" "default" {
  versioning {
    enabled = false 
  }
 ...
}

Expected Behavior

Bucket with versioning actually disabled

Steps to Reproduce

Try to create unversioned bucket

[imunhatep]

Correct me if i'm wrong, but it gets worse with disabled lifecycle rules, as now we get an ever growing bucket, as delete object request actually do not delete objects for buckets with suspended versioning.

[grimm26]

see hashicorp/terraform-provider-aws#12354 (comment)

[ekristen]

This is a pretty nasty bug, any chance of getting a fix anytime soon?

[nnsense]

I did some testing and it seems that this isn't related the module, but it's related terraform and what versioning attribute means into that module.

Apparently, versioning is a setting on its own, regardless of its value (which is what we can set with a var), just mention it, it will turn on versioning, leaving it in suspended mode, it's equivalent to setting enabled = false.

  versioning {}

This will turn it on and enable:

  versioning {
    enabled = true
  }

I really don't know how this can be sorted out, if just by mentioning an attribute will enable it (suspended), the only thing to do would be to remove it entirely based on a variable, but I'm not aware that's possible in terraform

[aknysh]

dynamic "versioning" {
    for_each = var.versioning_enabled ? [true] : []
    content {
      enabled = true
    }
  }

https://www.terraform.io/docs/language/expressions/dynamic-blocks.html

@nnsense ^

[nnsense]

Hey, that worked! Why you didn't add it into the module? If you don't have time I can create a PR :)

[nnsense]

...there's already a PR.. Open from February.. :'(

[aknysh]

@nnsense the PR is very old and convoluted.
Please open a new PR with just this feature

[nnsense]

I'm doing just that :) Question: there's a comment:

#bridgecrew:skip=BC_AWS_S3_16:Skipping `Ensure S3 bucket versioning is enabled` because dynamic blocks are not supported by checkov

But now I see that some basic handling for dynamic blocks has been added in checkov
bridgecrewio/checkov#836

So if you're using checkov it would be interesting to remove the comment and see if it works now

[korenyoni]

Closing this because #118 is merged