From 2fe30302b6894bdc6cd7b3f3e6415197901de043 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Fri, 9 Feb 2024 16:43:24 +0000 Subject: [PATCH 01/10] Add additional wait in TestAccFirestoreField_* tests --- .../firestore/resource_firestore_field_test.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go index 794b5f8369a5..55ade17b5a36 100644 --- a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go +++ b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go @@ -83,18 +83,26 @@ resource "google_project" "project" { org_id = "%{org_id}" } -resource "time_sleep" "wait_60_seconds" { +# 60s wait between creating the project and enabling the service +resource "time_sleep" "wait_60_sec_after_project" { depends_on = [google_project.project] create_duration = "60s" } +# 60s wait between enabling the service and trying to use it +resource "time_sleep" "wait_60_sec_after_service" { + depends_on = [google_project_service.firestore] + + create_duration = "60s" +} + resource "google_project_service" "firestore" { project = google_project.project.project_id service = "firestore.googleapis.com" # Needed for CI tests for permissions to propagate, should not be needed for actual usage - depends_on = [time_sleep.wait_60_seconds] + depends_on = [time_sleep.wait_60_sec_after_project] } resource "google_firestore_database" "database" { @@ -103,7 +111,7 @@ resource "google_firestore_database" "database" { location_id = "nam5" type = "FIRESTORE_NATIVE" - depends_on = [google_project_service.firestore] + depends_on = [time_sleep.wait_60_sec_after_service] } `, context) } else { From eb6eb9dd1526e384feeef196dcd9da91c85fc69a Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Tue, 13 Feb 2024 11:02:17 +0000 Subject: [PATCH 02/10] Boost wait in test to 6 minutes --- .../firestore/resource_firestore_field_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go index 55ade17b5a36..ed782823a387 100644 --- a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go +++ b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go @@ -84,17 +84,17 @@ resource "google_project" "project" { } # 60s wait between creating the project and enabling the service -resource "time_sleep" "wait_60_sec_after_project" { +resource "time_sleep" "wait_360_sec_after_project" { depends_on = [google_project.project] - create_duration = "60s" + create_duration = "360s" } # 60s wait between enabling the service and trying to use it -resource "time_sleep" "wait_60_sec_after_service" { +resource "time_sleep" "wait_360_sec_after_service" { depends_on = [google_project_service.firestore] - create_duration = "60s" + create_duration = "360s" } resource "google_project_service" "firestore" { @@ -102,7 +102,7 @@ resource "google_project_service" "firestore" { service = "firestore.googleapis.com" # Needed for CI tests for permissions to propagate, should not be needed for actual usage - depends_on = [time_sleep.wait_60_sec_after_project] + depends_on = [time_sleep.wait_360_sec_after_project] } resource "google_firestore_database" "database" { @@ -111,7 +111,7 @@ resource "google_firestore_database" "database" { location_id = "nam5" type = "FIRESTORE_NATIVE" - depends_on = [time_sleep.wait_60_sec_after_service] + depends_on = [time_sleep.wait_360_sec_after_service] } `, context) } else { From 492ed9a8346c931fab74057f6724a554e5201261 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 13 Feb 2024 12:45:33 +0000 Subject: [PATCH 03/10] Add dependency between database and service to control delete order --- .../firestore/resource_firestore_field_test.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go index ed782823a387..3c172b396782 100644 --- a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go +++ b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go @@ -83,18 +83,10 @@ resource "google_project" "project" { org_id = "%{org_id}" } -# 60s wait between creating the project and enabling the service -resource "time_sleep" "wait_360_sec_after_project" { +resource "time_sleep" "wait_60_sec" { depends_on = [google_project.project] - create_duration = "360s" -} - -# 60s wait between enabling the service and trying to use it -resource "time_sleep" "wait_360_sec_after_service" { - depends_on = [google_project_service.firestore] - - create_duration = "360s" + create_duration = "60s" } resource "google_project_service" "firestore" { @@ -102,7 +94,7 @@ resource "google_project_service" "firestore" { service = "firestore.googleapis.com" # Needed for CI tests for permissions to propagate, should not be needed for actual usage - depends_on = [time_sleep.wait_360_sec_after_project] + depends_on = [time_sleep.wait_60_sec] } resource "google_firestore_database" "database" { @@ -111,7 +103,7 @@ resource "google_firestore_database" "database" { location_id = "nam5" type = "FIRESTORE_NATIVE" - depends_on = [time_sleep.wait_360_sec_after_service] + depends_on = [google_project_service.firestore] # control delete order } `, context) } else { @@ -123,7 +115,7 @@ resource "google_firestore_database" "database" { type = "FIRESTORE_NATIVE" delete_protection_state = "DELETE_PROTECTION_DISABLED" - deletion_policy = "DELETE" + deletion_policy = "DELETE" } `, context) } From 320346e0fae43ffeb30f8d92c52c0ea4dd7b7a3a Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 13 Feb 2024 17:16:19 +0000 Subject: [PATCH 04/10] Update dependency to explicitly include project --- .../services/firestore/resource_firestore_field_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go index 3c172b396782..eb27107c9415 100644 --- a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go +++ b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go @@ -103,7 +103,11 @@ resource "google_firestore_database" "database" { location_id = "nam5" type = "FIRESTORE_NATIVE" - depends_on = [google_project_service.firestore] # control delete order + # used to control delete order + depends_on = [ + google_project_service.firestore, + google_project.project + ] } `, context) } else { From f701dbcbaa06b35be9803ae9ebfda645934dfd40 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Tue, 13 Feb 2024 19:35:44 +0000 Subject: [PATCH 05/10] Make firestore fields be removed from state when they're 'deleted' --- .../terraform/custom_delete/firestore_field_delete.go.erb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb b/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb index 7e78f7c53e0d..9a02ab2a0696 100644 --- a/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb +++ b/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb @@ -52,5 +52,7 @@ if err != nil { return err } +d.SetId("") // Remove the resource from state + log.Printf("[DEBUG] Finished deleting Field %q", d.Id()) return nil From da87da84086ae2799f4ae75b3d9927ab81ffffb8 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 14 Feb 2024 12:41:44 +0000 Subject: [PATCH 06/10] Add `destroy_duration` --- .../services/firestore/resource_firestore_field_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go index eb27107c9415..2b08e595b59d 100644 --- a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go +++ b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go @@ -87,6 +87,7 @@ resource "time_sleep" "wait_60_sec" { depends_on = [google_project.project] create_duration = "60s" + destroy_duration = "60s" } resource "google_project_service" "firestore" { From e21122f65e086ac203e9dacd8027545a66248960 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 14 Feb 2024 12:42:27 +0000 Subject: [PATCH 07/10] Remove from state after log line that uses id value --- .../terraform/custom_delete/firestore_field_delete.go.erb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb b/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb index 9a02ab2a0696..c69fdfb7b3c3 100644 --- a/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb +++ b/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb @@ -52,7 +52,8 @@ if err != nil { return err } +log.Printf("[DEBUG] Finished deleting Field %q", d.Id()) + d.SetId("") // Remove the resource from state -log.Printf("[DEBUG] Finished deleting Field %q", d.Id()) return nil From 5b552e9bbe89bc925705377dbbf0d4e60c7e4bc6 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 28 Feb 2024 12:53:28 +0000 Subject: [PATCH 08/10] Update destory check to accept a 403 as valid --- .../custom_check_destroy/firestore_field.go.erb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/mmv1/templates/terraform/custom_check_destroy/firestore_field.go.erb b/mmv1/templates/terraform/custom_check_destroy/firestore_field.go.erb index f3caa9775cca..7b5fe9d1fffc 100644 --- a/mmv1/templates/terraform/custom_check_destroy/firestore_field.go.erb +++ b/mmv1/templates/terraform/custom_check_destroy/firestore_field.go.erb @@ -16,7 +16,16 @@ res, err := transport_tpg.SendRequest(transport_tpg.SendRequestOptions{ UserAgent: config.UserAgent, }) if err != nil { - return err + e := err.(*googleapi.Error) + if e.Code == 403 && strings.Contains(e.Message, "Cloud Firestore API has not been used in project") { + // The acceptance test has provisioned the resources under test in a new project, and the destory check is seeing the + // effects of the project not existing. This means the service isn't enabled, and that the resource is definitely destroyed. + // We do not return the error in this case - destroy was successful + return nil + } + + // Return err in all other cases + return err } if v := res["indexConfig"]; v != nil { From 6532126090c1fbbc31b272669fdf2fb9a7899447 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Wed, 28 Feb 2024 12:54:59 +0000 Subject: [PATCH 09/10] Remove unneeded changes in PR --- .../services/firestore/resource_firestore_field_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go index 2b08e595b59d..8be524c6e99a 100644 --- a/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go +++ b/mmv1/third_party/terraform/services/firestore/resource_firestore_field_test.go @@ -83,11 +83,10 @@ resource "google_project" "project" { org_id = "%{org_id}" } -resource "time_sleep" "wait_60_sec" { +resource "time_sleep" "wait_60_seconds" { depends_on = [google_project.project] create_duration = "60s" - destroy_duration = "60s" } resource "google_project_service" "firestore" { @@ -95,7 +94,7 @@ resource "google_project_service" "firestore" { service = "firestore.googleapis.com" # Needed for CI tests for permissions to propagate, should not be needed for actual usage - depends_on = [time_sleep.wait_60_sec] + depends_on = [time_sleep.wait_60_seconds] } resource "google_firestore_database" "database" { From 65fac0f900a863594a967165edf83ff07a6512dc Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:34:49 +0000 Subject: [PATCH 10/10] Remove call to SetId --- .../terraform/custom_delete/firestore_field_delete.go.erb | 3 --- 1 file changed, 3 deletions(-) diff --git a/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb b/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb index c69fdfb7b3c3..7e78f7c53e0d 100644 --- a/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb +++ b/mmv1/templates/terraform/custom_delete/firestore_field_delete.go.erb @@ -53,7 +53,4 @@ if err != nil { } log.Printf("[DEBUG] Finished deleting Field %q", d.Id()) - -d.SetId("") // Remove the resource from state - return nil