dnsimple / terraform-provider-dnsimple

Terraform DNSimple provider.
https://www.terraform.io/docs/providers/dnsimple/
Mozilla Public License 2.0
21 stars 20 forks source link

Record not found error with prefetch enabled, but has no changes in default mode #192

Closed volodymyr-mykhailyk closed 8 months ago

volodymyr-mykhailyk commented 9 months ago

When prefetch is enabled on the provider config - two of our records receive a record not found error, and plan/apply is aborted. When you disable prefetch - no changes to resources are needed.

The issue seems to be related to the content of the record not exactly matching. When prefetch is disabled - the code probably using a normalized value and sees no need for updates.

Also, looking through the code of the provider and working with prefetch a little - there might be different normal scenarios (for example manually delete record on DNSimple and then run terraform plan) when current implementation will result in the error.

So, except for fixing the matching logic, should we also change the implementation so that the missing cache entry does not result in an error but rather falls back to regular API calls? I am happy to try and suggest a solution if you will suggest the right direction.

Terraform Version

Terraform v1.5.6
on darwin_arm64
+ provider registry.terraform.io/dnsimple/dnsimple v1.4.0

Also reproducible on provider version 1.3.1

Affected Resource(s)

Terraform Configuration Files

resource "dnsimple_zone_record" "matic_sales_dmarc" {
  type = "TXT"
  ttl  = 3600

  name  = "_dmarc.sales"
  value = "v=DMARC1; p=reject; pct=100; mailto:re+eeeee@dmarc.postmarkapp.com; aspf=r;"
  zone_name = "test.com"
}

Debug Output

Planning failed. Terraform encountered an error while generating this plan.

╷
│ Error: record not found
│ 
│   with dnsimple_zone_record.matic_sales_dmarc,
│   on ns_matic.com.tf line 172, in resource "dnsimple_zone_record" "matic_sales_dmarc":
│  172: resource "dnsimple_zone_record" "matic_sales_dmarc" {
│ 
│ failed to find DNSimple Zone Record in the zone cache: _dmarc.sales.test.com
╵

Expected Behavior

When prefetch is enabled:

Actual Behavior

When prefetch is enabled and cache does not have record exactly matching code/state - terraform run fails

DXTimer commented 8 months ago

Thank you, @volodymyr-mykhailyk, for bringing these issues to our attention.

Regarding the false negatives and cache misses due to content mismatches, you're spot on. We've been using the non-normalized value for cache lookups, but fortunately, we have access to the normalized value via .ValueNormalized. We can utilize this for a more accurate lookup as part of our bug-fix strategy.

Concerning the behaviour of cache misses, I'd appreciate some clarification on the use case you're referencing. To share my perspective: The cache for records is primarily utilized during the read operation of a resource, which is expected to occur only when the resource already exists within the state. During a resource's create action, the cache wouldn't be accessed since the read operation isn't performed at this stage. Therefore, the current approach of triggering an error on a cache miss seems to align with standard expectations, as the resource tracked by the state no longer exists, prompting the developer to clear their state via terraform state rm. Could you let me know if my understanding diverges from yours?

volodymyr-mykhailyk commented 8 months ago

Hi @DXTimer. Thanks for fixing the original bug. I would love to try it out once you release a new version.

As for my other comment - I mentioned a scenario:

  1. Create DNS record using Terraform
  2. Enable prefetch
  3. Delete the record manually on DNSimple ui (record present in state but will not be available in cache)
  4. Run terraform plan

Will such a scenario trigger this error? This is not a create action, but checking if provider configuration matching state/code. While the developer can perform terraform state rm and rerun a plan, a typical result for terraform is to detect that provider configuration is missing and suggest creating resources automatically on the first plan.

Sorry if this comment doesn't make sense or if the code works differently than I interpreted it while reading.

DXTimer commented 8 months ago

@volodymyr-mykhailyk you should be able to test out the fixes in the latest release 1.5.0.

Thank you for expanding on the scenario. Since as you've said a typical behaviour is to suggest the creation of the resource that was found to be missing, it would make sense for us to conform with the commonly expected behaviour.