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

Update aws_eip validation and documentation to prevent tagging non-VPC EIPs #14987

Closed
matthewhembree opened this issue Sep 2, 2020 · 3 comments · Fixed by #15070
Closed

Update aws_eip validation and documentation to prevent tagging non-VPC EIPs #14987

matthewhembree opened this issue Sep 2, 2020 · 3 comments · Fixed by #15070
Assignees
Labels
bug Addresses a defect in current functionality. documentation Introduces or discusses updates to documentation. service/ec2 Issues and PRs that pertain to the ec2 service.

Comments

@matthewhembree
Copy link

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

Terraform CLI and Terraform AWS Provider Version

terraform -v
Terraform v0.12.29
+ provider.aws v3.4.0

Affected Resource(s)

  • aws_eip

Terraform Configuration Files

locals {
  common_tags = {
    Application = var.StackName,
    ManagedBy   = "Terraform",
    RepoPath    = "xxx"
  }
}

resource "aws_eip" "PublicElasticIP" {
  vpc = true

  tags = merge(local.common_tags,
    {
      Name = "${var.StackName}-PublicElasticIP"
  })

  lifecycle {
    prevent_destroy = true
  }
}

resource "aws_eip" "ManagementElasticIP" {
  vpc = true

  tags = merge(local.common_tags,
    {
      Name = "${var.StackName}-ManagementElasticIP"
  })
  lifecycle {
    prevent_destroy = true
  }
}

resource "aws_eip" "ProdElasticIP" {
  vpc = false

  tags = merge(local.common_tags,
    {
      Name = "${var.StackName}-ProdElasticIP"
  })

  lifecycle {
    prevent_destroy = true
  }
}

resource "aws_eip" "TestElasticIP" {
  vpc = false

  tags = merge(local.common_tags,
    {
      Name = "${var.StackName}-TestElasticIP"
  })

  lifecycle {
    prevent_destroy = true
  }
}

Expected Behavior

Tagging of aws_eip resources.

Actual Behavior

Resources created: VPC and non-VPC aws_eip
Resources tagged: VPC aws_eip
Resources not tagged: non-VPC aws_eip

Error: error adding tags: error tagging resource (1.2.3.4): InvalidID: The ID '1.2.3.4' is not valid
        status code: 400, request id: 00000000-0000-0000-0000-000000000000



Error: error adding tags: error tagging resource (5.6.7.8): InvalidID: The ID '5.6.7.8' is not valid
        status code: 400, request id: 11111111-1111-1111-1111-111111111111

Steps to Reproduce

  1. terraform apply
Acquiring state lock. This may take a few moments...
aws_eip.ManagementElasticIP: Refreshing state... [id=eipalloc-xxxx]
aws_eip.PublicElasticIP: Refreshing state... [id=eipalloc-xxxx]

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_eip.ProdElasticIP will be created
  + resource "aws_eip" "ProdElasticIP" {
      + allocation_id     = (known after apply)
      + association_id    = (known after apply)
      + customer_owned_ip = (known after apply)
      + domain            = (known after apply)
      + id                = (known after apply)
      + instance          = (known after apply)
      + network_interface = (known after apply)
      + private_dns       = (known after apply)
      + private_ip        = (known after apply)
      + public_dns        = (known after apply)
      + public_ip         = (known after apply)
      + public_ipv4_pool  = (known after apply)
      + tags              = {
          + "Application" = "testing"
          + "ManagedBy"   = "Terraform"
          + "Name"        = "ProdElasticIP"
          + "RepoPath"    = "xxxx"
        }
      + vpc               = false
    }

  # aws_eip.TestElasticIP will be created
  + resource "aws_eip" "TestElasticIP" {
      + allocation_id     = (known after apply)
      + association_id    = (known after apply)
      + customer_owned_ip = (known after apply)
      + domain            = (known after apply)
      + id                = (known after apply)
      + instance          = (known after apply)
      + network_interface = (known after apply)
      + private_dns       = (known after apply)
      + private_ip        = (known after apply)
      + public_dns        = (known after apply)
      + public_ip         = (known after apply)
      + public_ipv4_pool  = (known after apply)
      + tags              = {
          + "Application" = "testing"
          + "ManagedBy"   = "Terraform"
          + "Name"        = "TestElasticIP"
          + "RepoPath"    = "xxxx"
        }
      + vpc               = false
    }

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

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_eip.TestElasticIP: Creating...
aws_eip.ProdElasticIP: Creating...

Error: error adding tags: error tagging resource (1.2.3.4): InvalidID: The ID '1.2.3.4' is not valid
        status code: 400, request id: 00000000-0000-0000-0000-000000000000



Error: error adding tags: error tagging resource (5.6.7.8): InvalidID: The ID '5.6.7.8' is not valid
        status code: 400, request id: 11111111-1111-1111-1111-111111111111


Releasing state lock. This may take a few moments...

Important Factoids

These are non-VPC aws_eip. They will not be associated to an ENI. They are for allocating globally unique addresses to be used within an EC2 VPN appliance as destination NAT addresses for partner VPN connections to address towards. Allocating non-VPC aws_eip should serve as a guardrail for preventing accidental ENI association.

@ghost ghost added the service/ec2 Issues and PRs that pertain to the ec2 service. label Sep 2, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Sep 2, 2020
@gdavison gdavison changed the title Error tagging non-VPC elastic IPs (aws_eip) Update aws_eip validation and documentation to prevent tagging non-VPC EIPs Sep 2, 2020
@gdavison
Copy link
Contributor

gdavison commented Sep 2, 2020

Thanks for the issue, @matthewhembree. Only EIPs in VPC scope can be tagged. We should update the resource validation and documentation to reflect that.

@gdavison gdavison added bug Addresses a defect in current functionality. documentation Introduces or discusses updates to documentation. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 2, 2020
@matthewhembree
Copy link
Author

@gdavison Thanks for the quick response! Based on that info, I feel that an effective guardrail will be adding tags with a very descriptive warning text to a VPC scoped EIP.

Thanks again!

@ghost
Copy link

ghost commented Oct 15, 2020

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 Oct 15, 2020
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. documentation Introduces or discusses updates to documentation. service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
2 participants