search

hashicorp / terraform-plugin-sdk

Terraform Plugin SDK enables building plugins (providers) to manage any service providers or custom in-house solutions
https://developer.hashicorp.com/terraform/plugin
Mozilla Public License 2.0
439 stars 232 forks source link

SDK's handling of unknown values in maps makes it impossible to implement the provider protocol correctly #1040

Open apparentlymart opened 2 years ago

apparentlymart commented 2 years ago

SDK version

github.com/hashicorp/terraform-plugin-sdk v1.17.2

Simplified Reproduction Case

package main

import (
    "log"

    "github.com/davecgh/go-spew/spew"
    "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
    "github.com/hashicorp/terraform-plugin-sdk/terraform"
    "github.com/zclconf/go-cty/cty"
)

func main() {
    s := schema.InternalMap{
        "tags": &schema.Schema{
            Type: schema.TypeMap,
            Elem: &schema.Schema{
                Type: schema.TypeString,
            },
            Optional: true,
        },
    }
    r := &schema.Resource{
        Schema: s,
    }
    diff, err := schema.DiffFromValues(
        cty.NullVal(cty.Object(map[string]cty.Type{"tags": cty.Map(cty.String)})),
        cty.ObjectVal(map[string]cty.Value{
            "tags": cty.MapVal(map[string]cty.Value{
                "Name":    cty.StringVal("foo"),
                "Unknown": cty.UnknownVal(cty.String),
            }),
        }),
        r,
    )
    if err != nil {
        log.Fatal(err)
    }

    d, err := s.Data(
        &terraform.InstanceState{},
        diff,
    )
    if err != nil {
        log.Fatal(err)
    }
    m := d.Get("tags")
    spew.Dump(m)
}

The above is using schema.DiffFromValues directly to narrow down to the specific shim function that seems to cause this problem. In practice there's a log more protocol shimming going on around this, but this seems to be the crucial part to observe the bug this issue is describing.

Expected Behavior

The map value returned to describe the tags argument should somehow indicate that the "Unknown" element has an unknown value.

Actual Behavior

The map value doesn't include the "Unknown" element at all:

(map[string]interface {}) (len=1) {
 (string) (len=4) "Name": (string) (len=3) "foo"
 // NOTE: Element "Unknown" is totally missing here
}

This makes it impossible to correctly implement of the provider protocol for map arguments, because the value for tags will be incorrect first during planning (in a way that Terraform Core glosses over as "tolerating it because the provider is using the legacy SDK"), and then again during apply, where it'll lead to an "inconsistent final plan" error once the unknown value becomes known:

╷
│ Error: Provider produced inconsistent final plan
│
│ When expanding the plan for example.example to include new values
│ learned so far during apply, provider "example.com/example/example"
│ produced an invalid new value for .tags: new element "Unknown" has
│ appeared.
│
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.
╵

To correctly implement the protocol the provider must either indicate that the whole tags attribute is unknown or that the tags attribute is known but its Unknown attribute is present and unknown itself. But that's impossible to achieve, because the provider logic has no way to see that the "Unknown" element was present in the map at all, let alone to see that it has an unknown value.

I don't think that it will actually be possible to fix this problem within the architecture of this SDK; I'm recording it only so that there's a place to refer to when someone recognizes this symptom in a provider.

Steps to Reproduce

I have prepared a Go Playground example containing the above code: https://go.dev/play/p/Ui7km-l49EW

However, the dependencies of this program are large enough that it's challenging to get the Go Playground to actually run it without hitting a build timeout. It will probably be necessary to bring the program locally and run it with go run instead.

References

There seems to be an equivalent problem for lists containing unknown values, but I've not yet confirmed with a simplified reproduction:

I've also seen variants talking about elements being missing from sets, which probably also has a similar root cause but the error message isn't so clear because a set element has no identity other than its own value, and so Terraform Core just says something like "this value wasn't in the set before". (I don't remember the error message well enough to quickly turn up examples right now.)

bflad commented 2 years ago

Hey @apparentlymart 👋 Thank you for raising this.

Just to quickly clarify things upfront, v1 is no longer supported as of last year, but I'm guessing this is being raised in the context of v2 also having this issue (which would not be surprising as I do not believe there were architecture/design changes in this particular functionality between v1 and v2).

Provider developers that opt to migrating to using terraform-plugin-framework have full access to the actual status of configuration, plan, and state values of any type (or element within), whether null, unknown, a known value. They can also set any type (or element within) to an unknown value during planning, assuming they are following Terraform's resource instance lifecycle constraints for the particular value.

apparentlymart commented 2 years ago

Ahh yes! I knew I must've done something weird here because the signatures of some functions changed when I reproduced it as an isolated program, but I didn't quite manage to spot that I'd forgotten the v2/ portions of those import paths! Late-in-the-day brain. :man_facepalming:

Here's an equivalent reproduction against SDKv2, for completeness:

package main

import (
    "context"
    "log"

    "github.com/davecgh/go-spew/spew"
    "github.com/hashicorp/go-cty/cty"
    "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
    "github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

func main() {
    s := schema.InternalMap{
        "tags": &schema.Schema{
            Type: schema.TypeMap,
            Elem: &schema.Schema{
                Type: schema.TypeString,
            },
            Optional: true,
        },
    }
    r := &schema.Resource{
        Schema: s,
    }
    diff, err := schema.DiffFromValues(
        context.Background(),
        cty.NullVal(cty.Object(map[string]cty.Type{"tags": cty.Map(cty.String)})),
        cty.ObjectVal(map[string]cty.Value{
            "tags": cty.MapVal(map[string]cty.Value{
                "Name":    cty.StringVal("foo"),
                "Unknown": cty.UnknownVal(cty.String),
            }),
        }),
        cty.ObjectVal(map[string]cty.Value{
            "tags": cty.MapVal(map[string]cty.Value{
                "Name":    cty.StringVal("foo"),
                "Unknown": cty.UnknownVal(cty.String),
            }),
        }),
        r,
    )
    if err != nil {
        log.Fatal(err)
    }

    d, err := s.Data(
        &terraform.InstanceState{},
        diff,
    )
    if err != nil {
        log.Fatal(err)
    }
    m := d.Get("tags")
    spew.Dump(m)
}

I tested the above with SDK v2.21.0 and got the same result I had inadvertently previously demonstrated with v1.17.2.