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

Terraform plan should fail if a project or a region value is missing #4071

Closed
mikhailshilkov opened this issue Jul 22, 2019 · 7 comments
Closed

Comments

@mikhailshilkov
Copy link

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.5
provider.google v2.11.0

Affected Resource(s)

  • google_*

Terraform Configuration Files

provider "google" {	
}

resource "google_compute_subnetwork" "network-with-private-secondary-ip-ranges" {
  name          = "test-subnetwork"
  ip_cidr_range = "10.2.0.0/16"
  network       = "${google_compute_network.custom-test.self_link}"
}

resource "google_compute_network" "custom-test" {
  name                    = "test-network"
  auto_create_subnetworks = false
}

Expected Behavior

terraform plan fails with "project": required field is not set.

Actual Behavior

If I forget to include the project value in my provider "google" resource, terraform plan works perfectly well. It's only terraform apply that is going to fail with project: required field is not set which is a bit unfortunate.

Likewise, plan won't fail for a regional resource like subnet with a missing region property. I have to wait until the actual apply to get Cannot determine region: set in this resource, or set provider-level 'region' or 'zone'..

Steps to Reproduce

  1. terraform plan

References

None

@emilymye
Copy link
Contributor

This is technically possible but very difficult, as it requires we touch every resource that has optional projects (i.e. all resources with parent projects) and add a customdiff to verify the presence of the provider field, since it is essentially validating a resource field based off a separate "resource" (the provider config). I'm inclined to say this isn't feasible to fix.

@mikhailshilkov
Copy link
Author

@emilymye Thanks for a quick response!

What makes it fundamentally different from AWS region and Azure location fields? They both fail the plan phase in case of missing required values. Is it that AWS region is always defined at the provider level, Azure location is at the resource level, but GCP is more flexible?

@emilymye
Copy link
Contributor

Based on a quick look and your comment, I'd say that yes, because AWS and Azure have the field marked as required per resource or per provider, they are able to validate at plan. Because GCP allows you generally to specify the project/location in either, they must also be marked as Optional in both and the validation provided through the TF Core Schema API isn't very good for verifying across resources/provider.

I'm not saying it's not possible, and we may do it sometime in the future! It's just not the nicest solution and requires us to touch every project/location-based resource, and we haven't run into this issue that much before. If more users are interested, we can look into it, but I'm not sure we have it at a high priority right now.

@emilymye emilymye added enhancement and removed bug labels Jul 23, 2019
@emilymye
Copy link
Contributor

I'm going to actually close this since we don't anticipate doing this anytime soon. We'll reopen if we reconsider.

@redbaron
Copy link

@emilymye , we've been hit by it and it was confusing. I realise that upstream is not going to fix it,but it doesn't mean Pulumi can't do it's part. One way would be to do merging with ambient defaults on Pulumi side and always pass project to TF explicitly or error before even calling TF is project is not set.

@mikhailshilkov
Copy link
Author

@redbaron You should probably move your comment to pulumi/pulumi-gcp#129

@ghost
Copy link

ghost commented Aug 23, 2019

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 Aug 23, 2019
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