From 460e787bf5f0e4b3499403f36f7754557b61eff8 Mon Sep 17 00:00:00 2001 From: Terrence Ryan Date: Fri, 11 Nov 2022 23:58:34 +0000 Subject: [PATCH 1/5] feat: adding deletion_policy=abandon to google_sql_database resource --- mmv1/products/sql/api.yaml | 9 ++ mmv1/products/sql/terraform.yaml | 2 + .../sql_database_deletion_policy.erb | 50 ++++++++++ .../tests/resource_sql_database_test.go | 94 +++++++++++++++++++ 4 files changed, 155 insertions(+) create mode 100644 mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb diff --git a/mmv1/products/sql/api.yaml b/mmv1/products/sql/api.yaml index de11738f6764..9798cab3f7d4 100644 --- a/mmv1/products/sql/api.yaml +++ b/mmv1/products/sql/api.yaml @@ -504,6 +504,15 @@ objects: description: | The name of the database in the Cloud SQL instance. This does not include the project ID or instance name. + - !ruby/object:Api::Type::String + name: 'deletion_policy' + required: false + description: | + (Optional) The deletion policy for the database. Setting ABANDON allows the resource + to be abandoned rather than deleted. This is useful for Postgres, where databases cannot be + deleted from the API if there are users other than cloudsqlsuperuser with access. Possible + values are: "ABANDON". + input: true - !ruby/object:Api::Resource name: 'User' kind: 'sql#user' diff --git a/mmv1/products/sql/terraform.yaml b/mmv1/products/sql/terraform.yaml index fc43af06824b..84993e252a5f 100644 --- a/mmv1/products/sql/terraform.yaml +++ b/mmv1/products/sql/terraform.yaml @@ -42,6 +42,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides charset: !ruby/object:Overrides::Terraform::PropertyOverride default_from_api: true diff_suppress_func: 'caseDiffSuppress' + custom_code: !ruby/object:Provider::Terraform::CustomCode + custom_delete: templates/terraform/custom_delete/sql_database_deletion_policy.erb SourceRepresentationInstance: !ruby/object:Overrides::Terraform::ResourceOverride examples: - !ruby/object:Provider::Terraform::Examples diff --git a/mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb b/mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb new file mode 100644 index 000000000000..6c124fe95748 --- /dev/null +++ b/mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb @@ -0,0 +1,50 @@ + +if deletionPolicy := d.Get("deletion_policy"); deletionPolicy == "ABANDON" { + // Allows for database to be abandoned without deletion to avoid deletion failing + // for Postgres databases in some circumstances due to existing SQL users + return nil +} + +billingProject := "" + +project, err := getProject(d, config) +if err != nil { + return fmt.Errorf("Error fetching project for Database: %s", err) +} +billingProject = project + +lockName, err := replaceVars(d, config, "google-sql-database-instance-{{project}}-{{instance}}") +if err != nil { + return err +} +mutexKV.Lock(lockName) +defer mutexKV.Unlock(lockName) + +url, err := replaceVars(d, config, "{{SQLBasePath}}projects/{{project}}/instances/{{instance}}/databases/{{name}}") +if err != nil { + return err +} + +var obj map[string]interface{} +log.Printf("[DEBUG] Deleting Database %q", d.Id()) + +// err == nil indicates that the billing_project value was found +if bp, err := getBillingProject(d, config); err == nil { + billingProject = bp +} + +res, err := sendRequestWithTimeout(config, "DELETE", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutDelete)) +if err != nil { + return handleNotFoundError(err, d, "Database") +} + +err = sqlAdminOperationWaitTime( + config, res, project, "Deleting Database", userAgent, + d.Timeout(schema.TimeoutDelete)) + +if err != nil { + return err +} + +log.Printf("[DEBUG] Finished deleting Database %q: %#v", d.Id(), res) +return nil \ No newline at end of file diff --git a/mmv1/third_party/terraform/tests/resource_sql_database_test.go b/mmv1/third_party/terraform/tests/resource_sql_database_test.go index e175c13fab82..75f111de1ce9 100644 --- a/mmv1/third_party/terraform/tests/resource_sql_database_test.go +++ b/mmv1/third_party/terraform/tests/resource_sql_database_test.go @@ -215,6 +215,71 @@ func testAccSqlDatabaseDestroyProducer(t *testing.T) func(s *terraform.State) er } } +func TestAccSqlDatabase_postgresAbandon(t *testing.T) { + t.Parallel() + + var database sqladmin.Database + + resourceName := "google_sql_database.database" + instanceName := fmt.Sprintf("sqldatabasetest-%d", randInt(t)) + dbName := fmt.Sprintf("sqldatabasetest-%d", randInt(t)) + + vcrTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccSqlDatabaseDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(testGoogleSqlDatabase_deletion_policy_abandon, instanceName, dbName), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlDatabaseExists(t, resourceName, &database), + testAccCheckGoogleSqlDatabaseEquals(resourceName, &database), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: resourceName, + ImportStateId: fmt.Sprintf("%s/%s", instanceName, dbName), + ImportState: true, + ImportStateVerify: true, + }, + + { + ResourceName: resourceName, + ImportStateId: fmt.Sprintf("instances/%s/databases/%s", instanceName, dbName), + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: resourceName, + ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), instanceName, dbName), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_protection"}, + }, + { + ResourceName: resourceName, + ImportStateId: fmt.Sprintf("projects/%s/instances/%s/databases/%s", getTestProjectFromEnv(), instanceName, dbName), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"deletion_protection"}, + }, + + { + // Abandon user + Config: fmt.Sprintf(testGoogleSqlDatabase_deletion_policy_abandon_no_database, instanceName), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlDatabaseExists(t, resourceName, &database), + ), + }, + }, + }) +} + var testGoogleSqlDatabase_basic = ` resource "google_sql_database_instance" "instance" { name = "%s" @@ -249,3 +314,32 @@ resource "google_sql_database" "database" { collation = "latin1_swedish_ci" } ` +var testGoogleSqlDatabase_deletion_policy_abandon = ` +resource "google_sql_database_instance" "instance" { + name = "%s" + region = "us-central1" + database_version = "MYSQL_5_7" + deletion_protection = false + settings { + tier = "db-f1-micro" + } +} + +resource "google_sql_database" "database" { + name = "%s" + instance = google_sql_database_instance.instance.name + deletion_policy = "ABANDON" +} +` + +var testGoogleSqlDatabase_deletion_policy_abandon_no_database = ` +resource "google_sql_database_instance" "instance" { + name = "%s" + region = "us-central1" + database_version = "MYSQL_5_7" + deletion_protection = false + settings { + tier = "db-f1-micro" + } +} +` From 85d629d43dc67f1f410e717112033ca114648322 Mon Sep 17 00:00:00 2001 From: Terrence Ryan Date: Sat, 12 Nov 2022 01:06:36 +0000 Subject: [PATCH 2/5] feat: honing in on correct behavior of google_sql_database --- mmv1/products/sql/api.yaml | 4 +- mmv1/products/sql/terraform.yaml | 11 +++ .../sql_database_deletion_policy.tf.erb | 19 ++++ .../tests/resource_sql_database_test.go | 92 ------------------- 4 files changed, 31 insertions(+), 95 deletions(-) create mode 100644 mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb diff --git a/mmv1/products/sql/api.yaml b/mmv1/products/sql/api.yaml index 9798cab3f7d4..f2780f18d7ff 100644 --- a/mmv1/products/sql/api.yaml +++ b/mmv1/products/sql/api.yaml @@ -506,13 +506,11 @@ objects: This does not include the project ID or instance name. - !ruby/object:Api::Type::String name: 'deletion_policy' - required: false description: | - (Optional) The deletion policy for the database. Setting ABANDON allows the resource + The deletion policy for the database. Setting ABANDON allows the resource to be abandoned rather than deleted. This is useful for Postgres, where databases cannot be deleted from the API if there are users other than cloudsqlsuperuser with access. Possible values are: "ABANDON". - input: true - !ruby/object:Api::Resource name: 'User' kind: 'sql#user' diff --git a/mmv1/products/sql/terraform.yaml b/mmv1/products/sql/terraform.yaml index 84993e252a5f..74668faec2b2 100644 --- a/mmv1/products/sql/terraform.yaml +++ b/mmv1/products/sql/terraform.yaml @@ -36,6 +36,17 @@ overrides: !ruby/object:Overrides::ResourceOverrides deletion_protection: "false" oics_vars_overrides: deletion_protection: "false" + - !ruby/object:Provider::Terraform::Examples + name: "sql_database_deletion_policy" + primary_resource_id: "database_deletion_policy" + vars: + database_name: "my-database" + database_instance_name: "my-database-instance" + deletion_protection: "true" + test_vars_overrides: + deletion_protection: "false" + oics_vars_overrides: + deletion_protection: "false" properties: collation: !ruby/object:Overrides::Terraform::PropertyOverride default_from_api: true diff --git a/mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb b/mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb new file mode 100644 index 000000000000..454f3eebe279 --- /dev/null +++ b/mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb @@ -0,0 +1,19 @@ +# [START cloud_sql_database_create] +resource "google_sql_database" "<%= ctx[:primary_resource_id] %>" { + name = "<%= ctx[:vars]['database_name'] %>" + instance = google_sql_database_instance.instance.name + deletion_policy = "ABANDON" +} +# [END cloud_sql_database_create] + +# See versions at https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/sql_database_instance#database_version +resource "google_sql_database_instance" "instance" { + name = "<%= ctx[:vars]['database_instance_name'] %>" + region = "us-central1" + database_version = "POSTGRES_14" + settings { + tier = "db-f1-micro" + } + + deletion_protection = "<%= ctx[:vars]['deletion_protection'] %>" +} diff --git a/mmv1/third_party/terraform/tests/resource_sql_database_test.go b/mmv1/third_party/terraform/tests/resource_sql_database_test.go index 75f111de1ce9..d2d20c2055b6 100644 --- a/mmv1/third_party/terraform/tests/resource_sql_database_test.go +++ b/mmv1/third_party/terraform/tests/resource_sql_database_test.go @@ -215,70 +215,6 @@ func testAccSqlDatabaseDestroyProducer(t *testing.T) func(s *terraform.State) er } } -func TestAccSqlDatabase_postgresAbandon(t *testing.T) { - t.Parallel() - - var database sqladmin.Database - - resourceName := "google_sql_database.database" - instanceName := fmt.Sprintf("sqldatabasetest-%d", randInt(t)) - dbName := fmt.Sprintf("sqldatabasetest-%d", randInt(t)) - - vcrTest(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccSqlDatabaseDestroyProducer(t), - Steps: []resource.TestStep{ - { - Config: fmt.Sprintf(testGoogleSqlDatabase_deletion_policy_abandon, instanceName, dbName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleSqlDatabaseExists(t, resourceName, &database), - testAccCheckGoogleSqlDatabaseEquals(resourceName, &database), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - }, - { - ResourceName: resourceName, - ImportStateId: fmt.Sprintf("%s/%s", instanceName, dbName), - ImportState: true, - ImportStateVerify: true, - }, - - { - ResourceName: resourceName, - ImportStateId: fmt.Sprintf("instances/%s/databases/%s", instanceName, dbName), - ImportState: true, - ImportStateVerify: true, - }, - { - ResourceName: resourceName, - ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), instanceName, dbName), - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_protection"}, - }, - { - ResourceName: resourceName, - ImportStateId: fmt.Sprintf("projects/%s/instances/%s/databases/%s", getTestProjectFromEnv(), instanceName, dbName), - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"deletion_protection"}, - }, - - { - // Abandon user - Config: fmt.Sprintf(testGoogleSqlDatabase_deletion_policy_abandon_no_database, instanceName), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleSqlDatabaseExists(t, resourceName, &database), - ), - }, - }, - }) -} var testGoogleSqlDatabase_basic = ` resource "google_sql_database_instance" "instance" { @@ -314,32 +250,4 @@ resource "google_sql_database" "database" { collation = "latin1_swedish_ci" } ` -var testGoogleSqlDatabase_deletion_policy_abandon = ` -resource "google_sql_database_instance" "instance" { - name = "%s" - region = "us-central1" - database_version = "MYSQL_5_7" - deletion_protection = false - settings { - tier = "db-f1-micro" - } -} - -resource "google_sql_database" "database" { - name = "%s" - instance = google_sql_database_instance.instance.name - deletion_policy = "ABANDON" -} -` -var testGoogleSqlDatabase_deletion_policy_abandon_no_database = ` -resource "google_sql_database_instance" "instance" { - name = "%s" - region = "us-central1" - database_version = "MYSQL_5_7" - deletion_protection = false - settings { - tier = "db-f1-micro" - } -} -` From 0c8f004776e85f781ad5367bd6bc71a1fe140d37 Mon Sep 17 00:00:00 2001 From: Terrence Ryan Date: Sat, 12 Nov 2022 06:19:56 +0000 Subject: [PATCH 3/5] feat: got tests to pass --- mmv1/products/sql/api.yaml | 1 + mmv1/products/sql/terraform.yaml | 2 +- .../terraform/examples/sql_database_deletion_policy.tf.erb | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mmv1/products/sql/api.yaml b/mmv1/products/sql/api.yaml index f2780f18d7ff..3c147fe80928 100644 --- a/mmv1/products/sql/api.yaml +++ b/mmv1/products/sql/api.yaml @@ -511,6 +511,7 @@ objects: to be abandoned rather than deleted. This is useful for Postgres, where databases cannot be deleted from the API if there are users other than cloudsqlsuperuser with access. Possible values are: "ABANDON". + url_param_only: true - !ruby/object:Api::Resource name: 'User' kind: 'sql#user' diff --git a/mmv1/products/sql/terraform.yaml b/mmv1/products/sql/terraform.yaml index 74668faec2b2..ed8a9a93f963 100644 --- a/mmv1/products/sql/terraform.yaml +++ b/mmv1/products/sql/terraform.yaml @@ -46,7 +46,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides test_vars_overrides: deletion_protection: "false" oics_vars_overrides: - deletion_protection: "false" + deletion_protection: "false" properties: collation: !ruby/object:Overrides::Terraform::PropertyOverride default_from_api: true diff --git a/mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb b/mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb index 454f3eebe279..891977d08291 100644 --- a/mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb +++ b/mmv1/templates/terraform/examples/sql_database_deletion_policy.tf.erb @@ -12,7 +12,7 @@ resource "google_sql_database_instance" "instance" { region = "us-central1" database_version = "POSTGRES_14" settings { - tier = "db-f1-micro" + tier = "db-g1-small" } deletion_protection = "<%= ctx[:vars]['deletion_protection'] %>" From 1d28a37013bb7965570916f0012868032e328be4 Mon Sep 17 00:00:00 2001 From: Terrence Ryan Date: Mon, 21 Nov 2022 23:53:25 +0000 Subject: [PATCH 4/5] fix: simplified adding deletion_policy to google_sql_database --- mmv1/products/sql/terraform.yaml | 2 +- .../sql_database_deletion_policy.erb | 50 ------------------- .../sql_database_deletion_policy.erb | 5 ++ .../tests/resource_sql_database_test.go | 2 - 4 files changed, 6 insertions(+), 53 deletions(-) delete mode 100644 mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb create mode 100644 mmv1/templates/terraform/pre_delete/sql_database_deletion_policy.erb diff --git a/mmv1/products/sql/terraform.yaml b/mmv1/products/sql/terraform.yaml index ed8a9a93f963..a6d82464c9df 100644 --- a/mmv1/products/sql/terraform.yaml +++ b/mmv1/products/sql/terraform.yaml @@ -54,7 +54,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides default_from_api: true diff_suppress_func: 'caseDiffSuppress' custom_code: !ruby/object:Provider::Terraform::CustomCode - custom_delete: templates/terraform/custom_delete/sql_database_deletion_policy.erb + pre_delete: templates/terraform/pre_delete/sql_database_deletion_policy.erb SourceRepresentationInstance: !ruby/object:Overrides::Terraform::ResourceOverride examples: - !ruby/object:Provider::Terraform::Examples diff --git a/mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb b/mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb deleted file mode 100644 index 6c124fe95748..000000000000 --- a/mmv1/templates/terraform/custom_delete/sql_database_deletion_policy.erb +++ /dev/null @@ -1,50 +0,0 @@ - -if deletionPolicy := d.Get("deletion_policy"); deletionPolicy == "ABANDON" { - // Allows for database to be abandoned without deletion to avoid deletion failing - // for Postgres databases in some circumstances due to existing SQL users - return nil -} - -billingProject := "" - -project, err := getProject(d, config) -if err != nil { - return fmt.Errorf("Error fetching project for Database: %s", err) -} -billingProject = project - -lockName, err := replaceVars(d, config, "google-sql-database-instance-{{project}}-{{instance}}") -if err != nil { - return err -} -mutexKV.Lock(lockName) -defer mutexKV.Unlock(lockName) - -url, err := replaceVars(d, config, "{{SQLBasePath}}projects/{{project}}/instances/{{instance}}/databases/{{name}}") -if err != nil { - return err -} - -var obj map[string]interface{} -log.Printf("[DEBUG] Deleting Database %q", d.Id()) - -// err == nil indicates that the billing_project value was found -if bp, err := getBillingProject(d, config); err == nil { - billingProject = bp -} - -res, err := sendRequestWithTimeout(config, "DELETE", billingProject, url, userAgent, obj, d.Timeout(schema.TimeoutDelete)) -if err != nil { - return handleNotFoundError(err, d, "Database") -} - -err = sqlAdminOperationWaitTime( - config, res, project, "Deleting Database", userAgent, - d.Timeout(schema.TimeoutDelete)) - -if err != nil { - return err -} - -log.Printf("[DEBUG] Finished deleting Database %q: %#v", d.Id(), res) -return nil \ No newline at end of file diff --git a/mmv1/templates/terraform/pre_delete/sql_database_deletion_policy.erb b/mmv1/templates/terraform/pre_delete/sql_database_deletion_policy.erb new file mode 100644 index 000000000000..c111f346b167 --- /dev/null +++ b/mmv1/templates/terraform/pre_delete/sql_database_deletion_policy.erb @@ -0,0 +1,5 @@ +if deletionPolicy := d.Get("deletion_policy"); deletionPolicy == "ABANDON" { + // Allows for database to be abandoned without deletion to avoid deletion failing + // for Postgres databases in some circumstances due to existing SQL users + return nil +} \ No newline at end of file diff --git a/mmv1/third_party/terraform/tests/resource_sql_database_test.go b/mmv1/third_party/terraform/tests/resource_sql_database_test.go index d2d20c2055b6..e175c13fab82 100644 --- a/mmv1/third_party/terraform/tests/resource_sql_database_test.go +++ b/mmv1/third_party/terraform/tests/resource_sql_database_test.go @@ -215,7 +215,6 @@ func testAccSqlDatabaseDestroyProducer(t *testing.T) func(s *terraform.State) er } } - var testGoogleSqlDatabase_basic = ` resource "google_sql_database_instance" "instance" { name = "%s" @@ -250,4 +249,3 @@ resource "google_sql_database" "database" { collation = "latin1_swedish_ci" } ` - From 717db50ebe9e54a471f3129169605bb2ce9f3db4 Mon Sep 17 00:00:00 2001 From: Terrence Ryan Date: Tue, 22 Nov 2022 20:01:33 +0000 Subject: [PATCH 5/5] chore: switch deletion_policy to virtual field --- mmv1/products/sql/api.yaml | 8 -------- mmv1/products/sql/terraform.yaml | 9 +++++++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/mmv1/products/sql/api.yaml b/mmv1/products/sql/api.yaml index 3c147fe80928..de11738f6764 100644 --- a/mmv1/products/sql/api.yaml +++ b/mmv1/products/sql/api.yaml @@ -504,14 +504,6 @@ objects: description: | The name of the database in the Cloud SQL instance. This does not include the project ID or instance name. - - !ruby/object:Api::Type::String - name: 'deletion_policy' - description: | - The deletion policy for the database. Setting ABANDON allows the resource - to be abandoned rather than deleted. This is useful for Postgres, where databases cannot be - deleted from the API if there are users other than cloudsqlsuperuser with access. Possible - values are: "ABANDON". - url_param_only: true - !ruby/object:Api::Resource name: 'User' kind: 'sql#user' diff --git a/mmv1/products/sql/terraform.yaml b/mmv1/products/sql/terraform.yaml index a6d82464c9df..7cd7181825d4 100644 --- a/mmv1/products/sql/terraform.yaml +++ b/mmv1/products/sql/terraform.yaml @@ -53,6 +53,15 @@ overrides: !ruby/object:Overrides::ResourceOverrides charset: !ruby/object:Overrides::Terraform::PropertyOverride default_from_api: true diff_suppress_func: 'caseDiffSuppress' + virtual_fields: + - !ruby/object:Api::Type::String + name: 'deletion_policy' + description: | + The deletion policy for the database. Setting ABANDON allows the resource + to be abandoned rather than deleted. This is useful for Postgres, where databases cannot be + deleted from the API if there are users other than cloudsqlsuperuser with access. Possible + values are: "ABANDON". + default_value: "ABANDON" custom_code: !ruby/object:Provider::Terraform::CustomCode pre_delete: templates/terraform/pre_delete/sql_database_deletion_policy.erb SourceRepresentationInstance: !ruby/object:Overrides::Terraform::ResourceOverride