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

Enhancement | Remove empty provider configuration block warning #16

Merged

Conversation

angelofenoglio
Copy link
Contributor

What

  • Update module to fix "Empty provider configuration block" warning.

Why

To get rid of the message:

╷
│ Warning: Empty provider configuration blocks are not required
│ 
│   on .terraform/modules/terraform_backend/config.tf line 5:
│    5: provider "aws" {
│ 
│ Remove the aws.main_region provider block from module.terraform_backend. Add aws.main_region to the list of configuration_aliases for aws in required_providers to define the provider
│ configuration name.
│ 
│ (and one more similar warning elsewhere)
╵

when using this module.


required_providers {
aws = "~> 3.0"
aws = {
Copy link
Contributor

Choose a reason for hiding this comment

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

@angelofenoglio So the fix is only this piece here, right?
I mean, to use the module you need to pass these providers:

providers = {
    aws           = aws
    aws.secondary = aws.secondary
}

So, I'm wondering if it'd be better to rather do it this way:

providers = {
    aws.primary   = aws
    aws.secondary = aws.secondary
}

That is, in order to follow the "Explicit is better than implicit." aphorism. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems reasonable, I'll make these changes then!

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, is that right? I would've thought that the following is implicit:

resource "foo" "bar" {
    ...
    provider = aws
    ...
}

And thus it does not need to be explicitly defined.

@exequielrafaela exequielrafaela self-requested a review April 13, 2022 13:43
Copy link
Member

@exequielrafaela exequielrafaela left a comment

Choose a reason for hiding this comment

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

@angelofenoglio the PR looks very good :)

Small comment regarding CI failing

I think you've already fixed this in another repo

pre-commit run --all-files
[INFO] Initializing environment for git://github.com/pre-commit/pre-commit-hooks.
An unexpected error has occurred: CalledProcessError: command: ('/usr/bin/git', 'fetch', 'origin', '--tags')
return code: 128
expected return code: 0
stdout: (none)
stderr:
    fatal: remote error: 
      The unauthenticated git protocol on port 9418 is no longer supported.
    Please see https://github.blog/2021-09-01-improving-git-protocol-security-github/ for more information.
    
Check the log at /home/circleci/.cache/pre-commit/pre-commit.log
make: *** [@bin/makefiles/terratest14/terratest14.mk:84: pre-commit] Error 3

Exited with code exit status 2
CircleCI received exit code 2

Regarding tests here https://github.com/binbashar/terraform-aws-tfstate-backend/blob/master/tests/verify_output_test.go we'll need to update the examples to cover the new module version

So if this will stop you from delivering your work please leave an issue in the repo and we can pull it as soon as possible. In general is very important to consider the tests effort, but we can do an exception this time.

CC: @binbashar/leverage-project-terraform-admin @binbashar/leverage-project-terraform-dev

@angelofenoglio
Copy link
Contributor Author

@exequielrafaela Oh sure! I've just fixed the pre-commit issue!
I just wanted to trigger the CI jobs, since this is my first contribution in a module and I wasn't sure how it would behave. I'll now update the tests accordingly to make them pass.

@angelofenoglio angelofenoglio merged commit 80f0579 into master May 3, 2022
@angelofenoglio angelofenoglio deleted the feature/remove_empy_provider_configuration_warning branch May 3, 2022 18:05
@exequielrafaela exequielrafaela changed the title Enhancement | Remove empy provider configuration block warning Enhancement | Remove empty provider configuration block warning Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants