allow updating bigtable table column families (#3411)

third_party/terraform/resources/resource_bigtable_instance.go

@@ -27,6 +27,10 @@ func resourceBigtableInstance() *schema.Resource {
 			resourceBigtableInstanceClusterReorderTypeList,
 		),
 
+		// ----------------------------------------------------------------------
+		// IMPORTANT: Do not add any additional ForceNew fields to this resource.
+		// Destroying/recreating instances can lead to data loss for users.
+		// ----------------------------------------------------------------------
 		Schema: map[string]*schema.Schema{
 			"name": {
 				Type:     schema.TypeString,

third_party/terraform/resources/resource_bigtable_table.go

@@ -12,12 +12,17 @@ func resourceBigtableTable() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceBigtableTableCreate,
 		Read:   resourceBigtableTableRead,
+		Update: resourceBigtableTableUpdate,
 		Delete: resourceBigtableTableDestroy,
 
 		Importer: &schema.ResourceImporter{
 			State: resourceBigtableTableImport,
 		},
 
+		// ----------------------------------------------------------------------
+		// IMPORTANT: Do not add any additional ForceNew fields to this resource.
+		// Destroying/recreating tables can lead to data loss for users.
+		// ----------------------------------------------------------------------
 		Schema: map[string]*schema.Schema{
 			"name": {
 				Type:     schema.TypeString,
@@ -28,7 +33,6 @@ func resourceBigtableTable() *schema.Resource {
 			"column_family": {
 				Type:     schema.TypeSet,
 				Optional: true,
-				ForceNew: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"family": {
@@ -153,6 +157,54 @@ func resourceBigtableTableRead(d *schema.ResourceData, meta interface{}) error {
 	return nil
 }
 
+func resourceBigtableTableUpdate(d *schema.ResourceData, meta interface{}) error {
+	config := meta.(*Config)
+	ctx := context.Background()
+
+	project, err := getProject(d, config)
+	if err != nil {
+		return err
+	}
+
+	instanceName := GetResourceNameFromSelfLink(d.Get("instance_name").(string))
+	c, err := config.bigtableClientFactory.NewAdminClient(project, instanceName)
+	if err != nil {
+		return fmt.Errorf("Error starting admin client. %s", err)
+	}
+	defer c.Close()
+
+	o, n := d.GetChange("column_family")
+	oSet := o.(*schema.Set)
+	nSet := n.(*schema.Set)
+	name := d.Get("name").(string)
+
+	// Add column families that are in new but not in old
+	for _, new := range nSet.Difference(oSet).List() {
+		column := new.(map[string]interface{})
+
+		if v, ok := column["family"]; ok {
+			log.Printf("[DEBUG] adding column family %q", v)
+			if err := c.CreateColumnFamily(ctx, name, v.(string)); err != nil {
+				return fmt.Errorf("Error creating column family %q: %s", v, err)
+			}
+		}
+	}
+
+	// Remove column families that are in old but not in new
+	for _, old := range oSet.Difference(nSet).List() {
+		column := old.(map[string]interface{})
+
+		if v, ok := column["family"]; ok {
+			log.Printf("[DEBUG] removing column family %q", v)
+			if err := c.DeleteColumnFamily(ctx, name, v.(string)); err != nil {
+				return fmt.Errorf("Error deleting column family %q: %s", v, err)
+			}
+		}
+	}
+
+	return resourceBigtableTableRead(d, meta)
+}
+
 func resourceBigtableTableDestroy(d *schema.ResourceData, meta interface{}) error {
 	config := meta.(*Config)
 	ctx := context.Background()

third_party/terraform/tests/resource_bigtable_table_test.go

@@ -104,6 +104,38 @@ func TestAccBigtableTable_familyMany(t *testing.T) {
 	})
 }
 
+func TestAccBigtableTable_familyUpdate(t *testing.T) {
+	t.Parallel()
+
+	instanceName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+	tableName := fmt.Sprintf("tf-test-%s", randString(t, 10))
+	family := fmt.Sprintf("tf-test-%s", randString(t, 10))
+
+	vcrTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckBigtableTableDestroyProducer(t),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccBigtableTable_familyMany(instanceName, tableName, family),
+			},
+			{
+				ResourceName:      "google_bigtable_table.table",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccBigtableTable_familyUpdate(instanceName, tableName, family),
+			},
+			{
+				ResourceName:      "google_bigtable_table.table",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
 func testAccCheckBigtableTableDestroyProducer(t *testing.T) func(s *terraform.State) error {
 	return func(s *terraform.State) error {
 		var ctx = context.Background()
@@ -219,3 +251,35 @@ resource "google_bigtable_table" "table" {
 }
 `, instanceName, instanceName, tableName, family, family)
 }
+
+func testAccBigtableTable_familyUpdate(instanceName, tableName, family string) string {
+	return fmt.Sprintf(`
+resource "google_bigtable_instance" "instance" {
+  name = "%s"
+
+  cluster {
+    cluster_id = "%s"
+    zone       = "us-central1-b"
+  }
+
+  instance_type = "DEVELOPMENT"
+}
+
+resource "google_bigtable_table" "table" {
+  name          = "%s"
+  instance_name = google_bigtable_instance.instance.name
+
+  column_family {
+    family = "%s-third"
+  }
+
+  column_family {
+    family = "%s-fourth"
+  }
+
+  column_family {
+    family = "%s-second"
+  }
+}
+`, instanceName, instanceName, tableName, family, family, family)
+}

third_party/terraform/website/docs/r/bigtable_instance.html.markdown

@@ -13,6 +13,11 @@ Creates a Google Bigtable instance. For more information see
 [the official documentation](https://cloud.google.com/bigtable/) and
 [API](https://cloud.google.com/bigtable/docs/go/reference).
 
+-> **Note**: It is strongly recommended to set `lifecycle { prevent_destroy = true }`
+on instances in order to prevent accidental data loss. See
+[Terraform docs](https://www.terraform.io/docs/configuration/resources.html#prevent_destroy)
+for more information on lifecycle parameters.
+
 
 ## Example Usage - Production Instance
 
@@ -26,6 +31,10 @@ resource "google_bigtable_instance" "production-instance" {
     num_nodes    = 1
     storage_type = "HDD"
   }
+
+  lifecycle {
+    prevent_destroy = true
+  }
 }
 ```
 
@@ -41,6 +50,10 @@ resource "google_bigtable_instance" "development-instance" {
     zone         = "us-central1-b"
     storage_type = "HDD"
   }
+
+  lifecycle {
+    prevent_destroy = true
+  }
 }
 ```
 

third_party/terraform/website/docs/r/bigtable_table.html.markdown

@@ -13,6 +13,11 @@ Creates a Google Cloud Bigtable table inside an instance. For more information s
 [the official documentation](https://cloud.google.com/bigtable/) and
 [API](https://cloud.google.com/bigtable/docs/go/reference).
 
+-> **Note**: It is strongly recommended to set `lifecycle { prevent_destroy = true }`
+on tables in order to prevent accidental data loss. See
+[Terraform docs](https://www.terraform.io/docs/configuration/resources.html#prevent_destroy)
+for more information on lifecycle parameters.
+
 
 ## Example Usage
 
@@ -26,12 +31,20 @@ resource "google_bigtable_instance" "instance" {
     num_nodes    = 3
     storage_type = "HDD"
   }
+
+  lifecycle {
+    prevent_destroy = true
+  }
 }
 
 resource "google_bigtable_table" "table" {
   name          = "tf-table"
   instance_name = google_bigtable_instance.instance.name
   split_keys    = ["a", "b", "c"]
+
+  lifecycle {
+    prevent_destroy = true
+  }
 }
 ```
 
@@ -44,6 +57,8 @@ The following arguments are supported:
 * `instance_name` - (Required) The name of the Bigtable instance.
 
 * `split_keys` - (Optional) A list of predefined keys to split the table on.
+!> **Warning:** Modifying the `split_keys` of an existing table will cause Terraform
+to delete/recreate the entire `google_bigtable_table` resource.
 
 * `column_family` - (Optional) A group of columns within a table which share a common configuration. This can be specified multiple times. Structure is documented below.