diff --git a/.changelog/6141.txt b/.changelog/6141.txt new file mode 100644 index 00000000000..c53a8d7e199 --- /dev/null +++ b/.changelog/6141.txt @@ -0,0 +1,6 @@ +```release-note:enhancement +spanner: Added field `version_retention_period` to `google_spanner_database` resource +``` +```release-note:enhancement +spanner: Fixed issue where `ddl` and `version_retention_period` could not be set when first creating `google_spanner_database` using POSTGRESQL +``` diff --git a/google/config_test.go b/google/config_test.go index 21dd0532d6e..eeb75aa57a6 100644 --- a/google/config_test.go +++ b/google/config_test.go @@ -2,7 +2,6 @@ package google import ( "context" - "fmt" "io/ioutil" "os" "testing" @@ -64,7 +63,7 @@ func TestConfigLoadAndValidate_accountFileJSONInvalid(t *testing.T) { func TestAccConfigLoadValidate_credentials(t *testing.T) { if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } testAccPreCheck(t) @@ -92,7 +91,7 @@ func TestAccConfigLoadValidate_credentials(t *testing.T) { func TestAccConfigLoadValidate_impersonated(t *testing.T) { if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } testAccPreCheck(t) @@ -122,7 +121,7 @@ func TestAccConfigLoadValidate_impersonated(t *testing.T) { func TestAccConfigLoadValidate_accessTokenImpersonated(t *testing.T) { if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } testAccPreCheck(t) @@ -162,7 +161,7 @@ func TestAccConfigLoadValidate_accessTokenImpersonated(t *testing.T) { func TestAccConfigLoadValidate_accessToken(t *testing.T) { if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } testAccPreCheck(t) diff --git a/google/resource_compute_disk_test.go b/google/resource_compute_disk_test.go index 48b102cd472..f8ca7b64dff 100644 --- a/google/resource_compute_disk_test.go +++ b/google/resource_compute_disk_test.go @@ -185,7 +185,7 @@ func TestAccComputeDisk_imageDiffSuppressPublicVendorsFamilyNames(t *testing.T) t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index af7a807ca35..3e33678a5d4 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -18,7 +18,7 @@ func TestAccComputeInstanceMigrateState(t *testing.T) { t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } cases := map[string]struct { StateVersion int @@ -122,7 +122,7 @@ func TestAccComputeInstanceMigrateState_empty(t *testing.T) { t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } var is *terraform.InstanceState var meta interface{} @@ -150,7 +150,7 @@ func TestAccComputeInstanceMigrateState_bootDisk(t *testing.T) { t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -218,7 +218,7 @@ func TestAccComputeInstanceMigrateState_v4FixBootDisk(t *testing.T) { t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -285,7 +285,7 @@ func TestAccComputeInstanceMigrateState_attachedDiskFromSource(t *testing.T) { t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -366,7 +366,7 @@ func TestAccComputeInstanceMigrateState_v4FixAttachedDiskFromSource(t *testing.T t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -446,7 +446,7 @@ func TestAccComputeInstanceMigrateState_attachedDiskFromEncryptionKey(t *testing t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -515,7 +515,7 @@ func TestAccComputeInstanceMigrateState_v4FixAttachedDiskFromEncryptionKey(t *te t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -583,7 +583,7 @@ func TestAccComputeInstanceMigrateState_attachedDiskFromAutoDeleteAndImage(t *te t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -656,7 +656,7 @@ func TestAccComputeInstanceMigrateState_v4FixAttachedDiskFromAutoDeleteAndImage( t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -728,7 +728,7 @@ func TestAccComputeInstanceMigrateState_scratchDisk(t *testing.T) { t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" @@ -794,7 +794,7 @@ func TestAccComputeInstanceMigrateState_v4FixScratchDisk(t *testing.T) { t.Parallel() if os.Getenv(TestEnvVar) == "" { - t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", TestEnvVar)) + t.Skipf("Network access not allowed; use %s=1 to enable", TestEnvVar) } config := getInitializedConfig(t) zone := "us-central1-f" diff --git a/google/resource_spanner_database.go b/google/resource_spanner_database.go index dcbf8eaedd4..10c67c033ce 100644 --- a/google/resource_spanner_database.go +++ b/google/resource_spanner_database.go @@ -19,6 +19,8 @@ import ( "fmt" "log" "reflect" + "regexp" + "strconv" "time" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -56,6 +58,44 @@ func resourceSpannerDBDdlCustomDiff(_ context.Context, diff *schema.ResourceDiff return resourceSpannerDBDdlCustomDiffFunc(diff) } +func validateDatabaseRetentionPeriod(v interface{}, k string) (ws []string, errors []error) { + value := v.(string) + valueError := fmt.Errorf("version_retention_period should be in range [1h, 7d], in a format resembling 1d, 24h, 1440m, or 86400s") + + r := regexp.MustCompile("^(\\d{1}d|\\d{1,3}h|\\d{2,5}m|\\d{4,6}s)$") + if !r.MatchString(value) { + errors = append(errors, valueError) + return + } + + unit := value[len(value)-1:] + multiple := value[:len(value)-1] + num, err := strconv.Atoi(multiple) + if err != nil { + errors = append(errors, valueError) + return + } + + if unit == "d" && (num < 1 || num > 7) { + errors = append(errors, valueError) + return + } + if unit == "h" && (num < 1 || num > 7*24) { + errors = append(errors, valueError) + return + } + if unit == "m" && (num < 1*60 || num > 7*24*60) { + errors = append(errors, valueError) + return + } + if unit == "s" && (num < 1*60*60 || num > 7*24*60*60) { + errors = append(errors, valueError) + return + } + + return +} + func resourceSpannerDatabase() *schema.Resource { return &schema.Resource{ Create: resourceSpannerDatabaseCreate, @@ -98,10 +138,7 @@ the instance is created. Values are of the form [a-z][-a-z0-9]*[a-z0-9].`, ForceNew: true, ValidateFunc: validateEnum([]string{"GOOGLE_STANDARD_SQL", "POSTGRESQL", ""}), Description: `The dialect of the Cloud Spanner Database. -If it is not provided, "GOOGLE_STANDARD_SQL" will be used. -Note: Databases that are created with POSTGRESQL dialect do not support -extra DDL statements in the 'CreateDatabase' call. You must therefore re-apply -terraform with ddl on the same database after creation. Possible values: ["GOOGLE_STANDARD_SQL", "POSTGRESQL"]`, +If it is not provided, "GOOGLE_STANDARD_SQL" will be used. Possible values: ["GOOGLE_STANDARD_SQL", "POSTGRESQL"]`, }, "ddl": { Type: schema.TypeList, @@ -132,6 +169,17 @@ in the same location as the Spanner Database.`, }, }, }, + "version_retention_period": { + Type: schema.TypeString, + Computed: true, + Optional: true, + ValidateFunc: validateDatabaseRetentionPeriod, + Description: `The retention period for the database. The retention period must be between 1 hour +and 7 days, and can be specified in days, hours, minutes, or seconds. For example, +the values 1d, 24h, 1440m, and 86400s are equivalent. Default value is 1h. +If this property is used, you must avoid adding new DDL statements to 'ddl' that +update the database's version_retention_period.`, + }, "state": { Type: schema.TypeString, Computed: true, @@ -167,6 +215,12 @@ func resourceSpannerDatabaseCreate(d *schema.ResourceData, meta interface{}) err } else if v, ok := d.GetOkExists("name"); !isEmptyValue(reflect.ValueOf(nameProp)) && (ok || !reflect.DeepEqual(v, nameProp)) { obj["name"] = nameProp } + versionRetentionPeriodProp, err := expandSpannerDatabaseVersionRetentionPeriod(d.Get("version_retention_period"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("version_retention_period"); !isEmptyValue(reflect.ValueOf(versionRetentionPeriodProp)) && (ok || !reflect.DeepEqual(v, versionRetentionPeriodProp)) { + obj["versionRetentionPeriod"] = versionRetentionPeriodProp + } extraStatementsProp, err := expandSpannerDatabaseDdl(d.Get("ddl"), d, config) if err != nil { return err @@ -259,6 +313,69 @@ func resourceSpannerDatabaseCreate(d *schema.ResourceData, meta interface{}) err } d.SetId(id) + // Note: Databases that are created with POSTGRESQL dialect do not support extra DDL + // statements at the time of database creation. To avoid users needing to run + // `terraform apply` twice to get their desired outcome, the provider does not set + // `extraStatements` in the call to the `create` endpoint and all DDL (other than + // ) is run post-create, by calling the `updateDdl` endpoint + + _, ok := opRes["name"] + if !ok { + return fmt.Errorf("Create response didn't contain critical fields. Create may not have succeeded.") + } + + retention, retentionPeriodOk := d.GetOk("version_retention_period") + retentionPeriod := retention.(string) + ddl, ddlOk := d.GetOk("ddl") + ddlStatements := ddl.([]interface{}) + + if retentionPeriodOk || ddlOk { + + obj := make(map[string]interface{}) + updateDdls := []string{} + + if ddlOk { + for i := 0; i < len(ddlStatements); i++ { + updateDdls = append(updateDdls, ddlStatements[i].(string)) + } + } + + if retentionPeriodOk { + dbName := d.Get("name") + retentionDdl := fmt.Sprintf("ALTER DATABASE `%s` SET OPTIONS (version_retention_period=\"%s\")", dbName, retentionPeriod) + if dialect, ok := d.GetOk("database_dialect"); ok && dialect == "POSTGRESQL" { + retentionDdl = fmt.Sprintf("ALTER DATABASE \"%s\" SET spanner.version_retention_period TO \"%s\"", dbName, retentionPeriod) + } + updateDdls = append(updateDdls, retentionDdl) + } + + log.Printf("[DEBUG] Applying extra DDL statements to the new Database: %#v", updateDdls) + + obj["statements"] = updateDdls + + url, err = replaceVars(d, config, "{{SpannerBasePath}}projects/{{project}}/instances/{{instance}}/databases/{{name}}/ddl") + if err != nil { + return err + } + + res, err = sendRequestWithTimeout(config, "PATCH", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutUpdate)) + if err != nil { + return fmt.Errorf("Error executing DDL statements on Database: %s", err) + } + + // Use the resource in the operation response to populate + // identity fields and d.Id() before read + var opRes map[string]interface{} + err = spannerOperationWaitTimeWithResponse( + config, res, &opRes, project, "Creating Database", userAgent, + d.Timeout(schema.TimeoutCreate)) + if err != nil { + // The resource didn't actually create + d.SetId("") + return fmt.Errorf("Error waiting to run DDL against newly-created Database: %s", err) + } + } + log.Printf("[DEBUG] Finished creating Database %q: %#v", d.Id(), res) return resourceSpannerDatabaseRead(d, meta) @@ -319,6 +436,9 @@ func resourceSpannerDatabaseRead(d *schema.ResourceData, meta interface{}) error if err := d.Set("name", flattenSpannerDatabaseName(res["name"], d, config)); err != nil { return fmt.Errorf("Error reading Database: %s", err) } + if err := d.Set("version_retention_period", flattenSpannerDatabaseVersionRetentionPeriod(res["versionRetentionPeriod"], d, config)); err != nil { + return fmt.Errorf("Error reading Database: %s", err) + } if err := d.Set("state", flattenSpannerDatabaseState(res["state"], d, config)); err != nil { return fmt.Errorf("Error reading Database: %s", err) } @@ -352,9 +472,15 @@ func resourceSpannerDatabaseUpdate(d *schema.ResourceData, meta interface{}) err d.Partial(true) - if d.HasChange("ddl") { + if d.HasChange("version_retention_period") || d.HasChange("ddl") { obj := make(map[string]interface{}) + versionRetentionPeriodProp, err := expandSpannerDatabaseVersionRetentionPeriod(d.Get("version_retention_period"), d, config) + if err != nil { + return err + } else if v, ok := d.GetOkExists("version_retention_period"); !isEmptyValue(reflect.ValueOf(v)) && (ok || !reflect.DeepEqual(v, versionRetentionPeriodProp)) { + obj["versionRetentionPeriod"] = versionRetentionPeriodProp + } extraStatementsProp, err := expandSpannerDatabaseDdl(d.Get("ddl"), d, config) if err != nil { return err @@ -478,6 +604,10 @@ func flattenSpannerDatabaseName(v interface{}, d *schema.ResourceData, config *C return NameFromSelfLinkStateFunc(v) } +func flattenSpannerDatabaseVersionRetentionPeriod(v interface{}, d *schema.ResourceData, config *Config) interface{} { + return v +} + func flattenSpannerDatabaseState(v interface{}, d *schema.ResourceData, config *Config) interface{} { return v } @@ -514,6 +644,10 @@ func expandSpannerDatabaseName(v interface{}, d TerraformResourceData, config *C return v, nil } +func expandSpannerDatabaseVersionRetentionPeriod(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) { + return v, nil +} + func expandSpannerDatabaseDdl(v interface{}, d TerraformResourceData, config *Config) (interface{}, error) { return v, nil } @@ -558,8 +692,16 @@ func resourceSpannerDatabaseEncoder(d *schema.ResourceData, meta interface{}, ob if dialect, ok := obj["databaseDialect"]; ok && dialect == "POSTGRESQL" { obj["createStatement"] = fmt.Sprintf("CREATE DATABASE \"%s\"", obj["name"]) } + + // Extra DDL statements are removed from the create request and instead applied to the database in + // a post-create action, to accommodate retrictions when creating PostgreSQL-enabled databases. + // https://cloud.google.com/spanner/docs/create-manage-databases#create_a_database + log.Printf("[DEBUG] Preparing to create new Database. Any extra DDL statements will be applied to the Database in a separate API call") + delete(obj, "name") + delete(obj, "versionRetentionPeriod") delete(obj, "instance") + delete(obj, "extraStatements") return obj, nil } @@ -574,8 +716,19 @@ func resourceSpannerDatabaseUpdateEncoder(d *schema.ResourceData, meta interface updateDdls = append(updateDdls, newDdls[i].(string)) } + //Add statement to update version_retention_period property, if needed + if d.HasChange("version_retention_period") { + dbName := d.Get("name") + retentionDdl := fmt.Sprintf("ALTER DATABASE `%s` SET OPTIONS (version_retention_period=\"%s\")", dbName, obj["versionRetentionPeriod"]) + if dialect, ok := d.GetOk("database_dialect"); ok && dialect == "POSTGRESQL" { + retentionDdl = fmt.Sprintf("ALTER DATABASE \"%s\" SET spanner.version_retention_period TO \"%s\"", dbName, obj["versionRetentionPeriod"]) + } + updateDdls = append(updateDdls, retentionDdl) + } + obj["statements"] = updateDdls delete(obj, "name") + delete(obj, "versionRetentionPeriod") delete(obj, "instance") delete(obj, "extraStatements") return obj, nil diff --git a/google/resource_spanner_database_generated_test.go b/google/resource_spanner_database_generated_test.go index 7e02081b564..3128bf40f70 100644 --- a/google/resource_spanner_database_generated_test.go +++ b/google/resource_spanner_database_generated_test.go @@ -60,6 +60,7 @@ resource "google_spanner_instance" "main" { resource "google_spanner_database" "database" { instance = google_spanner_instance.main.name name = "tf-test-my-database%{random_suffix}" + version_retention_period = "3d" ddl = [ "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", diff --git a/google/resource_spanner_database_test.go b/google/resource_spanner_database_test.go index c7de190f6ab..1c474f2ed6e 100644 --- a/google/resource_spanner_database_test.go +++ b/google/resource_spanner_database_test.go @@ -25,6 +25,7 @@ func TestAccSpannerDatabase_basic(t *testing.T) { Config: testAccSpannerDatabase_basic(instanceName, databaseName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + resource.TestCheckResourceAttr("google_spanner_database.basic", "version_retention_period", "1h"), // default set by API ), }, { @@ -38,6 +39,7 @@ func TestAccSpannerDatabase_basic(t *testing.T) { Config: testAccSpannerDatabase_basicUpdate(instanceName, databaseName), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + resource.TestCheckResourceAttr("google_spanner_database.basic", "version_retention_period", "2d"), ), }, { @@ -105,6 +107,7 @@ resource "google_spanner_instance" "basic" { resource "google_spanner_database" "basic" { instance = google_spanner_instance.basic.name name = "%s" + version_retention_period = "2d" # increase from default 1h ddl = [ "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", @@ -171,6 +174,11 @@ resource "google_spanner_database" "basic_spangres" { instance = google_spanner_instance.basic.name name = "%s-spangres" database_dialect = "POSTGRESQL" + // Confirm that DDL can be run at creation time for POSTGRESQL + version_retention_period = "2h" + ddl = [ + "CREATE TABLE t1 (t1 bigint NOT NULL PRIMARY KEY)", + ] deletion_protection = false } `, instanceName, instanceName, databaseName) @@ -189,8 +197,8 @@ resource "google_spanner_database" "basic_spangres" { instance = google_spanner_instance.basic.name name = "%s-spangres" database_dialect = "POSTGRESQL" + version_retention_period = "4d" ddl = [ - "CREATE TABLE t1 (t1 bigint NOT NULL PRIMARY KEY)", "CREATE TABLE t2 (t2 bigint NOT NULL PRIMARY KEY)", "CREATE TABLE t3 (t3 bigint NOT NULL PRIMARY KEY)", "CREATE TABLE t4 (t4 bigint NOT NULL PRIMARY KEY)", @@ -200,6 +208,156 @@ resource "google_spanner_database" "basic_spangres" { `, instanceName, instanceName, databaseName) } +func TestAccSpannerDatabase_versionRetentionPeriod(t *testing.T) { + t.Parallel() + + rnd := randString(t, 10) + instanceName := fmt.Sprintf("tf-test-%s", rnd) + databaseName := fmt.Sprintf("tfgen_%s", rnd) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckSpannerDatabaseDestroyProducer(t), + Steps: []resource.TestStep{ + { + // Test creating a database with `version_retention_period` set + Config: testAccSpannerDatabase_versionRetentionPeriod(instanceName, databaseName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + resource.TestCheckResourceAttr("google_spanner_database.basic", "version_retention_period", "2h"), + ), + }, + { + // Test removing `version_retention_period` and setting retention period to a new value with a DDL statement in `ddl` + Config: testAccSpannerDatabase_versionRetentionPeriodUpdate1(instanceName, databaseName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + resource.TestCheckResourceAttr("google_spanner_database.basic", "version_retention_period", "4h"), + ), + }, + { + // Test that adding `version_retention_period` controls retention time, regardless of any previous statements in `ddl` + Config: testAccSpannerDatabase_versionRetentionPeriodUpdate2(instanceName, databaseName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + resource.TestCheckResourceAttr("google_spanner_database.basic", "version_retention_period", "2h"), + ), + }, + { + // Test that changing the retention value via DDL when `version_retention_period` is set: + // - changes the value (from 2h to 8h) + // - is unstable; non-empty plan afterwards due to conflict + Config: testAccSpannerDatabase_versionRetentionPeriodUpdate3(instanceName, databaseName), + ExpectNonEmptyPlan: true, // is unstable + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + resource.TestCheckResourceAttr("google_spanner_database.basic", "version_retention_period", "8h"), + ), + }, + { + // Test that when the above config is reapplied: + // - changes the value (reverts to set value of `version_retention_period`, 2h) + // - is stable; no further conflict + Config: testAccSpannerDatabase_versionRetentionPeriodUpdate3(instanceName, databaseName), //same as previous step + ExpectNonEmptyPlan: false, // is stable + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet("google_spanner_database.basic", "state"), + resource.TestCheckResourceAttr("google_spanner_database.basic", "version_retention_period", "2h"), + ), + }, + }, + }) +} + +func testAccSpannerDatabase_versionRetentionPeriod(instanceName, databaseName string) string { + return fmt.Sprintf(` +resource "google_spanner_instance" "basic" { + name = "%s" + config = "regional-us-central1" + display_name = "%s-display" + num_nodes = 1 +} + +resource "google_spanner_database" "basic" { + instance = google_spanner_instance.basic.name + name = "%s" + version_retention_period = "2h" + ddl = [ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + ] + deletion_protection = false +} +`, instanceName, instanceName, databaseName) +} + +func testAccSpannerDatabase_versionRetentionPeriodUpdate1(instanceName, databaseName string) string { + return fmt.Sprintf(` +resource "google_spanner_instance" "basic" { + name = "%s" + config = "regional-us-central1" + display_name = "%s-display" + num_nodes = 1 +} + +resource "google_spanner_database" "basic" { + instance = google_spanner_instance.basic.name + name = "%s" + // Change 1/2 : deleted version_retention_period argument + ddl = [ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "ALTER DATABASE %s SET OPTIONS (version_retention_period=\"4h\")", // Change 2/2 : set retention with new DDL + ] + deletion_protection = false +} +`, instanceName, instanceName, databaseName, databaseName) +} + +func testAccSpannerDatabase_versionRetentionPeriodUpdate2(instanceName, databaseName string) string { + return fmt.Sprintf(` +resource "google_spanner_instance" "basic" { + name = "%s" + config = "regional-us-central1" + display_name = "%s-display" + num_nodes = 1 +} + +resource "google_spanner_database" "basic" { + instance = google_spanner_instance.basic.name + name = "%s" + version_retention_period = "2h" // Change : added version_retention_period argument + ddl = [ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "ALTER DATABASE %s SET OPTIONS (version_retention_period=\"4h\")", + ] + deletion_protection = false +} +`, instanceName, instanceName, databaseName, databaseName) +} + +func testAccSpannerDatabase_versionRetentionPeriodUpdate3(instanceName, databaseName string) string { + return fmt.Sprintf(` +resource "google_spanner_instance" "basic" { + name = "%s" + config = "regional-us-central1" + display_name = "%s-display" + num_nodes = 1 +} + +resource "google_spanner_database" "basic" { + instance = google_spanner_instance.basic.name + name = "%s" + version_retention_period = "2h" + ddl = [ + "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", + "ALTER DATABASE %s SET OPTIONS (version_retention_period=\"4h\")", + "ALTER DATABASE %s SET OPTIONS (version_retention_period=\"8h\")", // Change : set retention with new DDL + ] + deletion_protection = false +} +`, instanceName, instanceName, databaseName, databaseName, databaseName) +} + // Unit Tests for type spannerDatabaseId func TestDatabaseNameForApi(t *testing.T) { id := spannerDatabaseId{ @@ -290,6 +448,77 @@ func TestSpannerDatabase_resourceSpannerDBDdlCustomDiffFuncForceNew(t *testing.T } } +// Unit Tests for validation of retention period argument +func TestValidateDatabaseRetentionPeriod(t *testing.T) { + t.Parallel() + testCases := map[string]struct { + input string + expectError bool + }{ + // Not valid input + "empty_string": { + input: "", + expectError: true, + }, + "number_with_no_unit": { + input: "1", + expectError: true, + }, + "less_than_1h": { + input: "59m", + expectError: true, + }, + "more_than_7days": { + input: "8d", + expectError: true, + }, + // Valid input + "1_hour_in_secs": { + input: "3600s", + expectError: false, + }, + "1_hour_in_mins": { + input: "60m", + expectError: false, + }, + "1_hour_in_hours": { + input: "1h", + expectError: false, + }, + "7_days_in_secs": { + input: fmt.Sprintf("%ds", 7*24*60*60), + expectError: false, + }, + "7_days_in_mins": { + input: fmt.Sprintf("%dm", 7*24*60), + expectError: false, + }, + "7_days_in_hours": { + input: fmt.Sprintf("%dh", 7*24), + expectError: false, + }, + "7_days_in_days": { + input: "7d", + expectError: false, + }, + } + + for tn, tc := range testCases { + t.Run(tn, func(t *testing.T) { + _, errs := validateDatabaseRetentionPeriod(tc.input, "foobar") + var wantErrCount string + if tc.expectError { + wantErrCount = "1+" + } else { + wantErrCount = "0" + } + if (len(errs) > 0 && tc.expectError == false) || (len(errs) == 0 && tc.expectError == true) { + t.Errorf("failed, expected `%s` test case validation to have %s errors", tn, wantErrCount) + } + }) + } +} + func TestAccSpannerDatabase_deletionProtection(t *testing.T) { skipIfVcr(t) t.Parallel() diff --git a/website/docs/r/spanner_database.html.markdown b/website/docs/r/spanner_database.html.markdown index 07b8afcbdfb..833f737e728 100644 --- a/website/docs/r/spanner_database.html.markdown +++ b/website/docs/r/spanner_database.html.markdown @@ -38,10 +38,6 @@ On older versions, it is strongly recommended to set `lifecycle { prevent_destro on databases 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. -Note: Databases that are created with POSTGRESQL dialect do not support -extra DDL statements in the `CreateDatabase` call. You must therefore re-apply -terraform with ddl on the same database after creation. -
Open in Cloud Shell @@ -60,6 +56,7 @@ resource "google_spanner_instance" "main" { resource "google_spanner_database" "database" { instance = google_spanner_instance.main.name name = "my-database" + version_retention_period = "3d" ddl = [ "CREATE TABLE t1 (t1 INT64 NOT NULL,) PRIMARY KEY(t1)", "CREATE TABLE t2 (t2 INT64 NOT NULL,) PRIMARY KEY(t2)", @@ -86,6 +83,14 @@ The following arguments are supported: - - - +* `version_retention_period` - + (Optional) + The retention period for the database. The retention period must be between 1 hour + and 7 days, and can be specified in days, hours, minutes, or seconds. For example, + the values 1d, 24h, 1440m, and 86400s are equivalent. Default value is 1h. + If this property is used, you must avoid adding new DDL statements to `ddl` that + update the database's version_retention_period. + * `ddl` - (Optional) An optional list of DDL statements to run inside the newly created @@ -101,10 +106,7 @@ The following arguments are supported: * `database_dialect` - (Optional) The dialect of the Cloud Spanner Database. - If it is not provided, "GOOGLE_STANDARD_SQL" will be used. - Note: Databases that are created with POSTGRESQL dialect do not support - extra DDL statements in the `CreateDatabase` call. You must therefore re-apply - terraform with ddl on the same database after creation. + If it is not provided, "GOOGLE_STANDARD_SQL" will be used. Possible values are `GOOGLE_STANDARD_SQL` and `POSTGRESQL`. * `project` - (Optional) The ID of the project in which the resource belongs.