allow resources with an indirect project to use user project override…

…s, enable for kms cryptokey (#2737)

Merged PR #2737.

overrides/terraform/resource_override.rb

@@ -63,7 +63,11 @@ def self.attributes
           # An array of function names that determine whether an error is retryable.
           :error_retry_predicates,
 
-          :schema_version
+          :schema_version,
+
+          # This enables resources that get their project via a reference to a different resource
+          # instead of a project field to use User Project Overrides
+          :supports_indirect_user_project_override
         ]
       end
 
@@ -93,6 +97,7 @@ def validate
         check :timeouts, type: Api::Timeouts
         check :error_retry_predicates, type: Array, item_type: String
         check :schema_version, type: Integer
+        check :supports_indirect_user_project_override, type: :boolean, default: false
       end
 
       def apply(resource)

products/kms/terraform.yaml

@@ -59,6 +59,7 @@ overrides: !ruby/object:Overrides::ResourceOverrides
       to the resource to prevent accidental destruction.
     id_format: "{{key_ring}}/cryptoKeys/{{name}}"
     import_format: ["{{key_ring}}/cryptoKeys/{{name}}"]
+    supports_indirect_user_project_override: true
     schema_version: 1
     examples:
       - !ruby/object:Provider::Terraform::Examples

templates/terraform/resource.erb

@@ -185,7 +185,13 @@ func resource<%= resource_name -%>Create(d *schema.ResourceData, meta interface{
         return err
     }
 <%  end -%>
-    res, err := sendRequestWithTimeout(config, "<%= object.create_verb.to_s.upcase -%>", <% if has_project %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutCreate) <%= object.error_retry_predicates ? ", " + object.error_retry_predicates.join(',') : "" -%>)
+<%  if object.supports_indirect_user_project_override -%>
+    var project string
+    if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
+        project = parts[1]
+    }
+<%  end -%>
+    res, err := sendRequestWithTimeout(config, "<%= object.create_verb.to_s.upcase -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutCreate) <%= object.error_retry_predicates ? ", " + object.error_retry_predicates.join(',') : "" -%>)
     if err != nil {
         return fmt.Errorf("Error creating <%= object.name -%>: %s", err)
     }
@@ -230,7 +236,13 @@ func resource<%= resource_name -%>Read(d *schema.ResourceData, meta interface{})
         return err
     }
 <%  end -%>
-    res, err := sendRequest(config, "<%= object.read_verb.to_s.upcase -%>", <% if has_project %>project<% else %>""<% end %>, url, nil)
+<%  if object.supports_indirect_user_project_override -%>
+    var project string
+    if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
+        project = parts[1]
+    }
+<%  end -%>
+    res, err := sendRequest(config, "<%= object.read_verb.to_s.upcase -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, url, nil)
     if err != nil {
         return handleNotFoundError(err, d, fmt.Sprintf("<%= resource_name -%> %q", d.Id()))
     }
@@ -345,7 +357,13 @@ if <%= props.map { |prop| "d.HasChange(\"#{prop.name.underscore}\")" }.join ' ||
             return err
         }
 <%         end -%>
-        getRes, err := sendRequest(config, "<%= object.read_verb.to_s.upcase -%>", <% if has_project %>project<% else %>""<% end %>, getUrl, nil)
+<%         if object.supports_indirect_user_project_override -%>
+        var project string
+        if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
+            project = parts[1]
+        }
+<%         end -%>
+        getRes, err := sendRequest(config, "<%= object.read_verb.to_s.upcase -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, getUrl, nil)
         if err != nil {
             return handleNotFoundError(err, d, fmt.Sprintf("<%= resource_name -%> %q", d.Id()))
         }
@@ -397,10 +415,16 @@ if <%= props.map { |prop| "d.HasChange(\"#{prop.name.underscore}\")" }.join ' ||
         if err != nil {
             return err
         }
+<%      if object.supports_indirect_user_project_override -%>
+        var project string
+        if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
+            project = parts[1]
+        }
+<%      end -%>
 <%      if object.async.nil? -%>
-        _, err = sendRequestWithTimeout(config, "<%= key[:update_verb] -%>", <% if has_project %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
+        _, err = sendRequestWithTimeout(config, "<%= key[:update_verb] -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
 <%      else -%>
-        res, err := sendRequestWithTimeout(config, "<%= key[:update_verb] -%>", <% if has_project %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
+        res, err := sendRequestWithTimeout(config, "<%= key[:update_verb] -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
 <%      end -%>
         if err != nil {
             return fmt.Errorf("Error updating <%= object.name -%> %q: %s", d.Id(), err)
@@ -478,10 +502,16 @@ if <%= props.map { |prop| "d.HasChange(\"#{prop.name.underscore}\")" }.join ' ||
         return err
     }
 <%  end -%>
+<%  if object.supports_indirect_user_project_override -%>
+    var project string
+    if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
+        project = parts[1]
+    }
+<%  end -%>
 <%  if object.async.nil? -%>
-    _, err = sendRequestWithTimeout(config, "<%= object.update_verb -%>", <% if has_project %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
+    _, err = sendRequestWithTimeout(config, "<%= object.update_verb -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
 <%  else -%>
-    res, err := sendRequestWithTimeout(config, "<%= object.update_verb -%>", <% if has_project %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
+    res, err := sendRequestWithTimeout(config, "<%= object.update_verb -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutUpdate))
 <% end -%>
 
     if err != nil {
@@ -541,10 +571,16 @@ func resource<%= resource_name -%>Delete(d *schema.ResourceData, meta interface{
     if err != nil {
         return handleNotFoundError(err, d, "<%= object.name -%>")
     }
+<%  end -%>
+<%  if object.supports_indirect_user_project_override -%>
+    var project string
+    if parts := regexp.MustCompile(`projects\/([^\/]+)\/`).FindStringSubmatch(url); parts != nil {
+        project = parts[1]
+    }
 <%  end -%>
     log.Printf("[DEBUG] Deleting <%= object.name -%> %q", d.Id())
 
-    res, err := sendRequestWithTimeout(config, "<%= object.delete_verb.to_s.upcase -%>", <% if has_project %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutDelete))
+    res, err := sendRequestWithTimeout(config, "<%= object.delete_verb.to_s.upcase -%>", <% if has_project || object.supports_indirect_user_project_override %>project<% else %>""<% end %>, url, obj, d.Timeout(schema.TimeoutDelete))
     if err != nil {
         return handleNotFoundError(err, d, "<%= object.name -%>")
     }

templates/terraform/resource.html.markdown.erb

@@ -183,7 +183,7 @@ $ terraform import <% if object.min_version.name == 'beta' %>-provider=google-be
 -> If you're importing a resource with beta features, make sure to include `-provider=google-beta`
 as an argument so that Terraform uses the correct provider to import your resource.
 
-<% if object.base_url.include?("{{project}}")-%>
+<% if object.base_url.include?("{{project}}") || object.supports_indirect_user_project_override -%>
 ## User Project Overrides
 
 This resource supports [User Project Overrides](https://www.terraform.io/docs/providers/google/guides/provider_reference.html#user_project_override).

third_party/terraform/utils/kms_utils.go

@@ -177,15 +177,23 @@ func parseKmsCryptoKeyId(id string, config *Config) (*kmsCryptoKeyId, error) {
 func clearCryptoKeyVersions(cryptoKeyId *kmsCryptoKeyId, config *Config) error {
 	versionsClient := config.clientKms.Projects.Locations.KeyRings.CryptoKeys.CryptoKeyVersions
 
-	versionsResponse, err := versionsClient.List(cryptoKeyId.cryptoKeyId()).Do()
+	listCall := versionsClient.List(cryptoKeyId.cryptoKeyId())
+	if config.UserProjectOverride {
+		listCall.Header().Set("X-Goog-User-Project", cryptoKeyId.KeyRingId.Project)
+	}
+	versionsResponse, err := listCall.Do()
 
 	if err != nil {
 		return err
 	}
 
 	for _, version := range versionsResponse.CryptoKeyVersions {
 		request := &cloudkms.DestroyCryptoKeyVersionRequest{}
-		_, err = versionsClient.Destroy(version.Name, request).Do()
+		destroyCall := versionsClient.Destroy(version.Name, request)
+		if config.UserProjectOverride {
+			destroyCall.Header().Set("X-Goog-User-Project", cryptoKeyId.KeyRingId.Project)
+		}
+		_, err = destroyCall.Do()
 
 		if err != nil {
 			return err
@@ -197,10 +205,14 @@ func clearCryptoKeyVersions(cryptoKeyId *kmsCryptoKeyId, config *Config) error {
 
 func disableCryptoKeyRotation(cryptoKeyId *kmsCryptoKeyId, config *Config) error {
 	keyClient := config.clientKms.Projects.Locations.KeyRings.CryptoKeys
-	_, err := keyClient.Patch(cryptoKeyId.cryptoKeyId(), &cloudkms.CryptoKey{
+	patchCall := keyClient.Patch(cryptoKeyId.cryptoKeyId(), &cloudkms.CryptoKey{
 		NullFields: []string{"rotationPeriod", "nextRotationTime"},
 	}).
-		UpdateMask("rotationPeriod,nextRotationTime").Do()
+		UpdateMask("rotationPeriod,nextRotationTime")
+	if config.UserProjectOverride {
+		patchCall.Header().Set("X-Goog-User-Project", cryptoKeyId.KeyRingId.Project)
+	}
+	_, err := patchCall.Do()
 
 	return err
 }

third_party/terraform/utils/provider_test.go.erb

@@ -248,6 +248,49 @@ func TestAccProviderUserProjectOverride(t *testing.T) {
 	})
 }
 
+// Do the same thing as TestAccProviderUserProjectOverride, but using a resource that gets its project via
+// a reference to a different resource instead of a project field.
+func TestAccProviderIndirectUserProjectOverride(t *testing.T) {
+	t.Parallel()
+
+	org := getTestOrgFromEnv(t)
+	billing := getTestBillingAccountFromEnv(t)
+	pid := "terraform-" + acctest.RandString(10)
+	sa := "terraform-" + acctest.RandString(10)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		// No TestDestroy since that's not really the point of this test
+		Steps: []resource.TestStep{
+			{
+				Config: testAccProviderIndirectUserProjectOverride(pid, pname, org, billing, sa),
+				Check: func(s *terraform.State) error {
+					// The token creator IAM API call returns success long before the policy is
+					// actually usable. Wait a solid 2 minutes to ensure we can use it.
+					time.Sleep(2 * time.Minute)
+					return nil
+				},
+			},
+			{
+				Config:      testAccProviderIndirectUserProjectOverride_step2(pid, pname, org, billing, sa, false),
+				ExpectError: regexp.MustCompile(`Cloud Key Management Service \(KMS\) API has not been used`),
+			},
+			{
+				Config: testAccProviderIndirectUserProjectOverride_step2(pid, pname, org, billing, sa, true),
+			},
+			{
+				ResourceName:      "google_kms_crypto_key.project-2-key",
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccProviderIndirectUserProjectOverride_step3(pid, pname, org, billing, sa, true),
+			},
+		},
+	})
+}
+
 func testAccProviderBasePath_setBasePath(endpoint, name string) string {
 	return fmt.Sprintf(`
 provider "google" {
@@ -361,6 +404,109 @@ provider "google" {
 `, testAccProviderUserProjectOverride(pid, name, org, billing, sa), override)
 }
 
+// Set up two projects. Project 1 has a service account that is used to create a
+// kms crypto key in project 2. The kms API is only enabled in project 2,
+// which causes the create to fail unless user_project_override is set to true.
+func testAccProviderIndirectUserProjectOverride(pid, name, org, billing, sa string) string {
+	return fmt.Sprintf(`
+resource "google_project" "project-1" {
+	project_id      = "%s"
+	name            = "%s"
+	org_id          = "%s"
+	billing_account = "%s"
+}
+
+resource "google_service_account" "project-1" {
+	project    = google_project.project-1.project_id
+    account_id = "%s"
+}
+
+resource "google_project" "project-2" {
+	project_id      = "%s-2"
+	name            = "%s-2"
+	org_id          = "%s"
+	billing_account = "%s"
+}
+
+resource "google_project_service" "project-2-kms" {
+	project = google_project.project-2.project_id
+	service = "cloudkms.googleapis.com"
+}
+
+// Permission needed for user_project_override
+resource "google_project_iam_member" "project-2-serviceusage" {
+	project = google_project.project-2.project_id
+	role    = "roles/serviceusage.serviceUsageConsumer"
+	member  = "serviceAccount:${google_service_account.project-1.email}"
+}
+
+resource "google_project_iam_member" "project-2-kms" {
+	project = google_project.project-2.project_id
+	role    = "roles/cloudkms.admin"
+	member  = "serviceAccount:${google_service_account.project-1.email}"
+}
+
+data "google_client_openid_userinfo" "me" {}
+
+// Enable the test runner to get an access token on behalf of
+// the project 1 service account
+resource "google_service_account_iam_member" "token-creator-iam" {
+	service_account_id = google_service_account.project-1.name
+	role               = "roles/iam.serviceAccountTokenCreator"
+	member             = "serviceAccount:${data.google_client_openid_userinfo.me.email}"
+}
+`, pid, name, org, billing, sa, pid, name, org, billing)
+}
+
+func testAccProviderIndirectUserProjectOverride_step2(pid, name, org, billing, sa string, override bool) string {
+	return fmt.Sprintf(`
+// See step 3 below, which is really step 2 minus the kms resources.
+// Step 3 exists because provider configurations can't be removed while objects
+// created by that provider still exist in state. Step 3 will remove the
+// kms resources so the whole config can be deleted.
+%s
+
+resource "google_kms_key_ring" "project-2-keyring" {
+	provider = google.project-1-token
+	project  = google_project.project-2.project_id
+
+	name     = "%s"
+	location = "us-central1"
+}
+
+resource "google_kms_crypto_key" "project-2-key" {
+	provider = google.project-1-token
+	name     = "%s"
+	key_ring = google_kms_key_ring.project-2-keyring.self_link
+}
+`, testAccProviderIndirectUserProjectOverride_step3(pid, name, org, billing, sa, override), pid, pid)
+}
+
+func testAccProviderIndirectUserProjectOverride_step3(pid, name, org, billing, sa string, override bool) string {
+	return fmt.Sprintf(`
+%s
+
+data "google_service_account_access_token" "project-1-token" {
+	// This data source would have a depends_on to
+	// google_service_account_iam_binding.token-creator-iam, but depends_on
+	// in data sources makes them always have a diff in apply:
+	// https://www.terraform.io/docs/configuration/data-sources.html#data-resource-dependencies
+	// Instead, rely on the other test step completing before this one.
+
+	target_service_account = google_service_account.project-1.email
+	scopes                 = ["userinfo-email", "https://www.googleapis.com/auth/cloud-platform"]
+	lifetime               = "300s"
+}
+
+provider "google" {
+	alias = "project-1-token"
+
+	access_token          = data.google_service_account_access_token.project-1-token.access_token
+	user_project_override = %v
+}
+`, testAccProviderIndirectUserProjectOverride(pid, name, org, billing, sa), override)
+}
+
 // getTestRegion has the same logic as the provider's getRegion, to be used in tests.
 func getTestRegion(is *terraform.InstanceState, config *Config) (string, error) {
 	if res, ok := is.Attributes["region"]; ok {