-
Notifications
You must be signed in to change notification settings - Fork 381
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
Prevent nil pointer dereference in dashboard import #630
Prevent nil pointer dereference in dashboard import #630
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix! Added some questions.
e533853
to
8247db5
Compare
@@ -305,14 +305,14 @@ func buildTerraformTemplateVariables(datadogTemplateVariables *[]datadogV1.Dashb | |||
terraformTemplateVariables := make([]map[string]string, len(*datadogTemplateVariables)) | |||
for i, templateVariable := range *datadogTemplateVariables { | |||
terraformTemplateVariable := map[string]string{} | |||
if v, ok := templateVariable.GetNameOk(); ok { | |||
terraformTemplateVariable["name"] = *v | |||
if v := templateVariable.GetName(); len(v) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry, my suggestion only works on prefix, because it's marked nullable. Other fields don't have the same property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries :) I reverted the other two as they were before then, given that it's unlikely they'll map to a nil pointer anyway.
When importing a dashboard with a template variable with no prefix, the provider panics with the following error message: ``` panic: runtime error: invalid memory address or nil pointer dereference 2020-08-10T13:42:16.345+0200 [DEBUG] plugin.terraform-provider-datadog_v2.12.1: [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1bddcf1] 2020-08-10T13:42:16.345+0200 [DEBUG] plugin.terraform-provider-datadog_v2.12.1: 2020-08-10T13:42:16.345+0200 [DEBUG] plugin.terraform-provider-datadog_v2.12.1: goroutine 81 [running]: 2020-08-10T13:42:16.345+0200 [DEBUG] plugin.terraform-provider-datadog_v2.12.1: github.com/terraform-providers/terraform-provider-datadog/datadog.buildTerraformTemplateVariables(0xc0004a1a18, 0x1fa2ab9) 2020-08-10T13:42:16.345+0200 [DEBUG] plugin.terraform-provider-datadog_v2.12.1: github.com/terraform-providers/terraform-provider-datadog/datadog/resource_datadog_dashboard.go:296 +0x291 ``` This commit prevents by first checking if the pointer that we're going to dereference is nil. If that's the case, we assume that the template variable doesn't have a name, prefix or default, as appropriate.
8247db5
to
ea19573
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Thanks for fixing this. When will it be released in a new version of the provider? |
When importing a dashboard with a template variable with no prefix, the
provider panics with the following error message:
This commit prevents by first checking if the pointer that we're going
to dereference is nil. If that's the case, we assume that the template
variable doesn't have a name, prefix or default, as appropriate.