`az keyvault create` silently zaps existing access policy when run on existing vault

[ppanyukov]

Describe the bug

A bug or a "feature"? Regardless, makes az keyvault create rather unsafe.

When we run az keyvault create --name=EXISTING_VAULT and EXISTING_VAULT does exist, the command succeeds. However, any access policy set up in that vault is now gone and has to be recreated again.

The az keyvault create --name=EXISTING_VAULT does not produce any warning. Neither it does allow to fail is vault already exists.

To Reproduce

1. Run az keyvault create --name=EXISTING_VAULT
2. Set up extensive access policy on it in Azure Portal.
3. Run az keyvault create --name=EXISTING_VAULT again.
4. Observe in Azure Portal the policy is now all gone and is now in default state.

Expected behavior

1. Subsequent runs of az keyvault create --name=EXISTING_VAULT preserve all settings unless these differ as supplied by command line parameters. All access policies should be preserved. Essentially, this should be "don't change anything unless told" and should be safe to run at any times with the same command line args.

Environment summary
brew on OS X:

az --version
azure-cli (2.0.48)

[tjprescott]

This is expected behavior for nearly all create commands in Azure CLI. If you want it to preserve the existing settings, you must use the update command. You can check whether the vault already exists by running az keyvault show. If it does not, you will get exit code 3.

[ppanyukov]

@tjprescott

I'm not sure I agree with the summary closure of this issue due to "This is expected behavior for nearly all create commands" -- note nearly here.

Cases in point:

• az group create always succeeds but does not zap all resources if the resource group already exists
• az storage container create always succeeds but does not zap the contents of the existing blob container; and additionally has --fail-on-exist parameter

It would be hilarious it if the above commands followed az keyvault create behaviour, would you agree? :)

The problems with using "You can check whether the vault already exists by running az keyvault show. If it does not, you will get exit code 3" are several.

1. It's a pain in the butt to remember and code for this every time;
2. Running az keyvault create is very easy and existing behaviour makes it really unsafe and can break a lot of stuff;
3. It is it's racy -- just because the vault didn't exist when we checked for it does not mean it does not exist when we start running az keyvault create.

I think it would be better for everyone if at the very minimum all potentially destructive commands, including az keyvault create fail if the target resource exists, with advice error message to use update rather than silently zapping.

And for those who really want unsafe current behaviour -- add --force flag or something.

Please reconsider.

---

EDIT: The az keyvault create is also inconsistent in its current behaviour -- it zaps the access policies and changes tenant due to tenant bug (and who knows what else) but does not zap the secrets and keys -- thankfully of course, otherwise it'd be also very hilarious. Requiring from the users to wade through minute details of what exactly it changes/deletes and what it does not is putting extra pressure on us the users who are already pressed with many other things.

[tjprescott]

I'll leave it open to KeyVault team to respond. If their service supports precondition headers or something similar, this would be something they could incorporate.

[ppanyukov]

I've put out a more general feature/discussion issue which was prompted by this and is related. It would also address this if implemented :)

See #7613

[schaabs]

@ppanyukov @tjprescott After discussion with the key vault team, we have decided to change the behavior to not replace by default, but rather error, and add a --force parameter which can be specified to override this. This PR has the change, #7948.

[schaabs]

@ppanyukov I suppose I spoke too soon. From the feedback on this PR looks like there are plans to address this more broadly with #7613. Given that I've closed that PR and I'm closing this issue.