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

az storage account network-rule add is not idempotent #10673

Closed
jurjenoskam opened this issue Sep 27, 2019 · 9 comments
Closed

az storage account network-rule add is not idempotent #10673

jurjenoskam opened this issue Sep 27, 2019 · 9 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage az storage
Milestone

Comments

@jurjenoskam
Copy link

(azure-cli 2.0.74, I did not try other versions)

az storage account network-rule add is not idempotent. The first time I run it it works, subsequent invocations result in:

Values for request parameters are invalid: networkAcls.virtualNetworkRules[*].id(unique). For more information, see - https://aka.ms/storagenetworkruleset

In the spirit of https://github.com/Azure/azure-cli/blob/dev/doc/command_guidelines.md#standard-command-types I believe the subsequent operations should not result in an error, but be a no-op instead (making this command idempotent).

@maggiepint maggiepint added the Storage az storage label Sep 27, 2019
@maggiepint maggiepint added customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Attention This issue is responsible by Azure service team. triage labels Sep 27, 2019
@ghost
Copy link

ghost commented Sep 27, 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage

@jiasli jiasli removed the Service Attention This issue is responsible by Azure service team. label Sep 30, 2019
@Juliehzl
Copy link
Contributor

Hi @jurjenoskam , thanks for your feedback. We are looking into it.

@haroldrandom haroldrandom added customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage az storage labels Oct 25, 2019
@toddplu
Copy link

toddplu commented Dec 31, 2019

Any updates?

@yonzhan yonzhan added this to the S164 milestone Jan 1, 2020
@GeeWee
Copy link

GeeWee commented Jan 7, 2020

I have also had to create custom workarounds for this error.
Seems like there are no errors when removing network rules that don't exists, so the workaround (powershell) we're using is this function:

function Create-Storage-Container-Network-Rule {
    param($StorageAccountName, $VnetName, $SubnetName)

    # Adding network rules is not idempotent, see https://github.com/Azure/azure-cli/issues/10673
    # but removing them does not fail if they do not exist
    # so to avoid errors, we always remove them first and then add them
    echo "Adding subnet '${VnetName}/${SubnetName}' to storage container '${StorageAccountName}'"
    
    az storage account network-rule remove -n ${StorageAccountName} --vnet-name=${VnetName} --subnet=${SubnetName} -o none
    
    az storage account network-rule add -n ${StorageAccountName} --vnet-name=${VnetName} --subnet=${SubnetName} -o none

}

@yonzhan
Copy link
Collaborator

yonzhan commented Feb 15, 2020

add to S166.

@qianwens
Copy link
Member

Hi @jurjenoskam , this error is thrown from the service side. "add" a network-rule in a storage account is a patch operation because it is a partial update of a resource which is not guaranteed to be idempotent -> https://docs.microsoft.com/en-us/azure/architecture/best-practices/api-design.
You can run az storage account network-rule list first to check if the networkrule exists first.

@jurjenoskam
Copy link
Author

Hi @qianwens , thank you for your response. I understand the error is thrown from the service side, and I have been adding code to my scripts to circumvent that this operation is not idempotent. The issue is that I have to do this each time in every script where this operation is done, and this applies to everybody else writing Azure CLI scripts as well. In other words, lots and lots of duplicated work and code, while if this were done once (in Azure CLI itself) it would making writing scripts much simpler and make those script more robust.

Surely Azure CLI is not meant to be a direct passthrough of service API behavior? There are already many places where Azure CLI abstracts away service API behavior, from implementing retry logic when a service API call returns an error (e.g. many App Service commands) to building entire ARM templates and deploying them (VM commands).

It would really add value to Azure CLI if it would make its CLI commands idempotent, irrespective of the behavior of the underlying API call; so I hope you'll consider adding this logic in Azure CLI.

@qianwens qianwens added the bug This issue requires a change to an existing behavior in the product in order to be resolved. label Feb 23, 2020
@qianwens
Copy link
Member

@jurjenoskam , thanks for your feedback. This is a bug in Azure CLI and we will fix it.

@yonzhan
Copy link
Collaborator

yonzhan commented Mar 5, 2020

PR merged and will be released in this Sprint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. customer-reported Issues that are reported by GitHub users external to the Azure organization. Storage az storage
Projects
None yet
Development

No branches or pull requests

9 participants