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

A bug when enabling aws_s3_bucket.replication_configuration.rules.destination.metrics without ReplicationTime #22774

Open
husseinmimiHarri opened this issue Jan 26, 2022 · 7 comments
Labels
breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. bug Addresses a defect in current functionality. service/s3 Issues and PRs that pertain to the s3 service.

Comments

@husseinmimiHarri
Copy link

husseinmimiHarri commented Jan 26, 2022

Thanks @anGie44 for fixing similar issue #21895 with this fix #21901, but also i got similar problem when using aws_s3_bucket.replication_configuration.rules. destination.metrics block it was depends on aws_s3_bucket.replication_configuration.rules. destination.replication_time block.

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

Terraform CLI and Terraform AWS Provider Version

Terraform v1.0.1
on darwin_amd64

  • provider registry.terraform.io/hashicorp/aws v3.73.0

Affected Resource(s)

  • aws_s3_bucket

Terraform Configuration Files

resource.tf

resource "aws_s3_bucket" "source" {
    # settings

  replication_configuration {
    role = aws_iam_role.replication.arn

    rules {
            destination {
                        metrics {
                         status  = "Enabled"
                      }
                        # replication_time not specified
      }
    }
  }
}

Expected Behavior

Edit existing bucket to enable replication metrics

Actual Behavior

raise this error

module.assets_harridev_com.aws_s3_bucket.bucket: Modifying... [id=BUCKET_NAME]
╷
│ Error: Error putting S3 replication configuration: InvalidArgument: Metrics cannot contain an event threshold when ReplicationTime is not specified or Disabled
│ 	status code: 400, request id: ID, host id: ID
│
│   with module.BUCKET_NAME.aws_s3_bucket.bucket,
│   on .terraform/modules/BUCKET_NAME/resources.tf line 9, in resource "aws_s3_bucket" "bucket":
│    9: resource "aws_s3_bucket" "bucket" {
│
╵

Steps to Reproduce

  1. Create S3 bucket using aws_s3_bucket resource.
  2. Enable replication_configuration block inside it
  3. include metrics {
    status = "Enabled"
    }
  4. terragrunt apply (invoke terraform apply)

References

similar issue #21895
fix for issue fix #21901
Note: Fix done on aws_s3_bucket_replication_configuration resource not on aws_s3_bucket_resource.

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. service/s3 Issues and PRs that pertain to the s3 service. labels Jan 26, 2022
@anGie44 anGie44 self-assigned this Jan 27, 2022
@anGie44 anGie44 added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Jan 27, 2022
@anGie44
Copy link
Contributor

anGie44 commented Jan 27, 2022

Hi @husseinmimi , thank you again for creating this new issue. It looks like the metrics configuration block has a default value of 15 for minutes in the aws_s3_bucket resource so even if the value is not explicitly configured in terraform, as you've described above, that value will always be sent in the API request behind the scenes. To address this, we'd have to remove that default which is a breaking-change, so in the meantime I would strongly recommend using the aws_s3_bucket_replication_configuration resource to be able to configure it with more flexibility. Currently, the team is working on changes for the 4.0 release which will introduce breaking changes in the provider, so time permitting this could potentially be considered, however I cannot provide a guarantee at this time.

Reference:

"metrics": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"minutes": {
Type: schema.TypeInt,
Optional: true,
Default: 15,
ValidateFunc: validation.IntBetween(10, 15),

if metrics, ok := bd["metrics"].([]interface{}); ok && len(metrics) > 0 {
metricsConfig := &s3.Metrics{}
metricsValues := metrics[0].(map[string]interface{})
metricsConfig.EventThreshold = &s3.ReplicationTimeValue{}
metricsConfig.Status = aws.String(metricsValues["status"].(string))
metricsConfig.EventThreshold.Minutes = aws.Int64(int64(metricsValues["minutes"].(int)))
ruleDestination.Metrics = metricsConfig
}

@anGie44 anGie44 added the breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. label Jan 27, 2022
@anGie44 anGie44 removed their assignment Jan 27, 2022
@husseinmimiHarri
Copy link
Author

husseinmimiHarri commented Jan 27, 2022

@anGie44 thanks for your reply but i am not talking about the default value of time in metrics block, i am talking about this:

resource "aws_s3_bucket" "source" {
       ## settings
      destination {
        bucket        = aws_s3_bucket.destination.arn
        storage_class = "STANDARD"
          ########### replication_time block MUSTN'T be specified ########

         # replication_time {  # this block to configure S3 Replication Time Control (S3 RTC)
         #  status  = "Enabled"
         #  minutes = 15
         # }
        
        # I want to enable replication metrics only, and i don't have problem with minutes field to be 15 or anything else,
        # and i know it is 15 by default thats what i really want, but i want to enable replication metrics without
        # enable S3 Replication Time Control (S3 RTC) by replication_time block.
        metrics {
          status  = "Enabled"
        }
      }
    }
  }
}

