Unexpected Behavior With Plan Modifier (UseStateForUnknown) Prior State Under Lists and Sets

[rschmied]

Module version

github.com/hashicorp/terraform-plugin-framework v1.2.0
Terraform v1.4.2 on linux_amd64

Relevant provider source code

func LabGroup() map[string]schema.Attribute {
    return map[string]schema.Attribute{
        "id": schema.StringAttribute{
            Description: "Group ID (UUID).",
            Optional:    true,
            Computed:    true,
            PlanModifiers: []planmodifier.String{
                stringplanmodifier.UseStateForUnknown(),
            },
        },
        "name": schema.StringAttribute{
            Description: "Descriptive group name.",
            Computed:    true,
            PlanModifiers: []planmodifier.String{
                stringplanmodifier.UseStateForUnknown(),
            },
        },
        "permission": schema.StringAttribute{
            MarkdownDescription: "Permission, either `read_only` or `read_write`.",
            Optional:            true,
            Computed:            true,
            PlanModifiers: []planmodifier.String{
                stringplanmodifier.UseStateForUnknown(),
            },
            Validators: []validator.String{
                cmlvalidator.GroupPermission{},
            },
        },
    }
}

func Lab() map[string]schema.Attribute {
    return map[string]schema.Attribute{
        "id": schema.StringAttribute{
            Computed:    true,
            Description: "Lab identifier, a UUID.",
            PlanModifiers: []planmodifier.String{
                stringplanmodifier.UseStateForUnknown(),
            },
        },
        // attributes ommitted here
        "groups": schema.SetNestedAttribute{
            Optional:    true,
            Computed:    true,
            Description: "Groups assigned to the lab.",
            NestedObject: schema.NestedAttributeObject{
                Attributes: LabGroup(),
            },
            PlanModifiers: []planmodifier.Set{
                setplanmodifier.UseStateForUnknown(),
            },
        },
    }
}

Terraform Configuration Files

resource "cml2_group" "group1" {
    name       = "user_acc_lab_test_group1"
}

resource "cml2_group" "group2" {
    name       = "user_acc_lab_test_group2"
}

resource "cml2_lab" "test" {
    title       = "something"
    description = "something more"
    notes   = "somenotes"
    groups = [
        {
            id = cml2_group.group1.id
            permission = "read_only"
        },
        {
            id = cml2_group.group2.id
            permission = "read_only"
        }
    ]
}

Debug Output

Expected Behavior

Running with this configuration and removing one of the groups from the set should update the state to reflect the groups set change.

Actual Behavior

The update happens OK on the backend, the API returns all the right elements in the groups set (1 element) but it's a 50/50 chance that the state still has the other value.

My test removes the element with the ID cml2_group.group1.id from the set. When it fails, the resulting error is

=== RUN   TestAccLabResource
    lab_test.go:29: Step 3/3 error: Error running apply: exit status 1

        Error: Provider produced inconsistent result after apply

        When applying changes to cml2_lab.test, provider
        "provider[\"registry.terraform.io/hashicorp/cml2\"]" produced an unexpected
        new value: .groups: planned set element
        cty.ObjectVal(map[string]cty.Value{"id":cty.StringVal("ee272f5e-2886-43db-9591-1e60a235bf6e"),
        "name":cty.StringVal("user_acc_lab_test_group1"),
        "permission":cty.StringVal("read_write")}) does not correlate with any
        element in actual.

        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.
--- FAIL: TestAccLabResource (8.56s)
FAIL
FAIL    github.com/rschmied/terraform-provider-cml2/internal/provider/resource/lab      8.574s
FAIL

Apparently, the remaining element in the state/set is still the one that was removed from the HCL (and which does not appear in the element returned from the API). It's entirely possible that I do something wrong here as I was working with Lists before but had to move to Sets because of element order etc. As a workaround and in an attempt to mitigate / isolate the error, I added a SetUnknown to ModifyPlan like so:

    resp.Diagnostics.Append(
        resp.Plan.SetAttribute(
            ctx, path.Root("groups"),
            types.SetUnknown(
                types.ObjectType{
                    AttrTypes: cmlschema.LabGroupAttrType,
                },
            ),
        )...,
    )

When I add the above code, Terraform crashes:

=== RUN   TestAccLabResource
    lab_test.go:29: Step 1/3 error: Error running post-apply plan: exit status 11

        !!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

        Terraform crashed! This is always indicative of a bug within Terraform.
        Please report the crash with Terraform[1] so that we can fix this.

        When reporting bugs, please include your terraform version, the stack trace
        shown below, and any additional information which may help replicate the issue.

        [1]: https://github.com/hashicorp/terraform/issues

        !!!!!!!!!!!!!!!!!!!!!!!!!!! TERRAFORM CRASH !!!!!!!!!!!!!!!!!!!!!!!!!!!!

        value is not known
        goroutine 165 [running]:
        runtime/debug.Stack()
                /usr/local/go/src/runtime/debug/stack.go:24 +0x65
        runtime/debug.PrintStack()
                /usr/local/go/src/runtime/debug/stack.go:16 +0x19
        github.com/hashicorp/terraform/internal/logging.PanicHandler()
                /home/circleci/project/project/internal/logging/panic.go:55 +0x153
        panic({0x227e900, 0x2d49290})
                /usr/local/go/src/runtime/panic.go:890 +0x262
        github.com/zclconf/go-cty/cty.Value.LengthInt({{{0x2d74278?, 0xc0005f1ed0?}}, {0x23329a0?, 0x41b67c0?}})
                /home/circleci/go/pkg/mod/github.com/zclconf/go-cty@v1.12.1/cty/value_ops.go:1063 +0x1f8
        github.com/hashicorp/terraform/internal/plans/objchange.assertPlannedObjectValid(0xc0003f2640, {{{0x2d74278?, 0xc0005f10d0?}}, {0x2669b20?, 0xc000730d20?}}, {{{0x2d74278?, 0xc0005f0d60?}}, {0x2669b20?, 0xc000730c18?}}, {{{0x2d74278, ...}}, ...}, ...)
                /home/circleci/project/project/internal/plans/objchange/plan_valid.go:421 +0x5bf
        github.com/hashicorp/terraform/internal/plans/objchange.assertPlannedValueValid(0xc000da1440, {{{0x2d74278?, 0xc0005f10d0?}}, {0x2669b20?, 0xc000730d20?}}, {{{0x2d74278?, 0xc0005f0d60?}}, {0x2669b20?, 0xc000730c18?}}, {{{0x2d74278, ...}}, ...}, ...)
                /home/circleci/project/project/internal/plans/objchange/plan_valid.go:308 +0xa1b
        github.com/hashicorp/terraform/internal/plans/objchange.assertPlannedAttrValid({0xc000d884b8, 0x6}, 0xc000da1440, {{{0x2d74240?, 0xc0005f11a0?}}, {0x2360480?, 0xc000f8a420?}}, {{{0x2d74240, 0xc0005f0db0}}, {0x2360480, ...}}, ...)
                /home/circleci/project/project/internal/plans/objchange/plan_valid.go:264 +0x345
        github.com/hashicorp/terraform/internal/plans/objchange.assertPlannedAttrsValid(0x0?, {{{0x2d74240?, 0xc0005f11a0?}}, {0x2360480?, 0xc000f8a420?}}, {{{0x2d74240?, 0xc0005f0db0?}}, {0x2360480?, 0xc000f8a000?}}, {{{0x2d74240, ...}}, ...}, ...)
                /home/circleci/project/project/internal/plans/objchange/plan_valid.go:249 +0x24b
        github.com/hashicorp/terraform/internal/plans/objchange.assertPlanValid(0xc000e18600, {{{0x2d74240?, 0xc0005f11a0?}}, {0x2360480?, 0xc000f8a420?}}, {{{0x2d74240?, 0xc0005f0db0?}}, {0x2360480?, 0xc000f8a000?}}, {{{0x2d74240, ...}}, ...}, ...)
                /home/circleci/project/project/internal/plans/objchange/plan_valid.go:57 +0x3b2
        github.com/hashicorp/terraform/internal/plans/objchange.AssertPlanValid(...)
                /home/circleci/project/project/internal/plans/objchange/plan_valid.go:36
        github.com/hashicorp/terraform/internal/terraform.(*NodeAbstractResourceInstance).plan(0xc000357680, {0x2d8bfd8, 0xc0002af5e0}, 0x0, 0xc000980ba0, 0x0, {0x0, 0x0, 0x1?})
                /home/circleci/project/project/internal/terraform/node_resource_abstract_instance.go:830 +0x19fe
        github.com/hashicorp/terraform/internal/terraform.(*NodePlannableResourceInstance).managedResourceExecute(0xc000a01dc0, {0x2d8bfd8, 0xc0002af5e0})
                /home/circleci/project/project/internal/terraform/node_resource_plan_instance.go:223 +0xdae
        github.com/hashicorp/terraform/internal/terraform.(*NodePlannableResourceInstance).Execute(0x0?, {0x2d8bfd8?, 0xc0002af5e0?}, 0x0?)
                /home/circleci/project/project/internal/terraform/node_resource_plan_instance.go:60 +0x90
        github.com/hashicorp/terraform/internal/terraform.(*ContextGraphWalker).Execute(0xc000aba000, {0x2d8bfd8, 0xc0002af5e0}, {0x7f2c72185f68, 0xc000a01dc0})
                /home/circleci/project/project/internal/terraform/graph_walk_context.go:136 +0xc2
        github.com/hashicorp/terraform/internal/terraform.(*Graph).walk.func1({0x26e9960, 0xc000a01dc0})
                /home/circleci/project/project/internal/terraform/graph.go:75 +0x315
        github.com/hashicorp/terraform/internal/dag.(*Walker).walkVertex(0xc000980120, {0x26e9960, 0xc000a01dc0}, 0xc000a01f80)
                /home/circleci/project/project/internal/dag/walk.go:381 +0x2f6
        created by github.com/hashicorp/terraform/internal/dag.(*Walker).Update
                /home/circleci/project/project/internal/dag/walk.go:304 +0xf65
--- FAIL: TestAccLabResource (3.63s)
FAIL
FAIL    github.com/rschmied/terraform-provider-cml2/internal/provider/resource/lab      3.634s
FAIL

Steps to Reproduce

Testing this requires the provider to talk to some on-Prem software -- so, it's kind of difficult to reproduce without access to said software. It's entirely possible that the code does something utterly stupid. However, setting the Set to Unknown should NOT crash Terraform.

References

• Code is available on GitHub https://github.com/CiscoDevNet/terraform-provider-cml2/tree/dev
• Gist with a failed and an OK log https://gist.github.com/rschmied/10abecd7d4373bf574e17c281c98aa1a

[bflad]

Hi @rschmied 👋 Thank you for reporting these issues and sorry you are running into this unexpected behavior.

In an attempt to immediately unblock you, can you try removing the UseStateForUnknown() plan modifiers from the lab group nested attributes? This should at least prevent the framework logic from causing your plans to raise the Provider produced inconsistent result after apply Terraform error. There a few considerations the maintainers will need to work through to determine if a fix can be made for the underlying issue without breaking compatibility or if plan modifiers such as UseStateForUnknown() will need to raise an implementation error diagnostic to prevent their usage when under a list/set.

After some initial investigation, it appears the following setup is problematic:

• Attribute-based plan modifiers which attempt to read prior state (such as UseStateForUnknown())
• Implementing those attribute-based plan modifiers on attributes underneath a schema.ListNestedAttribute, schema.ListNestedBlock, schema.SetNestedAttribute, or schema.SetNestedBlock
• The resource is planning/applying an in-place update which rearranges and/or removes those elements within the list/set

For the Terraform core panic you mention, I have created a small reproduction and submitted a bug report to Terraform core. It is always invalid for a provider to attempt to set a known configuration value to unknown in a plan response. The framework doesn't have guardrails to raise its own error in this situation and Terraform core is currently raising a panic instead of a proper error (which will be the fix over there). Depending on the clarity of the future Terraform core error, the maintainers here can make a determination whether the framework should implement duplicate detection logic to raise a potentially clearer error.

[rschmied]

Thanks for looking into this -- I am currently on PTO and will try your proposed workaround when I am back.

[github-actions[bot]]

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.

[bflad]

I'm reopening this to signify that the maintainers were not able to rectify the second half of the changes needed before releasing v1.3.0 soon, so v1.3.0 will work with the same (potentially confusing) behavior as before. The hope is that this can be fully addressed in a subsequent release, but there is no timeline yet for that effort.