[Synapse] Update role assignment/definition related cmdlets by sunsw1994 · Pull Request #17476 · Azure/azure-cli

sunsw1994 · 2021-03-26T11:31:34Z

Description

Update 'az synapse role' commands to support new role base access control feature.

Testing Guide

az synapse role scope -h
az synapse role assignment -h
az synapse role definition -h

History Notes

[Synapse] az synapse role scope list: List all scopes synapse supports.
[Synapse] az synapse role assignment create/list/delete: Adding --scope/--item-type/--item arguments to support manage role assignments based on scope.
[Synapse] az synapse role assignment create/list/delete: Adding --assignee-object-id argument, it will bypass Graph API and uniquely determine principal object instead of deducing principal object using --assignee argument.
[Synapse] BREAKING CHANGE: az synapse role assignment create: Role names at old version are not allowed, Sql Admin, Apache Spark Admin, Workspace Admin
[Synapse] BREAKING CHANGE: az synapse role assignment create: When --assignee argument can't uniquely determine the principal object, the command will raise error instead of adding a role assignment for the uncertain principal object.

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

The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.

sunsw1994 · 2021-03-26T11:34:40Z

/azp run

azure-pipelines · 2021-03-26T11:34:54Z

Azure Pipelines successfully started running 2 pipeline(s).

yonzhan · 2021-03-26T14:49:09Z

Synapse

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py

zesluo · 2021-03-29T11:36:42Z

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py

+        # workspaces/{workspaceName}/bigDataPools/{bigDataPoolName}
+        scope = "workspaces/" + workspace_name + "/" + item_type + "/" + item
+    else:
+        scope = "workspaces/" + workspace_name


I just want to mention that these two may be not equal from my memory: 1.scope = "workspaces/" + workspace_name 2. scope = null

Hi @zesluo As far as my understand, the default scope is workspaces/{workspace_name}, we can discuss and confirm with @idear1203 .

Thanks for throwing it out.

src/azure-cli/azure/cli/command_modules/synapse/constant.py

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py

idear1203

sunsw1994 · 2021-04-02T09:31:44Z

Hi @jsntcy Could you help to review and merge this PR for Synapse RBAC commands. Thanks a lot.

Juliehzl · 2021-04-06T02:40:45Z

src/azure-cli/azure/cli/command_modules/synapse/_help.py

  - name: Create a role assignment using service principal name.
    text: |-
        az synapse role assignment create --workspace-name testsynapseworkspace \\
-        --role "Sql Admin" --assignee sp_name


does Sql Admin still work?

No, now Synapse is using new role name.
Synapse SQL Administrator
Synapse Apache Spark Administrator
...

https://docs.microsoft.com/en-us/azure/synapse-analytics/security/synapse-workspace-synapse-rbac-roles

In this way, it is breaking change to existing customers and we should try to avoid such breaking.
To not break customers, you should try to make previous definition work and add deprecation info to redirect to new definition.

Can we just allow this breaking change considering that the module is still in preview? Now Synapse moves to new names, we don't want to allow the old names.

Adding breaking change description at History Notes

[Synapse] BREAKING CHANGE: az synapse role assignment create: Role names at old version are not allowed, Sql Admin, Apache Spark Admin, Workspace Admin

I see. According to the telemetry, there is no much usage for this command as it is in preview. So we could accept the breaking change this time. Please note that we need to avoid breaking change as possible as we can in future design.

Juliehzl · 2021-04-06T02:42:52Z

src/azure-cli/azure/cli/command_modules/synapse/constant.py

+class PrincipalType(str, Enum):
+    user = "User"
+    group = "Group"
+    service_principal = "ServicePrincipal"
+
+
+class ItemType(str, Enum):
+    bigDataPools = "bigDataPools"
+    scopePools = "scopePools"
+    integrationRuntimes = "integrationRuntimes"
+    credentials = "credentials"
+    linkedServices = "linkedServices"


why not define in SDK?

@Juliehzl I think this not belong to SDK's scope, we define these item type for CLI using friendly. User can pass item type and CLI code will use this parameter to generate scope which is defined in SDK and received by service.

PrincipalType and ItemType 's purpose is only to limit the user's input

@idear1203 Do we have a plan to move it to SDK?

Currently we don't have such definitions in SDK. This is a good suggestion.

Juliehzl · 2021-04-06T02:45:46Z

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py

+def list_role_assignments(cmd, workspace_name, role=None, assignee=None, assignee_object_id=None,
+                          scope=None, item=None, item_type=None):
+    if bool(assignee) and bool(assignee_object_id):
+        raise CLIError('usage error: --assignee STRING | --assignee-object-id GUID')


Consider the following error type

azure-cli/src/azure-cli-core/azure/cli/core/azclierror.py

Lines 152 to 154 in 3bff786

class InvalidArgumentValueError(UserFault):

""" Argument value is not valid. """

pass

azure-cli/src/azure-cli-core/azure/cli/core/azclierror.py

Lines 157 to 161 in 3bff786

class ArgumentUsageError(UserFault):

""" Fallback of the argument usage related errors.

Avoid using this class unless the error can not be classified

into the Argument related specific error types. """

pass

Juliehzl · 2021-04-06T02:51:07Z

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py

+    if bool(assignee) and bool(assignee_object_id):
+        raise CLIError('usage error: --assignee STRING | --assignee-object-id GUID')
+
+    if bool(item) != bool(item_type):


same as above

src/azure-cli/azure/cli/command_modules/synapse/_help.py

src/azure-cli/azure/cli/command_modules/synapse/commands.py

src/azure-cli/azure/cli/command_modules/synapse/constant.py

Juliehzl · 2021-04-06T03:27:20Z

src/azure-cli/azure/cli/command_modules/synapse/commands.py

        g.custom_show_command('show', 'get_role_definition')

+    with self.command_group('synapse role scope', synapse_role_definitions_sdk,
+                            custom_command_type=get_custom_sdk('accesscontrol', None)) as g:


You could declare your client factory here and don't need to define in every fuction.

src/azure-cli/azure/cli/command_modules/synapse/commands.py

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py

…ue to server side issue.

sunsw1994 · 2021-04-07T07:37:29Z

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py

                           "If the assignee is a principal id, make sure the corresponding principal is created "
                           "with 'az ad sp create --id {assignee}'.".format(assignee=assignee))

+        if len(result) > 1:


Add breaking change description at History Notes:

[Synapse] BREAKING CHANGE: az synapse role assignment create: When --assignee can't uniquely determine the principal object, the command will raise error instead of adding a role assignment for the uncertain principal object.

sunsw1994 · 2021-04-07T08:01:43Z

@evelyn-ys @Juliehzl Thanks for your comments, these are helpful! I have fixed them and updated history notes with known breaking change information, could you help to review it and merge this PR. Hope we can catch up the latest release. Thanks a lot！

sunsw1994 added 3 commits March 26, 2021 19:09

update 'az synapse role' commands

38d6c61

update recording file

10f3a22

revert launch.json

c401ddb

sunsw1994 requested review from aim-for-better, idear1203 and zesluo March 26, 2021 11:31

sunsw1994 changed the title ~~Synapse rbac~~ [Synapse] Update role assignment/definition related cmdlets Mar 26, 2021

yonzhan requested a review from jsntcy March 26, 2021 14:49

yonzhan assigned jsntcy Mar 26, 2021

yonzhan added this to the S185 milestone Mar 26, 2021

zesluo reviewed Mar 29, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py Outdated Show resolved Hide resolved

zesluo reviewed Mar 29, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py Show resolved Hide resolved

zesluo reviewed Mar 29, 2021

View reviewed changes

idear1203 reviewed Mar 30, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/constant.py Outdated Show resolved Hide resolved

idear1203 reviewed Mar 30, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py Show resolved Hide resolved

idear1203 approved these changes Mar 30, 2021

View reviewed changes

sunsw1994 added 2 commits April 1, 2021 15:25

address comments

b5774e9

fix pylint error

a3c5667

jsntcy requested review from Juliehzl and evelyn-ys April 6, 2021 00:24

Juliehzl reviewed Apr 6, 2021

View reviewed changes

evelyn-ys reviewed Apr 6, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/_help.py Show resolved Hide resolved

evelyn-ys reviewed Apr 6, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/commands.py Outdated Show resolved Hide resolved

evelyn-ys reviewed Apr 6, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/constant.py Outdated Show resolved Hide resolved

Juliehzl reviewed Apr 6, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/commands.py Show resolved Hide resolved

evelyn-ys reviewed Apr 6, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py Outdated Show resolved Hide resolved

evelyn-ys reviewed Apr 6, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py Outdated Show resolved Hide resolved

evelyn-ys reviewed Apr 6, 2021

View reviewed changes

src/azure-cli/azure/cli/command_modules/synapse/operations/accesscontrol.py Outdated Show resolved Hide resolved

1. address comments 2. remove scope parameter of az role definition d…

4dc2a9b

…ue to server side issue.

evelyn-ys approved these changes Apr 7, 2021

View reviewed changes

sunsw1994 commented Apr 7, 2021

View reviewed changes

sunsw1994 added 2 commits April 7, 2021 15:44

Update help text for argument

6c3eb78

Update help text for argument

bdf53ca

yonzhan assigned Juliehzl Apr 7, 2021

Juliehzl approved these changes Apr 7, 2021

View reviewed changes

Juliehzl merged commit 2cef51d into Azure:dev Apr 8, 2021

	class InvalidArgumentValueError(UserFault):
	""" Argument value is not valid. """
	pass

	class ArgumentUsageError(UserFault):
	""" Fallback of the argument usage related errors.
	Avoid using this class unless the error can not be classified
	into the Argument related specific error types. """
	pass

Conversation

sunsw1994 commented Mar 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sunsw1994 commented Mar 26, 2021

Uh oh!

azure-pipelines bot commented Mar 26, 2021

Uh oh!

yonzhan commented Mar 26, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

idear1203 left a comment

Choose a reason for hiding this comment

Uh oh!

sunsw1994 commented Apr 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juliehzl Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sunsw1994 commented Apr 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sunsw1994 commented Mar 26, 2021 •

edited

Loading

Juliehzl Apr 6, 2021 •

edited

Loading