resource/aws_instance: Store sg as ID for default VPC

[darkowlzz]

Security group was being stored as name, although vpc_security_group_ids
accepts IDs. Due to this, there were mismatches right after applying.
This was happening only for default VPC, not a custom VPC.
This change enables saving default VPC sgs as IDs, not name.

fixes #1799

[klaus993]

Hi!

We had the same issue and I built the plugin myself with this changes, used it and definitely works!

Thank you, I hope this is swiftly merged 😄

[rhuddleston]

Any update on this getting merged?

[klaus993]

I don't know why this is not merged yet. @darkowlzz any problems here?

[darkowlzz]

@klaus993 not that I'm aware of 🤷‍♀️

[Yuxael]

First of all it's not a fix to #1799 because the main issue is not what is saved in security_groups but that it's populated instead of vpc_security_group_ids. I belive it should work as explained in hashicorp/terraform#6416 (comment) and should not change SG names to ids as it will possibly break something when used with EC2-Classic.

[tony612]

Any update? This is annoying.

[roman-vynar]

Still not merged... since Oct 16, 2017 😢

[bflad]

Hi everyone, I am actively reviewing PRs #1911 and #2338. The appropriate fix will land in the next version of the AWS provider. More soon!

[bflad]

For watchers of #1911 and especially to @darkowlzz for the initial PR, which looks like it was most likely correct from the beginning, we are very sorry this got lost in the shuffle. This likely should have been reviewed and potentially merged long ago. Our apologies on dropping the ball here. We have been working really hard to get the backlog under control and organized the last few weeks (e.g. you will notice a lot more repository labels now available and on every open issue so hopefully people can find related issues easier).

@darkowlzz specifically, you are going to notice this specific commit not landing since it did not add any additional testing to cover it, which the maintainers would have noted in an initial review had we been timely. That is my doing so we do not need to delay the fix any more to try and perform rebasing of #2338 and commits I am adding on top on top of this commit. I am really sorry. We really do not want this to be any sort of standard for people contributing to this repository. If there are any enhancements or bug fixes you would like fast tracked, please reach out to me and I will try to make them happen. I will also look through anything else existing of yours right now.

After some extensive acceptance testing we believe that #2338 will properly handle the issue here, which will be merged with an additional testing commit from me. This will be released in v1.9.0 of the AWS provider, which currently is planned for next week.

[bflad]

This has been released in terraform-provider-aws version 1.9.0. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

[vvitayau]

Now using

Terraform v0.11.13
+ provider.aws v2.7.0
+ provider.consul v2.0.0
+ provider.template v2.1.0

but still encountering this bug where the following is required

  lifecycle {
    ignore_changes = [
      "security_groups",
    ]
  }

Otherwise, I always encounter changes for every apply

      security_groups.#:            "0" => "1" (forces new resource)
      security_groups.3469484075:   "" => "sg-..." (forces new resource)

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!