genconfig: handle null nested attribute values without panic#38604
genconfig: handle null nested attribute values without panic#38604net0pyr wants to merge 2 commits into
Conversation
|
|
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
| // A null parent value here means the surrounding object has no value | ||
| // for this nested attribute (e.g. the entire resource state passed in | ||
| // from a list resource was cty.NullVal, or a null element was emitted | ||
| // inside a nested list/map). Treat it like a null nested value and |
There was a problem hiding this comment.
A null resource value means the resource instance does not exist, so it should not be returned from a list resource. You can't generate config for a null resource value, because it is not there to be confnigured.
There was a problem hiding this comment.
Moved the fix into generateHCLListResourceDef. The null parent that was reaching writeConfigNestedTypeAttributeFromExisting came from the explicit cty.NullVal(schema.Body.ImpliedType()) fallback there. Replaced it with schema.Body.EmptyValue() (and added a IsNull() check on the provider-supplied state attribute as well), so genconfig never sees a fully-null parent value.
| })), | ||
| expected: ` | ||
| resource "tfcoremock_simple_resource" "empty" { | ||
| list = null |
There was a problem hiding this comment.
This looks like you're testing for null attributes, but the linked issue is about a null nested block for timeouts.
There was a problem hiding this comment.
Replaced the two NestedType-attribute test cases with a single resource_with_empty_state case that mirrors the shape the caller now produces: a non-null object with null primitive attributes, null nested-type attributes (single/list/map) and a null nested block. That covers both the panic regression and the legacy-block timeouts shape you flagged, without claiming to fix a non-issue in genconfig itself.
|
@net0pyr please also disclose your use of GenAI tooling per our Contributing.md guidelines. Thanks! |
3 participants
Fixes #38569
Target Release
1.16.x
Rollback Plan
Changes to Security Controls
None.
CHANGELOG entry
Root cause
cty.Value.GetAttrasserts the value's internal representation tomap[string]interface{}; that assertion panics oncty.NullVal(...)because the representation isnil. Two functions ininternal/genconfig/generate_config.go—writeConfigNestedTypeAttributeFromExistingandwriteConfigBlocksFromExisting— calledGetAttron the incoming parent value without first checkingIsNull(). In theterraform query -generate-config-outflow,generateHCLListResourceDef(ininternal/terraform) deliberately hands genconfigcty.NullVal(schema.Body.ImpliedType())whenever the provider returned no state for a queried resource, which made the panic reachable from real usage.Fix
Add an
IsNull()guard before each problematicGetAttrcall. InwriteConfigNestedTypeAttributeFromExistingthe guard emits a literalnullfor the attribute when the parent is null, mirroring the existing handling for null primitives inwriteConfigAttributesFromExisting(which falls back toattrS.EmptyValue()for the same situation) and the per-nestingnestedVal.IsNull()branches further down in the same function. InwriteConfigBlocksFromExistingthe guard short-circuits the iteration becausewriteConfigNestedBlockFromExistingalready treats a null block value as "nothing to emit". The diff stays insideinternal/genconfig.Regression test
Adds two table-driven cases to
TestConfigGenerationininternal/genconfig/generate_config_test.go:resource_with_null_stateconstructscty.NullVal(...)as the resource state and exercises every nested-type nesting mode (single/list/map) plus a nested block, asserting that generation completes without panic and emits well-formed HCL.resource_with_nested_value_siblingcovers the inverse: a non-null parent with real nested values, to guard against the new null guard becoming too eager and silently dropping data.Both tests construct the
cty.Valuedirectly — no provider, no network, no CLI invocation.No public API changes
The change is internal to
internal/genconfig. No exported symbols, function signatures, or behaviours for non-null inputs are altered.AI Usage
This PR was developed with assistance from Claude (Anthropic) as a coding assistant. Specific role: I used Claude to help locate the panic site.