Fixes #34121 - Add/Delete a single Ansible role to Host(group)

[ofedoren]

This patch uses functionality from hammer-cli-foreman to allow associating/disassociating of a single resource, but the default
AssociatedCommand class is using host info endpoint and uses it's result to gather ids of the associated resource. Since we don't extend host info API response with Ansible roles, small override is needed and thus AssociatedAnsibleResource is present. It was written for general use in case it will be needed for ansible variables or potentionally others.

[ofedoren]

@adiabramovitch, would you have time to do a quick review here? :)

[adiabramovitch]

@ofedoren Maybe you should add some tests? :)
Moreover when I ran:

bundle exec hammer -r host ansible-roles add --id 4

resulted:

Ansible tole has been associated.

so I think that --ansible-role[-id] should be required ditto for hostgroup

[adiabramovitch]

typo

[adiabramovitch]

typo

[ofedoren]

Thanks, @adiabramovitch, updated. For tests to be running #11 is required.

[adamruzicka]

Could you please rebase so the tests start running?

[ofedoren]

@adamruzicka, 🍏 :)

[adamruzicka]

If you run hammer host ansible-roles list, it gives you a list of all roles, even those which are inherited from the host's hostgroup. If you tried to remove such a role, the command says the role has been disassociated, but nothing happens.

# bundle exec ruby bin/hammer host info --id 1 | grep 'Host Group'
Host Group:         HG1

# bundle exec ruby bin/hammer hostgroup ansible-roles list --id 1
---|-----------------|--------------------
ID | NAME            | IMPORTED AT        
---|-----------------|--------------------
2  | adriagalin.motd | 2022/03/14 14:07:07
---|-----------------|--------------------

# bundle exec ruby bin/hammer host ansible-roles list --id 1
---|-----------------|--------------------
ID | NAME            | IMPORTED AT        
---|-----------------|--------------------
2  | adriagalin.motd | 2022/03/14 14:07:07
---|-----------------|--------------------

# bundle exec ruby bin/hammer host ansible-roles remove --id 1 --ansible-role-id 2
Ansible role has been disassociated.

# bundle exec ruby bin/hammer host ansible-roles list --id 1
---|-----------------|--------------------
ID | NAME            | IMPORTED AT        
---|-----------------|--------------------
2  | adriagalin.motd | 2022/03/14 14:07:07
---|-----------------|--------------------

Is there something we could do about this?

[ofedoren]

Thanks, @adamruzicka, for the review!

Is there something we could do about this?

I've checked the UI and it seems we can't disassociate an inherited role from a host, so we shouldn't (actually can't) do that via hammer/API. The only thing we can do about it is to modify the command, so we can print a different message if one tries to remove an inherited role. But to distinguish between the inherited and own ones we would need to modify the /hosts/:id/ansible_roles endpoint with a flag param (something like explicit), so in the response we have marks for the inherited ones.

What do you think about that?

[adamruzicka]

I'd be fine with that. I just want to prevent showing that something happened when it in fact did not

[ofedoren]

@adamruzicka, updated. Now it requires theforeman/foreman_ansible#525. I've decided to raise an error, so the users are aware that we/they cannot disassociate an inherited role via API (nor in UI).

It will work without that patch as well. The changes in the main plugin are needed only to raise an error.

[adamruzicka]

This sadly does not catch all the cases when modifying hostgroup's roles. The list of roles for a hostgroup contains:
a) Roles explicitly assigned to the hostgroup
b) Roles assigned to hostgroup's ancestors
c) Roles explicitly assigned to hosts belonging to the hostgroup

Roles in group a) can be modified freely, roles in group b are caught by this check, but group c goes completely unnoticed and signals succesful removal even though nothing happens.

To me the easiest way out would be to treat "inherited" as "not directly assigned" instead of "passed from an ancestor of the same type", although it might be confusing if "inherited" has different meanings across the project. Maybe safer route would be renaming the field from "inherited" to "explicitly assigned"?

[ofedoren]

Thanks, @adamruzicka, for bearing with me. Somehow I missed more things than I'd expect. Updated, but requires theforeman/foreman_ansible#528 :/

P.S. Although I'm sure we're still not ready for merge, but just in case, could you please squash when we are? :)

[adamruzicka]

Almost there, but there's an issue when roles are specified by name

# hammer hostgroup ansible-roles list --id 4
---|-----------------|---------------------|-----------|------------------
ID | NAME            | IMPORTED AT         | INHERITED | DIRECTLY ASSIGNED
---|-----------------|---------------------|-----------|------------------
3  | gcoop-libre.ssh | 2022/03/14 15:15:20 | yes       | no               
---|-----------------|---------------------|-----------|------------------

# hammer hostgroup ansible-roles remove --id 4 --ansible-role-id 3
Could not disassociate the Ansible role:
  Ansible role gcoop-libre.ssh is not assigned directly and cannot be removed.

# hammer hostgroup ansible-roles remove --id 4 --ansible-role gcoop-libre.ssh
Ansible role has been disassociated.

# h hostgroup ansible-roles list --id 4
---|-----------------|---------------------|-----------|------------------
ID | NAME            | IMPORTED AT         | INHERITED | DIRECTLY ASSIGNED
---|-----------------|---------------------|-----------|------------------
3  | gcoop-libre.ssh | 2022/03/14 15:15:20 | yes       | no               
---|-----------------|---------------------|-----------|------------------

The same happens when trying to change host's roles

[ofedoren]

Thanks a lot, @adamruzicka, for the reviews! Updated. Previously I relied on option_sources to make name -> id resolution as with other commands, but in this case we have a really custom command, so I needed to actually override the default behaviour. Should work as expected now 🤞

[adamruzicka]

Works as expected, ACK

[adamruzicka]

Thank you @ofedoren !