Gather AWS labels and create CustomAttributes

[bronaghs]

This PR parses AWS Tags which are saved in the database as custom attributes.
Currently AWS only supports tagging VMs and Images.

This PR takes care of the parsing and depends on the model and post processing logic getting merged first:
ManageIQ/manageiq#14202 (Status: merged)

A UI PR will be made separately to display the custom attributes in the Labels table of the VMs and Images.

Steps to test:

1. Add tags to a VM or image through the AWS console.
2. Add an AWS provider to the MIQ UI
3. Open the Summary page for the VM selected in 1) and make sure a "Labels" table exists and displays the tags configured in 1)

https://www.pivotaltracker.com/story/show/139414085

[djberg96]

@bronaghs Interesting, I was not aware of this model, nor how it was used for labelling. This could obviate my approach completely. But, a few things:

Do labels hook into company policy at all the way tags and categories potentially do? I was told by @blomquisg that these should definitely hook into policy.

Also, should we try to create a two-way relationship by setting :resource_id and :resource_type as well?

[bronaghs]

@djberg96 - Create resource_id and resource_type are already columns in the custom_attributes table.
I can already execute: vm.custom_attributes

[djberg96]

@bronaghs right, but you can't go the other way, e.g. custom_attribute.resource. But, now that I think about it, this would have to be handled in post-processing, since the object wouldn't exist in the database yet, so we couldn't set the ID. You could set the resource_type, though.

[bronaghs]

@djberg96 - I have not been told that is a requirement

[gberginc]

@bronaghs I think you should also add this into the inventory. Especially because graph refresh is going to be the new default after #158 gets merged.

[djberg96]

@bronaghs Two things in the /app/models/custom_attribute.rb file I want to check on: https://github.com/ManageIQ/manageiq/blob/master/app/models/custom_attribute.rb

ALLOWED_API_SECTIONS = %w(metadata cluster_settings).freeze

However, this doesn't appear to be enforced in any way on the model itself, though it is checked at https://github.com/ManageIQ/manageiq/blob/master/app/controllers/api/providers_controller.rb#L94

The other thing that jumped out at me was this method:

def stored_on_provider?
  source == "VC"
end

This is used at https://github.com/ManageIQ/manageiq/blob/master/app/controllers/api/subcollections/custom_attributes.rb#L52-L65.

So, is this perhaps already reflected within the UI somehow? Since we're dealing with a tag on the provider side, should we set the source to "VC"?

[djberg96]

As a test, I tried creating a custom attribute, and then set the source to VC, as well as the resource_type and resource_id. This is the result within the UI:

Obviously this isn't VCenter, but we could change that text. The resource_id we could set in post-processing.

@abellotti I noticed that you added this method. Is there any reason we couldn't/shouldn't try to leverage this?

[abellotti]

@djberg96 I don't see why not, the code would need to be pulled out first, it's currently only for the Api, so maybe moving the logic as an api service (lib/services/api/) that you can instantiate and use on your side. /cc @imtayadeway

[imtayadeway]

@djberg96 @abellotti +1 on extracting services, but I don't know about putting it in lib/services/api if it's to be shared not only outside the API, but with another repo. Part of the Api namespace to me indicates that it is essentially private and that we are free to change at any time to meet our needs. I think lib/services would be a better option

[bronaghs]

@djberg96 - Im afraid I don't understand your point with your previous 2 comments. There is a source set to "amazon" in the custom attribute and the section is "labels". What are you looking to see changed?

[djberg96]

@bronaghs I guess it doesn't matter since we're not going to rely on that.

[bronaghs]

@blomquisg - review please and thank you.

[bronaghs]

thanks for the heads up @gberginc, I will add this to the new inventory code too.

[blomquisg]

@Ladas after this is merged, can you look at porting this over to the graph refresher for AWS?

[bronaghs]

@Ladas - Please review the porting of this PR to the new refresh Inventory code.

CustomAttributes are being saved in the DB for VMs and Images, vm.custom_attributes and image.custom_attributes both return correct AWS labels. However, ems.vm_or_template_labels only returns the custom_attributes on vms.
see my comment on the corresponding manageiq repo PR ManageIQ/manageiq#14281

[Ladas]

@bronaghs commented the ManageIQ/manageiq#14281, a correct relation in :through will fix it :-)

[Ladas]

would be nice if we could add 1 more tag to the testing data, but that can be done as follow up in the future, since it will require new VCRs

[Ladas]

seems like 1 method get_vm_or_template_labels should be enough

and just pass the 2 lazy_finds as args

[miq-bot]

Checked commits bronaghs/manageiq-providers-amazon@8ff07ca~...f9123d8 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
6 files checked, 0 offenses detected
Everything looks good. 🍰

[Ladas]

looks great 👍

[cben]

Do labels hook into company policy at all the way tags and categories potentially do? I was told by @blomquisg that these should definitely hook into policy.

Note that LabelTagMapping also currently applies tags at a low DB level, does not hook into policy the way "normal" tagging does!
(I agree it ought to, just never got around to even figure out how it's done. classification.rb has some methods calling policy. Looks synchronous, dunno if that'd be appropriate during refresh.)

[Ladas]

Hm, so there is a policy 'something has been tagged' ? Yeah calling policy as a part of the refresh would be definitely bad, we could check if this could be a post refresh action?