search

hashicorp / terraform-plugin-framework

A next-generation framework for building Terraform providers.
https://developer.hashicorp.com/terraform/plugin/framework
Mozilla Public License 2.0
299 stars 92 forks source link

Default values within sets cause crashes and/or incorrect behaviour #783

Open maxb opened 1 year ago

maxb commented 1 year ago

Module version

v1.3.1

Relevant provider source code

This is the entire source code of a single .go file provider that demonstrates the issues:

package main

import (
    "context"

    "github.com/hashicorp/terraform-plugin-framework/datasource"
    "github.com/hashicorp/terraform-plugin-framework/provider"
    "github.com/hashicorp/terraform-plugin-framework/providerserver"
    "github.com/hashicorp/terraform-plugin-framework/resource"
    "github.com/hashicorp/terraform-plugin-framework/resource/schema"
    "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
)

func main() {
    err := providerserver.Serve(
        context.Background(),
        func() provider.Provider {
            return new(bugProvider)
        },
        providerserver.ServeOpts{
            Address: "bug/bug/bug",
        },
    )
    if err != nil {
        panic(err)
    }
}

type bugProvider struct{}

func (p *bugProvider) Metadata(ctx context.Context, req provider.MetadataRequest, resp *provider.MetadataResponse) {
    resp.TypeName = "bug"
}

func (p *bugProvider) Schema(ctx context.Context, req provider.SchemaRequest, resp *provider.SchemaResponse) {
}

func (p *bugProvider) Configure(ctx context.Context, req provider.ConfigureRequest, resp *provider.ConfigureResponse) {
}

func (p *bugProvider) DataSources(ctx context.Context) []func() datasource.DataSource {
    return nil
}

func (p *bugProvider) Resources(ctx context.Context) []func() resource.Resource {
    return []func() resource.Resource{
        func() resource.Resource {
            return new(bugResource)
        },
    }
}

type bugResource struct{}

func (r bugResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
    resp.TypeName = req.ProviderTypeName + "_bug"
}

func (r bugResource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) {
    resp.Schema = schema.Schema{
        Attributes: map[string]schema.Attribute{
            "set": schema.SetNestedAttribute{
                Required: true,
                NestedObject: schema.NestedAttributeObject{
                    Attributes: map[string]schema.Attribute{
                        "fruit": schema.StringAttribute{
                            Optional: true,
                            Computed: true,
                            Default:  stringdefault.StaticString("orange"),
                        },
                        "other": schema.StringAttribute{
                            Optional: true,
                            Computed: true,
                            Default:  stringdefault.StaticString("other"),
                        },
                    },
                },
            },
        },
    }
}

func (r bugResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
}

func (r bugResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
    resp.State.Raw = req.Plan.Raw
}

func (r bugResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
    resp.State.Raw = req.Plan.Raw
}

func (r bugResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
}

Terraform Configuration Files

terraform {
  required_providers {
    bug = {
      source = "bug/bug/bug"
    }
  }
}

resource "bug_bug" "this" {
  set = [
    { fruit = "apple" },
    { fruit = "banana" },
    { fruit = "kumquat" },
  ]
}

Debug Output

https://gist.github.com/maxb/f0b606530531a1f7e89c1ac122f141da

Steps to Reproduce

  1. Copy the included provider source code to a file, add a go.mod including the latest version of terraform-plugin-framework (no other dependencies needed), build the provider.
  2. Set up dev_overrides so you can test the provider.
  3. Copy the included Terraform configuration to a file
  4. Run terraform apply -auto-approve (no initial state is needed)
  5. Run terraform apply -auto-approve again ... the provider panics/crashes

Secondary related bug:

  1. Remove any terraform.tfstate from the above reproduction
  2. Reduce the number of items in the set in the Terraform configuration from 3 to 1
  3. Repeatedly run terraform apply -auto-approve ... observe that provider repeatedly changes the value back and forth on each run, oscillating between the value actually written in the configuration, and the value set as a default in the code.

Partial diagnosis

The terraform-plugin-framework appears to use a bafflingly complex algorithm to apply defaults, involving correlating the state and the configuration.

This is a victim of its own complexity when dealing with sets of objects, as the identity of an object within a set incorporates its own value ... a value that may itself have defaults. This means the identity of a set member in the config may omit unspecified attributes, whilst the identity of the same set member in the state will include unspecified attributes, now set to their default values.

bflad commented 1 year ago

Related: https://github.com/hashicorp/terraform-plugin-framework/issues/709 (they will likely get fixed as best as possible at the same time)

bflad commented 1 year ago

For other maintainer's benefit, here's the relevant stack trace from framework v1.3.1:

