Add new module for AD Users

[coleneubauer]

SUMMARY

Addition of a module to return ad user information and create, update, or delete a user.

ISSUE TYPE

• New Module Pull Request

COMPONENT NAME

azure_rm_aduser_info

ADDITIONAL INFORMATION

This is the addition of a new module. The module supports getting information for an AD user or multiple users. The module supports multiple parameters for defining the scope of users to return as well as a direct OData filter for more complex queries.

Before a playbook with the following would fail because the module did not exist.
    - name: Using user_principle_name
      azure.azcollection.azure_rm_aduser_info:
        user_principle_name: [email protected]
        tenant: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

    - name: Using object_id
      azure.azcollection.azure_rm_aduser_info:
        object_id: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
        tenant: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

    - name: Using attribute mailNickname - not a collection
      azure.azcollection.azure_rm_aduser_info:
        attribute_name: mailNickname
        attribute_value: users_mailNickname
        tenant: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

    - name: Using attribute proxyAddresses - a collection
      azure.azcollection.azure_rm_aduser_info:
        attribute_name: proxyAddresses
        attribute_value: SMTP:[email protected]
        tenant: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

    - name: Using Filter mailNickname
      azure.azcollection.azure_rm_aduser_info:
        odata_filter: mailNickname eq '[email protected]'
        tenant: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

    - name: Using Filter proxyAddresses
      azure.azcollection.azure_rm_aduser_info:
        odata_filter: proxyAddresses/any(c:c eq 'SMTP:[email protected]')
        tenant: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx

With valid parameters, the playbook would now return information of the users who fit the scope defined.

[Fred-sun]

@coleneubauer Thank you for your contribution. This module is used to retrieve the Azure Recovery Services Vault policy. Can you add a module to manage the Azure Recovery Services Vault policy (azure_rm_aduser.py）? Use to implement the creation and deletion of a service library backup policy. Thank you very much!

[coleneubauer]

@coleneubauer Thank you for your contribution. This module is used to retrieve the Azure Recovery Services Vault policy. Can you add a module to manage the Azure Recovery Services Vault policy (azure_rm_aduser.py）? Use to implement the creation and deletion of a service library backup policy. Thank you very much!

@Fred-sun I think you may have mixed up my PRs. This is unrelated to the Azure Recovery Services Vault Backup Policy. That PR has both an info module and a management module. This one only is an info module to read AD User account information. I don't have a current business need for the management module and would like to not write it at this time.

[Fred-sun]

@coleneubauer Add a use case that successfully gets the AdUser information to the use case. Thank you very much!

[coleneubauer]

@Fred-sun The additional module has been added as requested.

[Fred-sun]

@coleneubauer Thanks for your update! I will test and push for merge!

[coleneubauer]

@haiyuazhang The metadata blocks have been removed.

[coleneubauer]

@Fred-sun The metaclass line was added as suggested

[Fred-sun]

@Fred-sun The metaclass line was added as suggested

@coleneubauer Do you execute your tests/integration/the targets/azure_rm_aduser/tasks/main.yml? Can you share your test results? Thank you very much!

[coleneubauer]

@Fred-sun The metaclass line was added as suggested

@coleneubauer Do you execute your tests/integration/the targets/azure_rm_aduser/tasks/main.yml? Can you share your test results? Thank you very much!

@Fred-sun I don't know how to execute the tests directly. I could not find in documentation how this was done and made the assumption this was intentional. What I do for the tests is copy the test to a local playbook, write a task at the top defining existing vars, and run the playbook. When I do it this way the results are that all the tasks pass.

[haiyuazhang]

The convention for a single ansible module is to manage a single resource instance. I checked your "CRUD" functions, seems they also support batch mode. It's bettor to fallback to the normal one module one resource instance way. Besides, The idempotency test is absent in the tests. I test the idempotency of the module locally, it doesn't work.

[coleneubauer]

The convention for a single ansible module is to manage a single resource instance. I checked your "CRUD" functions, seems they also support batch mode. It's bettor to fallback to the normal one module one resource instance way. Besides, The idempotency test is absent in the tests. I test the idempotency of the module locally, it doesn't work.

@haiyuazhang
"Besides, The idempotency test is absent in the tests. I test the idempotency of the module locally, it doesn't work."
This is a clear oversight on my part. Thanks for catching it, I'll update to fix.

"The convention for a single ansible module is to manage a single resource instance. I checked your "CRUD" functions, seems they also support batch mode. It's bettor to fallback to the normal one module one resource instance way"
This would be easy to change but it seems like a common use case for ad users to want to change multiple at a time. The alternate is to get a list of user's in ansible and update each one in a loop individually. I think allowing them to directly edit multiple would be more concise. What's your thought on the option of adding a flag to indicate a batch alteration should be done so it's more clear? Either way you want to take it I can update though. Let me know your thoughts, thanks.

[haiyuazhang]

The convention for a single ansible module is to manage a single resource instance. I checked your "CRUD" functions, seems they also support batch mode. It's bettor to fallback to the normal one module one resource instance way. Besides, The idempotency test is absent in the tests. I test the idempotency of the module locally, it doesn't work.

@haiyuazhang
"Besides, The idempotency test is absent in the tests. I test the idempotency of the module locally, it doesn't work."
This is a clear oversight on my part. Thanks for catching it, I'll update to fix.

"The convention for a single ansible module is to manage a single resource instance. I checked your "CRUD" functions, seems they also support batch mode. It's bettor to fallback to the normal one module one resource instance way"
This would be easy to change but it seems like a common use case for ad users to want to change multiple at a time. The alternate is to get a list of user's in ansible and update each one in a loop individually. I think allowing them to directly edit multiple would be more concise. What's your thought on the option of adding a flag to indicate a batch alteration should be done so it's more clear? Either way you want to take it I can update though. Let me know your thoughts, thanks.

If it won't take you too much effort, I suggest that we remove batch support from the module. Since simple loop in playbook can serve the same purpose. Keeping "the style" consistent among the modules in this collection might be a good practice.

[coleneubauer]

The convention for a single ansible module is to manage a single resource instance. I checked your "CRUD" functions, seems they also support batch mode. It's bettor to fallback to the normal one module one resource instance way. Besides, The idempotency test is absent in the tests. I test the idempotency of the module locally, it doesn't work.

@haiyuazhang
"Besides, The idempotency test is absent in the tests. I test the idempotency of the module locally, it doesn't work."
This is a clear oversight on my part. Thanks for catching it, I'll update to fix.
"The convention for a single ansible module is to manage a single resource instance. I checked your "CRUD" functions, seems they also support batch mode. It's bettor to fallback to the normal one module one resource instance way"
This would be easy to change but it seems like a common use case for ad users to want to change multiple at a time. The alternate is to get a list of user's in ansible and update each one in a loop individually. I think allowing them to directly edit multiple would be more concise. What's your thought on the option of adding a flag to indicate a batch alteration should be done so it's more clear? Either way you want to take it I can update though. Let me know your thoughts, thanks.

If it won't take you too much effort, I suggest that we remove batch support from the module. Since simple loop in playbook can serve the same purpose. Keeping "the style" consistent among the modules in this collection might be a good practice.

Sounds good, thanks for the feedback. I should be able to update this sometime this week.

[coleneubauer]

@Fred-sun @haiyuazhang Updated as requested and ready for review.

The ad_user module now only will operate on a single account. The info module does allow the users to retrieve information on multiple users as this seems consistent with other info modules see https://docs.ansible.com/ansible/latest/collections/azure/azcollection/azure_rm_resourcegroup_info_module.html#ansible-collections-azure-azcollection-azure-rm-resourcegroup-info-module or https://docs.ansible.com/ansible/latest/collections/azure/azcollection/azure_rm_subscription_info_module.html#ansible-collections-azure-azcollection-azure-rm-subscription-info-module or
https://docs.ansible.com/ansible/latest/collections/azure/azcollection/azure_rm_webapp_info_module.html#ansible-collections-azure-azcollection-azure-rm-webapp-info-module

There's a check now when attempting to update an existing account for idempotency and a test is added in the automated tests to check that an attempted update with no changes should come back as unchanged.