add support for tags on spot fleet requests

[taylorsmithgg]

Solves #1232

[catsby]

Hi! Thanks for your contribution here. I have some questions about the approach used and the 2 attributes. In addition this we'll need documentation written for the attribute(s) and acceptance tests. Please let me know if you'd like guidance there and we'll be happy to help!

[catsby]

I'm curious if you were able to examine other resource's support of EC2 tags before coming to this implementation? Most resources use our helper file aws/tags.go:

https://github.com/terraform-providers/terraform-provider-aws/blob/494221fd522c1d41c47790e1a04ce78fcd1c3ab6/aws/tags.go#L20-L25

Example usage from aws_instance:

https://github.com/terraform-providers/terraform-provider-aws/blob/494221fd522c1d41c47790e1a04ce78fcd1c3ab6/aws/resource_aws_instance.go#L262-L264

[taylorsmithgg]

I previewed aws_instance but with different specs and SpotFleetTagSpecification in the mix, opted to go with a similar model used here: https://github.com/terraform-providers/terraform-provider-aws/blob/master/aws/resource_aws_autoscaling_group.go

[catsby]

How does this attribute work, and how does it differ from tag set above? For instance, in the aws_instance resource, you see this for configuration:

tags {
  Name = "HelloWorld"
}

Can you elaborate on how this tags attribute is used?

[taylorsmithgg]

The class for aws-sdk is SpotFleetTagSpecification with uses key and value. I just attempted to follow along with the aws model without making too many modifications.

tags = [
  {
    key = "key"
    value = "${var.name}"
  }
]

[catsby]

It seems by the implementation above that a user cannot specify both tags and tag. By the looks of it, tags would take predecease if both are supplied , is the the intent? If so, they should use the helper/schema ConflictsWith attribute. An example of this usage can be found in aws_instance:

https://github.com/terraform-providers/terraform-provider-aws/blob/494221fd522c1d41c47790e1a04ce78fcd1c3ab6/aws/resource_aws_instance.go#L106-L110

[taylorsmithgg]

This was originally in the reference implementation, but caused some minor issues that I didn't have capacity to work on at the time. I intend to add it back in.

[moee]

First of all thanks for this PR, I'm looking very forward to be able to use this. If there is anything I can contribute (documentation, tests, ...) please let me know.

The PR now proposes this syntax:

tags = [
  {
    key = "key"
    value = "${var.name}"
  }
]

Is there a specific reason why it doesn't follow the SpotFleetLaunchSpecification naming from the AWS API and use the SpotFleetTagSpecification data structure? In this case, the tags would we wrapped around tag_specification

tag_specifications = [
  {
    resource_type = "instance"
    tags = [
      {
        key = "key"
        value = "${var.name}"
      }
    ]
  }
]

It adds some verbosity (considering that at the moment the only resource type that is supported is instance), but reading the API documentation, my first guess would have been to use tag_specifications in terraform. Also, if at some point more resource types are supported, you can provide this functionality without breaking compatibility.

[aaroncline]

Please revive this feature!

[nullck]

Please revive this feature[2]

[taylorsmithgg]

I'd be happy to, I'll make the requested changes in the next week. Sorry for the delays!

[davidfic]

Any update on this PR getting merged?

[Ninir]

Hy @taylorsmithgg

Did you find time to continue the work? :)
We would love to this merged! 🚀

[galindro]

@taylorsmithgg any return??

[ghost]

Is this the same as #2042 ?

[taylorsmithgg]

Apologies, I've been traveling a ton and haven't had time. Looks like #2042 wasn't working for some folks. I'll take a look now and see if I can get it in for you guys!

[bflad]

This pull request predates my maintainership by quite a bit so I'm really not sure the history here, but it appears this support has already been handled by #2042 and the related issue #1232 is closed. I'm going to close this pull request based on that to clean up the open PRs in the repository, but if I'm missing something, please let me know.

[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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!