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

feat: Remove region parameter for 3.0 aws provider #38

Merged

Conversation

jb-abbadie
Copy link
Contributor

@jb-abbadie jb-abbadie commented Aug 3, 2020

The 3.0 aws provider does not allow a region parameter for the aws_s3_bucket resource.

@jb-abbadie jb-abbadie changed the title Remove region parameter for 3.0 aws provider fix: Remove region parameter for 3.0 aws provider Aug 3, 2020
The 3.0 aws provider does not allow a region parameter to the
aws_s3_bucket resource.
@jb-abbadie jb-abbadie changed the title fix: Remove region parameter for 3.0 aws provider feat: Remove region parameter for 3.0 aws provider Aug 3, 2020
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

@bryantbiggs
Copy link
Member

thanks for the PR @jb-abbadie - with the changes in the 3.0 AWS provider we will have to take a look at how we handle this transition as not everyone will be making the jump to 3.0. it is a major change so we will need some time to figure out the best approach

@chris-mackubin
Copy link

In the short run, support both with a separate branch and version tagging. Similar to what terraform did with v11 to v12. Allows those of us that are using latest aws provider to just version stamp your module and not have to pull repo/build...

@antonbabenko antonbabenko merged commit c90355c into terraform-aws-modules:master Aug 13, 2020
@antonbabenko
Copy link
Member

Thanks @jb-abbadie !

I think there is little sense to support both branches for AWS provider v2 and v3 simultaneously, so this PR requires AWS provider v3, and all upcoming features (other PRs) will have to be compatible with that, too.

v1.10.0 has been just released.

If users want to use AWS provider v2 they will have to pin version of this module to be v1.9.0 or older.

@kimmoahokas
Copy link

Hey,

First of all, thanks for the module, it has been great.

But, a small feedback about this change: Many of us have pinned this module to version 1.x and aws provider to version 2.x. Now, my terraform init just failed as it tried to install 1.10 of this module, which is no longer compatible with aws 2.x. It would be nice if this was version 2.0 or something :)

@antonbabenko
Copy link
Member

In theory, yes, it is possible, but after reading CHANGELOG carefully I don't see anything that is breaking in version 3 except region which is fixed in this PR.

I recommend locking module version stricter to have it working as expected (for eg, version = "1.9.0").

@kimmoahokas
Copy link

I recommend locking module version stricter to have it working as expected (for eg, version = "1.9.0").

Yep, that's what we did now and everything is working again. I guess we just had a different expectation about versioning :)

@antonbabenko
Copy link
Member

Good that it works! I support you, and you are right about versioning. I would say that this time I was just lazier than usual to not make a major release. Saved me a few minutes of my life 👍

lgarron added a commit to thewca/wca.link that referenced this pull request May 9, 2021
This is a result of 03ba099.

If I understand terraform-aws-modules/terraform-aws-s3-bucket#38 correctly, we may need to upgrade our AWS provider for terraform to put this back in our config.
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants