[monitor] Fix panic when dynamic variables block is empty#3846
Open
phillip-dd wants to merge 2 commits into
Open
[monitor] Fix panic when dynamic variables block is empty#3846phillip-dd wants to merge 2 commits into
phillip-dd wants to merge 2 commits into
Conversation
Fix panic 'interface conversion: interface {} is nil, not
map[string]interface {}' in buildMonitorStruct when a variables block is
empty.
This shows up when an HCL dynamic block resolves to an empty
'variables {}' block — for example:
dynamic "variables" {
for_each = var.monitoring_variables != null ? [var.monitoring_variables] : []
content {
dynamic "cloud_cost_query" {
for_each = var.monitoring_variables.cloud_cost_queries != null ? var.monitoring_variables.cloud_cost_queries : []
content { ... }
}
}
}
with 'monitoring_variables = { cloud_cost_queries = [] }'. The outer
variables block exists but has no inner content, and SDKv2 surfaces it
during PlanResourceChange/CustomizeDiff as a list containing a single
nil element. The unsafe type assertion 'v.(map[string]interface{})'
then panics and terraform plan crashes.
Add the same defensive nil-guards we already use for data_quality_query
/ aggregate_*_query / data_jobs_query at three places:
- the outer 'variables' list iteration
- the inner 'event_query' list iteration
- the inner 'cloud_cost_query' list iteration
Nil entries are skipped, matching the existing pattern; if every entry
is nil, no variables are set on the monitor, which is the same result
as omitting the block.
Add unit tests that drive buildMonitorStruct with a stub Resource and
reproduce the original panic without the fix:
- TestBuildMonitorStruct_NilVariableEntry
- TestBuildMonitorStruct_NilCloudCostQueryEntry
- TestBuildMonitorStruct_NilEventQueryEntry
Replace the previous unit tests with TestAccDatadogMonitor_DynamicEmptyVariables, an acceptance test that exercises the wrapper-module pattern from issue #3149: an outer 'dynamic "variables"' that always produces one block, and an inner 'dynamic "cloud_cost_query"' whose for_each resolves to an empty list. The result is a single empty 'variables {}' block — exactly the input shape that crashed buildMonitorStruct on master with panic: interface conversion: interface {} is nil, not map[string]interface {} before the nil-guard fix in 2b011918. Verified by temporarily reverting the production fix: the test reproduces the exact panic from the issue, then passes once the fix is reapplied. The recorded cassette mirrors TestAccDatadogMonitor_EmptySchedulingOptions (same monitor shape, no priority, no variables field in any request body since the empty block is filtered out).
|
I can only run on private repositories. |
Contributor
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a panic that occurs when a
dynamic "variables"block in a Terraform configuration produces an emptyvariables {}block with no inner content. Fixes #3149Problem
When using the wrapper-module pattern with a
dynamic "variables"block that resolves to an empty block (nocloud_cost_query,event_query, ordata_quality_queryentries), the provider would crash duringterraform planwith:This occurred in the
buildMonitorStructfunction when attempting to type-assert nil values from the Terraform configuration.Solution
Added defensive checks in
buildMonitorStructto:variableslistThese changes allow empty
variables {}blocks to be gracefully handled, resulting in novariablesfield being sent to the API (same behavior as omitting the block entirely).Testing
Added
TestAccDatadogMonitor_DynamicEmptyVariablestest that verifies the provider can successfully create a monitor with an empty dynamic variables block, matching the real-world wrapper-module pattern from issue #3149.PR by Bits - View session in Datadog
Comment @DataDog to request changes