Adding compatibility with actual terraform version

dtan4 / terraforming

Export existing AWS resources to Terraform style (tf, tfstate) / No longer actively maintained

http://terraforming.dtan4.net/

MIT License

4.29k stars 659 forks source link

Adding compatibility with actual terraform version #484

Open fipciu1996 opened 5 years ago

fipciu1996 commented 5 years ago

Yesterday I tried to use terraforming with to get ec2 instances and there was some uncompatibility with new version of terraform. In fact tags aren't declared as resource, but as a map so it changes to "tags = {" instead of "tags {" and keys in this shouldn't be specified as a String. After rebuilding it works correctly

coveralls commented 5 years ago

Coverage remained the same at 100.0% when pulling 41e5930fb00d11b752a94f6beda98cd48f4a8ec9 on fipciu1996:feature/compliance-with-tags into 518879dfe36542b8bb1fbd37281ba4a862a920e4 on dtan4:master.

notjames commented 5 years ago

What is the risk of this change on backward compatibility? Seems like the way this change is committed could a potential problem.

fipciu1996 commented 5 years ago

What is the risk of this change on backward compatibility? Seems like the way this change is committed could a potential problem.

Actually as I find out it terraforming now supports terraform between 0.9.3 - 0.12.0-alpha2 (October 30, 2018) so probably there will be more and more people who will have potential problems using terraforming. In my opinion it should be compatible with newer versions or it should be mentioned in README.md that you need older version of terraform

notjames commented 5 years ago

What is the risk of this change on backward compatibility? Seems like the way this change is committed could a potential problem.

Actually as I find out it terraforming now supports terraform between 0.9.3 - 0.12.0-alpha2 (October 30, 2018) so probably there will be more and more people who will have potential problems using terraforming. In my opinion it should be compatible with newer versions or it should be mentioned in README.md that you need older version of terraform

I agree. I'd skip this commit to be in favor of a change to the README and maybe looking into supporting newer versions of terraform.

theaboutbox commented 5 years ago

I think that the tags = { } syntax works for earlier version of Terraform, it's just that Terraform stopped supporting the tags { ... } syntax in v0.12. (https://github.com/hashicorp/terraform/issues/19240) This would also make the output of this tool match the Terraform documentation.

It looks like PR #442 is trying to address the same issue.

I am definitely interested in seeing Terraformer output HCL that's valid in 0.12. If there are any issues with either of the existing PRs, I'd be happy to dig in and help fix them.