hashicorp / terraform-provider-time

Utility provider that provides Time-Based Resources
https://registry.terraform.io/providers/hashicorp/time/latest
Mozilla Public License 2.0
103 stars 31 forks source link

time_offset doesn't merge when multiple offsets are specified #257

Closed prubis closed 9 months ago

prubis commented 10 months ago

Terraform CLI and Provider Versions

Terraform v1.5.4 on windows_amd64

Terraform Configuration

resource "time_offset" "tomorrow_plus_eleven_style1" {
  offset_hours = 24 + 11
}

resource "time_offset" "tomorrow_plus_eleven_style2" {
  offset_hours = 11
  offset_days  = 1
}

output "tomorrow_plus_eleven_style1" {
  value = time_offset.tomorrow_plus_eleven_style1.rfc3339
}

output "tomorrow_plus_eleven_style2" {
  value = time_offset.tomorrow_plus_eleven_style2.rfc3339
}

Expected Behavior

I would expect both outputs to have the same value.

Actual Behavior

They have different values:

Apply complete! Resources: 2 added, 0 changed, 1 destroyed.

Outputs:

tomorrow_plus_eleven_style1 = "2023-11-03T12:54:36Z"
tomorrow_plus_eleven_style2 = "2023-11-02T12:54:36Z"

Steps to Reproduce

  1. terraform apply

How much impact is this issue causing?

Low

Logs

No response

Additional Information

The bug is easily worked around by specifying the desired sum offset in terms of the lowest denominator (e.g., I wanted days + hours, so I need to specify the days portion as additional hours), but it's inelegant and certainly undocumented - took me a little while to diagnose!

Suggested fix: Looking at the source, I suggest the issue is that the setOffsetValues() function doesn't take into account that it may already be assigning values to offsetTimestamp. For example, from lines 396-403:

    if plan.OffsetDays.ValueInt64() != 0 {
        offsetTimestamp = timestamp.AddDate(0, 0, int(plan.OffsetDays.ValueInt64()))
    }

    if plan.OffsetHours.ValueInt64() != 0 {
        hours := time.Duration(plan.OffsetHours.ValueInt64()) * time.Hour
        offsetTimestamp = timestamp.Add(hours)
    }

If both OffsetDays and OffsetHours are non-zero, then I think the offsetTimestamp = timestamp.Add(hours) step will overwrite the preceeding offsetTimestamp = timestamp.AddDate(...) step.

Code of Conduct

bflad commented 9 months ago

FYI @SBGoods, added this issue and the PR to the v0.10.0 milestone. The release process for all our providers is now setup to automatically close milestones matching the release names, similar to the terraform-plugin-* projects. 👍

github-actions[bot] commented 3 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.