From 0e121cfa62e9a30b623fb4c824166826a09d1b2e Mon Sep 17 00:00:00 2001 From: Sarah French Date: Thu, 28 Mar 2024 12:09:35 +0000 Subject: [PATCH 1/4] Add output `project_number` to google_storage_bucket resource and data source --- .../services/storage/resource_storage_bucket.go.erb | 9 +++++++++ .../services/storage/resource_storage_bucket_test.go.erb | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb b/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb index fc17d08790b8..29c050cae447 100644 --- a/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb +++ b/mmv1/third_party/terraform/services/storage/resource_storage_bucket.go.erb @@ -131,6 +131,12 @@ func ResourceStorageBucket() *schema.Resource { Description: `The ID of the project in which the resource belongs. If it is not provided, the provider project is used.`, }, + "project_number": { + Type: schema.TypeInt, + Computed: true, + Description: `The project number of the project in which the resource belongs.`, + }, + "self_link": { Type: schema.TypeString, Computed: true, @@ -1718,6 +1724,9 @@ func setStorageBucket(d *schema.ResourceData, config *transport_tpg.Config, res if err := d.Set("url", fmt.Sprintf("gs://%s", bucket)); err != nil { return fmt.Errorf("Error setting url: %s", err) } + if err := d.Set("project_number", res.ProjectNumber); err != nil { + return fmt.Errorf("Error setting project_number: %s", err) + } if err := d.Set("storage_class", res.StorageClass); err != nil { return fmt.Errorf("Error setting storage_class: %s", err) } diff --git a/mmv1/third_party/terraform/services/storage/resource_storage_bucket_test.go.erb b/mmv1/third_party/terraform/services/storage/resource_storage_bucket_test.go.erb index 22ef2baecef6..74bd5a9dc2ad 100644 --- a/mmv1/third_party/terraform/services/storage/resource_storage_bucket_test.go.erb +++ b/mmv1/third_party/terraform/services/storage/resource_storage_bucket_test.go.erb @@ -34,6 +34,10 @@ func TestAccStorageBucket_basic(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "google_storage_bucket.bucket", "force_destroy", "false"), + resource.TestCheckResourceAttr( + "google_storage_bucket.bucket", "project", envvar.GetTestProjectFromEnv()), + resource.TestCheckResourceAttrSet( + "google_storage_bucket.bucket", "project_number"), ), }, { From 2f711a104d6ca766656c37057b9a2c927f107430 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Thu, 28 Mar 2024 12:11:25 +0000 Subject: [PATCH 2/4] Let `project` argument be set on google_storage_bucket data source This does not impact the data source retrieving data about the bucket. Providing this argument only serves to prevent a call to the Compute API to populate that field after reading the bucket's data. Storage API only returns the project number, so Compute API is used to get the project id in the absence of a user-supplied value. --- .../services/storage/data_source_google_storage_bucket.go | 1 + 1 file changed, 1 insertion(+) diff --git a/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket.go b/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket.go index ed70c002f8cf..12e8e385d274 100644 --- a/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket.go +++ b/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket.go @@ -12,6 +12,7 @@ func DataSourceGoogleStorageBucket() *schema.Resource { dsSchema := tpgresource.DatasourceSchemaFromResourceSchema(ResourceStorageBucket().Schema) + tpgresource.AddOptionalFieldsToSchema(dsSchema, "project") tpgresource.AddRequiredFieldsToSchema(dsSchema, "name") return &schema.Resource{ From dc05979ad33ea4f385607104c1d9aeb7e6fc58f2 Mon Sep 17 00:00:00 2001 From: Sarah French Date: Thu, 28 Mar 2024 12:12:26 +0000 Subject: [PATCH 3/4] Add acceptance test for google_storage_bucket data source to show the project argument can be provided and is non-functional (therefore can be set to "foobar" without negatice impact) --- .../data_source_google_storage_bucket_test.go | 78 +++++++++++++++++-- 1 file changed, 71 insertions(+), 7 deletions(-) diff --git a/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket_test.go b/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket_test.go index 356a2d1d59cd..4628f6f002b9 100644 --- a/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket_test.go +++ b/mmv1/third_party/terraform/services/storage/data_source_google_storage_bucket_test.go @@ -1,17 +1,19 @@ package storage_test import ( - "fmt" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-provider-google/google/acctest" + "github.com/hashicorp/terraform-provider-google/google/envvar" ) func TestAccDataSourceGoogleStorageBucket_basic(t *testing.T) { t.Parallel() - bucket := "tf-bucket-" + acctest.RandString(t, 10) + context := map[string]interface{}{ + "bucket_name": "tf-bucket-" + acctest.RandString(t, 10), + } acctest.VcrTest(t, resource.TestCase{ PreCheck: func() { acctest.AccTestPreCheck(t) }, @@ -19,7 +21,7 @@ func TestAccDataSourceGoogleStorageBucket_basic(t *testing.T) { CheckDestroy: testAccStorageBucketDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccDataSourceGoogleStorageBucketConfig(bucket), + Config: testAccDataSourceGoogleStorageBucketConfig(context), Check: resource.ComposeTestCheckFunc( acctest.CheckDataSourceStateMatchesResourceStateWithIgnores("data.google_storage_bucket.bar", "google_storage_bucket.foo", map[string]struct{}{"force_destroy": {}}), ), @@ -28,18 +30,80 @@ func TestAccDataSourceGoogleStorageBucket_basic(t *testing.T) { }) } -func testAccDataSourceGoogleStorageBucketConfig(bucketName string) string { - return fmt.Sprintf(` +// Test that the data source can take a project argument, which is used as a way to avoid using Compute API to +// get project id for the project number returned from the Storage API. +func TestAccDataSourceGoogleStorageBucket_avoidComputeAPI(t *testing.T) { + // Cannot use t.Parallel() if using t.Setenv + + project := envvar.GetTestProjectFromEnv() + + context := map[string]interface{}{ + "bucket_name": "tf-bucket-" + acctest.RandString(t, 10), + "real_project_id": project, + "incorrect_project_id": "foobar", + } + + // Unset ENV so no provider default is available to the data source + t.Setenv("GOOGLE_PROJECT", "") + + acctest.VcrTest(t, resource.TestCase{ + // Removed PreCheck because it wants to enforce GOOGLE_PROJECT being set + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t), + CheckDestroy: testAccStorageBucketDestroyProducer(t), + Steps: []resource.TestStep{ + { + Config: testAccDataSourceGoogleStorageBucketConfig_setProjectInConfig(context), + Check: resource.ComposeTestCheckFunc( + // We ignore project to show that the project argument on the data source is retained and isn't impacted + acctest.CheckDataSourceStateMatchesResourceStateWithIgnores("data.google_storage_bucket.bar", "google_storage_bucket.foo", map[string]struct{}{"force_destroy": {}, "project": {}}), + + resource.TestCheckResourceAttrSet( + "google_storage_bucket.foo", "project_number"), + resource.TestCheckResourceAttr( + "google_storage_bucket.foo", "project", context["real_project_id"].(string)), + + resource.TestCheckResourceAttrSet( + "data.google_storage_bucket.bar", "project_number"), + resource.TestCheckResourceAttr( + "data.google_storage_bucket.bar", "project", context["incorrect_project_id"].(string)), + ), + }, + }, + }) +} + +func testAccDataSourceGoogleStorageBucketConfig(context map[string]interface{}) string { + return acctest.Nprintf(` +resource "google_storage_bucket" "foo" { + name = "%{bucket_name}" + location = "US" +} + +data "google_storage_bucket" "bar" { + name = google_storage_bucket.foo.name + depends_on = [ + google_storage_bucket.foo, + ] +} +`, context) +} + +func testAccDataSourceGoogleStorageBucketConfig_setProjectInConfig(context map[string]interface{}) string { + return acctest.Nprintf(` resource "google_storage_bucket" "foo" { - name = "%s" + project = "%{real_project_id}" + name = "%{bucket_name}" location = "US" } +// The project argument here doesn't help the provider retrieve data about the bucket +// It only serves to stop the data source using the compute API to convert the project number to an id data "google_storage_bucket" "bar" { + project = "%{incorrect_project_id}" name = google_storage_bucket.foo.name depends_on = [ google_storage_bucket.foo, ] } -`, bucketName) +`, context) } From 0f011928f737bea857c0efb693e82079f309da71 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Thu, 28 Mar 2024 18:48:49 +0000 Subject: [PATCH 4/4] Update `google_storage_bucket` data source docs with new `project` argument --- .../terraform/website/docs/d/storage_bucket.html.markdown | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mmv1/third_party/terraform/website/docs/d/storage_bucket.html.markdown b/mmv1/third_party/terraform/website/docs/d/storage_bucket.html.markdown index a6e88ab6207d..18fb11bde3f2 100644 --- a/mmv1/third_party/terraform/website/docs/d/storage_bucket.html.markdown +++ b/mmv1/third_party/terraform/website/docs/d/storage_bucket.html.markdown @@ -26,6 +26,8 @@ The following arguments are supported: * `name` - (Required) The name of the bucket. +* `project` - (Optional) The ID of the project in which the resource belongs. If it is not provided, the provider project is used. If no value is supplied in the configuration or through provider defaults then the data source will use the Compute API to find the project id that corresponds to the project number returned from the Storage API. Supplying a value for `project` doesn't influence retrieving data about the bucket but it can be used to prevent use of the Compute API. If you do provide a `project` value ensure that it is the correct value for that bucket; the data source will not check that the project id and project number match. + ## Attributes Reference See [google_storage_bucket](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/storage_bucket#argument-reference) resource for details of the available attributes.