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

Fix panic in azurerm_virtual_machine_scale_set resource with additional_unattend_config block #266

Merged

Conversation

dominik-lekse
Copy link
Contributor

@dominik-lekse dominik-lekse commented Aug 23, 2017

This pull request fixes a nil pointer dereference panic in the azurerm_virtual_machine_scale_set resource. The panic occurs when at least one additional_unattend_config block is defined in the os_profile_windows_config block. A similar issue has been fixed for the azurerm_virtual_machine resource in terraform #7453.

Affected Resource(s)

  • azurerm_virtual_machine_scale_set

Panic Output

panic: runtime error: invalid memory address or nil pointer dereference
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1a2f473]
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm:
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: goroutine 1163 [running]:
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: github.com/terraform-providers/terraform-provider-azurerm/azurerm.flattenAzureRmVirtualMachineScaleSetOsProfileWindowsConfig(0xc4203310b0, 0x1c6f5a8, 0x12, 0x1ad6bc0)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/Users/lekse/Projects/terraform/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/resource_arm_virtual_machine_scale_set.go:779 +0x3f3
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: github.com/terraform-providers/terraform-provider-azurerm/azurerm.resourceArmVirtualMachineScaleSetRead(0xc420183ab0, 0x1b983e0, 0xc4203e2500, 0xc420509fc0, 0x0)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/Users/lekse/Projects/terraform/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/resource_arm_virtual_machine_scale_set.go:656 +0x1c92
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: github.com/terraform-providers/terraform-provider-azurerm/azurerm.resourceArmVirtualMachineScaleSetCreate(0xc420183ab0, 0x1b983e0, 0xc4203e2500, 0x24, 0x2315c00)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/Users/lekse/Projects/terraform/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/resource_arm_virtual_machine_scale_set.go:603 +0x98b
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/helper/schema.(*Resource).Apply(0xc4205d3440, 0xc420394640, 0xc42051afe0, 0x1b983e0, 0xc4203e2500, 0x1, 0x28, 0xc42035bf20)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/Users/lekse/Projects/terraform/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/helper/schema/resource.go:193 +0x449
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/helper/schema.(*Provider).Apply(0xc420254f50, 0xc4203945f0, 0xc420394640, 0xc42051afe0, 0x261e4b0, 0x0, 0xc420129c80)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/Users/lekse/Projects/terraform/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/helper/schema/provider.go:259 +0x9b
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/plugin.(*ResourceProviderServer).Apply(0xc4205091a0, 0xc42051af80, 0xc420451e80, 0x0, 0x0)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/Users/lekse/Projects/terraform/src/github.com/terraform-providers/terraform-provider-azurerm/vendor/github.com/hashicorp/terraform/plugin/resource_provider.go:488 +0x57
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: reflect.Value.call(0xc4202d03c0, 0xc420121588, 0x13, 0x1c5febb, 0x4, 0xc420683f20, 0x3, 0x3, 0x0, 0xc4200396f8, ...)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/usr/local/Cellar/go/1.8.3/libexec/src/reflect/value.go:434 +0x91f
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: reflect.Value.Call(0xc4202d03c0, 0xc420121588, 0x13, 0xc420039720, 0x3, 0x3, 0xc4200397c0, 0x13f5fbe, 0xc420884720)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/usr/local/Cellar/go/1.8.3/libexec/src/reflect/value.go:302 +0xa4
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: net/rpc.(*service).call(0xc4209ff980, 0xc4209ff940, 0xc4206ac540, 0xc420122880, 0xc4204aa460, 0x1abf900, 0xc42051af80, 0x16, 0x1abf940, 0xc420451e80, ...)
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/usr/local/Cellar/go/1.8.3/libexec/src/net/rpc/server.go:387 +0x144
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: created by net/rpc.(*Server).ServeCodec
2017-08-23T21:15:20.953+0200 [DEBUG] plugin.terraform-provider-azurerm: 	/usr/local/Cellar/go/1.8.3/libexec/src/net/rpc/server.go:481 +0x404
2017/08/23 21:15:20 [TRACE] root: eval: *terraform.EvalWriteState
2017/08/23 21:15:20 [TRACE] root: eval: *terraform.EvalApplyProvisioners
2017/08/23 21:15:20 [TRACE] root: eval: *terraform.EvalIf
2017/08/23 21:15:20 [TRACE] root: eval: *terraform.EvalWriteState
2017/08/23 21:15:20 [TRACE] root: eval: *terraform.EvalWriteDiff
2017/08/23 21:15:20 [TRACE] root: eval: *terraform.EvalApplyPost
2017/08/23 21:15:20 [ERROR] root: eval: *terraform.EvalApplyPost, err: 1 error(s) occurred:

