Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3 refactor upgrade guide: Need for lifecycle on backports should be documented #26534

Open
ab opened this issue Aug 30, 2022 · 1 comment
Open
Labels
service/s3 Issues and PRs that pertain to the s3 service.

Comments

@ab
Copy link

ab commented Aug 30, 2022

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Summary

Hi folks,

The AWS provider 3.x series now has backported aws_s3_bucket_* resources from the 4.x provider. Notably, if you're going to use these, you must also add a lifecycle { ignore_changes = ... } to avoid a perpetual diff that will alternately destroy part of your configuration.

This is extremely surprising behavior, and there should be much more visible warnings about this in documentation in the upgrade guide and ideally in the latest AWS provider documentation.

Currently, the upgrade guide has numerous examples of using the new style resources, but zero examples of including the ignore_changes block necessary to make this safe for people who still pin to the 3.x provider. And anyone who does a web search for the new resources will be led first to the latest version of the docs, which also makes no mention of this warning or the necessary ignore_changes block.

I just had a minor incident due to this. And I was only able to uncover the cause after much searching by finding the old documentation for the new resources in the 3.75.2 AWS provider documentation.

This was really upsetting, particularly because I previously read the upgrade guide and followed the examples therein, which turned out not to be safe to follow.

There should be more caveats that these instructions are dangerous to follow if you are still using the 3.x provider!

Thank you.

Terraform CLI and Terraform AWS Provider Version

AWS provider 3.75.2

Affected Resource(s)

  • aws_s3_bucket_*

Terraform Configuration Files

Please include all Terraform configurations required to reproduce the bug. Bug reports without a functional reproduction may be closed without investigation.

⚠️ Unsafe in AWS provider version 3.x! ⚠️

resource "aws_s3_bucket" "example" {
  bucket = "my-tf-example-bucket"
}

resource "aws_s3_bucket_acl" "example_bucket_acl" {
  bucket = aws_s3_bucket.example.id
  acl    = "private"
}

Compare against lifecycle ignore_changes that isn't documented in the upgrade guide

resource "aws_s3_bucket" "example" {
  bucket = "my-tf-example-bucket"

  lifecycle {
    ignore_changes = [
      grant
    ]
  }
}

resource "aws_s3_bucket_acl" "example_bucket_acl" {
  bucket = aws_s3_bucket.example.id
  acl    = "private"
}

Expected Behavior

I expected deterministic behavior with the ACL set correctly.

Actual Behavior

A perpetual diff ensued, with Terraform creating the new resource in one apply and destroying it in the next apply.

Steps to Reproduce

  1. terraform apply

Important Factoids

References

@github-actions github-actions bot added the service/s3 Issues and PRs that pertain to the s3 service. label Aug 30, 2022
@thiagolsfortunato
Copy link

thiagolsfortunato commented Mar 23, 2023

Same Issue here

I fixed it when I added ignore_changes to remove all drift on the second execution.

Before:

# module.example.module.example[0].aws_s3_bucket.bucket will be updated in-place
  ~ resource "aws_s3_bucket" "bucket" {
        id                          = "example-logs-us-east-1"
        tags                        = {
            "Name" = "example-logs-us-east-1"
        }
        # (11 unchanged attributes hidden)

      - server_side_encryption_configuration {
          - rule {
              - bucket_key_enabled = true -> null

              - apply_server_side_encryption_by_default {
                  - kms_master_key_id = "arn:aws:kms:us-east-1:<account-id>:key/<key-id>" -> null
                  - sse_algorithm     = "aws:kms" -> null
                }
            }
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Add ignore_chages:

resource "aws_s3_bucket" "bucket" {
  bucket = var.bucket
  ...
  
  lifecycle {
    prevent_destroy = true

    ignore_changes = [
      server_side_encryption_configuration
    ]
  }
  ...
}

resource "aws_s3_bucket_server_side_encryption_configuration" "bucket_encrypt_config" {
  bucket = aws_s3_bucket.bucket.bucket

  rule {
    apply_server_side_encryption_by_default {
      kms_master_key_id = var.kms_key_arn != "" ? var.kms_key_arn : aws_kms_key.bucket_key[0].arn
      sse_algorithm     = var.server_side_encryption_algorithm
    }
    bucket_key_enabled = var.bucket_key_enabled
  }
}

Then the output of terraform plan:

...

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes
are needed.

After implementing them separately, I would like not to add all resources on the lifecycle.ignore_changes. it's a suggestion from you in the terraform documentation because they are deprecated parameters.

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service/s3 Issues and PRs that pertain to the s3 service.
Projects
None yet
Development

No branches or pull requests

2 participants