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

Panic for unset types.Number #89

Closed keldin-coding closed 3 years ago

keldin-coding commented 3 years ago

Module version

module terraform-provider-clubhouse

go 1.16

require (
    github.com/hashicorp/terraform-plugin-framework v0.2.0
    github.com/hashicorp/terraform-plugin-go v0.3.1
    golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 // indirect
    golang.org/x/sys v0.0.0-20210510120138-977fb7262007 // indirect
    golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
)

Relevant provider source code

https://gist.github.com/lirossarvet/8be0ce992f2ac44edf60ff28d31681b0

This is the relevant chunk, I think, or at least enough. The whole resource is in the gist, though.

package clubhouse

import (
    "context"
    "fmt"
    "terraform-provider-clubhouse/client"

    // "github.com/hashicorp-demoapp/hashicups-client-go"
    "github.com/hashicorp/terraform-plugin-framework/schema"
    "github.com/hashicorp/terraform-plugin-framework/tfsdk"
    "github.com/hashicorp/terraform-plugin-framework/types"

    // "github.com/hashicorp/terraform-plugin-framework/types"
    "github.com/hashicorp/terraform-plugin-go/tfprotov6"
)

type resourceStoryType struct{}

// Order Resource schema
func (r resourceStoryType) GetSchema(_ context.Context) (schema.Schema, []*tfprotov6.Diagnostic) {
    return schema.Schema{
        Attributes: map[string]schema.Attribute{
            "name": {
                Type:     types.StringType,
                Required: true,
            },
            "project_id": {
                Type:     types.NumberType,
                Required: true,
            },
            "description": {
                Type:     types.StringType,
                Required: true,
            },
            "epic_id": {
                Type:     types.NumberType,
                Optional: true,
            },
            "story_type": {
                Type:     types.StringType,
                Optional: true,
                Computed: true,
            },
            "group_id": {
                Type:     types.StringType,
                Optional: true,
            },
            "id": {
                Type:     types.NumberType,
                Computed: true,
            },
        },
    }, nil
}

// New resource instance
func (r resourceStoryType) NewResource(_ context.Context, p tfsdk.Provider) (tfsdk.Resource, []*tfprotov6.Diagnostic) {
    return resourceStory{
        p: *(p.(*provider)),
    }, nil
}

type resourceStory struct {
    p provider
}

type resourceStoryData struct {
    Name        types.String `tfsdk:"name"`
    ProjectId   types.Number `tfsdk:"project_id"`
    Description types.String `tfsdk:"description"`
    EpicId      types.Number `tfsdk:"epic_id"`
    Id          types.Number `tfsdk:"id"`
    GroupId     types.String `tfsdk:"group_id"`
    StoryType   types.String `tfsdk:"story_type"`
}

