diff --git a/.changelog/3947.txt b/.changelog/3947.txt new file mode 100644 index 00000000000..d64a33b58d3 --- /dev/null +++ b/.changelog/3947.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +spanner: Added schema update/update ddl support for `google_spanner_database` +``` diff --git a/google/resource_spanner_database.go b/google/resource_spanner_database.go index d6b131bee09..d9be83d38ee 100644 --- a/google/resource_spanner_database.go +++ b/google/resource_spanner_database.go @@ -18,15 +18,49 @@ import ( "fmt" "log" "reflect" + "strings" "time" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" ) +// customizeDiff func for additional checks on google_spanner_database properties: +func resourceSpannerDBDdlCustomDiffFunc(diff TerraformResourceDiff) error { + old, new := diff.GetChange("ddl") + oldDdls := old.([]interface{}) + newDdls := new.([]interface{}) + var err error + + if len(newDdls) < len(oldDdls) { + err = diff.ForceNew("ddl") + if err != nil { + return fmt.Errorf("ForceNew failed for ddl, old - %v and new - %v", oldDdls, newDdls) + } + return nil + } + + for i := range oldDdls { + if newDdls[i].(string) != oldDdls[i].(string) { + err = diff.ForceNew("ddl") + if err != nil { + return fmt.Errorf("ForceNew failed for ddl, old - %v and new - %v", oldDdls, newDdls) + } + return nil + } + } + return nil +} + +func resourceSpannerDBDdlCustomDiff(diff *schema.ResourceDiff, meta interface{}) error { + // separate func to allow unit testing + return resourceSpannerDBDdlCustomDiffFunc(diff) +} + func resourceSpannerDatabase() *schema.Resource { return &schema.Resource{ Create: resourceSpannerDatabaseCreate, Read: resourceSpannerDatabaseRead, + Update: resourceSpannerDatabaseUpdate, Delete: resourceSpannerDatabaseDelete, Importer: &schema.ResourceImporter{ @@ -35,9 +69,12 @@ func resourceSpannerDatabase() *schema.Resource { Timeouts: &schema.ResourceTimeout{ Create: schema.DefaultTimeout(4 * time.Minute), + Update: schema.DefaultTimeout(4 * time.Minute), Delete: schema.DefaultTimeout(4 * time.Minute), }, + CustomizeDiff: resourceSpannerDBDdlCustomDiff, + Schema: map[string]*schema.Schema{ "instance": { Type: schema.TypeString, @@ -57,7 +94,6 @@ the instance is created. Values are of the form [a-z][-a-z0-9]*[a-z0-9].`, "ddl": { Type: schema.TypeList, Optional: true, - ForceNew: true, Description: `An optional list of DDL statements to run inside the newly created database. Statements can create tables, indexes, etc. These statements execute atomically with the creation of the database: if there is an @@ -231,6 +267,66 @@ func resourceSpannerDatabaseRead(d *schema.ResourceData, meta interface{}) error return nil } +func resourceSpannerDatabaseUpdate(d *schema.ResourceData, meta interface{}) error { + config := meta.(*Config) + + billingProject := "" + + project, err := getProject(d, config) + if err != nil { + return err + } + billingProject = project + + d.Partial(true) + + if d.HasChange("ddl") { + obj := make(map[string]interface{}) + + extraStatementsProp, err := expandSpannerDatabaseDdl(d.Get("ddl"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("ddl"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, extraStatementsProp)) { + obj["extraStatements"] = extraStatementsProp + } + + obj, err = resourceSpannerDatabaseUpdateEncoder(d, meta, obj) + if err != nil { + return err + } + + url, err := replaceVars(d, config, "{{SpannerBasePath}}projects/{{project}}/instances/{{instance}}/databases/{{name}}/ddl") + if err != nil { + return err + } + + // err == nil indicates that the billing_project value was found + if bp, err := getBillingProject(d, config); err == nil { + billingProject = bp + } + + res, err := sendRequestWithTimeout(config, "PATCH", billingProject, url, obj, d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return fmt.Errorf("Error updating Database %q: %s", d.Id(), err) + } else { + log.Printf("[DEBUG] Finished updating Database %q: %#v", d.Id(), res) + } + + err = spannerOperationWaitTime( + config, res, project, "Updating Database", + d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return err + } + + d.SetPartial("ddl") + } + + d.Partial(false) + + return resourceSpannerDatabaseRead(d, meta) +} + func resourceSpannerDatabaseDelete(d *schema.ResourceData, meta interface{}) error { config := meta.(*Config) @@ -260,6 +356,14 @@ func resourceSpannerDatabaseDelete(d *schema.ResourceData, meta interface{}) err return handleNotFoundError(err, d, "Database") } + err = spannerOperationWaitTime( + config, res, project, "Deleting Database", + d.Timeout(schema.TimeoutDelete)) + + if err != nil { + return err + } + log.Printf("[DEBUG] Finished deleting Database %q: %#v", d.Id(), res) return nil } @@ -326,6 +430,24 @@ func resourceSpannerDatabaseEncoder(d *schema.ResourceData, meta interface{}, ob return obj, nil } +func resourceSpannerDatabaseUpdateEncoder(d *schema.ResourceData, meta interface{}, obj map[string]interface{}) (map[string]interface{}, error) { + old, new := d.GetChange("ddl") + oldDdls := old.([]interface{}) + newDdls := new.([]interface{}) + updateDdls := []string{} + + //Only new ddl statments to be add to update call + for i := len(oldDdls); i < len(newDdls); i++ { + updateDdls = append(updateDdls, newDdls[i].(string)) + } + + obj["statements"] = strings.Join(updateDdls, ",") + delete(obj, "name") + delete(obj, "instance") + delete(obj, "extraStatements") + return obj, nil +} + func resourceSpannerDatabaseDecoder(d *schema.ResourceData, meta interface{}, res map[string]interface{}) (map[string]interface{}, error) { config := meta.(*Config) d.SetId(res["name"].(string)) diff --git a/google/resource_spanner_database_test.go b/google/resource_spanner_database_test.go index c8d237c75f1..54ba9f1efce 100644 --- a/google/resource_spanner_database_test.go +++ b/google/resource_spanner_database_test.go @@ -28,27 +28,44 @@ func TestAccSpannerDatabase_basic(t *testing.T) { }, { // Test import with default Terraform ID - ResourceName: "google_spanner_database.basic", - ImportState: true, - ImportStateVerify: true, + ResourceName: "google_spanner_database.basic", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"ddl"}, }, { - ResourceName: "google_spanner_database.basic", - ImportStateId: fmt.Sprintf("projects/%s/instances/%s/databases/%s", project, instanceName, databaseName), - ImportState: true, - ImportStateVerify: true, + Config: testAccSpannerDatabase_basicUpdate(instanceName, databaseName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + ), + }, + { + // Test import with default Terraform ID + ResourceName: "google_spanner_database.basic", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"ddl"}, }, { - ResourceName: "google_spanner_database.basic", - ImportStateId: fmt.Sprintf("instances/%s/databases/%s", instanceName, databaseName), - ImportState: true, - ImportStateVerify: true, + ResourceName: "google_spanner_database.basic", + ImportStateId: fmt.Sprintf("projects/%s/instances/%s/databases/%s", project, instanceName, databaseName), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"ddl"}, }, { - ResourceName: "google_spanner_database.basic", - ImportStateId: fmt.Sprintf("%s/%s", instanceName, databaseName), - ImportState: true, - ImportStateVerify: true, + ResourceName: "google_spanner_database.basic", + ImportStateId: fmt.Sprintf("instances/%s/databases/%s", instanceName, databaseName), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"ddl"}, + }, + { + ResourceName: "google_spanner_database.basic", + ImportStateId: fmt.Sprintf("%s/%s", instanceName, databaseName), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"ddl"}, }, }, }) @@ -66,6 +83,31 @@ resource "google_spanner_instance" "basic" { resource "google_spanner_database" "basic" { instance = google_spanner_instance.basic.name name = "%s" + ddl = [ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", + ] +} +`, instanceName, instanceName, databaseName) +} + +func testAccSpannerDatabase_basicUpdate(instanceName, databaseName string) string { + return fmt.Sprintf(` +resource "google_spanner_instance" "basic" { + name = "%s" + config = "regional-us-central1" + display_name = "display-%s" + num_nodes = 1 +} + +resource "google_spanner_database" "basic" { + instance = google_spanner_instance.basic.name + name = "%s" + ddl = [ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", + "CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)", + ] } `, instanceName, instanceName, databaseName) } @@ -81,3 +123,81 @@ func TestDatabaseNameForApi(t *testing.T) { expected := "projects/project123/instances/instance456/databases/db789" expectEquals(t, expected, actual) } + +// Unit Tests for ForceNew when the change in ddl +func TestSpannerDatabase_resourceSpannerDBDdlCustomDiffFuncForceNew(t *testing.T) { + t.Parallel() + + cases := map[string]struct { + before interface{} + after interface{} + forcenew bool + }{ + "remove_old_statements": { + before: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"}, + after: []interface{}{ + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)"}, + forcenew: true, + }, + "append_new_statements": { + before: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"}, + after: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", + }, + forcenew: false, + }, + "no_change": { + before: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"}, + after: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)"}, + forcenew: false, + }, + "order_of_statments_change": { + before: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", + "CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)", + }, + after: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)", + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", + }, + forcenew: true, + }, + "missing_an_old_statement": { + before: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", + "CREATE TABLE t3 (t3 INT64 NOT NULL,) PRIMARY KEY(t3)", + }, + after: []interface{}{ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", + }, + forcenew: true, + }, + } + + for tn, tc := range cases { + d := &ResourceDiffMock{ + Before: map[string]interface{}{ + "ddl": tc.before, + }, + After: map[string]interface{}{ + "ddl": tc.after, + }, + } + err := resourceSpannerDBDdlCustomDiffFunc(d) + if err != nil { + t.Errorf("failed, expected no error but received - %s for the condition %s", err, tn) + } + if d.IsForceNew != tc.forcenew { + t.Errorf("ForceNew not setup correctly for the condition-'%s', expected:%v;actual:%v", tn, tc.forcenew, d.IsForceNew) + } + } +} diff --git a/google/test_utils.go b/google/test_utils.go index 3ad7b46b23d..f43e067327d 100644 --- a/google/test_utils.go +++ b/google/test_utils.go @@ -62,9 +62,10 @@ func (d *ResourceDataMock) Id() string { } type ResourceDiffMock struct { - Before map[string]interface{} - After map[string]interface{} - Cleared map[string]struct{} + Before map[string]interface{} + After map[string]interface{} + Cleared map[string]struct{} + IsForceNew bool } func (d *ResourceDiffMock) GetChange(key string) (interface{}, interface{}) { @@ -83,6 +84,11 @@ func (d *ResourceDiffMock) Clear(key string) error { return nil } +func (d *ResourceDiffMock) ForceNew(key string) error { + d.IsForceNew = true + return nil +} + func checkDataSourceStateMatchesResourceState(dataSourceName, resourceName string) func(*terraform.State) error { return checkDataSourceStateMatchesResourceStateWithIgnores(dataSourceName, resourceName, map[string]struct{}{}) } diff --git a/google/utils.go b/google/utils.go index d5b3cdc8e9b..dddef1c8db7 100644 --- a/google/utils.go +++ b/google/utils.go @@ -28,6 +28,7 @@ type TerraformResourceDiff interface { GetChange(string) (interface{}, interface{}) Get(string) interface{} Clear(string) error + ForceNew(string) error } // getRegionFromZone returns the region from a zone for Google cloud. diff --git a/website/docs/r/spanner_database.html.markdown b/website/docs/r/spanner_database.html.markdown index e0b6b6981e8..fcbb8681fd1 100644 --- a/website/docs/r/spanner_database.html.markdown +++ b/website/docs/r/spanner_database.html.markdown @@ -102,6 +102,7 @@ This resource provides the following [Timeouts](/docs/configuration/resources.html#timeouts) configuration options: - `create` - Default is 4 minutes. +- `update` - Default is 4 minutes. - `delete` - Default is 4 minutes. ## Import