So the bug is that Metrics cannot contain an event threshold when ReplicationTime is not specified or Disabled message shouldn't be raised with settings i made, because in aws console i can enable replication metrics even when S3 Replication Time Control (S3 RTC) is disabled.

I hope that issue is more clear now.

@anGie44
Copy link
Contributor

anGie44 commented Jan 27, 2022

Hi again @husseinmimi ! Yes, unfortunately it's not super clear from the terrraform CLI side but the problem goes back to the data sent in the API request of PutBucketReplication which is determined by how the AWS provider interprets the terraform configuration which does not work well for all use cases. The error you are seeing is directly from AWS API response. Hopefully the following details will show what occurs behind the scenes and why the error is popping up. All-in-all the default value of time in metrics proves to be problematic and I believe that's why the aws_s3_bucket_replication_configuration resource by design does not have that default.

So when the terraform config is the following:

resource "aws_s3_bucket" "source" {
  ## settings
  destination {
    bucket        = aws_s3_bucket.destination.arn
    storage_class = "STANDARD"
     
   metrics {
     status  = "Enabled"
   }
  }
}

By the time that configuration gets communicated to the AWS provider, the destination configuration block looks a bit like the following data structure:

bd := []interface{}{
  "bucket": "...".
  "storage_class" : "STANDARD",
  "metrics": []interface{}{
    "status": "Enabled",
    "minutes": "15", // because the aws_s3_bucket resource schema has defined this value so it gets passed into here
  }
}

which then causes the API request of PutBucketReplication to look like the following snippet:

...
ruleDestination := &s3.Destination{}

if metrics, ok := bd["metrics"].([]interface{}); ok && len(metrics) > 0 { 
 	metricsConfig := &s3.Metrics{} 
 	metricsValues := metrics[0].(map[string]interface{}) 
 	metricsConfig.EventThreshold = &s3.ReplicationTimeValue{} 
 	metricsConfig.Status = aws.String(metricsValues["status"].(string)) 
 	metricsConfig.EventThreshold.Minutes = aws.Int64(int64(metricsValues["minutes"].(int))) <-- we are setting this even though practitioners don't intend to in their terraform configurations
 	ruleDestination.Metrics = metricsConfig 
 } 

rule := &s3.ReplicationRule{
  Destination: ruleDestination,
}

i := &s3.PutBucketReplicationInput{
		Bucket:                   aws.String(bucket),
		ReplicationConfiguration: &s3.ReplicationConfiguration{
                   Rules: []*s3.Rule{}{rule},
               },
	}

One way to work around this would be to follow the logic expressed in the error message which wouldn't introduce a breaking change:

ruleDestination := &s3.Destination{}

// replication time control (RTC) <-- move this logic up here
if rtc, ok := bd["replication_time"].([]interface{}); ok && len(rtc) > 0 {
  rtcValues := rtc[0].(map[string]interface{})
  rtcConfig := &s3.ReplicationTime{}
  rtcConfig.Status = aws.String(rtcValues["status"].(string))
  rtcConfig.Time = &s3.ReplicationTimeValue{}
  rtcConfig.Time.Minutes = aws.Int64(int64(rtcValues["minutes"].(int)))
  ruleDestination.ReplicationTime = rtcConfig
}

if metrics, ok := bd["metrics"].([]interface{}); ok && len(metrics) > 0 { 
 	metricsConfig := &s3.Metrics{} 
 	metricsValues := metrics[0].(map[string]interface{}) 
 	metricsConfig.Status = aws.String(metricsValues["status"].(string)) 
        // only set Metrics EventThreshold when ReplicationTime is  specified and not Disabled
        if ruleDestination.ReplicationTime != nil && ruleDestination.ReplicationTime.Status != "Disabled" {
          metricsConfig.EventThreshold = &s3.ReplicationTimeValue{
            Minutes: aws.Int64(int64(metricsValues["minutes"].(int))), 
          }
       }
       ruleDestination.Metrics = metricsConfig 
 } 

rule := &s3.ReplicationRule{
  Destination: ruleDestination,
}

i := &s3.PutBucketReplicationInput{
		Bucket:                   aws.String(bucket),
		ReplicationConfiguration: &s3.ReplicationConfiguration{
                   Rules: []*s3.Rule{}{rule},
               },
	}

But with the downside that you'll always see a perpetual diff due to the expected default (15) as follows:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
          ~ update in-place

        Terraform will perform the following actions:

          # aws_s3_bucket.bucket will be updated in-place
          ~ resource "aws_s3_bucket" "bucket" {
                id                          = "tf-test-bucket-6118547079748010386"
                tags                        = {}
                # (10 unchanged attributes hidden)

              ~ replication_configuration {
                    # (1 unchanged attribute hidden)

                  - rules {
                      - id       = "foobar" -> null
                      - priority = 0 -> null
                      - status   = "Enabled" -> null

                      - destination {
                          - bucket        = "arn:aws:s3:::tf-test-bucket-destination-6118547079748010386" -> null
                          - storage_class = "STANDARD" -> null

                          - metrics {
                              - minutes = 0 -> null
                              - status  = "Enabled" -> null
                            }
                        }

                      - filter {
                          - prefix = "foo" -> null
                          - tags   = {} -> null
                        }
                    }
                  + rules {
                      + id     = "foobar"
                      + status = "Enabled"

                      + destination {
                          + bucket        = "arn:aws:s3:::tf-test-bucket-destination-6118547079748010386"
                          + storage_class = "STANDARD"

                          + metrics {
                              + minutes = 15
                              + status  = "Enabled"
                            }
                        }

                      + filter {
                          + prefix = "foo"
                        }
                    }
                }

                # (1 unchanged block hidden)
            }

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

To address the above diff (just brainstorming here), we could make a code change to remove the plan-time validation on metrics minutes , as atm the value must be between 10 and 15. so that in your terraform config the following could be configured and no subsequent diff would occur. The 0 value never gets sent over in the API request but helps match the API response value.

resource "aws_s3_bucket" "bucket" {
...
metrics {
          minutes = 0
          status    = "Enabled"
        }
}

@haytham-salhi
Copy link

Hi @anGie44, any updates on this? Can you recommend some workaround instead of using the aws_s3_bucket_replication_configuration resource block?

@gari199
Copy link

gari199 commented Jun 7, 2022

Hi @husseinmimi , any news on this issue? I am having the same problem when trying to update 3 buckets with terraform after having defined the replication metrics via the UI and cannot run my pipelines properly.

I am using the deprecated replication_configuration option within s3 and not the resource block aws_s3_bucket_replication_configuration

Thank you very much for your time!

@husseinmimiHarri
Copy link
Author

Hello @gari1995,

No, there is no new updates for this issue

@mr-niche
Copy link

mr-niche commented Mar 5, 2024

Has anyone come up with a reasonable work-around here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Introduces a breaking change in current functionality; usually deferred to the next major release. bug Addresses a defect in current functionality. service/s3 Issues and PRs that pertain to the s3 service.
Projects
None yet
Development

No branches or pull requests

5 participants