Deduplicate role task in roles role

[Tompage1994]

What does this PR do?

De-duplicates the role task in the roles role by using defaults and a more clever loop

Unlike the example in the issue, I've simplified slightly as it would have been messy doing the if/else stuff in the task. Instead I've used defaults which is much neater.

How should this be tested?

CI already added

Is there a relevant Issue open for this?

resolves #661

Other Relevant info, PRs, etc

There hasn't been a release since the PR which this is updating so no additional changelog required.

[djdanielsson]

LGTM

[ivarmu]

Looks better, but there's a situation that wouldn't be desired... if the input contains the following... it would silently bypass the wrong formatted entry (it is missing both role and roles parameters):

controller_roles:
  - team: "team1"
    credential: "credential1"

It's not a stopper, but may need to add an extra check for that situation...

[ivarmu]

Looks better, but there's a situation that wouldn't be desired... if the input contains the following... it would silently bypass the wrong formatted entry (it is missing both role and roles parameters):

controller_roles:
  - team: "team1"
    credential: "credential1"

It's not a stopper, but may need to add an extra check for that situation...

Adding the following fail task would be enough to raise a failure when the input items has a lack of both role and roles fields:

---
- hosts: localhost
  connection: local
  gather_facts: false
  vars:
    - data:
        - name: row1
          roles:
            - role1
            - role2
        - name: row2
          role: roleA
        - name: row3
  tasks:
    - fail:
        msg: "The following input is incomplete: {{ current_bad_input }}"
      vars:
        bad_input: "{{ data | selectattr('role', 'undefined') | selectattr('roles', 'undefined') }}"
      loop: "{{ bad_input }}"
      loop_control:
        loop_var: current_bad_input

    - debug:
        msg:
          - "{{ item.1 if item.1 is defined else item.role }}"
      loop: "{{ (data | selectattr('role', 'defined')) + (data | subelements(['roles'], skip_missing=true)) }}"
...

What do you think?

[Tompage1994]

Looks better, but there's a situation that wouldn't be desired... if the input contains the following... it would silently bypass the wrong formatted entry (it is missing both role and roles parameters):

controller_roles:
  - team: "team1"
    credential: "credential1"

It's not a stopper, but may need to add an extra check for that situation...

Adding the following fail task would be enough to raise a failure when the input items has a lack of both role and roles fields:

---
- hosts: localhost
  connection: local
  gather_facts: false
  vars:
    - data:
        - name: row1
          roles:
            - role1
            - role2
        - name: row2
          role: roleA
        - name: row3
  tasks:
    - fail:
        msg: "The following input is incomplete: {{ current_bad_input }}"
      vars:
        bad_input: "{{ data | selectattr('role', 'undefined') | selectattr('roles', 'undefined') }}"
      loop: "{{ bad_input }}"
      loop_control:
        loop_var: current_bad_input

    - debug:
        msg:
          - "{{ item.1 if item.1 is defined else item.role }}"
      loop: "{{ (data | selectattr('role', 'defined')) + (data | subelements(['roles'], skip_missing=true)) }}"
...

What do you think?

I think you can go without as the mandatory filter on the input essentially does that job. If you don't specify role or roles you get the following output:

fatal: [localhost]: FAILED! => {"msg": "Mandatory variable 'role'  not defined."}

I think that will be fine to be honest.

[ivarmu]

I think that will be fine to be honest.

Ok, just to be sure that my concern is clear... given the following example:

---
- hosts: localhost
  connection: local
  gather_facts: false
  vars:
    - data:
        - name: row1
          roles:
            - role1
            - role2
        - name: row2
          role: roleA
        - name: row3
  tasks:
    # - fail:
    #     msg: "The following input is incomplete: {{ current_bad_input }}"
    #   vars:
    #     bad_input: "{{ data | selectattr('role', 'undefined') | selectattr('roles', 'undefined') }}"
    #   loop: "{{ bad_input }}"
    #   loop_control:
    #     loop_var: current_bad_input

    - debug:
        msg:
          - "{{ item.1 if item.1 is defined else item.role }}"
      loop: "{{ (data | selectattr('role', 'defined')) + (data | subelements(['roles'], skip_missing=true)) }}"
...

This will skip the data -> row3 item, as loop is selecting only the list items having defined either of role or roles, discarding any posible item without both of them, so the mandatory term will not have the opportunity to shine:

ansible-playbook /tmp/test.yaml
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] *****************************************************************************************************************************************************************************************************************************

TASK [debug] *********************************************************************************************************************************************************************************************************************************
ok: [localhost] => (item={'name': 'row2', 'role': 'roleA'}) => {
    "msg": [
        "roleA"
    ]
}
ok: [localhost] => (item=[{'name': 'row1', 'roles': ['role1', 'role2']}, 'role1']) => {
    "msg": [
        "role1"
    ]
}
ok: [localhost] => (item=[{'name': 'row1', 'roles': ['role1', 'role2']}, 'role2']) => {
    "msg": [
        "role2"
    ]
}

PLAY RECAP ***********************************************************************************************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

Do we want to let that kind of incomplete configuration entries in the input configuration as code?

[Tompage1994]

That doesn't happen because instead of testing if role is defined, I've tested if roles is undefined, meaning if neither is set then it will be picked up and will spit out the error.

The only possible issue is when BOTH are defined, but in that case it will just use the roles option and role will be ignored

This:

tasks:
  - name: Include roles role at ../roles/roles
      ansible.builtin.include_role:
        name: ../roles/roles
      vars:
        controller_roles:
          - user: user1
            job_template: Demo Job Template
            roles:
              - read
              - execute
          - inventory: Demo Inventory
            user: user1
            role: read
          - job_template: Demo Job Template

Outputs this:

...
fatal: [localhost]: FAILED! => {
    "msg": "Mandatory variable 'role'  not defined."
}