Azure / azure_preview_modules

Azure preview modules for Ansible
https://galaxy.ansible.com/azure/azure_preview_modules
43 stars 48 forks source link

azure_rm_virtualmachineextension always changed #326

Open UnwashedMeme opened 4 years ago

UnwashedMeme commented 4 years ago

An azure_rm_virtualmachineextension always reports as changed even when nothing changes.

I have used this module to make 3 extensions and 2 are always listed as having changes-- the two that use protected_settings. I noticed in the output it shows state.protected_settings is null even though I am setting that in my ansible config.

To reproduce:

e.g.

- name: "Create VM Extension to join AD"
  delegate_to: localhost
  tags:
    - vm
    - base
  vars:
    extension_settings:
      Name: "{{ domain_name }}"
      User: "{{ domain_admin_user }}"
      Restart: "true"
      Options: "3"
      OUPath: "{{ domain_server_ou }}"
    protected_extension_settings:
      Password: "{{ domain_admin_password }}"
  azure_rm_virtualmachineextension:
    name: "join-domain"
    virtual_machine_extension_type: "JsonADDomainExtension"
    resource_group: "{{ resource_group_vm }}"
    virtual_machine_name: "{{ inventory_hostname }}"
    publisher: "Microsoft.Compute"
    type_handler_version: "1.3"
    auto_upgrade_minor_version: true
    settings: "{{ extension_settings | to_json }}"
    protected_settings: "{{ protected_extension_settings | to_json }}"

I'm guessing that the Azure API doesn't return protected_settings so when the code looks for a change (right about here) it is always different.

Fred-sun commented 4 years ago

@UnwashedMeme Thank you for reporting the pr. Do you mean we need to add the return value for azure_rm_virtualmahineextension module?

UnwashedMeme commented 4 years ago

@Fred-sun I edited the top description to try and be more clear. The issue is that it always reports changes even when nothing changes.

I don't want it to print out the protected_settings, I just suspect that the bug lies in the fact that when I use protected_settings and the code compares the configured value vs the value returned from the azure api they are always and will always be different since (it looks like) azure doesn't give that value back.

Fred-sun commented 4 years ago

@UnwashedMeme I see, Thank for your info, As you said, there is no idempotent when use 'protected_settig

Fred-sun commented 4 years ago

@mybayern1974 Please help take a look this issue when you're free! There is no idempotent when create azure_rm_virtualmachineextension by 'protected_setting' attribute. Thank you very much!

Fred-sun commented 4 years ago

@haiyuazhang Could help take a look this issue? The Azure API doesn't return protected_settings when create virtualmachine extension. Thank you very much!


{'auto_upgrade_minor_version': True, 'virtual_machine_extension_type': u'CustomScript', 'name': u'testVMExtension', 'publisher': u'Microsoft.Azure.Extensions', 'type': u'Microsoft.Compute/virtualMachines/extensions', 'force_update_tag': None, 'tags': None, 'provisioning_state': u'Succeeded', 'additional_properties': {}, 'location': u'eastus', 'protected_settings': None, 'instance_view': None, 'settings': None, 'type_handler_version': u'2.0', 'id': u'/subscriptions/xxxxxxxxxxxx/resourceGroups/v-xisuRG02/providers/Microsoft.Compute/virtualMachines/testVM/extensions/testVMExtension'}
UnwashedMeme commented 4 years ago

The API not giving the protected_settings back makes sense, that's the protection. Perhaps if it could give back a hash though?

Fred-sun commented 4 years ago

The API not giving the protected_settings back makes sense, that's the protection. Perhaps if it could give back a hash though?

Thanks for your update, I see!

Fred-sun commented 4 years ago

@UnwashedMeme So there are two ways to solve this problem. First: the protected_settings return value is not checked in the module, or not updated when empty (easy import). Second: update the Azure API so that the protected_settings return value is an encrypted sequence(not easy import). Which do you think is more suitable?

UnwashedMeme commented 4 years ago
Fred-sun commented 4 years ago

@UnwashedMeme If we follow the first assumption, can we do this?Add a parameter force_update to force the update. Such as:


if self.force_update =='yes' and self.protected_settings is not None:
    response['protected_settings'] = self.protected_settings
    to_be_updated = True
UnwashedMeme commented 4 years ago

I've not done too much with writing ansible modules but i would suspect the parameter parsing will translate force_update to a boolean hence it should just be if self.force_update and self.protected_settings is not None. If I write force_update: true (or any of the other truthy values describes on this page I expect it to work, not requiring the string yes explicitly.

Aside from that maybe set the default value of to_be_updated = self.force_update. Make force_update always force an update not specific to protected_settings, that's just the most obvious use case.

Fred-sun commented 4 years ago

@UnwashedMeme I see, Thanks for your info.

Fred-sun commented 4 years ago

@haiyuazhang Do you think so? If there is no problem, I will try to give a PR fix this problem. Thank you very much!

Fred-sun commented 4 years ago

@haiyuazhang It should not need to be changed, as it is designed to force updates when protected_settings is set. Could you please help confirm it? Thank you very much!

Fred-sun commented 4 years ago

Kindly ping!

Fred-sun commented 4 years ago

@haiyuazhang Would you pelase help take a look this issue when you're free? Thank you very much!

Fred-sun commented 4 years ago

@UnwashedMeme Thank you very much for your interest in Ansible. This repo is no longer maintained in this repository and has been migrated to https://github.com/ansible-collections/azure Please re-submit this Issue in the above repository and closed this. Thank you very much!