From 395bdc3098cd055a87d0bcc94d2b6cdf2b8ce9ea Mon Sep 17 00:00:00 2001 From: Harshika Jain Date: Sat, 28 Sep 2024 15:01:47 +0530 Subject: [PATCH 1/8] Update the resource getter method when instance is not active --- mmv1/products/sql/Database.yaml | 1 + .../pre_read/sql_database_activation_policy.tmpl | 8 ++++++++ .../terraform/services/sql/resource_sql_user.go | 12 +++++++----- 3 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl diff --git a/mmv1/products/sql/Database.yaml b/mmv1/products/sql/Database.yaml index 01f4a62d6247..089cb3384f0c 100644 --- a/mmv1/products/sql/Database.yaml +++ b/mmv1/products/sql/Database.yaml @@ -48,6 +48,7 @@ async: collection_url_key: 'items' custom_code: pre_delete: 'templates/terraform/pre_delete/sql_database_deletion_policy.tmpl' + pre_read: 'templates/terraform/pre_read/sql_database_activation_policy.tmpl' # Sweeper skipped as this resource has customized deletion. exclude_sweeper: true read_error_transform: 'transformSQLDatabaseReadError' diff --git a/mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl b/mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl new file mode 100644 index 000000000000..582504e94e11 --- /dev/null +++ b/mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl @@ -0,0 +1,8 @@ +instance := d.Get("instance").(string) +databaseInstance, err := config.NewSqlAdminClient(userAgent).Instances.Get(project, instance).Do() + if err != nil { + return err + } + if databaseInstance.Settings.ActivationPolicy != "ALWAYS" { + return nil + } \ No newline at end of file diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_user.go b/mmv1/third_party/terraform/services/sql/resource_sql_user.go index c507f883f122..f54e1fdac50a 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_user.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_user.go @@ -326,6 +326,13 @@ func resourceSqlUserRead(d *schema.ResourceData, meta interface{}) error { instance := d.Get("instance").(string) name := d.Get("name").(string) host := d.Get("host").(string) + databaseInstance, err := config.NewSqlAdminClient(userAgent).Instances.Get(project, instance).Do() + if err != nil { + return err + } + if databaseInstance.Settings.ActivationPolicy != "ALWAYS" { + return nil + } var users *sqladmin.UsersListResponse err = nil @@ -342,11 +349,6 @@ func resourceSqlUserRead(d *schema.ResourceData, meta interface{}) error { } var user *sqladmin.User - databaseInstance, err := config.NewSqlAdminClient(userAgent).Instances.Get(project, instance).Do() - if err != nil { - return err - } - for _, currentUser := range users.Items { var username string if !(strings.Contains(databaseInstance.DatabaseVersion, "POSTGRES") || currentUser.Type == "CLOUD_IAM_GROUP") { From e88c5fa5ddd2225338a39d0e5c454a5c56499b90 Mon Sep 17 00:00:00 2001 From: Harshika Jain Date: Wed, 9 Oct 2024 12:34:21 +0530 Subject: [PATCH 2/8] unit tests for activation policy check in user --- .../services/sql/resource_sql_user_test.go | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go index 80c50c7363ce..8e6bdf89c124 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go @@ -324,6 +324,64 @@ func TestAccSqlUser_mysqlPasswordPolicy(t *testing.T) { }) } +func TestAccSqlUser_instanceWithActivationPolicy(t *testing.T) { + // Multiple fine-grained resources + acctest.SkipIfVcr(t) + t.Parallel() + + instance := fmt.Sprintf("tf-test-%d", acctest.RandInt(t)) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccSqlUserDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "ALWAYS"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), + ), + }, + // Step 2: Update activation_policy to NEVER + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), + ), + }, + // Step 3: Refresh to verify no errors + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), + ), + }, + }, + }) +} + +func testGoogleSqlUser_instanceWithActivationPolicy(instance, activationPolicy string) string { + return fmt.Sprintf(` +resource "google_sql_database_instance" "test_instance" { + name = "%s" + database_version = "MYSQL_5_7" + region = "us-central1" + + settings { + tier = "db-f1-micro" + availability_type = "ZONAL" + activation_policy = "%s" + } +} + +resource "google_sql_user" "user" { + name = "admin" + instance = google_sql_database_instance.instance.name + password = "password" + } +`, instance, activationPolicy) +} + func testGoogleSqlUser_mysql(instance, password string) string { return fmt.Sprintf(` resource "google_sql_database_instance" "instance" { From b9acb258375749f2bbf1dc8652ea63193764aacd Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Thu, 10 Oct 2024 12:37:49 +0100 Subject: [PATCH 3/8] Fix resource reference --- .../terraform/services/sql/resource_sql_user_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go index 8e6bdf89c124..df68563ecc6f 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go @@ -362,7 +362,7 @@ func TestAccSqlUser_instanceWithActivationPolicy(t *testing.T) { func testGoogleSqlUser_instanceWithActivationPolicy(instance, activationPolicy string) string { return fmt.Sprintf(` -resource "google_sql_database_instance" "test_instance" { +resource "google_sql_database_instance" "instance" { name = "%s" database_version = "MYSQL_5_7" region = "us-central1" From 3a6dd998d962072d7e553089e71fa38fb0e76c74 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Thu, 10 Oct 2024 15:33:52 +0100 Subject: [PATCH 4/8] Apply suggestions from code review This includes: - Update to allow the test's post-test destroy code to delete resources without error - Removes checks that tried to read the user's data and triggered the "Error 400: Invalid request: Invalid request since instance is not running., invalid" API error --- .../terraform/services/sql/resource_sql_user_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go index df68563ecc6f..b2ee190473bc 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go @@ -345,13 +345,14 @@ func TestAccSqlUser_instanceWithActivationPolicy(t *testing.T) { // Step 2: Update activation_policy to NEVER { Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), - ), }, // Step 3: Refresh to verify no errors { Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), + }, + // Step 4: Update activation_policy to ALWAYS so that post-test destroy code is able to delete the google_sql_user resource + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "ALWAYS"), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), ), @@ -366,6 +367,7 @@ resource "google_sql_database_instance" "instance" { name = "%s" database_version = "MYSQL_5_7" region = "us-central1" + deletion_protection = false settings { tier = "db-f1-micro" From 046ab2baec6bf884abf1b6d2fa53a9e65b318285 Mon Sep 17 00:00:00 2001 From: Harshika Jain Date: Sat, 28 Sep 2024 15:01:47 +0530 Subject: [PATCH 5/8] Update the resource getter method when instance is not active --- mmv1/products/sql/Database.yaml | 1 + .../pre_read/sql_database_activation_policy.tmpl | 8 ++++++++ .../terraform/services/sql/resource_sql_user.go | 12 +++++++----- 3 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl diff --git a/mmv1/products/sql/Database.yaml b/mmv1/products/sql/Database.yaml index 01f4a62d6247..089cb3384f0c 100644 --- a/mmv1/products/sql/Database.yaml +++ b/mmv1/products/sql/Database.yaml @@ -48,6 +48,7 @@ async: collection_url_key: 'items' custom_code: pre_delete: 'templates/terraform/pre_delete/sql_database_deletion_policy.tmpl' + pre_read: 'templates/terraform/pre_read/sql_database_activation_policy.tmpl' # Sweeper skipped as this resource has customized deletion. exclude_sweeper: true read_error_transform: 'transformSQLDatabaseReadError' diff --git a/mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl b/mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl new file mode 100644 index 000000000000..582504e94e11 --- /dev/null +++ b/mmv1/templates/terraform/pre_read/sql_database_activation_policy.tmpl @@ -0,0 +1,8 @@ +instance := d.Get("instance").(string) +databaseInstance, err := config.NewSqlAdminClient(userAgent).Instances.Get(project, instance).Do() + if err != nil { + return err + } + if databaseInstance.Settings.ActivationPolicy != "ALWAYS" { + return nil + } \ No newline at end of file diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_user.go b/mmv1/third_party/terraform/services/sql/resource_sql_user.go index c507f883f122..f54e1fdac50a 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_user.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_user.go @@ -326,6 +326,13 @@ func resourceSqlUserRead(d *schema.ResourceData, meta interface{}) error { instance := d.Get("instance").(string) name := d.Get("name").(string) host := d.Get("host").(string) + databaseInstance, err := config.NewSqlAdminClient(userAgent).Instances.Get(project, instance).Do() + if err != nil { + return err + } + if databaseInstance.Settings.ActivationPolicy != "ALWAYS" { + return nil + } var users *sqladmin.UsersListResponse err = nil @@ -342,11 +349,6 @@ func resourceSqlUserRead(d *schema.ResourceData, meta interface{}) error { } var user *sqladmin.User - databaseInstance, err := config.NewSqlAdminClient(userAgent).Instances.Get(project, instance).Do() - if err != nil { - return err - } - for _, currentUser := range users.Items { var username string if !(strings.Contains(databaseInstance.DatabaseVersion, "POSTGRES") || currentUser.Type == "CLOUD_IAM_GROUP") { From 29588004bddbb4733093d423bda61f22d862023f Mon Sep 17 00:00:00 2001 From: Harshika Jain Date: Wed, 9 Oct 2024 12:34:21 +0530 Subject: [PATCH 6/8] unit tests for activation policy check in user --- .../services/sql/resource_sql_user_test.go | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go index 80c50c7363ce..8e6bdf89c124 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go @@ -324,6 +324,64 @@ func TestAccSqlUser_mysqlPasswordPolicy(t *testing.T) { }) } +func TestAccSqlUser_instanceWithActivationPolicy(t *testing.T) { + // Multiple fine-grained resources + acctest.SkipIfVcr(t) + t.Parallel() + + instance := fmt.Sprintf("tf-test-%d", acctest.RandInt(t)) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccSqlUserDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "ALWAYS"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), + ), + }, + // Step 2: Update activation_policy to NEVER + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), + ), + }, + // Step 3: Refresh to verify no errors + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), + ), + }, + }, + }) +} + +func testGoogleSqlUser_instanceWithActivationPolicy(instance, activationPolicy string) string { + return fmt.Sprintf(` +resource "google_sql_database_instance" "test_instance" { + name = "%s" + database_version = "MYSQL_5_7" + region = "us-central1" + + settings { + tier = "db-f1-micro" + availability_type = "ZONAL" + activation_policy = "%s" + } +} + +resource "google_sql_user" "user" { + name = "admin" + instance = google_sql_database_instance.instance.name + password = "password" + } +`, instance, activationPolicy) +} + func testGoogleSqlUser_mysql(instance, password string) string { return fmt.Sprintf(` resource "google_sql_database_instance" "instance" { From 1cc2ff923f1afc0e710bf5d1acc0d901bfbf6347 Mon Sep 17 00:00:00 2001 From: Harshika Jain Date: Wed, 16 Oct 2024 09:36:26 +0530 Subject: [PATCH 7/8] Added the tests for sql database resource if instance activation policy is never. --- .../sql/resource_sql_database_test.go | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go b/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go index aabe707bc983..080d3c0ea9ab 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go @@ -183,6 +183,74 @@ func testAccSqlDatabaseDestroyProducer(t *testing.T) func(s *terraform.State) er } } +func TestAccSqlDatabase_instanceWithActivationPolicy(t *testing.T) { + t.Parallel() + + var database sqladmin.Database + + instance_name := fmt.Sprintf("tf-test-%d", acctest.RandInt(t)) + database_name := fmt.Sprintf("tf-test-%d", acctest.RandInt(t)) + + acctest.VcrTest(t, resource.TestCase{ + PreCheck: func() { acctest.AccTestPreCheck(t) }, + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccSqlDatabaseDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name , "ALWAYS"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlDatabaseExists( + t, "google_sql_database.database", &database), + testAccCheckGoogleSqlDatabaseEquals( + "google_sql_database.database", &database), + ), + }, + // Step 2: Update activation_policy to NEVER + { + Config: testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name, "NEVER"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlDatabaseExists( + t, "google_sql_database.database", &database), + testAccCheckGoogleSqlDatabaseEquals( + "google_sql_database.database", &database), + ), + }, + // Step 3: Refresh to verify no errors + { + Config: testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name, "NEVER"), + Check: resource.ComposeTestCheckFunc( + testAccCheckGoogleSqlDatabaseExists( + t, "google_sql_database.database", &database), + testAccCheckGoogleSqlDatabaseEquals( + "google_sql_database.database", &database), + ), + }, + }, + }) +} + +func testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name, activationPolicy string) string { + return fmt.Sprintf(` +resource "google_sql_database_instance" "test_instance" { + name = "%s" + database_version = "MYSQL_5_7" + region = "us-central1" + + settings { + tier = "db-f1-micro" + availability_type = "ZONAL" + activation_policy = "%s" + } +} + +resource "google_sql_database" "database" { + name = "%s" + instance = google_sql_database_instance.instance.name + } +`, instance_name, database_name, activationPolicy) +} + + var testGoogleSqlDatabase_basic = ` resource "google_sql_database_instance" "instance" { name = "%s" From 67a854f064274811c2095c32a95fa99f12ec8d5c Mon Sep 17 00:00:00 2001 From: Harshika Jain Date: Thu, 17 Oct 2024 21:59:57 +0530 Subject: [PATCH 8/8] Unit test for activation policy check in database --- .../sql/resource_sql_database_test.go | 26 +++++++------------ .../services/sql/resource_sql_user_test.go | 11 ++++---- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go b/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go index 080d3c0ea9ab..939a8fa37fdc 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_database_test.go @@ -157,9 +157,9 @@ func testAccCheckGoogleSqlDatabaseExists(t *testing.T, n string, database *sqlad *database = *found - return nil - } -} + return nil + } + } func testAccSqlDatabaseDestroyProducer(t *testing.T) func(s *terraform.State) error { return func(s *terraform.State) error { @@ -201,28 +201,22 @@ func TestAccSqlDatabase_instanceWithActivationPolicy(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckGoogleSqlDatabaseExists( t, "google_sql_database.database", &database), - testAccCheckGoogleSqlDatabaseEquals( - "google_sql_database.database", &database), ), }, // Step 2: Update activation_policy to NEVER { Config: testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name, "NEVER"), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleSqlDatabaseExists( - t, "google_sql_database.database", &database), - testAccCheckGoogleSqlDatabaseEquals( - "google_sql_database.database", &database), - ), }, // Step 3: Refresh to verify no errors { Config: testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name, "NEVER"), + }, + // Step 4: Update activation_policy to ALWAYS so that post-test destroy code is able to delete the google_sql_database resource + { + Config: testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name, "ALWAYS"), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleSqlDatabaseExists( t, "google_sql_database.database", &database), - testAccCheckGoogleSqlDatabaseEquals( - "google_sql_database.database", &database), ), }, }, @@ -231,11 +225,11 @@ func TestAccSqlDatabase_instanceWithActivationPolicy(t *testing.T) { func testGoogleSqlDatabase_instanceWithActivationPolicy(instance_name, database_name, activationPolicy string) string { return fmt.Sprintf(` -resource "google_sql_database_instance" "test_instance" { +resource "google_sql_database_instance" "instance" { name = "%s" database_version = "MYSQL_5_7" region = "us-central1" - + deletion_protection = false settings { tier = "db-f1-micro" availability_type = "ZONAL" @@ -247,7 +241,7 @@ resource "google_sql_database" "database" { name = "%s" instance = google_sql_database_instance.instance.name } -`, instance_name, database_name, activationPolicy) +`, instance_name, activationPolicy, database_name,) } diff --git a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go index 8e6bdf89c124..a14ad85a4fe2 100644 --- a/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go +++ b/mmv1/third_party/terraform/services/sql/resource_sql_user_test.go @@ -345,13 +345,14 @@ func TestAccSqlUser_instanceWithActivationPolicy(t *testing.T) { // Step 2: Update activation_policy to NEVER { Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), - Check: resource.ComposeTestCheckFunc( - testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), - ), }, // Step 3: Refresh to verify no errors { Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "NEVER"), + }, + // Step 4: Update activation_policy to ALWAYS so that post-test destroy code is able to delete the google_sql_user resource + { + Config: testGoogleSqlUser_instanceWithActivationPolicy(instance, "ALWAYS"), Check: resource.ComposeTestCheckFunc( testAccCheckGoogleSqlUserExists(t, "google_sql_user.user"), ), @@ -362,11 +363,11 @@ func TestAccSqlUser_instanceWithActivationPolicy(t *testing.T) { func testGoogleSqlUser_instanceWithActivationPolicy(instance, activationPolicy string) string { return fmt.Sprintf(` -resource "google_sql_database_instance" "test_instance" { +resource "google_sql_database_instance" "instance" { name = "%s" database_version = "MYSQL_5_7" region = "us-central1" - + deletion_protection = false settings { tier = "db-f1-micro" availability_type = "ZONAL"