michael-sterling commented 6 months ago

Summary

I am trying to adopt this provider for an existing Redirect Pizza instance. I would like to import existing redirects to be managed by Terraform.

However the sources array and destination blocks do not get populated with the existing values when importing an existing redirect from from the API.

Further, when I test making a change to a redirect already managed by Terraform, refreshing the Terraform state using terraform plan -refresh-only does not pick up any changes I have made to Sources or Destinations using the https://redirect.pizza website UI for that redirect. This means that Terraform will not know that it needs to update that redirect in order to cause the redirect to match Terraform configuration again.

I suspect that this is due to mapping errors translating the API JSON for a redirect into the shape expected by Terraform state.

Relevant Code

This code appears to be mapping the sources array.
- Terraform expects the sources property to be an array of string, but
- the sources property gets assigned an array of { url: string } objects.
- This results in an [ERROR] setting state: sources.0: '' expected type 'string', got unconvertible type 'map[string]interface {}' error.
This code appears to be generating the destination blocks property-by-property.
- Apparently this isn't how Terraform expects to create blocks; this results in [ERROR] setting state: destination.0.url: can only set full list errors.

Steps to Reproduce

Provider Version: 0.2.0 Terraform Version: 1.6.6

Note: To reproduce outside of my environment, you will need your own API token and the ID of an existing redirect. The ID I used in the below example is 1624710909.

Example Terraform

terraform {
  required_providers {
    redirectpizza = {
      source  = "enflow/redirectpizza"
      version = "0.2.0"
    }
  }
}

variables "token" {
  type = string 
}

provider "redirectpizza" {
  token = var.token
}

resource "redirectpizza_redirect" "example" {

  sources = ["www.example.com"]

  destination {
    url = "https://example.com"
  }

  tracking       = false
  uri_forwarding = true
}

import {
  to = redirectpizza_redirect.example
  id = 1624710909
}

Command to Reproduce

terraform plan

Output

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:

  # redirectpizza_redirect.example will be updated in-place
  # (imported from "1624710909")
  ~ resource "redirectpizza_redirect" "example" {
        id                = "1624710909"
        keep_query_string = false
        redirect_type     = "permanent"
      + sources           = [
          + "www.example.com",
        ]
        tags              = []
      ~ tracking          = true -> false
      ~ uri_forwarding    = false -> true

      + destination {
          + monitoring = "inherit"
          + url        = "https://example.com"
        }
    }

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

Note that:

The provider was able to successfully import the existing values for keep_query_string, redirect_type, and tags
The provider was able to successfully detect we were changing the existing values for tracking and uri_forwarding
The provider was not able to import the existing values for sources or destination
- Instead of simply adding the new values, I would expect the plan to show the old values being removed.

Terraform State

If I run the import manually using terraform import redirectpizza_redirect.example 1624710909, the resulting resource state looks like this (again without sources or destination):

{
  "version": 4,
  "terraform_version": "1.6.6",
  "serial": 1,
  "lineage": "d57bdeb0-84c9-75cd-cb9c-9eb14265206f",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "redirectpizza_redirect",
      "name": "example",
      "provider": "provider[\"registry.terraform.io/enflow/redirectpizza\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "destination": [],
            "id": "1624710909",
            "keep_query_string": false,
            "redirect_type": "permanent",
            "sources": null,
            "tags": [],
            "tracking": true,
            "uri_forwarding": false
          },
          "sensitive_attributes": [],
          "private": "eyJzY2hlbWFfdmVyc2lvbiI6IjAifQ=="
        }
      ]
    }
  ],
  "check_results": null
}

Logs

Using TF_LOG = 'DEBUG' I see these events in the logs:

[...]
redirectpizza_redirect.example: Refreshing state... [id=1624710909]
2024-05-02T16:06:41.170-0700 [INFO]  provider.terraform-provider-redirectpizza_v0.2.0.exe: 2024/05/02 16:06:41 [ERROR] setting state: destination.0.url: can only set full list: timestamp=2024-05-02T16:06:41.170-0700
2024-05-02T16:06:41.170-0700 [INFO]  provider.terraform-provider-redirectpizza_v0.2.0.exe: 2024/05/02 16:06:41 [ERROR] setting state: destination.0.expression: can only set full list: timestamp=2024-05-02T16:06:41.170-0700
2024-05-02T16:06:41.170-0700 [INFO]  provider.terraform-provider-redirectpizza_v0.2.0.exe: 2024/05/02 16:06:41 [ERROR] setting state: destination.0.monitoring: can only set full list: timestamp=2024-05-02T16:06:41.170-0700     
2024-05-02T16:06:41.170-0700 [WARN]  Provider "registry.terraform.io/enflow/redirectpizza" produced an unexpected new value for redirectpizza_redirect.example during refresh.
      - .uri_forwarding: was null, but now cty.False
      - .keep_query_string: was null, but now cty.False
      - .redirect_type: was null, but now cty.StringVal("permanent")
      - .tags: was null, but now cty.SetValEmpty(cty.String)
      - .tracking: was null, but now cty.True
2024-05-02T16:06:41.171-0700 [INFO]  provider.terraform-provider-redirectpizza_v0.2.0.exe: 2024/05/02 16:06:41 [ERROR] setting state: sources.0: '' expected type 'string', got unconvertible type 'map[string]interface {}', value: 'map[url:[snip]]': timestamp=2024-05-02T16:06:41.170-0700
2024-05-02T16:06:41.173-0700 [WARN]  Provider "registry.terraform.io/enflow/redirectpizza" produced an invalid plan for redirectpizza_redirect.example, but we are tolerating it because it is using the legacy plugin SDK.        
    The following problems may be the cause of any confusing errors from downstream operations:
      - .keep_query_string: planned value cty.False for a non-computed attribute
      - .redirect_type: planned value cty.StringVal("permanent") for a non-computed attribute
      - .tags: planned value cty.SetValEmpty(cty.String) for a non-computed attribute
      - .destination[0].monitoring: planned value cty.StringVal("inherit") for a non-computed attribute
  [...]

mbardelmeijer commented 6 months ago

Thanks, @michael-sterling, for the well-documented issue! We will investigate and circle back 👍