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

Feature/Discussion: provide safety net for create commands and make them consistent #7613

Open
ppanyukov opened this issue Oct 18, 2018 · 5 comments
Assignees
Labels
Core CLI core infrastructure Discussion feature-request idempotence Infrastructure Service Attention This issue is responsible by Azure service team.
Milestone

Comments

@ppanyukov
Copy link

ppanyukov commented Oct 18, 2018

Is your feature request related to a problem? Please describe.

This is prompted by "#7608: az keyvault create silently zaps existing access policy when run on existing vault"

The crux of the problem is:

  • Some/most create commands are destructive and will silently do their destructive deed.

  • There is no safety net.

  • This is not the default behaviour I want in production -- it's too scary.

  • Relying on humans not forget to check if resource already exists before
    running create is not the answer in the absence of safety net.

  • With some create commands like az storage container create being
    safe to run at any time, it makes it easier to forget that other commands
    are not safe.

  • Destructive commands are inconsistent and undocumented in what they destroy:
    e.g. az keyvault create zaps access policies but leaves the secrets intact.
    Practical test seems to be the only way to be 100% sure and it's a poor option.

  • It is well within Azure CLI's capabilities to do the "remembering" and
    safety net for us and thus provice a better experience here.


Describe the solution you'd like

The goal of the solution is:

  • Make Azure CLI much safer to use;
  • Bring consistency to the all commands;
  • Do the "remembering" work for us humans;
  • No nasty surprises.

The proposal is:

For all safe non-destructive create commands:

  • A safe create command is a command which:

    • Safe to run at any time
    • Never destroys or changes anything if target resource exists
    • Actually creates target resource only if it does not exist
    • Note that if we run create with parameters such that
      it would lead to any chages in target resource it is not
      a safe command.
    • Example: az group create (it's safe, right?)
  • We can run it at any time just like now.

  • Consider for all such commands to have --fail-on-exist flag for
    consistency (see az storage container create).

For all non-safe destructive create commands:

  • A non-safe create command is any command which:

    • Destroys all or part of the target resource (see az keyvault create)
    • Makes any changes to the target resource if it exists
    • Some normally safe commands may become unsafe depending on paramteres.
  • Default behaviour for these should be:

    • Command always fails if the target resource already exists.
    • This reminds people to use if resource exists; then update; else create pattern.
    • This provides much needed safety net.
  • For legacy and "I know what I'm doing" scenarios:

    • Supply --force or similar flag.
    • This will enable to current destructive behaviour for
      compatility/whatever other reasons.

Describe alternatives you've considered

None really, open to suggestions which work towards the goals stated above.


Additional context

  • We have lots of people who use Azure CLI infrequently and in production.
    It's very hard for them to remember what is safe and what isn't
    and exact behaviour of each command, plus remembering to check for
    resource existence when required. I include myself in this category.

  • We have lots of automation and unattended deployments going on into all
    environments, any human error in scripting Azure CLI is a potential disaster.
    Making things safer would make things way way better.


EDIT1: corrected link to the issue which prompted this

@ukphillips
Copy link

We just stumbled across this, specifically #7608 and these guardrails would be greatly appreciated. Our initial expectation was that it would have failed if something already existed (especially something that has an externally unique requirement - like a DNS name). I think the proposed behavior is much more natural and is what a developer would expect.

@tjprescott tjprescott mentioned this issue Nov 29, 2018
2 tasks
@tjprescott
Copy link
Member

CLI commands are intended to be idempotent (though some fall short of this standard) and thus failing a create because a resource already exists by default is not something we would entertain if for no other reason that it would be a massively breaking change.

However, we are open to the idea of exposing an opt-in existence check like --fail-if-exists that create commands can opt into, and we can even consider allowing users to configure this as TRUE by default through az configure.

@yonzhan
Copy link
Collaborator

yonzhan commented Dec 9, 2019

add to S164.

@yonzhan yonzhan added the Core CLI core infrastructure label Dec 25, 2019
@jiasli jiasli assigned bim-msft, jiasli, mmyyrroonn and Juliehzl and unassigned bim-msft and jiasli Jan 15, 2020
@jiasli
Copy link
Member

jiasli commented Aug 10, 2021

Keyvault creation API Vaults - Create Or Update fails if the request body doesn't include accessPolicies:

PUT https://management.azure.com/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourceGroups/rg0810/providers/Microsoft.KeyVault/vaults/kv0810?api-version=2021-04-01-preview

{
  "id": "/subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/resourceGroups/rg0810/providers/Microsoft.KeyVault/vaults/kv0810",
  "location": "westus",
  "name": "kv0810",
  "properties": {
    "createMode": null,
    ... // No accessPolicies
  }
}
400 Bad Request

{
    "error": {
        "code": "BadRequest",
        "message": "The parameter accessPolicies is not specified."
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core CLI core infrastructure Discussion feature-request idempotence Infrastructure Service Attention This issue is responsible by Azure service team.
Projects
None yet
Development

No branches or pull requests