public/terraform-provider-google
0
0
Fork 0
mirror of https://github.com/letic/terraform-provider-google.git synced 2024-10-01 16:21:06 +00:00
Code Issues Projects Releases Wiki Activity

Fix panic on empty list for authorized masters' cidr_blocks (#1904)

Browse Source 
* test empty authorized masters' cidr_blocks

When the `cidr_block` isn't simply blank but contains an empty list as in

```
master_authorized_networks_config {
  cidr_blocks = []
}
```

a panic occurs looking something like

```
goroutine 26 [running]:
github.com/terraform-providers/terraform-provider-google/google.expandMasterAuthorizedNetworksConfig(0x15a4f80, 0xc4202586e0, 0x21)
	/tmp/GOPATH/src/github.com/terraform-providers/terraform-provider-google/google/resource_container_cluster.go:1355 +0x4f2
github.com/terraform-providers/terraform-provider-google/google.resourceContainerClusterCreate(0xc420146a80, 0x16b1800, 0xc4200b8000, 0x0, 0x0)
	/tmp/GOPATH/src/github.com/terraform-providers/terraform-provider-google/google/resource_container_cluster.go:520 +0x2848
github.com/terraform-providers/terraform-provider-google/vendor/github.com/hashicorp/terraform/helper/schema.(*Resource).Apply(0xc420495490, 0xc420341310, 0xc4202582c0, 0x16b1800, 0xc4200b8000, 0x1, 0xc42024eae0, 0xc4201e3650)
	/tmp/GOPATH/src/github.com/terraform-providers/terraform-provider-google/vendor/github.com/hashicorp/terraform/helper/schema/resource.go:227 +0x364
github.com/terraform-providers/terraform-provider-google/vendor/github.com/hashicorp/terraform/helper/schema.(*Provider).Apply(0xc4204c6700, 0xc4203412c0, 0xc420341310, 0xc4202582c0, 0x14ee1441a000, 0x0, 0x18)
	/tmp/GOPATH/src/github.com/terraform-providers/terraform-provider-google/vendor/github.com/hashicorp/terraform/helper/schema/provider.go:283 +0xa4
github.com/terraform-providers/terraform-provider-google/vendor/github.com/hashicorp/terraform/plugin.(*ResourceProviderServer).Apply(0xc4202d7c40, 0xc42035de80, 0xc42025c160, 0x0, 0x0)
	/tmp/GOPATH/src/github.com/terraform-providers/terraform-provider-google/vendor/github.com/hashicorp/terraform/plugin/resource_provider.go:527 +0x57
reflect.Value.call(0xc4203feae0, 0xc42000e038, 0x13, 0x19e88a8, 0x4, 0xc42015ff20, 0x3, 0x3, 0xc420047ee8, 0xc4204c6798, ...)
	/usr/local/go/src/reflect/value.go:434 +0x905
reflect.Value.Call(0xc4203feae0, 0xc42000e038, 0x13, 0xc420047f20, 0x3, 0x3, 0xc400000001, 0x0, 0x0)
	/usr/local/go/src/reflect/value.go:302 +0xa4
net/rpc.(*service).call(0xc420418600, 0xc42007c140, 0xc42001e798, 0xc4200c4000, 0xc4202d6c40, 0x1557f80, 0xc42035de80, 0x16, 0x1557fc0, 0xc42025c160, ...)
	/usr/local/go/src/net/rpc/server.go:381 +0x142
created by net/rpc.(*Server).ServeCodec
	/usr/local/go/src/net/rpc/server.go:475 +0x36b
```

which we trigger by altering the first step to contain the HCL notation
for an empty list instead of simply an empty string.

In order to accomplish this, the tests had to be modified to accept an
emptyValue string as well which contains the content of the
`emptyValue` string when the cidrBlocks array is empty. This maintains
the old behavior of the original tests when `emptyValue` is an empty
string, while also facilating differing behavior for the new testcase by
setting `emptyValue` to whichever string we want to test instead. I
don't think this is very clean, but I guess it's pragmatic enough.

I'll hear if this is a thorn in the side to someone 😏.

* avoid panic on cidr_block type assertion

