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

Remove provider config #49

Closed
wants to merge 14 commits into from

Conversation

heathsnow
Copy link

@heathsnow heathsnow commented Jun 4, 2021

what

  • Remove the requester/accepter provider configuration from the module.
  • Update minimum Terraform version to 0.15.0 (using configuration_aliases wasn't working with 0.14.x)

why

  • Provider configuration should always be passed into modules instead of configured within. When the module itself configures the provider(s) then the for_each and count functionality cannot be used with the module which greatly reduces it's utility.

references

@heathsnow heathsnow requested review from a team as code owners June 4, 2021 19:10
@heathsnow heathsnow requested a review from a team as a code owner June 4, 2021 19:11
@heathsnow heathsnow requested review from Makeshift and 3h4x and removed request for a team June 4, 2021 19:11
accepter.tf Outdated Show resolved Hide resolved
@nitrocode
Copy link
Member

/test all

@nitrocode
Copy link
Member

Please update the tests

- Remove `requester_aws_assume_role_arn`
- Remove `requester_region`
…rm-aws-vpc-peering-multi-account into remove-provider-config
@heathsnow heathsnow requested review from nitrocode and removed request for a team June 7, 2021 21:08
@heathsnow
Copy link
Author

I've addressed the errors that were reported in the tests (ie removing now defunct variables).

@nitrocode
Copy link
Member

/test all

@nitrocode
Copy link
Member

nitrocode commented Jun 8, 2021

@heathsnow see https://www.terraform.io/docs/language/modules/develop/providers.html#passing-providers-explicitly

  • The tests are failing due to the providers not set on the instantiation of the module in examples/complete/main.tf
  • The configuration_aliases should be set for the aws provider in versions.tf
        aws = {
          source                = "hashicorp/aws"
          version               = ">= 2.0"
          configuration_aliases = [ aws.accepter, aws.requester ]
        }

Comment on lines +87 to +88
aws.accepter = aws.accepter
aws.requester = aws.requester
Copy link
Member

Choose a reason for hiding this comment

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

Showing how to set these providers above would be helpful for copying and pasting the example.

@heathsnow heathsnow requested a review from nitrocode June 8, 2021 20:21
@heathsnow heathsnow marked this pull request as draft June 29, 2021 19:01
@nitrocode
Copy link
Member

/test all

@mergify
Copy link

mergify bot commented Nov 13, 2021

This pull request is now in conflict. Could you fix it @heathsnow? 🙏

@nitrocode nitrocode dismissed a stale review via e0c0419 November 13, 2021 05:46
@heathsnow
Copy link
Author

I'm closing this draft. No time right now to attend to it.

@heathsnow heathsnow closed this Jun 10, 2022
@heathsnow heathsnow deleted the remove-provider-config branch June 10, 2022 20:02
@heathsnow heathsnow restored the remove-provider-config branch June 10, 2022 23:48
@nitrocode nitrocode reopened this Nov 21, 2022
@heathsnow
Copy link
Author

Closing, won't fix.

@heathsnow heathsnow closed this Nov 30, 2022
@heathsnow heathsnow deleted the remove-provider-config branch November 30, 2022 20:25
@heathsnow heathsnow restored the remove-provider-config branch November 30, 2022 20:25
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.

Module does not support for_each due to hard coded providers
3 participants