ansible-collections / azure

Development area for Azure Collections
https://galaxy.ansible.com/azure/azcollection
GNU General Public License v3.0
246 stars 332 forks source link

azure_rm_securitygroup should support diff output #1093

Open Nothing4You opened 1 year ago

Nothing4You commented 1 year ago
SUMMARY

https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_checkmode.html#using-diff-mode

Diff mode should be supported to be able to review the changes performed.

ISSUE TYPE
COMPONENT NAME

azure_rm_securitygroup

ADDITIONAL INFORMATION

Diff mode allows reviewing the changes performed. For example, it allows to easily determine which changes had been done outside of the ansible-managed configuration, such as someone manually changing it in Azure directly. While the module currently returns whether there were changes, it's not possible to see which attributes/rules have changed. I haven't checked for diff support in other modules but this might be a collection-wide topic.

- name: Update NSG
  diff: true
  azure.azcollection.azure_rm_securitygroup:
    subscription_id: "{{ nsg_subscription_id }}"
    resource_group: "{{ nsg_resource_group }}"
    name: testing-ansible
    purge_rules: yes
    rules:
      ## Inbound
      - name: Managed-rule
        priority: 100
        access: Deny
        source_address_prefix: 192.168.252.251/32
        destination_address_prefix: 192.168.252.251/32
        direction: Inbound
        description: test ansible rule

Example (admittedly much simpler on a simple text diff):

- hosts: localhost
  connection: local
  gather_facts: false

  tasks:
    - ansible.builtin.lineinfile:
        path: ./ansible-diff-demo
        create: true
        regexp: ^diff demo
        line: "diff demo {{ 100 | random }}"
      diff: true
image
Fred-sun commented 1 year ago

@Nothing4You The diff parameter is very convenient for the change of document, but I think it is not too convenient for the resource deployment, because we need to use the relevant "_info" module to get out the deployment or update, and then we can determine the change result, which cannot be directly returned in diff. Thank you very much!

Nothing4You commented 1 year ago

Hi @Fred-sun, I haven't looked into the code, but isn't this information already available? I only peeked at your changes in #1096 and it looks like you're at least comparing the names of the rules there already, so at least some diffable information is already present? If it does indeed need more information, you can check if you're running in diff mode and only then do additional requests when running in diff mode, see e.g. https://blog.networktocode.com/post/generating-diff-with-ansible/:

If you want to know if the module is running in diff mode, you can check the variable module._diff.

Fred-sun commented 1 year ago

@Nothing4You I understand your idea and we will discuss what to do. Thank you!

Mohammad-Atif-Khan commented 1 year ago

This would be very useful (and important) feature to have, which would make this collection more holistic and in line with the 'plan' step from terraform (& bicep). This is not only applicable to NSGs but other resources as well, since modules in this collection already check if changes are needed to make them idempotent - adding the 'print changes' part (when diff is set to true) to it will make even more useful and help implement 'Drift detection' playbooks, which in the cloud are quite a non-trivial issue.

Fred-sun commented 7 months ago

@Mohammad-Atif-Khan I am very sorry that the collection azure. azcollection does not support diff mode. However, we can get the current resource information using the *_info.py module. Then consider whether to update, configured in the parameters of the .py module. Thank you! thank you!

Mohammad-Atif-Khan commented 7 months ago

Hi @Fred-sun , as an interim solution/workaround, yes that makes sense but I like @Nothing4You believe that we should really aim for a 'diff' argument compatible collection of modules - that makes it a fully inline with the other enterprise collections and much more usable. Imagine being able to do print out changes and record them in the ticketing system as part of the change itself. This also supports the 'check' mode doesn't it?

thanks.