-
Notifications
You must be signed in to change notification settings - Fork 332
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
Ignore comparing protected_settings #580
Conversation
Howdy @Fred-sun , For context, I'm familiar with the bug that prompted this PR, and how the vm extension "resource" works, but am not very knowledgeable about how this module works. The one concern I have with this modification is how would it work if I need to update a vm extension and the only modification is the protected setting? The specific hypothetical example I can think of is the case of a log-analytics secret key being rotated, This may not be an actual issue in practice. I don't have much experience with rotating keys and updating the extension, so I don't know how it would impact in this specific example, but the general concern would apply to any extension where an update may be made with the only change being a protected setting. |
@paultaiton Yes, maybe we should find a way to check the update, I will hold this PR and find a solution, thank you! |
Initial discussion of this issue in Azure/azure_preview_modules#326 mentioned an approach where an additional @Fred-sun I am happy to help work on this issue. Would you mind granting me contributor access to your repo/branch so I can update this PR? Or I can open a separate PR with the new changes built on top of your ones. Thanks! |
@l3ender Invitations have been sent! |
Thanks @Fred-sun! I have updated the PR. A few notes:
I ran test cases with the following playbook: ---
- name: "Playbook for testing."
hosts: "localhost"
connection: "local"
gather_facts: false
vars:
resource_group: "automated-testing"
resource_group_secondary: "automated-testing-secondary"
collections:
- azure.azcollection
tasks:
- name: "Include tests"
include_tasks: "tests/integration/targets/azure_rm_virtualmachineextension/tasks/main.yml"
vars:
protected_settings_file: "tests/integration/targets/azure_rm_virtualmachineextension/files/test-protected-settings.json"
public_settings_file: "tests/integration/targets/azure_rm_virtualmachineextension/files/test-public-settings.json" Please take a look and let me know if you have any comments. Thank you! |
Hello @xuzhang3, can you please check on this one? Thank you! |
Hello, any update on this PR? Thank you! |
I have not looked over the entirety of the PR, but the aproach that @l3ender suggested of a forced-update tag would completely elimanate any of my prior concerns. |
@l3ender LGTM shipping it |
SUMMARY
Ignore comparing protected_settings, Try to fixes #473
ISSUE TYPE
COMPONENT NAME
azure_rm_virtualmachineextension.py
ADDITIONAL INFORMATION