2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: panic: runtime error: invalid memory address or nil pointer dereference
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x642887]
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: 
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: goroutine 14 [running]:
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).PlanResourceChange(0xc0001d54a0, {0xc54480, 0xc0000f2c60}, 0xc000144780, 0xc0001ff4f0)
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/github.com/hashicorp/terraform-plugin-framework@v1.3.1/internal/fwserver/server_planresourcechange.go:210 +0x20c7
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: github.com/hashicorp/terraform-plugin-framework/internal/proto6server.(*Server).PlanResourceChange(0xc0001d54a0, {0xc54480?, 0xc0000f2b10?}, 0xc000144640)
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/github.com/hashicorp/terraform-plugin-framework@v1.3.1/internal/proto6server/server_planresourcechange.go:55 +0x41a
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: github.com/hashicorp/terraform-plugin-go/tfprotov6/tf6server.(*server).PlanResourceChange(0xc000252500, {0xc54480?, 0xc0000f2150?}, 0xc0002ce070)
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/github.com/hashicorp/terraform-plugin-go@v0.16.0/tfprotov6/tf6server/server.go:784 +0x574
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/tfplugin6._Provider_PlanResourceChange_Handler({0xb2eee0?, 0xc000252500}, {0xc54480, 0xc0000f2150}, 0xc0002ce000, 0x0)
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/github.com/hashicorp/terraform-plugin-go@v0.16.0/tfprotov6/internal/tfplugin6/tfplugin6_grpc.pb.go:404 +0x170
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: google.golang.org/grpc.(*Server).processUnaryRPC(0xc0002481e0, {0xc58458, 0xc00040a1a0}, 0xc0000f4000, 0xc0003a8000, 0x108a008, 0x0)
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/google.golang.org/grpc@v1.56.0/server.go:1337 +0xdf3
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: google.golang.org/grpc.(*Server).handleStream(0xc0002481e0, {0xc58458, 0xc00040a1a0}, 0xc0000f4000, 0x0)
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/google.golang.org/grpc@v1.56.0/server.go:1714 +0xa36
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: google.golang.org/grpc.(*Server).serveStreams.func1.1()
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/google.golang.org/grpc@v1.56.0/server.go:959 +0x98
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug: created by google.golang.org/grpc.(*Server).serveStreams.func1
2023-06-18T17:37:34.030+0100 [DEBUG] provider.terraform-provider-bug:   /home/maxb/.cache/go-modules/google.golang.org/grpc@v1.56.0/server.go:957 +0x18c

And the associated code:

https://github.com/hashicorp/terraform-plugin-framework/blob/d33e7ae875c6894dae3294fcd0a21fa10d80faa1/internal/fwserver/server_planresourcechange.go#L202-L215

While plannedState could/should have a nil check there, ignoring the errors likely would indicate something else amiss.

bflad commented 1 year ago

As part of further triaging this, returning the resp.PlannedState.GetAttribute diagnostic yields:

  | Error: Duplicate Set Element
  |
  |   with framework_bug.test,
  |   on terraform_plugin_test.tf line 2, in resource "framework_bug" "test":
  |    2: \t\t\t\t\tset = [
  |    3: \t\t\t\t\t\t{ fruit = "apple" },
  |    4: \t\t\t\t\t\t{ fruit = "banana" },
  |    5: \t\t\t\t\t\t{ fruit = "kumquat" },
  |    6: \t\t\t\t\t]
  |
  | This attribute contains duplicate values of:
  | tftypes.Object["fruit":tftypes.String,
  | "other":tftypes.String]<"fruit":tftypes.String<"orange">,
  | "other":tftypes.String<"other">>

Which is due to the (basetypes.SetValue).Validate() implementation. The precursor logging being:

2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: setting attribute set[Value({"fruit":"apple","other":"other"})].other to default value: "other": tf_resource_type=framework_bug tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: setting attribute set[Value({"fruit":"apple","other":"other"})].fruit to default value: "orange": tf_resource_type=framework_bug tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: attribute is a non-schema attribute, not setting default: tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange tf_resource_type=framework_bug
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: setting attribute set[Value({"fruit":"banana","other":"other"})].fruit to default value: "orange": tf_resource_type=framework_bug tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: setting attribute set[Value({"fruit":"banana","other":"other"})].other to default value: "other": tf_resource_type=framework_bug tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: attribute is a non-schema attribute, not setting default: tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange tf_resource_type=framework_bug
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: setting attribute set[Value({"fruit":"kumquat","other":"other"})].fruit to default value: "orange": tf_rpc=PlanResourceChange tf_resource_type=framework_bug tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: setting attribute set[Value({"fruit":"kumquat","other":"other"})].other to default value: "other": tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange tf_resource_type=framework_bug tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83
2023-07-11T14:02:11.824-0400 [TRACE] sdk.framework: attribute is a non-schema attribute, not setting default: tf_req_id=0cc087b6-31dc-9fcb-aca4-ff58d9a5aa83 tf_provider_addr=registry.terraform.io/hashicorp/framework tf_rpc=PlanResourceChange tf_resource_type=framework_bug

As you mention @maxb, there's a consternation of set values where the value itself represents its identity. While this property is due to the design of Terraform's type system, the default implementation is clearly not doing the right thing here. One potential option here would be walking set elements as slice indexes during this transformation, which matches how other internal framework logic handles other walks/transformations to avoid the set identity issue. The longer term option is disregarding Terraform's ProposedNewState data completely and re-implementing core's logic for list/set type alignment to generate our own ProposedNewState data, which was the unspoken plan for #709.

austinvalle commented 2 months ago

Another instance of this bug was reported in our discuss forums: https://discuss.hashicorp.com/t/setnestedattribute-executing-terraform-apply-gets-an-error-error-provider-produced-inconsistent-result-after-apply/67886/