Skip to content

Commit

Permalink
[review] remove enabled flag / restructure schema
Browse files Browse the repository at this point in the history
- remove `google_container_cluster.master_authorized_networks_config.enabled`
- add `display_name` and restructure schema as follows:
    master_authorized_networks_config {
        cidr_blocks {
            cidr_block   = "0.0.0.0/0"
            display_name = "foo"
        }
    }
- amend tests
  • Loading branch information
davidquarles committed Nov 1, 2017
1 parent 382be19 commit b34c5f7
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 78 deletions.
105 changes: 57 additions & 48 deletions google/resource_container_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func resourceContainerCluster() *schema.Resource {
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validation.CIDRNetwork(11, 19),
ValidateFunc: validateRFC1918Network(8, 32),
},

"description": {
Expand Down Expand Up @@ -218,18 +218,23 @@ func resourceContainerCluster() *schema.Resource {
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"enabled": {
Type: schema.TypeBool,
Required: true,
},
"cidr_blocks": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
MaxItems: 10,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validation.CIDRNetwork(0, 32),
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"cidr_block": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.CIDRNetwork(0, 32),
},
"display_name": {
Type: schema.TypeString,
Optional: true,
},
},
},
},
},
Expand Down Expand Up @@ -549,27 +554,26 @@ func resourceContainerClusterUpdate(d *schema.ResourceData, meta interface{}) er
d.Partial(true)

if d.HasChange("master_authorized_networks_config") {
if c, ok := d.GetOk("master_authorized_networks_config"); ok {
req := &container.UpdateClusterRequest{
Update: &container.ClusterUpdate{
DesiredMasterAuthorizedNetworksConfig: expandMasterAuthorizedNetworksConfig(c),
},
}
op, err := config.clientContainer.Projects.Zones.Clusters.Update(
project, zoneName, clusterName, req).Do()
if err != nil {
return err
}

// Wait until it's updated
waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE cluster master authorized networks", timeoutInMinutes, 2)
if waitErr != nil {
return waitErr
}
log.Printf("[INFO] GKE cluster %s master authorized networks config has been updated", d.Id())
c := d.Get("master_authorized_networks_config")
req := &container.UpdateClusterRequest{
Update: &container.ClusterUpdate{
DesiredMasterAuthorizedNetworksConfig: expandMasterAuthorizedNetworksConfig(c),
},
}
op, err := config.clientContainer.Projects.Zones.Clusters.Update(
project, zoneName, clusterName, req).Do()
if err != nil {
return err
}

d.SetPartial("master_authorized_networks_config")
// Wait until it's updated
waitErr := containerOperationWait(config, op, project, zoneName, "updating GKE cluster master authorized networks", timeoutInMinutes, 2)
if waitErr != nil {
return waitErr
}
log.Printf("[INFO] GKE cluster %s master authorized networks config has been updated", d.Id())

d.SetPartial("master_authorized_networks_config")
}

// The master must be updated before the nodes
Expand Down Expand Up @@ -863,17 +867,20 @@ func expandClusterAddonsConfig(configured interface{}) *container.AddonsConfig {
}

func expandMasterAuthorizedNetworksConfig(configured interface{}) *container.MasterAuthorizedNetworksConfig {
config := configured.([]interface{})[0].(map[string]interface{})
cidrBlocks := config["cidr_blocks"].(*schema.Set).List()
result := &container.MasterAuthorizedNetworksConfig{
Enabled: config["enabled"].(bool),
CidrBlocks: make([]*container.CidrBlock, 0),
}
if result.Enabled {
for _, v := range cidrBlocks {
result.CidrBlocks = append(result.CidrBlocks, &container.CidrBlock{
CidrBlock: v.(string),
})
result := &container.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([]*container.CidrBlock, 0)
for _, v := range cidrBlocks {
cidrBlock := v.(map[string]interface{})
result.CidrBlocks = append(result.CidrBlocks, &container.CidrBlock{
CidrBlock: cidrBlock["cidr_block"].(string),
DisplayName: cidrBlock["display_name"].(string),
})
}
}
}
return result
Expand Down Expand Up @@ -920,17 +927,19 @@ func flattenClusterNodePools(d *schema.ResourceData, config *Config, c []*contai
}

func flattenMasterAuthorizedNetworksConfig(c *container.MasterAuthorizedNetworksConfig) []map[string]interface{} {
cidrBlocks := make([]string, 0, len(c.CidrBlocks))
for _, v := range c.CidrBlocks {
cidrBlocks = append(cidrBlocks, v.CidrBlock)
}
result := []map[string]interface{}{
map[string]interface{}{
"enabled": c.Enabled,
"cidr_blocks": cidrBlocks,
},
result := make(map[string]interface{})
if c.Enabled && len(c.CidrBlocks) > 0 {
cidrBlocks := make([]map[string]interface{}, 0, len(c.CidrBlocks))
for _, v := range c.CidrBlocks {
cidrBlocks = append(cidrBlocks, map[string]interface{}{
"cidr_block": v.CidrBlock,
"display_name": v.DisplayName,
})
}
// now does this adhere to the schema?
result["cidr_blocks"] = cidrBlocks
}
return result
return []map[string]interface{}{result}
}

func resourceContainerClusterStateImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
Expand Down
39 changes: 17 additions & 22 deletions google/resource_container_cluster_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package google

import (
"bytes"
"fmt"
"reflect"
"sort"
Expand Down Expand Up @@ -117,31 +118,25 @@ func TestAccContainerCluster_withMasterAuthorizedNetworksConfig(t *testing.T) {
CheckDestroy: testAccCheckContainerClusterDestroy,
Steps: []resource.TestStep{
{
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, true, []string{"0.0.0.0/0"}),
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{"0.0.0.0/0"}),
Check: resource.ComposeTestCheckFunc(
testAccCheckContainerCluster("google_container_cluster.with_master_authorized_networks"),
resource.TestCheckResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.enabled", "true"),
resource.TestCheckResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.cidr_blocks.#", "1"),
),
},
{
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, true, []string{}),
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{}),
Check: resource.ComposeTestCheckFunc(
testAccCheckContainerCluster("google_container_cluster.with_master_authorized_networks"),
resource.TestCheckResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.enabled", "true"),
resource.TestCheckNoResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.cidr_blocks"),
),
},
{
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, false, []string{"8.8.8.8/32"}),
Config: testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName, []string{"8.8.8.8/32"}),
Check: resource.ComposeTestCheckFunc(
testAccCheckContainerCluster("google_container_cluster.with_master_authorized_networks"),
resource.TestCheckResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.enabled", "false"),
resource.TestCheckNoResourceAttr("google_container_cluster.with_master_authorized_networks",
"master_authorized_networks_config.0.cidr_blocks"),
),
Expand Down Expand Up @@ -723,13 +718,6 @@ func testAccCheckContainerCluster(n string) resource.TestCheckFunc {
}
}

masterAuthorizedNetworksEnabled := false
if cluster.MasterAuthorizedNetworksConfig != nil {
masterAuthorizedNetworksEnabled = cluster.MasterAuthorizedNetworksConfig.Enabled
clusterTests = append(clusterTests,
clusterTestField{"master_authorized_networks_config.0.enabled", masterAuthorizedNetworksEnabled})
}

for _, attrs := range clusterTests {
if c := checkMatch(attributes, attrs.tf_attr, attrs.gcp_attr); c != "" {
return fmt.Errorf(c)
Expand Down Expand Up @@ -933,10 +921,18 @@ resource "google_container_cluster" "with_master_auth" {
}
}`, acctest.RandString(10))

func testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName string, enabled bool, cidrBlocks []string) string {
func testAccContainerCluster_withMasterAuthorizedNetworksConfig(clusterName string, cidrs []string) string {

for i, cidr := range cidrBlocks {
cidrBlocks[i] = strconv.Quote(cidr)
cidrBlocks := ""
if len(cidrs) > 0 {
var buf bytes.Buffer
for _, c := range cidrs {
buf.WriteString(fmt.Sprintf(`
cidr_blocks {
cidr_block = "%s"
}`, c))
}
cidrBlocks = buf.String()
}

return fmt.Sprintf(`
Expand All @@ -946,10 +942,9 @@ resource "google_container_cluster" "with_master_authorized_networks" {
initial_node_count = 1
master_authorized_networks_config {
enabled = %v
cidr_blocks = [%s]
%s
}
}`, clusterName, enabled, strings.Join(cidrBlocks, ","))
}`, clusterName, cidrBlocks)
}

func testAccContainerCluster_withAdditionalZones(clusterName string) string {
Expand Down
18 changes: 10 additions & 8 deletions website/docs/r/container_cluster.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ resource "google_container_cluster" "primary" {
Kubernetes master. Structure is documented below.

* `master_authorized_networks_config` - (Optional) The desired configuration options
for master authorized networks
for master authorized networks. Omit the nested `cidr_blocks` attribute to disallow
external access (except the cluster node IPs, which GKE automatically whitelists).

* `min_master_version` - (Optional) The minimum version of the master. GKE
will auto-update the master to new versions, so this does not guarantee the
Expand Down Expand Up @@ -163,16 +164,17 @@ The `master_auth` block supports:

The `master_authorized_networks_config` block supports:

* `enabled` - (Required) Whether or not master authorized networks is enabled

* `cidr_blocks` - (Optional) Defines up to 10 external networks that can access
Kubernetes master through HTTPS. To avoid upstream failures, an empty list is
passed when this feature is disabled, irrespective of values passed here.
Kubernetes master through HTTPS.

The `node_config` block supports:
The `master_authorized_networks_config.cidr_blocks` block supports:

* `machine_type` - (Optional) The name of a Google Compute Engine machine type.
Defaults to `n1-standard-1`.
* `cidr_block` - (Optional) External network that can access Kubernetes master through HTTPS.
Must be specified in CIDR notation.

* `display_name` - (Optional) Field for users to identify CIDR blocks.

The `node_config` block supports:

* `disk_size_gb` - (Optional) Size of the disk attached to each node, specified
in GB. The smallest allowed disk size is 10GB. Defaults to 100GB.
Expand Down

0 comments on commit b34c5f7

Please sign in to comment.