* azurerm_virtual_machine_scale_set.staging_app: unexpected EOF
2017/08/23 21:15:20 [ERROR] root: eval: *terraform.EvalSequence, err: 1 error(s) occurred:

* azurerm_virtual_machine_scale_set.staging_app: unexpected EOF
2017/08/23 21:15:20 [TRACE] [walkApply] Exiting eval tree: azurerm_virtual_machine_scale_set.staging_app
2017/08/23 21:15:20 [TRACE] dag/walk: upstream errored, not walking "provider.azurerm (close)"
2017/08/23 21:15:20 [TRACE] dag/walk: upstream errored, not walking "meta.count-boundary (count boundary fixup)"
2017/08/23 21:15:20 [TRACE] dag/walk: upstream errored, not walking "root"
2017-08-23T21:15:20.958+0200 [DEBUG] plugin: plugin process exited: path=/Users/lekse/Projects/terraform/bin/terraform-provider-azurerm
2017/08/23 21:15:20 [TRACE] Preserving existing state lineage "9cd3087b-7fee-4744-8b6a-4a7ccfbbfca2"
2017/08/23 21:15:20 [TRACE] Preserving existing state lineage "9cd3087b-7fee-4744-8b6a-4a7ccfbbfca2"
2017/08/23 21:15:20 [TRACE] Preserving existing state lineage "9cd3087b-7fee-4744-8b6a-4a7ccfbbfca2"
2017/08/23 21:15:20 [DEBUG] plugin: waiting for all plugin processes to complete...
2017-08-23T21:15:20.989+0200 [WARN ] plugin: error closing client during Kill: err="connection is shut down"

Expected Behavior

  • New virtual machine scale set resource is created in Azure
  • New virtual machine scale set is stored in Terraform state
  • terraform apply finishes successfully

Actual Behavior

  • New virtual machine scale set resource is created in Azure
  • New virtual machine scale set is not stored in Terraform state
  • terraform apply exits with panic

Steps to Reproduce

  1. Define a azurerm_virtual_machine_scale_set resource which contains at least one additional_unattend_config block in the os_profile_windows_config block
  2. terraform apply

Acceptance tests

=== RUN   TestAccAzureRMVirtualMachineScaleSet_basicWindows_managedDisk
--- PASS: TestAccAzureRMVirtualMachineScaleSet_basicWindows_managedDisk (574.01s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	574.030s

References

…t resource when azure-sdk-for-go client returns nil in AdditionalUnattendContent.Content
@dominik-lekse
Copy link
Contributor Author

After scanning the open issues, I add that this fixes #220.

@tombuildsstuff
Copy link
Contributor

Hey @dominik-lekse

Thanks for this PR - this LGTM! Taking a look at the VMSS tests I can't see any test covering this right now - as such would it be possible to add one a test for this as part of this PR, so that we can test this automatically going forward? :)

Thanks!

@dominik-lekse
Copy link
Contributor Author

@tombuildsstuff You are right, looks like there is no test case covering a scale set consisting of Windows machines. I will add one which then also includes the os_profile_windows_config block.

…`additional_unattend_config` in `os_profile_windows_config` as already fixed for `azurerm_virtual_machine` in 443dd11
…ource `azurerm_virtual_machine_scale_set` when using `os_profile_linux_config`
@dominik-lekse
Copy link
Contributor Author

I have added the test TestAccAzureRMVirtualMachineScaleSet_basicWindows_managedDisk to this pull request. When running the test, I was able to reproduce #219. The commit a9b29f4 changes the type of additional_unattend_config in the schema from TypeSet to TypeList as it has been done for azurerm_virtual_machine in 443dd11. Since there was no schema migration required in 443dd11, I assume this is also the case for azurerm_virtual_machine_scale_set. Would be great if you could check this assumption @tombuildsstuff.

@dominik-lekse
Copy link
Contributor Author

Hi @tombuildsstuff, just wanted to ping if you had a chance to take a look at this bug fix?

@tombuildsstuff
Copy link
Contributor

👋 @dominik-lekse sorry for the delay - I'm going to take a look into this today :)

@tombuildsstuff
Copy link
Contributor

Hey @dominik-lekse

Thanks for this PR - I've taken a look through and this LGTM :)

Tests pass:

screen shot 2017-09-18 at 15 27 20

Thanks!

@tombuildsstuff tombuildsstuff merged commit 0d25ba2 into hashicorp:master Sep 18, 2017
tombuildsstuff added a commit that referenced this pull request Sep 18, 2017
@dominik-lekse dominik-lekse deleted the bugfix/vmss-unattendconfig-panic branch September 30, 2017 11:40
@ghost
Copy link

ghost commented Apr 1, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants