SetNew in CustomizeDiff does not trigger diff on computed field when map value is set to empty string

invidian commented 4 years ago

SDK version

{
  "Path": "github.com/hashicorp/terraform-plugin-sdk",
  "Version": "v1.9.0"
}

Relevant provider source code

package main

import (
    "log"

    "github.com/google/go-cmp/cmp"
    "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
    "github.com/hashicorp/terraform-plugin-sdk/plugin"
    "github.com/hashicorp/terraform-plugin-sdk/terraform"
)

func main() {
    plugin.Serve(&plugin.ServeOpts{
        ProviderFunc: Provider})
}

func Provider() terraform.ResourceProvider {
    return &schema.Provider{
        ResourcesMap: map[string]*schema.Resource{
            "mapdiff_test": resourceTest(),
        },
    }
}

func resourceTest() *schema.Resource {
    return &schema.Resource{
        Create:        resourceTestCreate,
        Read:          resourceTestRead,
        Delete:        resourceTestDelete,
        Update:        resourceTestCreate,
        CustomizeDiff: resourceTestDiff,
        Schema: map[string]*schema.Schema{
            "input": &schema.Schema{
                Type:     schema.TypeMap,
                Required: true,
                Elem: &schema.Schema{
                    Type: schema.TypeString,
                },
            },
            "result": &schema.Schema{
                Type:     schema.TypeMap,
                Computed: true,
                Elem: &schema.Schema{
                    Type: schema.TypeString,
                },
            },
        },
    }
}

func resourceTestCreate(d *schema.ResourceData, m interface{}) error {
    d.SetId("dooo")

    return resourceTestRead(d, m)
}

func resourceTestRead(d *schema.ResourceData, m interface{}) error {
    return nil
}

func resourceTestDelete(d *schema.ResourceData, m interface{}) error {
    return nil
}

func resourceTestDiff(d *schema.ResourceDiff, m interface{}) error {
    d.SetNew("result", d.Get("input"))

    o, n := d.GetChange("result")

    log.Printf(cmp.Diff(o, n))

    return nil
}

Terraform Configuration Files

// Example to reproduce.
resource "mapdiff_test" "foo" {
  input = { 
    "baz" = "hahn"
    "a"   = ""
  } 
}

// Real world example.
provider "tls" {
  version = "~> 2.1"
} 

resource "tls_private_key" "example" {
  algorithm   = "ECDSA"
  ecdsa_curve = "P384"
}

resource "mapdiff_test" "foo" {
  input = { 
    "foo" = tls_private_key.example.private_key_pem
    "baz" = "hahn"
  } 
}

Debug Output

https://gist.github.com/invidian/4f1d9d0b6f74f352aad892dab857a8fa

Expected Behavior

When using SetNew from schema.ResourceDiff on fields which are TypeMap and Computed, Terraform should properly write them and show them in diff, not ignore them.

Actual Behavior

Field input, containing user input properly shows diff and gets updated in the state, but field result gets ignored and it's never corrected. This might be problematic, when one needs a Map, where the value can actually be an empty string.

Steps to Reproduce

Save provider code to main.go file.
Build using go build -o terraform-provider-diffmap.

Create main.tf file with following content:

resource "mapdiff_test" "foo" {
input = { 
"baz" = "hahn"
"a"   = ""
} 
}

Run terraform init.
Run terraform apply -auto-approve.

Inspect created Terraform state with cat terraform.tfstate. Example output:

{
"version": 4,
"terraform_version": "0.12.24",
"serial": 38,
"lineage": "9f56ba63-345b-ca92-75c4-7e9cbd2d5da1",
"outputs": {},
"resources": [
{
  "mode": "managed",
  "type": "mapdiff_test",
  "name": "foo",
  "provider": "provider.mapdiff",
  "instances": [
    {
      "schema_version": 0,
      "attributes": {
        "id": "dooo",
        "input": {
          "a": "",
          "baz": "hahn"
        },
        "result": {
          "baz": "hahn"
        }
      },
      "private": "bnVsbA=="
    }
  ]
}
]
}

What actual output should be:

{
"version": 4,
"terraform_version": "0.12.24",
"serial": 38,
"lineage": "9f56ba63-345b-ca92-75c4-7e9cbd2d5da1",
"outputs": {},
"resources": [
{
  "mode": "managed",
  "type": "mapdiff_test",
  "name": "foo",
  "provider": "provider.mapdiff",
  "instances": [
    {
      "schema_version": 0,
      "attributes": {
        "id": "dooo",
        "input": {
          "a": "",
          "baz": "hahn"
        },
        "result": {
          "a": "",
          "baz": "hahn"
        }
      },
      "private": "bnVsbA=="
    }
  ]
}
]
}

Context

With terraform-provider-flexkube, I take user configuration, including TLS certificates and then transpile it to more complex structure, which is written to Terraform state to provide user friendly diff. Unfortunately, currently re-generating the certificates triggers inconsistent state, as certificate content is not known while planning. I would expect computed fields which has value set to empty string ("") to be seen in diff as "Known after apply", instead of just being ignored. Using SetNewComputed on sub-field produces error too.

With "real world example", running Terraform twice or with proper -target serves as a workaround. To trigger inconsistent plan issue, one needs to run terraform taint tls_private_key.example and then terraform apply.

If this is like this by design, how can I avoid getting inconsistent plan in such case?

References

Might be related to #282

invidian commented 4 years ago

I'm also doing some testing now with NewValueKnown and it seems, that this function incorrectly returns true for input field in the example I provided:

func resourceTestDiff(d *schema.ResourceDiff, m interface{}) error {
  if d.NewValueKnown("input") {
    log.Printf("new value known")
  }

And if I run terraform plan, I get the message logged every time. That doesn't seem like the right behavior either.

mingfang commented 4 years ago

@invidian I'm not entirely sure but I was seeing something similar and this is my hackaround https://github.com/mingfang/terraform-provider-k8s/blob/master/k8s/k8s.go#L99

invidian commented 4 years ago

@invidian I'm not entirely sure but I was seeing something similar and this is my hackaround https://github.com/mingfang/terraform-provider-k8s/blob/master/k8s/k8s.go#L99

Thanks for the hint @mingfang. Unfortunately, from my testing, it seems that what you're doing is to remove the extra parameters from diff, and what I need is to trigger additional diff on map field. I've tried injecting some attributes, rather than removing them, but I don't know Terraform internals very well, so I still didn't figure out how to do that.

I think for the workaround I'll just set top field as NewComputed, to at least avoid Terraform giving the error to the user. The downside is not so accurate diff provided to the user.

hashicorp / terraform-plugin-sdk