// Create a new resource
func (r resourceStory) Create(ctx context.Context, req tfsdk.CreateResourceRequest, resp *tfsdk.CreateResourceResponse) {
    if !r.p.configured {
        resp.Diagnostics = append(resp.Diagnostics, &tfprotov6.Diagnostic{
            Severity: tfprotov6.DiagnosticSeverityError,
            Summary:  "Provider not configured",
            Detail:   "The provider hasn't been configured before apply, likely because it depends on an unknown value from another resource. This leads to weird stuff happening, so we'd prefer if you didn't do that. Thanks!",
        })
        return
    }

    var plan resourceStoryData
    err := req.Plan.Get(ctx, &plan)
    if err != nil {
        resp.Diagnostics = append(resp.Diagnostics, &tfprotov6.Diagnostic{
            Severity: tfprotov6.DiagnosticSeverityError,
            Summary:  "Error reading plan",
            Detail:   "An unexpected error was encountered while reading the plan: " + err.Error(),
        })
        return
    }

    rawStory := client.Story{
        Name:        plan.Name.Value,
        ProjectId:   convertBigFloatToInt64(plan.ProjectId.Value),
        Description: plan.Description.Value,
    }

    if !plan.StoryType.Unknown && !plan.StoryType.Null {
        rawStory.StoryType = &(plan.StoryType.Value)
    }

    if !plan.EpicId.Null && !plan.EpicId.Unknown {
        i := convertBigFloatToInt64(plan.EpicId.Value)
        rawStory.EpicId = &i
    }

    if !plan.GroupId.Null {
        rawStory.GroupId = &plan.GroupId.Value
    }

    created, httpErr := r.p.client.CreateStory(ctx, rawStory)
    if httpErr != nil {
        resp.Diagnostics = append(resp.Diagnostics, &tfprotov6.Diagnostic{
            Severity: tfprotov6.DiagnosticSeverityError,
            Summary:  "Failed to create",
            Detail:   "An unexpected error was encountered while creating the resource: " + httpErr.Error(),
        })
        return
    }

    forState := resourceStoryData{
        Id:          types.Number{Value: convertInt64ToBigFloat(*created.Id)},
        ProjectId:   types.Number{Value: convertInt64ToBigFloat(created.ProjectId)},
        Name:        types.String{Value: created.Name},
        Description: types.String{Value: created.Description},
    }

    if created.StoryType != nil {
        forState.StoryType = types.String{Value: *created.StoryType}
    }

    if created.EpicId != nil {
        forState.EpicId = types.Number{Value: convertInt64ToBigFloat(*created.EpicId)}
    } else {
        forState.EpicId = types.Number{Null: true}
    }

    if created.GroupId != nil {
        forState.GroupId = types.String{Value: *created.GroupId}
    }

    resp.Diagnostics = append(resp.Diagnostics, &tfprotov6.Diagnostic{
        Severity: tfprotov6.DiagnosticSeverityWarning,
        Summary:  "What're we doing",
        Detail:   fmt.Sprintf("HERE WE GO\nCreatead: %+v\nForState %+v\n", created, forState),
    })
    return

    stateError := resp.State.Set(ctx, forState)
    if stateError != nil {
        resp.Diagnostics = append(resp.Diagnostics, &tfprotov6.Diagnostic{
            Severity: tfprotov6.DiagnosticSeverityError,
            Summary:  "Failed to set state",
            Detail:   "An unexpected error was encountered while setting the state: " + stateError.Error(),
        })
        return
    }
}

API Client struct

type Story struct {
    Name        string `json:"name"`
    ProjectId   int64  `json:"project_id"`
    Description string `json:"description"`

    EpicId    *int64  `json:"epic_id,omitempty"`
    Id        *int64  `json:"id,omitempty"`
    StoryType *string `json:"story_type,omitempty"`
    GroupId   *string `json:"group_id"`
    Archived  *bool   `json:"archived,omitempty"`
}

Terraform Configuration Files

terraform {
  required_providers {
    clubhouse = {
      version = ">= 0.0.1"
      source  = "github.com/firehydrant/clubhouse"
    }
  }
  required_version = "~> 1.0.3"
}

provider "clubhouse" {}

// Notice that epic_id, an Optional Attribute, is unset.
resource "clubhouse_story" "safety" {
  name        = "Hooty Hoo"
  project_id  = 1 // Not a real ID
  group_id    = "aca6a0e5-690e-4b5c-9fbd-dc73f8109a9d" // not a real ID in this case
  description = <<-EOT
  HOO HOO
  EOT
}

Debug Output

Panic output is in the gist posted above. I can share the trace logs if needed.

Expected Behavior

An unset Optional types.Number attribute would not cause a panic and would instead not set the value in state or ignore it.

Actual Behavior

Panic! (in the terraform provider)

Steps to Reproduce

  1. Create a Terraform Provider using the framework
  2. Have an attribute of type types.NumberType and Optional: true
  3. Construct the struct to pass into State.Set with something like:
type MyResourceData struct {
  MyAttribute types.Number `tfsdk:my_attribute"
}

// in Create method
m := MyResourceData

err := resp.State.Set(ctx, &m)

References

bflad commented 3 years ago

Hi @lirossarvet 👋 Thank you so much for filing this detailed bug report. This is certainly not expected behavior! We have a fix for this that will be proposed soon and we will let you know when this is resolved.

In the meantime, using *big.Float directly (or another number pointer type, such as *int64) should allow this use case to work as expected. e.g.

type MyResourceData struct {
  MyAttribute *big.Float `tfsdk:my_attribute"
}
bflad commented 3 years ago

The fix for this has been merged and will be part of the next terraform-plugin-framework release, either v0.4.3 (if shipped) or v0.5.0, in the coming weeks. 🎉

github-actions[bot] commented 2 years ago

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.