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: terraform always specify project #384

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

alexlo03
Copy link
Contributor

@alexlo03 alexlo03 commented Sep 9, 2024

  • This includes terraform format changes

@nielm
Copy link
Collaborator

nielm commented Sep 16, 2024

Can you split the mac-os (--null -> -0) change into a separate PR please.

Regarding the 'always specify project ID' -- is this causing you issues?

In the TF parent where we call these modules, there is always a Provider section that specifies the default project ID and region globally for he Google provider

eg: https://github.com/cloudspannerecosystem/autoscaler/blob/main/terraform/cloud-functions/per-project/main.tf

provider "google" {
  project = var.project_id
  region  = var.region
}

@alexlo03
Copy link
Contributor Author

Xargs PR split off: #392

Re project_id as variable vs project_id as implicit input from Google Provider.

Right now the modules I am updating are split brain on the project_id input. Some of the project_id spec comes from the variable, some come from the implicit Google Provider. This move consolidates to use the variable, which in my opinion is superior.

Some parts of Google has kind of started to understand that the implicit is trouble, so for example many resources will not infer them from the provider. Here is an example: https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam

project - (Required) The project id of the target project. This is not inferred from the provider.

Modules like https://github.com/apigee/terraform-modules also don't rely on the provider project.

@nielm
Copy link
Collaborator

nielm commented Sep 19, 2024

Generally in the Google Terraform provider, project is optional, and implicit from the provider config

There are only 9 resources where project is required,

The principle seems to be that project ID is required when you are specifying resources that affect the project itself, such as project level IAM...

@alexlo03
Copy link
Contributor Author

I may be overstating the direction at the Google Provider level, at the Module level I think it should have one way in. If you think it should be consolidated the other way I think that's a reasonable approach.

@alexlo03
Copy link
Contributor Author

alexlo03 commented Sep 19, 2024

Also I am realizing this may be a breaking change if these modules are used with a provider set to projectA and an project input of projectB (which I think would be "bad" but maybe someone is doing it).

@henrybell henrybell self-assigned this Oct 10, 2024
@alexlo03
Copy link
Contributor Author

I think blocked by #390

@henrybell
Copy link
Collaborator

/gcbrun

@henrybell henrybell merged commit de5adb0 into cloudspannerecosystem:main Oct 10, 2024
9 checks passed
@henrybell
Copy link
Collaborator

Thanks @alexlo03!

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.

3 participants