core: prevent panics with null objects in nested attrs #35090
Conversation
jbardin
added
the
1.8-backport
jbardin
force-pushed
the
jbardin/plan-valid-nested-null
branch
from
d904032
to
f511356
Compare
When descending into nested structural attributes, don't try to extract attributes from null objects. Unlike with blocks, nested attributes allow the possibility of assigning null values. While these technically aren't allowed to be altered, we need to accept these for compatibility.
jbardin
force-pushed
the
jbardin/plan-valid-nested-null
branch
from
f511356
to
2562420
Compare
There was a problem hiding this comment.
This looks good, I'm pretty sure these null checks make sense. Just to clarify: in a situation like the following,
resource "pf9_cluster" "cluster" {
name = var.cluster_name
addons = {
"kubernetes-dashboard" = null
"something-else" = {
# ...nested object attrs
}
}
}...setting an entire key to null in that map of nested objects (e.g. "kubernetes-dashboard") is the act that results in attrS.NestedType being nil. Right?
Edit: Ahh that's not quite right. James filled me in in slack — NestedType is nil if it's a plain attr rather than a nested objects attr, so the changed behavior to stop the panic is all relevant to the non-nil NestedType case. The real issue with the example config would be something like: the config value is a null value of the relevant object type, but (due to a provider framework bug) the planned value is a non-null value of that object type whose keys are all null. So we need to cope with the potential nullness of the various value sources independently.
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
When descending into structural attributes, don't try to extract attributes from null objects. Unlike with blocks, nested attributes allow the possibility of assigning null values which could be overridden by the provider.
Technically nested object attributes like this should not be allowed to to change from the configuration (
ComputedandOptionaldo not apply to the individual container elements, only to the parent attribute), but due to existing providers we need to continue to let these pass through. Luckily the cases where it might cause problems are relatively few, since a corresponding configuration entry must have been assigned asnullto allow the new value through in the first place, and any non-null attributes would already have to be defined asComputedto pass further validation. This behavior of the attribute acting like it's computed by virtue of having computed attributes itself is less surprising than if it actually had a schema setting that were being contradicted, and the fact that it is resolved early during the plan means it's less likely that fatal errors will pop up during apply.Fixes #35083