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(cai): modules must not define providers #1132

Merged

Conversation

oikarinen
Copy link
Contributor

As per Terraform documentation, provider configurations can be defined only in a root module.

@oikarinen oikarinen requested review from rjerrems, gtsorbo and a team as code owners March 3, 2024 08:45
As per Terraform
[documentation](https://developer.hashicorp.com/terraform/language/modules/develop/providers),
provider configurations can be defined only in a root module.
Copy link

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the Stale label May 15, 2024
@daniel-cit daniel-cit requested a review from eeaton as a code owner May 20, 2024 18:09
@daniel-cit
Copy link
Contributor

/gcbrun

@github-actions github-actions bot removed the Stale label May 20, 2024
Copy link
Collaborator

@eeaton eeaton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

This module was originally deployed from a separate repo, we added it in v4 so as to deploy as part of the process for 1-org stage. It seems we missed a few pieces to modify as part of making it smoothly part of this repo.

  • convention of not using a provider in a module (fixed by this PR)
  • unnecessary steps to specify a service account, should already be configured in the pipeline like other resources (fixed by this PR)
  • strange to configure CMEK only for these resources, inconsistent with how CMEK is handled everywhere else. (I'll make a separate issue to track this)

@eeaton eeaton enabled auto-merge (squash) May 21, 2024 16:12
@eeaton eeaton merged commit f396741 into terraform-google-modules:master May 21, 2024
5 checks passed
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.

None yet

3 participants