Skip to content

Fix incorrect provider error#3

Merged
jsf9k merged 16 commits intodevelopfrom
bugfix/fix-incorrect-provider-error
May 14, 2020
Merged

Fix incorrect provider error#3
jsf9k merged 16 commits intodevelopfrom
bugfix/fix-incorrect-provider-error

Conversation

@jsf9k
Copy link
Copy Markdown
Member

@jsf9k jsf9k commented May 13, 2020 •
edited
Loading

🗣 Description

This pull request removes the dummy region arguments inside the provider blocks. I also pulled in upstream changes. The key commit is 5da4a4c.

💭 Motivation and Context

The dummy region arguments inside the provider blocks were causing the code in examples/basic_usage to fail to apply. This is because providing the region argument actually results in an explicit provider (using the default profile) being created. As stated in the penultimate paragraph in this section, "a proxy provider block is one that is either completely empty or that contains only the alias argument."

🧪 Testing

The code inside of examples/basic_usage successfully applies again.

🚥 Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (causes existing functionality to change)

✅ Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

hillaryj and others added 14 commits April 14, 2020 12:43
@hillaryj
Update CODEOWNERS
c8b4bcf
@hillaryj
Merge pull request #42 from cisagov/hillaryj-owner
175ae71 
Update CODEOWNERS
@jsf9k
Merge remote-tracking branch 'skeleton-generic/develop' into improvem…
1fc08b3 
…ent/upstream-changes
@jsf9k
Merge pull request #18 from cisagov/improvement/upstream-changes
3fb32f1 
Pull in upstream changes from cisagov/skeleton-generic
@felddy
Allow events from apb to rebuild this repository weekly.
8c99c52
@felddy
Merge pull request #43 from cisagov/improvement/apb-events
3301dfa 
Allow events from apb to rebuild this repository weekly.
@dav3r
Merge remote-tracking branch 'skeleton-generic/develop' into improvem…
f715a2f 
…ent/merge_upstream_changes
@dav3r
Use latest version of GitHub action for Go and latest stable version …
585d9cd 
…of Go.
@dav3r
Explicitly turn on GO111MODULE environment variable.
10d7c90
@dav3r
Merge pull request #19 from cisagov/improvement/merge_upstream_changes
2b903e7 
Fix terraform-docs install, update Go, merge upstream changes
@felddy
Add Lineage configuration.
ddd06c5
@felddy
Merge pull request #20 from cisagov/improvement/lineage
365edc1 
Add Lineage configuration.
@jsf9k
Remove dummy region from providers
5da4a4c 
terraform validate wants this, but adding any key inside the provider
block other than "alias" appears to actually _define_ a provider
instead of allowing one to be passed in from the parent module.

In this case, a provider is created from the default profile and the
specified region, and this is used _instead_ of the aws.images-staging
and aws.images-production providers passed in by the parent module.
@jsf9k
Merge remote-tracking branch 'skeleton-tf-module/develop' into bugfix…
05034c2 
…/fix-incorrect-provider-error
@jsf9k jsf9k added the bugfix label May 13, 2020
@jsf9k jsf9k requested review from dav3r and mcdonnnj May 13, 2020 17:51
@jsf9k jsf9k requested review from a team and felddy as code owners May 13, 2020 17:51
@jsf9k jsf9k self-assigned this May 13, 2020
@jsf9k jsf9k requested a review from hillaryj May 13, 2020 17:51
@jsf9k
Copy link
Copy Markdown
Member Author

jsf9k commented May 13, 2020

@dav3r and @mcdonnnj, it appears that what makes terraform validate happy leads to incorrect behavior. What should we do with the terraform validate pre-commit hook? It should be OK to run it on any directory inside of examples, and that should check the module code indirectly, but it cannot be run on the actual module code.

In short, module provider blocks can contain no arguments other than alias.

hillaryj
hillaryj approved these changes May 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@hillaryj hillaryj left a comment

Choose a reason for hiding this comment

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

🎉

@hillaryj
Copy link
Copy Markdown
Contributor

hillaryj commented May 14, 2020

It's now failing to build/lint on the terraform validate step because the region is missing.

mcdonnnj added 2 commits May 14, 2020 10:13
@mcdonnnj @jsf9k
Disable the terraform_validate hook.
beb1d0e 
We have seen a number of issues related to this hook ever since it was
re-enabled. It will need to remain disabled until at least the 0.13 Terraform
release, and can only be re-enabled if all issues we have seen have been
resolved in how `terraform validate` operates.
@mcdonnnj @jsf9k
Add final statement to descriptionf or why terraform_validate is disa…
fa40f68 
…bled.

Review noticed that there lacked a determination for what we were doing about
the problem with the terraform_validate hook. I described the problems but
failed to mention what our path forward would be. This commit rectifies that
oversight.
@jsf9k
Copy link
Copy Markdown
Member Author

jsf9k commented May 14, 2020 •
edited
Loading

It's now failing to build/lint on the terraform validate step because the region is missing.

Yep. I was waiting on cisagov/skeleton-generic#44 to trickle down. I got impatient, though, so I went ahead and git cherry-picked the relevant commits from there and applied them here.

dav3r
dav3r approved these changes May 14, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

This does the trick! 🍰

felddy
felddy approved these changes May 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@felddy felddy left a comment

Choose a reason for hiding this comment

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

🐕 🔫 😭

@jsf9k jsf9k merged commit 7ab7c50 into develop May 14, 2020
@jsf9k jsf9k deleted the bugfix/fix-incorrect-provider-error branch May 14, 2020 14:24
This was referenced May 18, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felddy felddy felddy approved these changes

@dav3r dav3r dav3r approved these changes

@mcdonnnj mcdonnnj Awaiting requested review from mcdonnnj mcdonnnj is a code owner

+1 more reviewer

@hillaryj hillaryj hillaryj approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jsf9k jsf9k

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jsf9k @hillaryj @felddy @dav3r @mcdonnnj