Skip to content

Conversation

@bim-msft
Copy link
Contributor

@bim-msft bim-msft commented Feb 6, 2020

Support for #11268

  • Waiting for Swagger/Python SDK, release time: Apr 15.
  • Waiting for service public.

How to test:

  1. Run az keyvault create -n {} -g {} --enable-rbac-authorization
  2. Check the value of properties.enableRbacAuthorization

This checklist is used to make sure that common guidelines for a pull request are followed.

Comment on lines 217 to 228
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to hard-code our testing environment in the public CLI code. We can register a new cloud instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to hard-code our testing environment in the public CLI code. We can register a new cloud instead.

Good idea, I'll deprecate this part.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The keyvault may be created via Azure Portal, so customer may not used --enable-rbac-authorization to enable RBAC authorization. So how about using

Cannot set policies for a vault with RBAC authorization enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The keyvault may be created via Azure Portal, so customer may not used --enable-rbac-authorization to enable RBAC authorization. So how about using

Cannot set policies for a vault with RBAC authorization enabled

Agree, this is more reasonable, will refine it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In error message we usually use single quote '. Grave accent ` is only for markdown.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In error message we usually use single quote '. Grave accent ` is only for markdown.

Good point, will refine this.

@yonzhan yonzhan added this to the S169 - For Build milestone Apr 11, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Apr 11, 2020

add to S169

type: command
short-summary: Create a key vault.
long-summary: Default permissions are created for the current user or service principal unless the `--no-self-perms` flag is specified.
long-summary: If `--enable-rbac-authorization` is not specified, then default permissions are created for the current user or service principal unless the `--no-self-perms` flag is specified.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long summary is not shown in CLI reference. May you can move it to short summary.

Copy link
Contributor Author

@bim-msft bim-msft Apr 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long summary is not shown in CLI reference. May you can move it to short summary.

@qwordy Good point, but I think it's ok to keep it as long summary, for the docs display issue, we'd better push docs team to display our long summaries as well. Now at least we can see the long summary using -h.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. No problem.

@bim-msft
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@bim-msft bim-msft merged commit 89cf88c into Azure:dev Apr 22, 2020
@bim-msft bim-msft deleted the bim_kv_rbac branch April 22, 2020 12:44
type: command
short-summary: Create a key vault.
long-summary: Default permissions are created for the current user or service principal unless the `--no-self-perms` flag is specified.
long-summary: If `--enable-rbac-authorization` is not specified, then default permissions are created for the current user or service principal unless the `--no-self-perms` flag is specified.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement is cumbersome.

Suggested change
long-summary: If `--enable-rbac-authorization` is not specified, then default permissions are created for the current user or service principal unless the `--no-self-perms` flag is specified.
long-summary: Default permissions are created for the current user or service principal unless the `--no-self-perms` or `--enable-rbac-authorization` flag is specified.

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

Successfully merging this pull request may close these issues.

5 participants