Add schema update support to spanner db 2082 (#3947)

* eoncders and customdiff added for spanner DB ddl update

* config update test case added

* customdiff modified to handle out-of-index issue

* new lines added

* indent fixed

* indent fixed for tests

* test added for ddl update condition

* mock added Terraformresourcediff, unit tests added

* test fixed

* more unit tests added

* tests fixed

* PR comments implemented

* unit tests converted to table driven tests

* ImportStateVerifyIgnore flag added to tests

* syntax corrected in test

products/spanner/api.yaml

@@ -122,15 +122,15 @@ objects:
   - !ruby/object:Api::Resource
     name: 'Database'
     base_url: projects/{{project}}/instances/{{instance}}/databases
+    input: true
     description: |
       A Cloud Spanner Database which is hosted on a Spanner instance.
-    input: true
     references: !ruby/object:Api::Resource::ReferenceLinks
       guides:
         'Official Documentation': 'https://cloud.google.com/spanner/'
       api: 'https://cloud.google.com/spanner/docs/reference/rest/v1/projects.instances.databases'
     async: !ruby/object:Api::OpAsync
-      actions: ['create', 'update']
+      actions: ['create','update','delete']
       operation: !ruby/object:Api::OpAsync::Operation
         path: 'name'
         base_url: '{{op_id}}'
@@ -168,12 +168,13 @@ objects:
       - !ruby/object:Api::Type::Array
         item_type: Api::Type::String
         name: 'extraStatements'
+        update_url: projects/{{project}}/instances/{{instance}}/databases/{{name}}/ddl
+        update_verb: :PATCH
         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
           error in any statement, the database is not created.
-        input: true
       - !ruby/object:Api::Type::Enum
         name: 'state'
         description: An explanation of the status of the database.

products/spanner/terraform.yaml

@@ -45,8 +45,11 @@ overrides: !ruby/object:Overrides::ResourceOverrides
       state: !ruby/object:Overrides::Terraform::PropertyOverride
         exclude: false
     custom_code: !ruby/object:Provider::Terraform::CustomCode
+      constants: 'templates/terraform/constants/spanner_database.go.erb'
       encoder: templates/terraform/encoders/spanner_database.go.erb
       decoder: templates/terraform/decoders/spanner_database.go.erb
+      update_encoder: templates/terraform/update_encoder/spanner_database.go.erb
+      resource_definition: 'templates/terraform/resource_definition/spanner_database.go.erb'
   InstanceConfig: !ruby/object:Overrides::Terraform::ResourceOverride
     # This is appropriate for a datasource - doesn't have a create method.
     exclude: true
@@ -92,7 +95,6 @@ overrides: !ruby/object:Overrides::ResourceOverrides
       update_encoder: templates/terraform/encoders/spanner_instance_update.go.erb
       decoder: templates/terraform/decoders/spanner_instance.go.erb
       post_create: templates/terraform/post_create/sleep.go.erb
-
 # This is for copying files over
 files: !ruby/object:Provider::Config::Files
   # These files have templating (ERB) code that will be run.

templates/terraform/constants/spanner_database.go.erb

@@ -0,0 +1,45 @@
+<%- # the license inside this block applies to this file
+	# Copyright 2020 Google Inc.
+	# Licensed under the Apache License, Version 2.0 (the "License");
+	# you may not use this file except in compliance with the License.
+	# You may obtain a copy of the License at
+	#
+	#     http://www.apache.org/licenses/LICENSE-2.0
+	#
+	# Unless required by applicable law or agreed to in writing, software
+	# distributed under the License is distributed on an "AS IS" BASIS,
+	# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+	# See the License for the specific language governing permissions and
+	# limitations under the License.
+-%>
+// 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)
+}

templates/terraform/resource_definition/spanner_database.go.erb

@@ -0,0 +1,15 @@
+<%# The license inside this block applies to this file.
+	# Copyright 2020 Google Inc.
+	# Licensed under the Apache License, Version 2.0 (the "License");
+	# you may not use this file except in compliance with the License.
+	# You may obtain a copy of the License at
+	#
+	#     http://www.apache.org/licenses/LICENSE-2.0
+	#
+	# Unless required by applicable law or agreed to in writing, software
+	# distributed under the License is distributed on an "AS IS" BASIS,
+	# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+	# See the License for the specific language governing permissions and
+	# limitations under the License.
+-%>
+CustomizeDiff: resourceSpannerDBDdlCustomDiff,

templates/terraform/update_encoder/spanner_database.go.erb

@@ -0,0 +1,29 @@
+<%# The license inside this block applies to this file.
+	# Copyright 2020 Google Inc.
+	# Licensed under the Apache License, Version 2.0 (the "License");
+	# you may not use this file except in compliance with the License.
+	# You may obtain a copy of the License at
+	#
+	#     http://www.apache.org/licenses/LICENSE-2.0
+	#
+	# Unless required by applicable law or agreed to in writing, software
+	# distributed under the License is distributed on an "AS IS" BASIS,
+	# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+	# See the License for the specific language governing permissions and
+	# limitations under the License.
+-%>
+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

third_party/terraform/tests/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)
+		}
+	}
+}

third_party/terraform/utils/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{}{})
 }

third_party/terraform/utils/utils.go.erb

@@ -33,6 +33,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.