Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cty: WithoutOptionalAttributesDeep does not panic on cty.NilType #170

Closed
wants to merge 1 commit into from

Conversation

ryancragun
Copy link

Fix an issue where WithoutOptionalAttributesDeep() on a cty.NilType would result in a panic. This was discovered when using gohcl to decode a block that has attributes that are set to null.

panic: WithoutOptionalAttributesDeep does not support the given type

goroutine 46 [running]:
github.com/zclconf/go-cty/cty.Type.WithoutOptionalAttributesDeep({{0x0?, 0x0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/type.go:152 +0x565
github.com/zclconf/go-cty/cty/convert.conversionObjectToObject.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {0x0, 0x0, 0x0})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion_object.go:86 +0x3a5
github.com/zclconf/go-cty/cty/convert.getConversion.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {0x0, 0x0, 0x0})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion.go:57 +0x2b3
github.com/zclconf/go-cty/cty/convert.GetConversionUnsafe.retConversion.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion.go:199 +0x34
github.com/zclconf/go-cty/cty/convert.Convert({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {{0x197f548?, 0xc0006288b0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/public.go:51 +0x151
github.com/hashicorp/hcl/v2/gohcl.DecodeExpression({0x197f698, 0xc0003b27e0}, 0x197f580?, {0x1720c40, 0xc0001cdcf8})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/hashicorp/hcl/v2@v2.18.0/gohcl/decode.go:299 +0xb0
github.com/hashicorp/enos/internal/flightplan.(*Scenario).decodeAndValidateTerraformCLIAttribute(0xc0001cdce0, 0xc0000cac00, 0xc001ac7998)
  ¦ ¦ ¦ /home/runner/actions-runner/_work/enos/enos/internal/flightplan/scenario.go:349 +0x610
github.com/hashicorp/enos/internal/flightplan.(*Scenario).decode(...)
  ¦ ¦ ¦ /home/runner/actions-runner/_work/enos/enos/internal/flightplan/scenario.go:187
github.com/hashicorp/enos/internal/flightplan.(*ScenarioDecoder).decodeScenario(0xc0005b36c8, 0xc000bc8bd0, 0xc00085cf70)
panic: WithoutOptionalAttributesDeep does not support the given type

Fix an issue where `WithoutOptionalAttributesDeep()` on a `cty.NilType`
would result in a panic. This was discovered when using `gohcl` to
decode a block that has attributes that are set to null.

```
panic: WithoutOptionalAttributesDeep does not support the given type

goroutine 46 [running]:
github.com/zclconf/go-cty/cty.Type.WithoutOptionalAttributesDeep({{0x0?, 0x0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/type.go:152 +0x565
github.com/zclconf/go-cty/cty/convert.conversionObjectToObject.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {0x0, 0x0, 0x0})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion_object.go:86 +0x3a5
github.com/zclconf/go-cty/cty/convert.getConversion.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {0x0, 0x0, 0x0})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion.go:57 +0x2b3
github.com/zclconf/go-cty/cty/convert.GetConversionUnsafe.retConversion.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion.go:199 +0x34
github.com/zclconf/go-cty/cty/convert.Convert({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {{0x197f548?, 0xc0006288b0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/public.go:51 +0x151
github.com/hashicorp/hcl/v2/gohcl.DecodeExpression({0x197f698, 0xc0003b27e0}, 0x197f580?, {0x1720c40, 0xc0001cdcf8})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/hashicorp/hcl/v2@v2.18.0/gohcl/decode.go:299 +0xb0
github.com/hashicorp/enos/internal/flightplan.(*Scenario).decodeAndValidateTerraformCLIAttribute(0xc0001cdce0, 0xc0000cac00, 0xc001ac7998)
  ¦ ¦ ¦ /home/runner/actions-runner/_work/enos/enos/internal/flightplan/scenario.go:349 +0x610
github.com/hashicorp/enos/internal/flightplan.(*Scenario).decode(...)
  ¦ ¦ ¦ /home/runner/actions-runner/_work/enos/enos/internal/flightplan/scenario.go:187
github.com/hashicorp/enos/internal/flightplan.(*ScenarioDecoder).decodeScenario(0xc0005b36c8, 0xc000bc8bd0, 0xc00085cf70)
panic: WithoutOptionalAttributesDeep does not support the given type
```

Signed-off-by: Ryan Cragun <me@ryan.ec>
@apparentlymart
Copy link
Collaborator

Hi @ryancragun! Thanks for working on this.

cty.NilType is not a valid type -- it represents the absence of one -- and so I think it's reasonable for this to be a panic although I would agree that the panic message is not communicating that well.

The name NilType is intended to communicate that it's similar to Go's native nil for types like interfaces, where it literally represents "nothing" and most interactions with it will panic. The same is true for NilValue which is the analogous concept for values. These "nothing" values exist for situations such as in functions that return cty.Type, error where the error and the type are mutually exclusive. No "real" type should ever include cty.NilType anywhere in it.

@ryancragun
Copy link
Author

Sounds good. Here's another attempt of fixing the panic #171

@ryancragun ryancragun closed this Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants