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

Tags not being assigned to Elastic IP instances #18756

Closed
tomerickson-es opened this issue Apr 12, 2021 · 14 comments · Fixed by #18909
Closed

Tags not being assigned to Elastic IP instances #18756

tomerickson-es opened this issue Apr 12, 2021 · 14 comments · Fixed by #18909
Assignees
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/ec2 Issues and PRs that pertain to the ec2 service.
Milestone

Comments

@tomerickson-es
Copy link

tomerickson-es commented Apr 12, 2021

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Regression
Relates #17612

Terraform CLI and Terraform AWS Provider Version

ubuntu@ip-10-1-10-161:~/sandbox$ terraform -v
Terraform v0.14.5

  • provider registry.terraform.io/hashicorp/aws v3.36.0

Affected Resource(s)

  • aws_eip

Terraform Configuration Files

data "aws_ami" "ubuntu" {
  most_recent = true
  filter {
    name   = "name"
    values = ["ami-ubuntu-16.04-*"]
  }
  owners = ["285398391915"] # Canonical
}

locals {
  common_tags = {"Deployment" = "custom", "email" = "qa@editshare.com"}
}

resource aws_instance "nodes" {
  ami = data.aws_ami.ubuntu.id
  instance_type = "t2.medium"
  key_name = "TO_BE_SUPPLIED"
  availability_zone = "us-east-2a"
  subnet_id = "TO_BE_SUPPLIED"
  tags = {
    Name = "tome-foo"
  }
}

resource "aws_eip" "node_eip" {
  instance = aws_instance.nodes.id
  tags = {
      Name = "foo-node-01-eip"
      NodeName = "foo-node-01"
    }
}

Debug Output

N/A

Panic Output

N/A

Expected Behavior

The two tags specified in the aws_eip declaration should be present when viewing the Tags section of the Elastic IP from the AWS Console.

Actual Behavior

No Tags are present.

Steps to Reproduce

  1. terraform apply

Important Factoids

  1. the tags are assigned correctly when this test module is run from Mac OS X deployment machines
  2. the tags are assigned correctly when this test module is run from Terraform Cloud
  3. the tags are NOT assigned at all when this test module is run from Ubuntu 20.04 LTS, recreated on a server in our Dev Ops pipeline or one Launched from the AWS Console
  4. the tags are NOT assigned at all when this test module is run from Ubuntu 18.04 LTS ec2 instance running in AWS
  5. if you run terraform apply again from the Ubuntu 20.04 test machine, terraform indicates the aws_eip will be updated in-place, with the changes being to add the two tags. looks like the provider supports the tags, but somehow not on the initial deployment of the resource. output from that apply operation is below
  6. IMPORTANT! the tags are properly assigned to the EIP when it is created if we pin the aws provider to 3.35.0. therefore we believe this was introduced in recent changes that are part of aws provider 3.36.0
ubuntu@ip-10-1-10-161:~/sandbox$ terraform apply
aws_instance.nodes: Refreshing state... [id=i-01f47a7fb12aca60e]
aws_eip.node_eip: Refreshing state... [id=eipalloc-02c6281e8fe1a3479]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # aws_eip.node_eip will be updated in-place
  ~ resource "aws_eip" "node_eip" {
        id                   = "eipalloc-02c6281e8fe1a3479"
      ~ tags                 = {
          + "Name"     = "foo-node-01-eip"
          + "NodeName" = "foo-node-01"
        }
        # (11 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

References

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/eip

  • #0000
@ghost ghost added the service/ec2 Issues and PRs that pertain to the ec2 service. label Apr 12, 2021
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Apr 12, 2021
@YakDriver YakDriver added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Apr 13, 2021
@YakDriver
Copy link
Member

@tomerickson-es Thanks for raising this issue! Also, thanks for all the excellent detail. That will make it easier to track down. Sometimes these problems are intermittent. It is rare to have differences between OS flavors but it can happen. Have you been able to replicate the results consistently? Again, thanks and sorry for the trouble.

@tomerickson-es
Copy link
Author

@YakDriver i pinned our code to use hashicorp/aws 3.36.0 and deployed from my macbook, and that also results in the error where the tags are not assigned to the EIP. i think your suspicion that this is a bug in 3.36.0 regardless of OS is probably correct. thanks for following up!

@bflad
Copy link
Contributor

bflad commented Apr 14, 2021

Related: #18847

@tomerickson-es which AWS region is this configuration running in?

@rick-masters
Copy link
Contributor

I am the author of the change to EIP tagging which was released in 3.36.0. I just saw this issue so I'll be trying to reproduce this today and hopefully help find a resolution soon. More details on the change can be found here:
#16881

The only information I can add at this point is that the change was tested manually on both an older EC2 Classic account and a newer VPC style account. Also, the relevant acceptance tests were run. But it appears that something was missed so I'll try to figure out what's going on.

By the way, I know it is a pain to redact sensitive data, but debug output TF_LOG=TRACE terraform ... can often pinpoint the problem. If anyone has redacted debug output they are willing to share that might speed things up.

@tomerickson-es
Copy link
Author

@rick-masters i updated my sample program to be fully standalone, and recreated the issue. the new module is below and the resulting logs for TF_LOG=TRACE are attached. I ran this by setting env vars

AWS_PROFILE=default
AWS_DEFAULT_REGION=us-east-2

Then I copied the module to a new working directory, ran terraform init followed by terraform apply

I did notice the following somewhat ominous lines in the trace log output, very near the end

2021/04/15 13:23:53 [WARN] Provider "registry.terraform.io/hashicorp/aws" produced an unexpected new value for aws_eip.node_eip, but we are tolerating it because it is using the legacy plugin SDK.
    The following problems may be the cause of any confusing errors from downstream operations:
      - .customer_owned_ipv4_pool: was null, but now cty.StringVal("")
      - .tags: element "Name" has vanished
      - .tags: element "NodeName" has vanished

I hope all this helps

data "aws_ami" "ubuntu" {
  most_recent = true
  filter {
    name   = "name"
    values = ["ami-ubuntu-16.04-*"]
  }
  owners = ["285398391915"] # Canonical
}

resource "aws_vpc" "vpc" {
  cidr_block = "10.1.0.0/16"
  tags = {
      Name = "test-foo-VPC"
    }
}

resource "aws_internet_gateway" "igw" {
  vpc_id = aws_vpc.vpc.id
  tags = {
      Name = "test-foo-IGW"
    }
}

resource "aws_subnet" "public-subnet" {
  availability_zone       = "us-east-2a"
  cidr_block              = "10.1.0.0/16"
  map_public_ip_on_launch = true # makes it public
  vpc_id                  = aws_vpc.vpc.id
  tags = {
      Name = "test-foo-vpc-Public1"
    }
}

resource "aws_route_table" "route-table" {
  vpc_id = aws_vpc.vpc.id
  route {
    cidr_block = "0.0.0.0/0"
    gateway_id = aws_internet_gateway.igw.id
  }
  tags = {
      Name = "test-foo-Route-Table"
    }
}

resource "aws_route_table_association" "public-subnet-associations-managed" {
  route_table_id = aws_route_table.route-table.id
  subnet_id      = aws_subnet.public-subnet.id
}

# EC2 section
resource aws_instance "nodes" {
  ami = data.aws_ami.ubuntu.id
  instance_type = "t2.medium"
  availability_zone = "us-east-2a"
  subnet_id = aws_subnet.public-subnet.id
  tags = {
    Name = "test-foo"
  }
}

resource "aws_eip" "node_eip" {
  instance = aws_instance.nodes.id
  tags = {
      Name = "test-foo-node-01-eip"
      NodeName = "test-foo-node-01"
    }
}

terraform {
  required_version = "~> 0.14.5"
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = "3.36.0"
    }
  }
}

eip_no_tags.trace.txt

@rick-masters
Copy link
Contributor

Thanks @tomerickson-es that helps so I can make sure I have the same sdk version and so forth. I have the log file if you want to delete it now.

@rick-masters
Copy link
Contributor

@tomerickson-es
I just reproduced this and then it works for me if I add vpc = true to the eip configuration. I think that's the right setting for your configuration.

What is strange is that your experience appears to be that terraform was able to set the tags for "standard" eips in 3.35.0. That was not my understanding of the old behavior and there was code in the provider which explicitly avoided setting tags for "standard" eips:

		if d.Get("domain").(string) == ec2.DomainTypeStandard {
			return fmt.Errorf("tags can not be set for an EIP in EC2 Classic")
		}

Upon further inspection, I'm suspecting that this statement never actually worked and then when I rewrote it as part of the tagging change [1], I made it work correctly, but that actually caused trouble. It will take me some time to confirm that, and if so, that check will need to be removed entirely, but for now I think vpc = true will fix your problem and is the correct setting anyway.

  1. https://github.com/hashicorp/terraform-provider-aws/pull/17612/files#diff-5cc53a93a886b4b2474bd02b135f8aabd7f4aeb82d77e8ac0378386eaa01ae84L186

@rick-masters
Copy link
Contributor

I know what this issue is. If your account/region does not support EC2 Classic, then the EIP will default to type vpc (not standard) even if you do not specify vpc = true. That is why the tagging worked for you in 3.35.0 without vpc = true. I have an EC2 classic account and so I must specify vpc = true or it does not work for me in some regions.

It was a mistake on my part for overlooking that behavior. Sorry about that. I've written a new change which only errors out if there are tags and vpc = true is missing and EC2 classic is a supported platform.

This issue should be flagged as a regression. A pull request is forthcoming.

@YakDriver YakDriver added the regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. label Apr 15, 2021
@rick-masters
Copy link
Contributor

It will take me a while to put together a pull request and test it properly, but if anyone would like to see the fix right now, this is what I have in progress:

--- a/aws/resource_aws_eip.go
+++ b/aws/resource_aws_eip.go
@@ -145,7 +145,11 @@ func resourceAwsEipCreate(d *schema.ResourceData, meta interface{}) error {
                Domain: aws.String(domainOpt),
        }
 
-       if domainOpt == ec2.DomainTypeVpc {
+       if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
+               supportedPlatforms := meta.(*AWSClient).supportedplatforms
+               if domainOpt != ec2.DomainTypeVpc && len(supportedPlatforms) > 0 && hasEc2Classic(supportedPlatforms) {
+                       return fmt.Errorf("tags can not be set for an EIP in EC2 Classic")
+               }
                allocOpts.TagSpecifications = ec2TagSpecificationsFromMap(d.Get("tags").(map[string]interface{}), ec2.ResourceTypeElasticIp)
        }
 
@@ -183,10 +187,6 @@ func resourceAwsEipCreate(d *schema.ResourceData, meta interface{}) error {
 
        log.Printf("[INFO] EIP ID: %s (domain: %v)", d.Id(), *allocResp.Domain)
 
-       if v := d.Get("tags").(map[string]interface{}); len(v) > 0 && d.Get("domain").(string) == ec2.DomainTypeStandard {
-               return fmt.Errorf("tags can not be set for an EIP in EC2 Classic")
-       }
-
        return resourceAwsEipUpdate(d, meta)
 }

@YakDriver
Copy link
Member

YakDriver commented Apr 15, 2021

@rick-masters That makes sense as to why it is not affecting everyone. Thank you for the explanation.

If you are able to put together even a simple PR, before you finish testing, I can start testing on my end as well. Ideally, I'd like to get a fix in today. The release has been pushed to tomorrow. If not, no worries. We can work up a PR on this end also. Just let me know.

@YakDriver YakDriver self-assigned this Apr 15, 2021
@rick-masters
Copy link
Contributor

@YakDriver ok, working on a PR...

@rick-masters
Copy link
Contributor

@YakDriver PR posted, started more testing

@github-actions github-actions bot added this to the v3.37.0 milestone Apr 16, 2021
@ghost
Copy link

ghost commented Apr 16, 2021

This has been released in version 3.37.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented May 16, 2021

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!

@ghost ghost locked as resolved and limited conversation to collaborators May 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants