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

App Engine design around removal is sub-optimal #1973

Closed
sjungwirth opened this issue Aug 31, 2018 · 5 comments
Closed

App Engine design around removal is sub-optimal #1973

sjungwirth opened this issue Aug 31, 2018 · 5 comments

Comments

@sjungwirth
Copy link
Contributor

I know this issue was raised previously in #1561, as others encountered this with the rollout of app engine support, but I believe the current behavior is simply broken.

Recreating the project in order to "remove" app engine will not actually work and simply creates a broken TF project. Projects don't really get deleted from GCP for maybe ~30 days, so when TF "destroys" a project it just gets moved to a pending deletion status, and you can't create a new project with the same ID, because the old project with that same ID still exists.

From @paddycarver in #1561 (comment)

I could mark App Engine as computed, so it wouldn't treat it missing from your config as a diff, and would just accept the API response as fine.

This seems like the most reasonable action to me, although I agree it is not ideal, just that the other options make even less sense.

Terraform Version

Terraform v0.11.8

Affected Resource(s)

  • google_project

Terraform Configuration Files

provider "google" {
  region  = "us-central1"
  version = "~> 1.15"
}

resource "google_project" "project" {
  name            = "App Engine Test"
  project_id      = "project-recreate-bug-ae-test"
}

Debug Output

https://gist.github.com/sjungwirth/e9907cc4603ed5041bba0cd7e770148b

Expected Behavior

Terraform should not attempt to destroy and re-create the project.

Actual Behavior

Applying the plan fails, because a google project is only soft deleted (pending deletion status), and can't be immediately recreated because a project with the same ID already exists.

...

Error: Error applying plan:

1 error(s) occurred:

* google_project.project: 1 error(s) occurred:

* google_project.project: error creating project xxxxxxxxx (App Engine Test): googleapi: Error 409: Requested entity already exists, alreadyExists. If you received a 403 error, make sure you have the `roles/resourcemanager.projectCreator` permission

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform apply
  2. Enable app engine through GCP console
  3. terraform apply
@paddycarver paddycarver changed the title TF tries to destroy/recreate projects if GAE is enabled but not in TF config App Engine design around removal is sub-optimal Aug 31, 2018
@paddycarver
Copy link
Contributor

Hi @sjungwirth, thanks for opening the new issue! Happy to discuss the design of this.

There's definitely a trade-off being made here. I'm very hesitant to implement the option you called out, because we have no way to message to the user that we didn't actually delete their app, which is alarming, because Terraform will say that it did. The accuracy of Terraform's plans is something I think is super important.

Recreating the project in order to "remove" app engine will not actually work and simply creates a broken TF project.

Could you expand on this for me? I understand the deletion period, and the ID-in-use-error problem (my less-than-ideal recommendation is to change the ID of the project when plan tells you it's going to be re-created, but that's kind of a gross workaround) but I don't understand how that leads to:

  • Not removing the project? The project is removed, or at least as far as the API is concerned. It can be restored for 30 days, and its ID isn't usable, yes, but that's just what "removed" means in terms of the project. I agree it's not a great definition, but we gotta work with the APIs we're given.
  • Creating a broken TF project? There shouldn't be a new TF project created, and the old project should be pending deletion, not broken. Is something else happening?

I'd love a little more elaboration on what those mean.

I'm also exploring additional designs for App Engine, to weigh some of the trade-offs being made here, so people can choose the trade-offs that make sense for them. One option I'm considering is a standalone resource, with a acknowledge_no_delete (or similar, name to be bikesheded) field that must be set to true for it to be able to be deleted. I'm also holding out hope we get a resolution to the upstream issue that will allow us to delete an app without deleting a project, which would also solve this for us.

@sjungwirth
Copy link
Contributor Author

Maybe I misunderstood the idea of marking App Engine as computed, but I guess what I would suggest is that terraform ignore or doesn't complain if the project has app engine enabled but it isn't in the TF config. Yes this means you might think app engine was removed if you had it in TF configuration previously and removed it, but that really isn't possible on GCP currently anyway.

Not removing the project?

Sure, the project is "removed" in any sense that TF can tell, sorry if I wasn't clear; but we know that any attempt to destroy and re-create a project with the same id simply won't work.

Creating a broken TF project?

After proceeding with the TF plan to destroy and re-create the project, I am left in an unrecoverable state. In order to continue to be able to use TF with my current configurations I would need to cancel the scheduled project deletion in GCP console, then use terraform import on the project (and any other resources I had previously) and update the project config to have the app_engine configuration.

my less-than-ideal recommendation is to change the ID of the project

Maybe this is fine for some projects, but we have many other integrations that reference a project id outside of TF, so not really an option in other cases.

I guess the core of what I'm saying is that because it is not possible to disable app engine once it has been enabled for a project, TF shouldn't try to do so either; as even if I try to go through with the TF plan to re-create my project, it won't work and leaves me in a totally unrecoverable state.

@paddycarver
Copy link
Contributor

I am left in an unrecoverable state.

What does "unrecoverable" mean as you're using it here? I think I'm missing something about the state you're ending in, because as far as I know, you should end up with the project removed from state and pending deletion. I'm not clear on what "recovery" means in that situation.

Yes this means you might think app engine was removed if you had it in TF configuration previously and removed it, but that really isn't possible on GCP currently anyway.

Right, but Terraform is supposed to do what it says in the plan. If the plan says that it is going to remove an app, and it says it did so successfully, but the app is still running, potentially costing people money or compromising their security posture, that's a bug in Terraform's behavior, because it didn't do what it said it did. Conversely, if it says it's going to delete and recreate a project, then proceeds to do so, and runs into an error recreating the project--which, I agree, isn't great, and I'm definitely thinking on how else we could do this--it at least did exactly what it said it was going to, and when it couldn't do what it said it would, it told the user that it couldn't, and why. Which lets the user make an informed choice at every step: do I want to delete my whole project? Ok, I can't recreate with that ID. Should I change my ID, or should I undo the deletion and re-import the project?

I think we're getting into the weeds here, though: what are you trying to do that you can't, right now? I understand the change you want, i think, I just don't understand what it will enable you to do that you can't achieve currently.

@paddycarver
Copy link
Contributor

The 1.19.0 release provided a new app_engine_application resource, so I think we can call this one closed. Feel free to comment if I'm wrong.

@ghost
Copy link

ghost commented Nov 17, 2018

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 Nov 17, 2018
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