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

[Fix] Failed to recreate APigee organisation in the same Project #156

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mhdjomaa3450
Copy link

@mhdjomaa3450 mhdjomaa3450 commented Sep 2, 2024

Once an Apigee organization is created in a specific project, two resources (Key Rings) are also created within that project. These resources are managed by the apigee-x-core module, specifically the kms-org-db and kms-inst-disk . However, when you destroy the environment using the terraform destroy command, these two resources are not deleted permanently

KeyRings cannot be deleted from Google Cloud Platform. Destroying a Terraform-managed KeyRing will remove it from state but will not delete the resource from the project. for more details pls check out this link: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/kms_key_ring

Delete an Apigee organization.

For organizations with BillingType EVALUATION, an immediate deletion is performed. For paid organizations (Subscription or Pay-as-you-go), a soft-deletion is performed. The organization can be restored within the soft-deletion period, which is specified using the retention field in the request or by filing a support ticket with Apigee. During the data retention period specified in the request, the Apigee organization cannot be recreated in the same Google Cloud project.
The default data retention setting for this operation is 7 days. To permanently delete the organization in 24 hours, set the retention parameter to MINIMUM. https://cloud.google.com/apigee/docs/reference/apis/apigee/rest/v1/organizations/delete#deletionretention

When running terraform apply again to recreate the apigee in the same project , you might encounter the following error:

