Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some KMS ressources created in the wrong project #4828

Closed
SkYNewZ opened this issue Nov 6, 2019 · 6 comments
Closed

Some KMS ressources created in the wrong project #4828

SkYNewZ opened this issue Nov 6, 2019 · 6 comments

Comments

@SkYNewZ
Copy link

SkYNewZ commented Nov 6, 2019

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
  • If an issue is assigned to the "modular-magician" user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to "hashibot", a community member has claimed the issue already.

Terraform Version

➜ terraform -v
Terraform v0.12.13

Affected Resource(s)

  • google_kms_crypto_key
  • google_kms_secret_ciphertext

Terraform Configuration Files

---
resource "google_kms_key_ring" "keyring" {
  project = var.project_id

  name       = format("%s-%s", var.kms_keyring_name, random_string.keyring.result)
  location   = var.kms_keyring_location
  depends_on = [google_project_service.kms_service_service_acccount_project] # —> Cloud KMS activation
}

resource "google_kms_crypto_key" "key" {
  name     = var.kms_crypto_key_name
  key_ring = google_kms_key_ring.keyring.self_link
  purpose  = "ENCRYPT_DECRYPT"

  labels = {
    creator = var.label
  }

  depends_on = [google_project_service.kms_service_service_acccount_project] # —> Cloud KMS activation
}

data "google_kms_secret_ciphertext" "citadelle_token" {
  crypto_key = google_kms_crypto_key.key.self_link
  plaintext  = var.token
  depends_on = [google_project_service.kms_service_service_acccount_project] # —> Cloud KMS activation
}
---

Expected Behavior

These Terraform files should create a google_kms_secret_ciphertext and google_kms_crypto_key in the given var.project_id.

Actual Behavior

⚠️ Here, given GOOGLE_CREDENTIALS are a project service account with full rights on all organization (in order to create projects).

google_kms_secret_ciphertext and google_kms_crypto_key are created in the service account's project… We will see this with a Terraform error which tell us a wrong project number.

We also had this issue on the google_kms_key_ring ressource —> fixed with user_project_override = true in provider.tf. Nevertheless, it's not fixed the google_kms_secret_ciphertext and google_kms_crypto_key creation issue.

Steps to Reproduce

  • Create a project and a owner service account
  • Give it full rights on ORG
  • Use this code with a brand new created project, and its project_id in var.project_id.

Important Factoids

Regarding the terraform-provider-google or terraform-provider-google-beta providers generated by https://github.com/GoogleCloudPlatform/magic-modules, we can see project is not used in theses ressources and user_project_override = true is not available

@jnahelou

@ghost ghost added the bug label Nov 6, 2019
@danawillow danawillow self-assigned this Nov 6, 2019
@danawillow
Copy link
Contributor

