aminueza / terraform-provider-minio

Terraform provider for managing MinIO S3 buckets and IAM Users.
https://registry.terraform.io/providers/aminueza/minio
GNU Affero General Public License v3.0
243 stars 73 forks source link

Lifecycle rule with noncurrent_version_expiration_days causes a update every plan/apply in 2.0.0 #543

Closed sdejong629 closed 1 year ago

sdejong629 commented 1 year ago

Prerequisites

Description

When using a ilm resource and using the noncurrent_version_expiration_days rule, a plan and apply always updates the resource, even when no changes have been made to terraform code.

terraform config

resource "minio_s3_bucket" "bucket" {
  bucket = ""bucket_name"
}

resource "minio_ilm_policy" "bucket" {
  bucket = minio_s3_bucket.bucket.bucket

  rule {
    id                                                       = "expire-noncurrent-versions-60days"
    noncurrent_version_expiration_days = 60
  }
}

Terraform plan output:

~ resource "minio_ilm_policy" "bucket" {
        id     = "bucket_name"
        # (1 unchanged attribute hidden)
      ~ rule {
          - expiration                         = "0001-01-01" -> null
            id                                 = "expire-noncurrent-versions-60days"
            tags                               = {}
            # (2 unchanged attributes hidden)
        }
    }

Steps to Reproduce

  1. Add minio_ilm_policy resource with a noncurrent_version_expiration_days rule
  2. Run a terraform plan & apply
  3. Run another terraform plan and apply
  4. It wil show changes like stated above

Expected behavior: No changes should occur after the initial

Actual behavior: Expiration date is updated to null

Reproduces how often: [What percentage of the time does it reproduce?] 100%

Versions

2.0.0

Additional Information

I have not tested any other ILM policy rules, just the noncurrent_version_expiration_days. I'm creating buckets trough a custom module, which just creates serveral of these sets.

pjsier commented 1 year ago

Thanks for reporting this issue! It looks like this is because #526 isn't returning null for expiration. I should be able to put in a PR to fix that

pjsier commented 1 year ago

Just created #545 if you have a second to test @sdejong629. I was able to reproduce in the unit tests and this change resolved the issue

sdejong629 commented 1 year ago

Just created #545 if you have a second to test @sdejong629. I was able to reproduce in the unit tests and this change resolved the issue

If you have a lead on how I can use your repo instead of the registered one...

And isn't it dangerous to set an expiration date if none is set? Should it just be empty? It isn't supposed to expire items at all.

felladrin commented 1 year ago

Thanks for the detailed report @sdejong629!

If you have a lead on how I can use your repo instead of the registered one...

To test it directly from @pjsier's branch, you can clone his repo and checkout his branch fix/543-lifecycle-days, then, having Task installed, you just need to run task install and the provider should be ready to be used (terraform gives preference to locally installed providers).

isn't it dangerous to set an expiration date if none is set? Should it just be empty? It isn't supposed to expire items at all.

🤔 As I see @pjsier's change is not for setting a date if it's empty. Instead, it's to format the Date field of Expiration in the format "YYYY-MM-DD" only if Expiration is not null. Because if we apply fortmat in a null value, it sets the expiration date to 0001-01-01, causing this bug.