segmentio / stack

A set of Terraform modules for configuring production infrastructure with AWS
https://open.segment.com
MIT License
2.1k stars 420 forks source link

Use role instead of roles (deprecated) #127

Closed sgrodzicki closed 7 years ago

sgrodzicki commented 7 years ago

WARNING: This is deprecated since version 0.9.3 (April 12, 2017), as >= 2 roles are not possible. See issue #11575.

https://www.terraform.io/docs/providers/aws/r/iam_instance_profile.html#roles

achille-roussel commented 7 years ago

Oh Terraform...

It seems like role is a new property that didn't exist prior to 0.9.3 tho, which means this change would force everyone to upgrade to Terraform 0.9 in order to use stack (we just did here at Segment and it broke a bunch of our workflows). => https://github.com/hashicorp/terraform/pull/13130/files

Should we wait a bit before getting this merged?

sgrodzicki commented 7 years ago

It looks like we have two options:

  1. wait a bit
  2. use and promote versioning (which probably should go in pair with Terraform releases)

There was a 0.1 release on 15 Jun 2016. What was the purpose of this release?

achille-roussel commented 7 years ago

The purpose of 0.1 was to have versioning, but we didn't follow up... TF introduces backward incompatible changes so often that it's a nightmare to follow.

I'm open to tagging the current state as 0.2, then we can update the README saying we're going to introduce this breaking change.

How about we wait 6 weeks then and merge this?

sgrodzicki commented 7 years ago

Looking at the release cycle of Terraform 6 weeks is a lot and 0.10.x could be released until then. Should I include README updates in this PR reflecting these changes?

achille-roussel commented 7 years ago

They release a patch every 2 weeks, which means it would be 0.9.8 or 0.9.9, let's say 6 weeks or 0.10.0, whichever comes first?

I'll make the README changes and the release separately.

sgrodzicki commented 7 years ago

Agree :)

sgrodzicki commented 7 years ago

It's been 6 weeks. What's next?

achille-roussel commented 7 years ago

Alright let's get it merged! Thanks for your contribution.