search

hashicorp / terraform-plugin-go

A low-level Go binding for the Terraform protocol for integrations to be built on top of.
Mozilla Public License 2.0
128 stars 33 forks source link

tftypes: Adjust `(tftypes.Value).Equal` function to support dynamic value comparisons #383

Closed austinvalle closed 8 months ago

austinvalle commented 8 months ago

Ref: https://github.com/hashicorp/terraform-plugin-framework/pull/931

This change adjusts (tftypes.Value).Equal to not panic if the concrete values don't match 😄 .

This is an edge-case that was originally uncovered in the linked dynamic PR because it's possible that the types may be equal, but the concrete values are different when compared: https://github.com/hashicorp/terraform-plugin-framework/pull/931/files?show-viewed-files=true&file-filters%5B%5D=#diff-d827ef82ad83975f9b9a8cd0128db4a0798d0fb6878aea019ad4e1d642cb07b1

This can be caused with a types.Dynamic when changing the type during a refresh, i.e. ReadResource RPC

austinvalle commented 8 months ago

I'd love to release a v0.22.1 with this change whenever the PR is approved 🙂

austinvalle commented 8 months ago

@bflad Looks good to me, although I would also suggest adding a unit test case which previously panicked. 🚀

Whoops, completely forgot about the tests 😅 . Added a bunch of tests (including some missing equality type diffs that weren't related to the panic) in 72f9762623556b385a9450a9147204c87c48186e.

I'm also going to hold off on creating a v0.22.1 release to avoid creating too much noise around the function changes. I don't need the release until we get closer to merging the dynamic PR 👍🏻

Before changes (tuple test)

--- FAIL: TestValueEqual (0.00s)
    --- FAIL: TestValueEqual/DynamicPseudoType-tupleDiff-different-value-types (0.00s)
panic: ElementKeyInt(0): cannot convert bool into string [recovered]
    panic: ElementKeyInt(0): cannot convert bool into string

goroutine 37 [running]:
testing.tRunner.func1.2({0x10446ba20, 0x1400020ea50})
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1545 +0x1c8
testing.tRunner.func1()
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1548 +0x360
panic({0x10446ba20?, 0x1400020ea50?})
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/runtime/panic.go:914 +0x218
github.com/hashicorp/terraform-plugin-go/tftypes.Value.Equal({{0x10448d868?, 0x14000184000?}, {0x10442aae0?, 0x1400000c948?}}, {{0x10448d868?, 0x140001840f0?}, {0x10442aae0?, 0x1400000c978?}})
    /Users/austin.valle/code/terraform-plugin-go/tftypes/value.go:226 +0x114
github.com/hashicorp/terraform-plugin-go/tftypes.TestValueEqual.func1(0x140001b41a0)
    /Users/austin.valle/code/terraform-plugin-go/tftypes/value_test.go:1213 +0x6c
testing.tRunner(0x140001b41a0, 0x14000010c80)
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1595 +0xe8
created by testing.(*T).Run in goroutine 6
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1648 +0x33c
FAIL    github.com/hashicorp/terraform-plugin-go/tftypes    0.463s
FAIL

Before changes (object test)

--- FAIL: TestValueEqual (0.00s)
    --- FAIL: TestValueEqual/DynamicPseudoType-objectDiff-different-value-types (0.00s)
panic: AttributeName("dyn_val"): cannot convert *big.Float into string [recovered]
    panic: AttributeName("dyn_val"): cannot convert *big.Float into string

goroutine 36 [running]:
testing.tRunner.func1.2({0x102513a20, 0x1400028e0c0})
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1545 +0x1c8
testing.tRunner.func1()
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1548 +0x360
panic({0x102513a20?, 0x1400028e0c0?})
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/runtime/panic.go:914 +0x218
github.com/hashicorp/terraform-plugin-go/tftypes.Value.Equal({{0x1025357b8?, 0x140001be300?}, {0x1024e91c0?, 0x140001be240?}}, {{0x1025357b8?, 0x140001be480?}, {0x1024e91c0?, 0x140001be390?}})
    /Users/austin.valle/code/terraform-plugin-go/tftypes/value.go:226 +0x114
github.com/hashicorp/terraform-plugin-go/tftypes.TestValueEqual.func1(0x140001ce680)
    /Users/austin.valle/code/terraform-plugin-go/tftypes/value_test.go:1213 +0x6c
testing.tRunner(0x140001ce680, 0x14000130780)
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1595 +0xe8
created by testing.(*T).Run in goroutine 18
    /opt/homebrew/Cellar/go/1.21.4/libexec/src/testing/testing.go:1648 +0x33c
FAIL    github.com/hashicorp/terraform-plugin-go/tftypes    0.502s
FAIL
bflad commented 8 months ago

Nice work on those tests 👍

github-actions[bot] commented 5 months ago

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.