Plan-time project validation

[preston-hf]

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

v0.12.21

Affected Resource(s)

• google_*

Terraform Configuration Files

resource "google_project_iam_member" "dev_access" {
  member = "user:[email protected]"
  role = "roles/editor"
}

Debug Output

At plan time, shows adding a member as expected, all is good.
When applying:

Error: project: required field is not set

Expected Behavior

The project not being present should be known when the plan is run.

Actual Behavior

Plan works fine, the error only appears when applying.

Steps to Reproduce

1. terraform apply with no provider-configured project and no project specified.

Important Factoids

References

• #0000

[venkykuberan]

@preston-hf, during plan there is no syntax error thus it allowed. During apply provider checks if project attribute has any value passed, if not it checks env variable GOOGLE_PROJECT is set. If none of them applied then throws an error.

You need to pass the project name in the config or use the environment variable for apply command to be successful.

[preston-hf]

I understand how to use it, it doesn't mean this isn't an issue. This breaks git-ops, especially with modules. It should be smart enough to figure out if the project needed when planning instead of breaking the "build" after it's merged. Are you saying it's totally impossible to fix this? Could the check not be done when the plan is run?

[preston-hf]

And there are other errors that aren't syntax errors that fail the plan, like not including a required parameter, which this is, or a string not fitting a regex.

[venkykuberan]

Actually we can't make the project attribute as mandatory for this resource, as it would break the code for others using the project at provider level or as environment variable. Let me discuss with my team and see the possibilities of adding that validation to plan phase.

[preston-hf]

Thanks for re-opening! I hope there's some way to resolve it because the only thing I see is defining a provider block in every module to make configure it, and that gets messy with a bunch of modules. It's also a footgun because most of the examples in the docs don't have project set so you don't find out until after the code is merged in, then it requires manual admin intervention.

[venkykuberan]

@emilymye what are your thoughts on this issue ? How would we ensure project id isn't blank during the plan run ?

[emilymye]

Hi @preston-hf - this is true for almost all our resources with project/region/zone fields that also have provider-level defaults (for, etc). While we have the ability to plan-time validate this, it's neither a nice nor simple solution - we are limited by the plan-time hooks the provider SDK provides for us - and so we've generally had it as a lower-priority task.

In fact, we ended up making project explictly required for google_project_iam_policy just because it relies heavily on the project as the main resource. I'm not sure why we ended up not doing that for binding/member, but it might be the route we end up taking here instead. It is a breaking change though, so we'd probably do it on a major release.

Thanks for re-opening! I hope there's some way to resolve it because the only thing I see is defining a provider block in every module to make configure it, and that gets messy with a bunch of modules. It's also a footgun because most of the examples in the docs don't have project set so you don't find out until after the code is merged in, then it requires manual admin intervention.

Can you elaborate how this is breaking your workflow? I'm not sure I follow.

[preston-hf]

I would prefer to be explicit for every resource. It's just a simple line or two of code, that is especially helpful at preventing footguns if you've got more than one project in the module. I think the best of both worlds would be to allow explicit configuration of the default (instead of inferring it from gcloud/service account credentials), let me pull it from an environment variable or config variable myself, if I want that behavior. If it's not explicitly provided in the provider config, do not infer it and there should be no default. I believe this is the current behavior, and it should not change.

This should also be able to be checked at plan time, so these issues are found before merging using a gitops workflow as outlined roughly in GCP's docs here. When following this roughly, you do your plan when a pull request is sent, and it passes with the changes you expect shown in the output.

Many of the resources do not have a project setting, so it's not obvious looking at the code that something is missing, and the plan passes. You only learn that project was not specified once it's merged, and that requires another pull request to fix, meanwhile your terraform "build" is broken until that can get merged.

If you have a provider default somewhere, the behavior is even worse. Whatever resource you omitted silently uses the wrong project if you forgot to specify one.

[emilymye]

Split this issue, so we'll track adding plan-time validation of projects here (as it's a non-straightforward task to do so, we can't promise we'll get to it ASAP). I've filed a separate issue (#5787) to track making project required for _member/_binding, since that will need to be released as a breaking change.

[preston-hf]

Just reporting in more resources: google_bigquery_dataset_iam_member also doesn't detect there's no project until apply-time.

[wyardley]

Maybe the proposed #11339 would help? Now (I'm getting a better understanding of why it's a breaking change, though.

With the beta provider, I'm actually seeing a somewhat similar issue (though with the project implicitly present, in that case) even on resources where the beta provider marks project as an optional attribute (I think this is a bug, probably, though the symptom of when it bails is maybe working as intended).

Based on what @rileykarson told me on Slack, though, I thought plans should fail when project is required but not set? I agree that the cases where it's required but validation and plans both succeed are extremely unfortunate and annoying. In fact, I would want these to be caught on terraform validate so that they could be caught by pre-commit hooks, IDEs, or CI checks before even being planned.

Of course there will always be some cases where a plan bails for reasons terraform cannot predict, but this seems like a clear case where apply time surprises could (and should) be easily avoided.

[wyardley]

Also, I personally like / rely on it being implicit from the provider set project (and optional on resources), and I can understand why the "magic" behavior of using GOOGLE_PROJECT (though I think GCP_PROJECT is the current "official" standard, and GCLOUD_PROJECT a deprecated one, per https://cloud.google.com/functions/docs/configuring/env-var?) may be expected by some folks and / or tools, but I would imagine it's somewhat fair to expect that the environment between terraform validate and terraform plan will be somewhat consistent with that of terraform apply - I know I've seen some other cases where I had to mock in env vars for validation to work successfully....