This is basically the fix. Since the value can be nil, we want to ensure
we handle a failure during the assertion since we know that asserting
`nil` conforms to `map[string]interface{}` will cause a run-time panic.

* flatten to config on empty list for cidr_blocks

since an empty list for cidrBlocks constitutes valid input, one should
return a map containing an empty list for the cidr_blocks field instead
of a nil value.

The nil value is only appropriate when the input Config is also nil.
This commit is contained in:
David Asabina 2018-08-21 04:29:37 +02:00 committed by emily
parent b05998781d
commit bf7626a5c6
2 changed files with 25 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
google/resource_container_cluster.go
View File

@ -1468,16 +1468,17 @@ func expandMasterAuthorizedNetworksConfig(configured interface{}) *containerBeta
result := &containerBeta.MasterAuthorizedNetworksConfig{}
if len(configured.([]interface{})) > 0 {
result.Enabled = true
config := configured.([]interface{})[0].(map[string]interface{})
if _, ok := config["cidr_blocks"]; ok {
cidrBlocks := config["cidr_blocks"].(*schema.Set).List()
result.CidrBlocks = make([]*containerBeta.CidrBlock, 0)
for _, v := range cidrBlocks {
cidrBlock := v.(map[string]interface{})
result.CidrBlocks = append(result.CidrBlocks, &containerBeta.CidrBlock{
CidrBlock: cidrBlock["cidr_block"].(string),
DisplayName: cidrBlock["display_name"].(string),
})
if config, ok := configured.([]interface{})[0].(map[string]interface{}); ok {
if _, ok := config["cidr_blocks"]; ok {
cidrBlocks := config["cidr_blocks"].(*schema.Set).List()
result.CidrBlocks = make([]*containerBeta.CidrBlock, 0)
for _, v := range cidrBlocks {
cidrBlock := v.(map[string]interface{})
result.CidrBlocks = append(result.CidrBlocks, &containerBeta.CidrBlock{
CidrBlock: cidrBlock["cidr_block"].(string),
DisplayName: cidrBlock["display_name"].(string),
})
}
}
}
}
@ -1629,9 +1630,6 @@ func flattenMasterAuthorizedNetworksConfig(c *containerBeta.MasterAuthorizedNetw
if c == nil {
return nil
}
if len(c.CidrBlocks) == 0 {
return nil
}
result := make(map[string]interface{})
if c.Enabled {
cidrBlocks := make([]interface{}, 0, len(c.CidrBlocks))

19
google/resource_container_cluster_test.go
View File

@ -276,7 +276,16 @@ func TestAccContainerCluster_withMasterAuthorizedNetworksConfig(t *testing.T) {
CheckDestroy: testAccCheckContainerClusterDestroy,
Steps: []resource.TestStep{
{
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{"8.8.8.8/32"}),
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{}, "cidr_blocks = []"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.#", "1"),
resource.TestCheckResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.cidr_blocks.#", "0"),
),
},
{
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{"8.8.8.8/32"}, ""),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.cidr_blocks.#", "1"),
@ -289,7 +298,7 @@ func TestAccContainerCluster_withMasterAuthorizedNetworksConfig(t *testing.T) {
ImportStateIdPrefix: "us-central1-a/",
},
{
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{"10.0.0.0/8", "8.8.8.8/32"}),
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{"10.0.0.0/8", "8.8.8.8/32"}, ""),
},
{
ResourceName: "google_container_cluster.with_master_authorized_networks",
@ -298,7 +307,7 @@ func TestAccContainerCluster_withMasterAuthorizedNetworksConfig(t *testing.T) {
ImportStateIdPrefix: "us-central1-a/",
},
{
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{}),
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{}, ""),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckNoResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.cidr_blocks"),
@ -1591,9 +1600,9 @@ resource "google_container_cluster" "with_network_policy_enabled" {
}`, clusterName)
}
func testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName string, cidrs []string) string {
func testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName string, cidrs []string, emptyValue string) string {
cidrBlocks := ""
cidrBlocks := emptyValue
if len(cidrs) > 0 {
var buf bytes.Buffer
buf.WriteString("cidr_blocks = [")