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

Disabled serviceusage.googleapis.com API leads to tainted project #6222

Closed
morgante opened this issue Apr 27, 2020 · 9 comments
Closed

Disabled serviceusage.googleapis.com API leads to tainted project #6222

morgante opened this issue Apr 27, 2020 · 9 comments
Assignees
Labels
bug persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work

Comments

@morgante
Copy link

morgante commented Apr 27, 2020

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 v0.12.24
+ provider.google v3.19.0
+ provider.google-beta v3.19.0
+ provider.null v2.1.2
+ provider.random v2.2.1

Affected Resource(s)

  • google_project

Terraform Configuration Files

From Project Factory:

resource "google_project" "main" {
  name                = var.name
  project_id          = local.temp_project_id
  org_id              = local.project_org_id
  folder_id           = local.project_folder_id
  billing_account     = var.billing_account
  auto_create_network = false

  labels = var.labels

  depends_on = [null_resource.preconditions]
}

Debug Output

https://gist.github.com/morgante/cbb394bcadd6e81d90acd2dc2dab86d5

Expected Behavior

When creating a project with auto_create_network = false, Terraform needs to activate the compute API on the project. This requires the seed project have the serviceusage API active.

I would expect this failure to be recoverable so Terraform bails out early (ie. before creating the project) if the correct APIs are not active similar to #5671.

Actual Behavior

Terraform creates the project, then attempts to delete the network - leading to a tainted project it attempts to recreate (which is impossible because projects are unique).

Steps to Reproduce

  1. Disable the Service Usage APi on your credential project
  2. terraform apply and observe failure
  3. terraform plan and observe tainted project

References

@ghost ghost added the bug label Apr 27, 2020
@morgante
Copy link
Author

morgante commented Apr 27, 2020

I've observed a fair number of issues and failures can happen during the network deletion, which is potentially a source of pain for users. While the simplest fix would be to add a precheck (similar to #5671), I wonder if we should instead split the network deletion into a separate resource.

My reasoning is that failures in the google_project resource are very expensive as they taint the project and projects cannot easily be recreated. Splitting deletion into a separate resource would move it into a safer place where failures are less catastrophic. It doesn't necessarily match Terraform's theoretical model but in practice it would be more resilient. @danawillow Thoughts?

For example

resource "google_default_network" "main" {
  project_id  = local.temp_project_id
  delete        = true
}

@edwardmedia
Copy link
Contributor

Yes, this is an issue. It does prompt project replacement

  # google_project.main is tainted, so must be **replaced**
-/+ resource "google_project" "main" {
        auto_create_network = false
        billing_account     = "111111-222222-333333"
      + folder_id           = (known after apply)
      ~ id                  = "projects/my-new-project" -> (known after apply)
      - labels              = {} -> null
        name                = "my-new-project"
      ~ number              = "22915xxxxxxx" -> (known after apply)
        org_id              = "999999999999"
        project_id          = "my-new-project"
      + skip_delete         = (known after apply)
    }

@umairidris
Copy link
Contributor

One idea I had for this which I did not test was creating a service API client and calling the list (or any valid function) on a non existent project. If the API is not enabled on the service account this might return an error indicating so, while if it was enabled then you should get an error saying the project does not exist. Worth exploring this option if there is no other easy way to find out enabled services on the billed project.

@emilymye emilymye added the persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work label Jun 17, 2020
@thiagonache
Copy link
Contributor

thiagonache commented Oct 3, 2020

@umairidris and @morgante I've tested @umairidris idea and should work as below.

package main

import (
	"fmt"
	"log"
	"os"

	"golang.org/x/oauth2"
	"golang.org/x/oauth2/google"
	serviceusage "google.golang.org/api/serviceusage/v1beta1"
)

func failCreation(e *serviceusage.Service) {
	log.Fatalf("Unexpected API state %s", e.State)
}

func handleErrorFromAPI(e error) {
	fmt.Println("handle error from API")
	// if match Error 403: Project '\d+\' not found the project does not exist
	// if match Service Usage API has not been used in project 28203909211 before or it is disabled it is disabled
	log.Fatal(e)
}

var credentialsPath = "/Users/tnache/Downloads/test-serviceusage-go-b9a421db0f3f.json"
var clientScope = "https://www.googleapis.com/auth/cloud-platform"
var projectNumber = "28203909211"

func main() {
	os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", credentialsPath)
	client, err := google.DefaultClient(oauth2.NoContext, clientScope)
	if err != nil {
		log.Fatal(err)
	}
	svc, err := serviceusage.New(client)
	resp, err := svc.Services.Get("projects/" + projectNumber + "/services/serviceusage.googleapis.com").Do()
	if err != nil {
		handleErrorFromAPI(err)
	}
        // test edge case when API call works but status is not ENABLED.
	if resp.State != "ENABLED" {
		failCreation(resp)
	}
	fmt.Println("passed")
}

Would you guys willing to review a PR for precheck function?
Note: Code is just an example, I'd not write like that the production code. Also, I still need to check the scope needed.

@umairidris
Copy link
Contributor

Hi @thiagonache,

Thanks for testing the idea out. Neither myself nor @morgante actually work on the provider, so we can't give you approval. I would ask @danawillow to assign someone to look at this.

Regarding the code, I would not use a real project #, instead use one that would not exist like '000'. Do note that the provider already has a way to instantiate clients (the project resource already creates serviceusage clients in the code), so you can probably simplify the instantiation.

@thiagonache
Copy link
Contributor

thiagonache commented Oct 5, 2020

@umairidris Thanks for your answer. I'll wait for @danawillow .
Yes, this code snippet is more like a "just test the idea/poc". If Dana would be willing to review a PR I'll write the proper code and submit it.

@danawillow
Copy link
Contributor

@thiagonache seems reasonable to me! There's already a function called resourceGoogleProjectCheckPreRequisites that I think would be a good place for this. I can't guarantee I'll actually be the one to review it though- we have a bot that chooses a member of our team randomly.

@rileykarson
Copy link
Collaborator

Closed by #7447

@ghost
Copy link

ghost commented Nov 8, 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 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Nov 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work
Projects
None yet
Development

No branches or pull requests

7 participants