Hi @SkYNewZ, I'm taking a look at the code for the resource/datasource and I don't see anything obviously wrong yet. Can you post debug logs (https://www.terraform.io/docs/internals/debugging.html) for a run that failed? That'll help me see the exact requests/responses that were sent to/from the GCP API.

@JordanP
Copy link

JordanP commented Nov 13, 2019

I can confirm this. KMS resources are created in the service account project (the one set in credentials = file("service-account.json") ) and not in the provider project (the one set in project = ...).

EDIT: the only resource that seems to be looked up in the wrong project is

resource "google_project_service" "key_management_system" {
  service            = "cloudkms.googleapis.com"
  disable_on_destroy = false
}

@ghost ghost removed the waiting-response label Nov 13, 2019
@SkYNewZ
Copy link
Author

SkYNewZ commented Nov 13, 2019

Project A has a service account to create KMS resources on project B.
To do that we need X-Goog-User-Project header on each KMS resource requests otherwise project A will require KMS API enabled.

I found that X-Goog-User-Project has been implemented during GoogleCloudPlatform/magic-modules#2145 and require config.UserProjectOverride && project != ""

But currently in KMS resources, project parameter is set to "" : https://github.com/terraform-providers/terraform-provider-google-beta/blob/690235f3e35b31e3f300900a5bc2ac79cd04ed2d/google-beta/resource_kms_crypto_key.go#L152
So this header is not add.

To keep homogeneity between resource, I add the project attribute in Terraform resource schem:

-  has_project = object.base_url.include?('{{project}}')
+  has_project = object.base_url.include?('{{project}}') || object.enable_project_attribute

Here is my draft to do that:
https://github.com/jnahelou/magic-modules/tree/terraform-kms-project

Creation works with patch:
(before)

2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: POST /v1/projects/int-lz1-dtp/locations/europe-west1/keyRings/automatic_citadelle_keyring-rCA6h5pOEjNHfNhLrMe5il5MRVtJxM/cryptoKeys/automatic_citadelle_key:encrypt?alt=json&prettyPrint=false HTTP/1.1
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: Host: cloudkms.googleapis.com
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: User-Agent: google-api-go-client/0.5 HashiCorp Terraform/0.12.10 (+https://www.terraform.io) Terraform Plugin SDK/1.0.0 terraform-provider-google-beta/dev
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: Content-Length: 37
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: Content-Type: application/json
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: X-Goog-Api-Client: gl-go/1.11.0 gdcl/20191007
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: Accept-Encoding: gzip
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: 
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: {
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2:  "plaintext": "Y2l0YWRlbGxlX3Rva2Vu"
2019-11-07T16:58:07.438+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.18.2: }
(after)
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: POST /v1/projects/int-lz1-dtp/locations/europe-west1/keyRings/automatic_citadelle_keyring-rCA6h5pOEjNHfNhLrMe5il5MRVtJxM/cryptoKeys/automatic_citadelle_key:encrypt?alt=json HTTP/1.1
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: Host: cloudkms.googleapis.com
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: User-Agent: HashiCorp Terraform/0.12.10 (+https://www.terraform.io) Terraform Plugin SDK/1.0.0 terraform-provider-google-beta/dev
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: Content-Length: 37
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: Content-Type: application/json
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: X-Goog-User-Project: int-lz1-dtp
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: Accept-Encoding: gzip
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: 
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: {
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0:  "plaintext": "XXXX"
2019-11-07T17:31:33.582+0100 [DEBUG] plugin.terraform-provider-google-beta_v2.19.0: }

But the upgrade from existing resource fail because version 1 don't have project_id attribute.

google_kms_crypto_key.key: Refreshing state... [id=projects/int-lz1-dtp/locations/europe-west1/keyRings/automatic_citadelle_keyring-rCA6h5pOEjNHfNhLrMe5il5MRVtJxM/cryptoKeys/automatic_citadelle_key]
2019/11/07 17:19:56 [ERROR] <root>: eval: *terraform.EvalRefresh, err: project: required field is not set
2019/11/07 17:19:56 [ERROR] <root>: eval: *terraform.EvalSequence, err: project: required field is not set

@danawillow
Copy link
Contributor

Cool, that makes sense. I think we can fix this without adding additional fields (either to magic modules itself or to a resource). I'll be able to look at this again next week (maybe as early as Friday of this week), or if you have a PR you'd like reviewed before then, feel free to tag me to review.

@danawillow
Copy link
Contributor

It sounds like there are two separate issues being discussed in this bug, and I want to make sure both of them get resolved.

Issue 1 is that certain KMS resources are being created in the project where the service account lives, instead of the one defined in the Terraform config. I can't resolve this issue until I've seen debug logs (https://www.terraform.io/docs/internals/debugging.html) for a failed run. It would help to have the config, plan output, and debug logs together in one gist. Feel free to obfuscate any information you don't feel comfortable sharing publicly, as long as I can tell the difference between the two projects in question.

Issue 2 is that the KMS resources / data sources without project attributes don't support user_project_override, which means that it's asking you to enable the APIs on the wrong project. I can get started on a fix for this right now that won't involve adding any additional fields to the resources / data sources.

@SkYNewZ and @JordanP, can you confirm that my explanation of the two issues sounds correct, and if so, post the debug logs in question when you have a chance?

@ghost
Copy link

ghost commented May 13, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators May 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants