search

hashicorp / packer-plugin-amazon

Packer plugin for Amazon AMI Builder
https://www.packer.io/docs/builders/amazon
Mozilla Public License 2.0
76 stars 112 forks source link

Support AMI Tag Copying #295

Closed rifelpet closed 1 year ago

rifelpet commented 2 years ago

Please search the existing issues for relevant feature requests, and use the reaction feature (https://blog.github.com/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/) to add upvotes to pre-existing requests.

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

AWS just announced support for including tags when copying AMIs. This is supported by a new CopyImageTags boolean in the ec2.CopyImage action.

It would be useful to expose this in Packer's AMI configuration settings.

Use Case(s)

Using ami_regions to copy the resulting AMI to other regions leaves those copies without tags. Tags can be useful for downstream processes to identify the intended AMI to use, for example with the aws_ami Terraform datasource's filtering. Historically, tagging AMI copies in other regions required an external process. AWS now supports this with the new boolean, so users will no longer need to tag the AMIs themselves.

Potential configuration

copy_image_tags = true

Potential References

https://github.com/aws/aws-sdk-go/compare/v1.44.141...v1.44.142#diff-1082fdae378a5abc33332afae3c56453249b51a8309b4e3d15278e8c23471d2fR61657

lbajolet-hashicorp commented 1 year ago

Hi @rifelpet,

According to the documentation of the plugin, tags and attributes should be copied along with the AMI's. If that is not the case, this is a misconception on our part, and we should always copy them.

We probably won't need an extra configuration attribute for this, but the call should definitely be amended to include this in the output AMIs. That is, unless I'm missing something, in which case please let me know.

I'll open a PR shortly to fix this, thanks for the report!