Default values within sets cause crashes and/or incorrect behaviour #783
Labels
bug
Something isn't working
Comments
|
Related: #709 (they will likely get fixed as best as possible at the same time) |
|
For other maintainer's benefit, here's the relevant stack trace from framework v1.3.1: And the associated code: terraform-plugin-framework/internal/fwserver/server_planresourcechange.go Lines 202 to 215 in d33e7ae While |
|
As part of further triaging this, returning the Which is due to the 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 |
bflad
added a commit
that referenced
this issue
Jul 11, 2023
…th logging Reference: #783 Prior to the logic update: ``` --- FAIL: TestServerPlanResourceChange (0.01s) --- FAIL: TestServerPlanResourceChange/update-set-default-values (0.00s) panic: runtime error: invalid memory address or nil pointer dereference [recovered] panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x2 addr=0x18 pc=0x1043b0484] goroutine 32 [running]: testing.tRunner.func1.2({0x1048d4e60, 0x104c08f90}) /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1526 +0x1c8 testing.tRunner.func1() /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1529 +0x384 panic({0x1048d4e60, 0x104c08f90}) /opt/homebrew/Cellar/go/1.20.5/libexec/src/runtime/panic.go:884 +0x204 github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).PlanResourceChange(0x140001f4a00, {0x10494ddb0, 0x14000098008}, 0x140001eeeb0, 0x14000592540) /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange.go:219 +0x19e4 github.com/hashicorp/terraform-plugin-framework/internal/fwserver_test.TestServerPlanResourceChange.func37(0x14000231860) /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:5849 +0x70 testing.tRunner(0x14000231860, 0x140000ccde0) /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1576 +0x10c created by testing.(*T).Run /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x368 FAIL github.com/hashicorp/terraform-plugin-framework/internal/fwserver 0.457s ``` This change verifies and handles the more critical bug fix of the panic report. The handling of defaults for set nested attributes will require further logic updates, which will be handled in a future change.
bflad
added a commit
that referenced
this issue
Jul 12, 2023
…th logging (#797) Reference: #783 Prior to the logic update: ``` --- FAIL: TestServerPlanResourceChange (0.01s) --- FAIL: TestServerPlanResourceChange/update-set-default-values (0.00s) panic: runtime error: invalid memory address or nil pointer dereference [recovered] panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x2 addr=0x18 pc=0x1043b0484] goroutine 32 [running]: testing.tRunner.func1.2({0x1048d4e60, 0x104c08f90}) /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1526 +0x1c8 testing.tRunner.func1() /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1529 +0x384 panic({0x1048d4e60, 0x104c08f90}) /opt/homebrew/Cellar/go/1.20.5/libexec/src/runtime/panic.go:884 +0x204 github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).PlanResourceChange(0x140001f4a00, {0x10494ddb0, 0x14000098008}, 0x140001eeeb0, 0x14000592540) /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange.go:219 +0x19e4 github.com/hashicorp/terraform-plugin-framework/internal/fwserver_test.TestServerPlanResourceChange.func37(0x14000231860) /Users/bflad/src/github.com/hashicorp/terraform-plugin-framework/internal/fwserver/server_planresourcechange_test.go:5849 +0x70 testing.tRunner(0x14000231860, 0x140000ccde0) /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1576 +0x10c created by testing.(*T).Run /opt/homebrew/Cellar/go/1.20.5/libexec/src/testing/testing.go:1629 +0x368 FAIL github.com/hashicorp/terraform-plugin-framework/internal/fwserver 0.457s ``` This change verifies and handles the more critical bug fix of the panic report. The handling of defaults for set nested attributes will require further logic updates, which will be handled in a future change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Module version
Relevant provider source code
This is the entire source code of a single .go file provider that demonstrates the issues:
Terraform Configuration Files
Debug Output
https://gist.github.com/maxb/f0b606530531a1f7e89c1ac122f141da
Steps to Reproduce
go.modincluding the latest version of terraform-plugin-framework (no other dependencies needed), build the provider.dev_overridesso you can test the provider.terraform apply -auto-approve(no initial state is needed)terraform apply -auto-approveagain ... the provider panics/crashesSecondary related bug:
terraform.tfstatefrom the above reproductionterraform 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.
The text was updated successfully, but these errors were encountered: