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

Introduce Mechanism to Tag AMIs #5898

Closed
sargun opened this issue Sep 17, 2018 · 9 comments
Closed

Introduce Mechanism to Tag AMIs #5898

sargun opened this issue Sep 17, 2018 · 9 comments
Labels
new-resource Introduces a new resource. service/ec2 Issues and PRs that pertain to the ec2 service.
Milestone

Comments

@sargun
Copy link
Contributor

sargun commented Sep 17, 2018

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 "me too" comments, 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

Description

Currently, there's not a good way to tag existing AMIs in Terraform. This is valuable when you use aws_launch_configuration. Right now, you have to import the aws_ami back into terraform from outside terraform, and tag it, since create explicitly makes a new resource, and there is not an existing resource type like aws_tag.

New or Affected Resource(s)

  • aws_ami
  • aws_ami_copy
  • aws_launch_configuration

Potential Terraform Configuration

resource "aws_ami_tag" "foo" {
  provider = "aws.account2"
  name = "my_exposed_ami"
  tag {
    name = "foo"
    value = "bar"
  }
}
@bflad bflad added new-resource Introduces a new resource. service/ec2 Issues and PRs that pertain to the ec2 service. labels Sep 17, 2018
@sargun
Copy link
Contributor Author

sargun commented Sep 17, 2018

@bflad Any opinions on how you'd like to approach this? I can imagine this being problematic if you do this on a aws_ami_copy AMI where the destination AMI ID has a tag that conflicts with the source. If you're okay with assuming the user is going to be reasonable, then I can go ahead an implement this as a new resource.

@bflad
Copy link
Contributor

bflad commented Sep 18, 2018

@sargun I'm personally not the biggest fan of two methods to configure the same "objects" because it can lead to management conflicts where two resources will show a perpetual difference of configuration as this scenario suggests making something like this possible:

  • aws_ami tags (exclusive management) and this new resource (non-exclusive management)

Here are some examples of management conflict issues between the same "objects" we see very often (despite warnings in the documentation):

  • aws_security_group ingress/egress (exclusive management) and aws_security_group_rule (non-exclusive management)
  • aws_route_table route (exclusive management) and aws_route (non-exclusive management)
  • aws_network_acl ingress/egress (exclusive management) and aws_network_acl_rule (non-exclusive management)

But that certainly does not mean we should not implement something like this, just a word of caution. 😄

This similar previous issue has some ideas as well: #3143

If it me personally implementing this or reviewing a PR, I would be looking for generic aws_ec2_tag resource following the EC2 API for CreateTags and DeleteTags:

# Not implemented, details may change during development
resource "aws_ec2_tag" "example" {
  resource_id = "" # (Required, ForceNew) ami-12345678, i-12345678, etc
  key = "" # (Required, ForceNew)
  value = "" # (Required)
}

Hope this helps!

@sargun
Copy link
Contributor Author

sargun commented Sep 18, 2018

If we make it one resource per AMI -- it becomes an absolute mess, because its count is the the multiplicative product of the number of AMIs and tags. Terraform 0.12 will fix this, but for now, it wont.

If it's a single map, we can look at the set of keys, and do our own diffing of when keys are removed and deleted.

@bflad
Copy link
Contributor

bflad commented Sep 18, 2018

How would it determine the difference between something being missing from the map or already defined?

If a resource has existing tags, e.g. in HCL syntax

tags {
  key1 = value1
  key2 = value2
}

And this resource defines:

tags {
  key2 = value2updated
  key3 = value3
}

What happens to key1 and key2? There are nuances and complexity to keep this type of resource in line with the design philosophies of Terraform when managing multiple API "objects" which in this case is each individual tag (e.g. it should not overwrite key2 except with a flag or importing it first as the CreateTags API does not have a flag to prevent overwrite by itself)

@sargun
Copy link
Contributor Author

sargun commented Sep 19, 2018

I suggest we scrap this until Terraform 0.12 comes around.

@tksrc
Copy link

tksrc commented Mar 25, 2020

I wonder what's the latest here?

I am unable to tag VPN attachments to the Transit Gateway. No workarounds from what I can see.

I think a separate tag resource would help for these edge cases.

@bflad
Copy link
Contributor

bflad commented Jun 13, 2020

A new aws_ec2_tag resource for managing individual EC2 resource tags has been merged and will release with version 2.67.0 of the Terraform AWS Provider, later next week. This resource should only be used in cases where EC2 resources are created outside Terraform (e.g. AMIs), being shared via Resource Access Manager (RAM), or implicitly created by other means (e.g. Transit Gateway VPN Attachments).

# Example configuration in Terraform 0.12 and later syntax
resource "aws_ec2_transit_gateway" "example" {}

resource "aws_customer_gateway" "example" {
  bgp_asn    = 65000
  ip_address = "172.0.0.1"
  type       = "ipsec.1"
}

resource "aws_vpn_connection" "example" {
  customer_gateway_id = aws_customer_gateway.example.id
  transit_gateway_id  = aws_ec2_transit_gateway.example.id
  type                = aws_customer_gateway.example.type
}

resource "aws_ec2_tag" "example" {
  resource_id = aws_vpn_connection.example.transit_gateway_attachment_id
  key         = "Name"
  value       = "Hello World"
}

As with any Terraform 0.12.6 or later configuration, this resource can be combined with for_each support to manage multiple resource tags, if necessary.

Thanks to @joestump and others who made the implementation possible. 👍

@bflad bflad closed this as completed Jun 13, 2020
@ghost
Copy link

ghost commented Jun 19, 2020

This has been released in version 2.67.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 Jul 13, 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 and limited conversation to collaborators Jul 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-resource Introduces a new resource. service/ec2 Issues and PRs that pertain to the ec2 service.
Projects
None yet
Development

No branches or pull requests

3 participants