bug: AWS EIP (dis)association fails on non-EC2-Classic accounts

[radeksimko]

As @pmoust mentioned in #1596

Test plan

All tested in a fresh new account (no EC2 Classic), it may need some testing in EC2 Classic too, but I don't have access to any.

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=EIP' 2>/dev/null

go generate ./...
TF_ACC=1 go test ./builtin/providers/aws -v -run=EIP -timeout 45m
=== RUN TestAccAWSEIP_normal
--- PASS: TestAccAWSEIP_normal (2.25s)
=== RUN TestAccAWSEIP_instance
--- PASS: TestAccAWSEIP_instance (256.66s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    258.929s

---

15b68fe

I'm happy to send 15b68fe as a separate PR, but it depends on the other commits as it's changing the same file.

How to replicate

resource "aws_vpc" "default" {
  cidr_block = "10.10.0.0/16"
}

resource "aws_subnet" "public" {
  vpc_id = "${aws_vpc.default.id}"
  cidr_block = "10.10.1.0/24"
  availability_zone = "us-east-1c"
}

resource "aws_instance" "web" {
  ami = "ami-323b195a"
  availability_zone = "us-east-1c"
  instance_type = "t2.small"
  key_name = "coreos-test"
  subnet_id = "${aws_subnet.public.id}"
  associate_public_ip_address = true
}

resource "aws_eip" "lb" {
  instance = "${aws_instance.web.id}"
  vpc = true
}

Output (that's still valid):

* Failure associating instances: Gateway.NotAttached: Network vpc-7c9abd19 is not attached to any internet gateway

destroy:

* MissingParameter: Either public IP or association id must be specified

The instance ID is saved to state file even though the API request ends up in error, which then results in trying to remove association which doesn't actually exist = corrupted state.

[radeksimko]

Looking at #1555 ...
@fatih if you're free to test this, it would help too.

[catsby]

I just hit this and solved it another way:

-       assocIds := []*string{}
-       publicIps := []*string{}
+       var publicIps []*string
+       var assocIds []*string
        if domain == "vpc" {
                assocIds = []*string{aws.String(id)}
        } else {

the correct one is still populated via the domain == "vpc" check, and the other remains []*string(nil) instead of []*string{}, which AWS draws a clear distinction.

I did not test your scenario, I just recall seeing it. Does my attempt fix yours as well (at least in this context), and it one way better? Thanks 😄

[radeksimko]

@catsby Just left a comment in your PR. I can remove two commits and just leave the bugfix for the IGW issue, but I will need you to merge your fix first, then I can rebase and do all the git magic. 🏄

[catsby]

Sorry for the delay – we'll get this sorted today

[catsby]

@radeksimko I just merged #1618

[radeksimko]

Bugfix separated into #1776

[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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.