Aws Tags

[wrightp]

Support tagging for AWS resources which are "taggable".

[wrightp]

@tyler-ball this is ready for a review

[wrightp]

@chef/ociv work re: aws tagging

[tyler-ball]

The tag example is still failing like #189 correct?

[tyler-ball]

Hmm - I totally didn't think about the fact that most people are going to use machine and that doesn't support tag attributes. This solution (having to use aws_instance with the same name as the machine) seems less than intuitive. And I don't want to add a tag attribute to machine because that would require a new release of chef-provisioning... And whatever solution we use will have to work for machine, machine_batch and load_balancer.

Its not awesome, but I think we should read a tags field in the machine_options and load_balancer_options like so:

machine 'test' do
  machine_options bootstrap_options: {
    aws_tags: {'key' => 'value'}
  }
end
machine_batch 'test-batch' do
  machine_options bootstrap_options: {
    aws_tags: {'key' => 'value'}
  }
  machine 'test1'
  machine 'test2'
end
machine_image 'test-image' do
  machine_options bootstrap_options: {
    aws_tags: {'key' => 'value'}
  }
  image_options bootstrap_options: {
    aws_tags: {'key' => 'value'}
  }
end
load_balancer 'test_lb' do
  load_balancer_options aws_tags: {'key' => 'value'}
end

@jkeiser what do you think of this? Its a weird dual-user-interface problem, but I don't want to add a base provider_tags attribute to these resources in chef-provisioning because that implies all providers support tags.

Long term, I want to fix this by having machine in a recipe lookup the correct AwsInstance resource using the new resource mapping introduced in Chef 12. Then

machine 'ref-machine-1` do
  aws_tags :marco => 'polo'
end

would correctly instantiate a AwsInstance resource and apply the aws_tags to it.

[wrightp]

I'm +1 on using *_options for instances and load balancers over how it is currently.

[wrightp]

I'd also like to add that even though tagging machines and load balancers isn't ideal with this branch, I would press that we still merge these changes pending a good review. This is a feature that is much needed and we can improve it in a new branch.

[wrightp]

Yes, that's how I discovered it. The aws tag spec that tags an aws_instance is terminated properly. I was hoping it was some issue with the recipe I am overlooking.

[tyler-ball]

Can you add some yarddoc here?

[tyler-ball]

The specs and the matcher look great! There is only a few missing specs I would like to see:

aws_ebs_volume 'ref' do
  aws_tags {'aaa' => 'a'}
end
aws_ebs_volume 'ref' do
  aws_tags {'bbb' => 'b'}
end

should only have the 'bbb' tag on it after convergence. This shows that deleting a tag by not putting it in the tag list works.

The other spec should test that when the aws_tags is missing we don't modify any of the current tags:

aws_ebs_volume 'ref' do
  aws_tags {'aaa' => 'a'}
end
aws_ebs_volume 'ref'

should still contain the tags 'aaa'

[wrightp]

Good call on the tests. The first case I was handling, but the second case uncovered a bug where the tags were being wiped out because the default aws_tags attribute is {}. Since we may want that functionality I removed the default {} and handle the nil attribute in #converge_tags.

[tyler-ball]

@Patrick-Wright you should be able to rebase against master and have your ref work!

[tyler-ball]

👍 Great job!

[danieljimenez]

It doesn't seem to work on security groups when you use 'add_machine_options'

[wrightp]

@danieljimenez you need to use the aws_tags attribute with the resource.

aws_security_group 'blah' do
  aws_tags :tag_name => 'value'
 # other attrs
end

the readme has some additional details: https://github.com/chef/chef-provisioning-aws#aws-resources

[danieljimenez]

@Patrick-Wright I did this:

add_machine_options aws_tags: { 'my-tag' => 'tag' }
... build machines ...
aws_security_group 'sg-12345'

   NoMethodError
    -------------
    undefined method `aws_tags' for Chef::Resource::AwsSecurityGroup

[wrightp]

@danieljimenez good find.
@tyler-ball check this out: https://github.com/chef/chef-provisioning-aws/blob/master/lib/chef/resource/aws_security_group.rb#L1-L5

Looks like AwsSecurityGroup resource is requiring _with_entry, but is subclassing AWSResource. I'll try updating this locally, but do you see an immediate issue changing that to AWSResourceWithEntry?

[tyler-ball]

@Patrick-Wright Ah! I updated security groups to subclass AWSResource instead of AWSResourceWithEntry in #194. I did this because the name of an aws security group is specific to a VPC, so we don't need to store a mapping entry.

It looks like this exposed an issue with tags! I'll file a bug.

[tyler-ball]

Tracking this new issue in #204