Error: Error creating KeyRing: googleapi: Error 409: KeyRing projects/bespin-apigee-test-2-433209/locations/me-central2/keyRings/apigee-instance already exists.
with module.apigee-x-core.module.kms-inst-disk["instance"].google_kms_key_ring.default[0], on .terraform/modules/apigee-x-core.kms-inst-disk/modules/kms/main.tf line 32, in resource "google_kms_key_ring" "default":
32: resource "google_kms_key_ring" "default" {

Error: Error creating KeyRing: googleapi: Error 409: KeyRing projects/bespin-apigee-test-2-433209/locations/me-central2/keyRings/apigee-x-org already exists.
with module.apigee-x-core.module.kms-org-db.google_kms_key_ring.default[0],
on .terraform/modules/apigee-x-core.kms-org-db/modules/kms/main.tf line 32, in resource "google_kms_key_ring" "default":
32: resource "google_kms_key_ring" "default" {

The solution to remove the project and create the Apigee in a new project is not considered best practice. To address this issue, We can add a random_string resource and use it as a postfix in the names of these two resources as shown below.

resource "random_string" "key_random_suffix" {
  length  = 6
  special = false
}


module "kms-org-db" {
  source     = "github.com/terraform-google-modules/cloud-foundation-fabric//modules/kms?ref=v28.0.0"
  project_id = var.project_id
  iam = {
    "roles/cloudkms.cryptoKeyEncrypterDecrypter" = ["serviceAccount:${google_project_service_identity.apigee_sa.email}"]
  }
  keyring = {
    location = coalesce(var.org_kms_keyring_location, var.ax_region)
    # name     = var.org_kms_keyring_name --> delete this line
     name     = "${var.org_kms_keyring_name}-${random_string.key_random_suffix.result}"
  }
  keyring_create = var.org_kms_keyring_create
  keys = {
    org-db = { rotation_period = var.org_key_rotation_period, labels = null }
  }
}

module "kms-inst-disk" {
  for_each   = var.apigee_instances
  source     = "github.com/terraform-google-modules/cloud-foundation-fabric//modules/kms?ref=v28.0.0"
  project_id = var.project_id
  iam = {
    "roles/cloudkms.cryptoKeyEncrypterDecrypter" = ["serviceAccount:${google_project_service_identity.apigee_sa.email}"]
  }
  keyring = {
    location = coalesce(each.value.keyring_location, each.value.region)
    name     = "${coalesce(each.value.keyring_name, "apigee-${each.key}")}-${random_string.key_random_suffix.result}"
   # name     = coalesce(each.value.keyring_name, "apigee-${each.key}") --> delete this line
  }
  keyring_create = each.value.keyring_create
  keys = {
    (each.value.key_name) = {
      rotation_period = each.value.key_rotation_period
      labels          = each.value.key_labels
    }
  }
}

[Fix] Failed to recreate APigee organisation in the same Project
apigee#155
Copy link

google-cla bot commented Sep 2, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@mhdjomaa3450 mhdjomaa3450 reopened this Sep 2, 2024
@danistrebel
Copy link
Collaborator

danistrebel commented Sep 3, 2024

Many thanks @mhdjomaa3450 for this great PR and the detailed description.

@g-greatdevaks and/or I will take a look.

If we commit on the approach the following actions would have to be taken before merging:

  • Re-generate the docs via ./tools/update-docs.sh
  • Fix the tests resource counts to include the random string resource (+=1 on pretty much all counts) as detailed in the test output

The Conventional Commits Lint Check can be ignored. We use Conventional commits to generate release notes but we can do this manually.

@mhdjomaa3450
Copy link
Author

mhdjomaa3450 commented Sep 4, 2024

@danistrebel @g-greatdevaks , Hi can you please review it.

@g-greatdevaks
Copy link
Collaborator

g-greatdevaks commented Sep 4, 2024

Let me review and do the needful, shortly.
There seems to be some checks that failed.

@g-greatdevaks
Copy link
Collaborator

g-greatdevaks commented Sep 5, 2024

Hey @mhdjomaa3450, thanks for the great PR and for explaining the issue in an elaborate manner.

As far as recreation of the Apigee Organization in the same GCP Project is concerned, there are two provisions which are already present as part of the existing apigee-x-core Terraform Module, as stated below:

  1. For Apigee Organization specific KMS Key Ring, we have one variable exposed, named org_kms_keyring_name, which can be fed via terraform.tfvars, as shown here. Whenever recreating an Apigee Organization, we can have the value of the variable changed to avoid hitting the error (Error creating KeyRing: googleapi: Error 409) stated above.
  2. For Apigee X Runtime Instances, we have the names of the keys present as part of the apigee_instances map taken for naming the KMS Key Ring (respectively for each region). This means that when the Apigee X Runtime Instances are to be recreated, regardless of whether the recreation is part of the Apigee X Organization recreation operation or is ran independently at Apigee X Runtime Instance level, one can change the name of the key(s) corresponding to the region(s) for which the Apigee X Runtime Instance(s) are to be recreated.

We can still go ahead and abstract this dependency of the user needing to update the said names.
However, in case a user needs to perform some operations in future by referencing these KMS Key Ring names, the user, if using Terraform (which is highly unlikely for the use-cases that I have described next; gcloud or Google APIs would be better for such operations) will need to make use of Terraform Data Sources or expose the names of the KMS Key Rings in the output of Terraform. I don't see such a requirement coming that frequently, though there can be some use-cases (for governance purposes) to disable or destroy old versions of a KMS Key corresponding to a KMS Key Ring.

@mhdjomaa3450 Kindly let me know your thoughts.

cc: @danistrebel

@mhdjomaa3450
Copy link
Author

@g-greatdevaks Hi, thanks for your reply! I completely agree with you. However, in my case, I aimed to make the process more automated for myself, so I thought it would be helpful to contribute it to the community. As you mentioned, if we proceed with this, we would still need to utilize Terraform Data Sources or expose the KMS Key Ring names in the Terraform output.

@danistrebel
Copy link
Collaborator

Thanks @g-greatdevaks great points! I'd tend to lean towards keeping the current state and mentioning the fact that upon re-creation the key ring names would have to be overridden in the module docs / variable description.

If we decide to go ahead i'd like to keep the ability to control the org keyname (i.e. via org_kms_keyring_name) without randomness such that users can use imports to migrate existing manual setups to TF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants