Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azure_rm_virtualmachineextension always changed #326

Open
UnwashedMeme opened this issue Oct 30, 2019 · 17 comments
Open

azure_rm_virtualmachineextension always changed #326

UnwashedMeme opened this issue Oct 30, 2019 · 17 comments

Comments

@UnwashedMeme
Copy link

UnwashedMeme commented Oct 30, 2019

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:

  • Try to add the JsonADDomainExtension to a windows VM, passing the password as a protected_setting
  • Don't modify anything and repeat -- it still reports changes.

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
Copy link
Collaborator

Fred-sun commented Nov 1, 2019

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

@UnwashedMeme
Copy link
Author

@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
Copy link
Collaborator

Fred-sun commented Nov 4, 2019

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

@Fred-sun
Copy link
Collaborator

Fred-sun commented Nov 4, 2019

@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
Copy link
Collaborator

Fred-sun commented Dec 9, 2019

@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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Author

  • protected_settings not checked -- I think this is satisfactory provided it is clearly documented on the module. Having a force_update flag or something like that to force it through even if the check didn't show any changes might help some.
  • Azure api change -- this would be awesome to have, and could also help powershell, terraform, etc. As you say, I expect this to be harder to get done.

@Fred-sun
Copy link
Collaborator

@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
Copy link
Author

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
Copy link
Collaborator

@UnwashedMeme I see, Thanks for your info.

@Fred-sun
Copy link
Collaborator

@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
Copy link
Collaborator

Fred-sun commented Jan 6, 2020

@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
Copy link
Collaborator

Fred-sun commented Feb 6, 2020

Kindly ping!

@Fred-sun
Copy link
Collaborator

Fred-sun commented Mar 6, 2020

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

@Fred-sun
Copy link
Collaborator

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants