Skip to content

Commit 097e10f

Browse files
ABZhang0Alan Zhang
andauthored
AMG: fix issue with create role assignment step (#8244)
* adjust create validation and role assignment logic * Update test recording * Fix amw cmd role assignment --------- Co-authored-by: Alan Zhang <[email protected]>
1 parent f0498d0 commit 097e10f

File tree

9 files changed

+1498
-1255
lines changed

9 files changed

+1498
-1255
lines changed

src/amg/HISTORY.rst

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,11 @@ Release History
7979
2.0.0
8080
++++++
8181
* Move existing Grafana CRUD command implementations to AAZ CodeGen
82-
* `az grafana create`: Migrate to AAZDev Tool & implicitly support role assignment principal types
83-
* `az grafana update`: Migrate to AAZDev Tool
84-
* `az grafana list`: Migrate to AAZDev Tool
85-
* `az grafana show`: Migrate to AAZDev Tool
86-
* `az grafana delete`: Migrate to AAZDev Tool
82+
* `az grafana create`: move implementation to AAZDev Tool & implicitly support role assignment principal types
83+
* `az grafana update`: move implementation to AAZDev Tool
84+
* `az grafana list`: move implementation to AAZDev Tool
85+
* `az grafana show`: move implementation to AAZDev Tool
86+
* `az grafana delete`: move implementation to AAZDev Tool
8787

8888
2.1.0
8989
++++++
@@ -114,3 +114,7 @@ Release History
114114
2.5.1
115115
++++++
116116
* `az grafana dashboard import`: validate JSON file content prior to import
117+
118+
2.5.2
119+
++++++
120+
* `az grafana create`: fix issue with principal type implicit selection during role assignment step

src/amg/azext_amg/custom.py

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,18 @@
99
from knack.log import get_logger
1010

1111
from azure.cli.core.commands.client_factory import get_mgmt_service_client, get_subscription_id
12-
from azure.cli.core.profiles import ResourceType, get_sdk
1312
from azure.cli.core.util import should_disable_connection_verify
1413
from azure.cli.core.azclierror import ArgumentUsageError, CLIInternalError, InvalidArgumentValueError, ManualInterrupt
15-
from ._validators import process_grafana_create_namespace
14+
from azure.mgmt.authorization import AuthorizationManagementClient
15+
from azure.mgmt.authorization.models import RoleAssignmentCreateParameters, PrincipalType
1616

1717
from azure.cli.core.aaz import AAZBoolArg, AAZListArg, AAZStrArg
1818
from .aaz.latest.grafana._create import Create as _GrafanaCreate
1919
from .aaz.latest.grafana._delete import Delete as _GrafanaDelete
2020
from .aaz.latest.grafana._update import Update as _GrafanaUpdate
2121

2222
from ._client_factory import cf_amg
23-
from .utils import get_yes_or_no_option, MGMT_SERVICE_CLIENT_API_VERSION
23+
from .utils import get_yes_or_no_option
2424

2525
logger = get_logger(__name__)
2626

@@ -62,8 +62,6 @@ def pre_operations(self):
6262
if not args.skip_system_assigned_identity:
6363
args.identity = {"type": "SystemAssigned"}
6464

65-
process_grafana_create_namespace(self.ctx, self.ctx.args)
66-
6765
# override the output method to create role assignments after instance creation
6866
def _output(self, *args, **kwargs):
6967
from azure.cli.core.commands.arm import resolve_role_id
@@ -86,14 +84,12 @@ def _output(self, *args, **kwargs):
8684
grafana_admin_role_id = resolve_role_id(cli_ctx, "Grafana Admin", subscription_scope)
8785

8886
for principal_id in principal_ids:
89-
principal_types = {"User", "Group"}
90-
_create_role_assignment(cli_ctx, principal_id, principal_types, grafana_admin_role_id,
87+
_create_role_assignment(cli_ctx, principal_id, grafana_admin_role_id,
9188
self.ctx.vars.instance.id)
9289

9390
if self.ctx.vars.instance.identity:
9491
monitoring_reader_role_id = resolve_role_id(cli_ctx, "Monitoring Reader", subscription_scope)
95-
principal_types = {"ServicePrincipal"}
96-
_create_role_assignment(cli_ctx, self.ctx.vars.instance.identity.principal_id, {"ServicePrincipal"},
92+
_create_role_assignment(cli_ctx, self.ctx.vars.instance.identity.principal_id,
9793
monitoring_reader_role_id, subscription_scope)
9894

9995
result = self.deserialize_output(self.ctx.vars.instance, client_flatten=True)
@@ -163,23 +159,22 @@ def _get_login_account_principal_id(cli_ctx):
163159
return result[0]['id']
164160

165161

166-
def _create_role_assignment(cli_ctx, principal_id, principal_types, role_definition_id, scope):
162+
def _create_role_assignment(cli_ctx, principal_id, role_definition_id, scope):
167163
import time
168164
from azure.core.exceptions import HttpResponseError, ResourceExistsError
169165

170-
assignments_client = get_mgmt_service_client(cli_ctx, ResourceType.MGMT_AUTHORIZATION,
171-
api_version=MGMT_SERVICE_CLIENT_API_VERSION).role_assignments
172-
RoleAssignmentCreateParameters = get_sdk(cli_ctx, ResourceType.MGMT_AUTHORIZATION,
173-
'RoleAssignmentCreateParameters', mod='models',
174-
operation_group='role_assignments')
175-
parameters = RoleAssignmentCreateParameters(role_definition_id=role_definition_id,
176-
principal_id=principal_id, principal_type=principal_types.pop())
166+
assignments_client = get_mgmt_service_client(cli_ctx, AuthorizationManagementClient).role_assignments
167+
principal_types = [p.value for p in PrincipalType]
168+
current_principal_type = principal_types.pop(0)
177169

178170
logger.info("Creating an assignment with a role '%s' on the scope of '%s'", role_definition_id, scope)
179171
retry_times = 36
180172
assignment_name = _gen_guid()
181173
for retry_time in range(0, retry_times):
182174
try:
175+
parameters = RoleAssignmentCreateParameters(role_definition_id=role_definition_id,
176+
principal_id=principal_id,
177+
principal_type=current_principal_type)
183178
assignments_client.create(scope=scope, role_assignment_name=assignment_name,
184179
parameters=parameters)
185180
break
@@ -188,9 +183,11 @@ def _create_role_assignment(cli_ctx, principal_id, principal_types, role_definit
188183
break
189184
except HttpResponseError as ex:
190185
if 'UnmatchedPrincipalType' in ex.message: # try each principal_type until we get the right one
191-
parameters = RoleAssignmentCreateParameters(role_definition_id=role_definition_id,
192-
principal_id=principal_id,
193-
principal_type=principal_types.pop())
186+
logger.debug("Principal type '%s' is not matched", current_principal_type)
187+
try:
188+
current_principal_type = principal_types.pop(0)
189+
except:
190+
raise CLIInternalError("Failed to create a role assignment. No matching principal types found.")
194191
continue
195192
if 'role assignment already exists' in ex.message: # Exception from Track-1 SDK
196193
logger.info('Role assignment already exists')
@@ -204,8 +201,7 @@ def _create_role_assignment(cli_ctx, principal_id, principal_types, role_definit
204201

205202

206203
def _delete_role_assignment(cli_ctx, principal_id, role_definition_id=None, scope=None):
207-
assignments_client = get_mgmt_service_client(cli_ctx, ResourceType.MGMT_AUTHORIZATION,
208-
api_version=MGMT_SERVICE_CLIENT_API_VERSION).role_assignments
204+
assignments_client = get_mgmt_service_client(cli_ctx, AuthorizationManagementClient).role_assignments
209205
f = f"principalId eq '{principal_id}'"
210206

211207
if role_definition_id and scope:

src/amg/azext_amg/integrations.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,10 @@ def link_amw_to_amg(cmd, grafana_name, monitor_name, grafana_resource_group_name
4747
if not skip_role_assignments:
4848
subscription_scope = '/'.join(monitor['id'].split('/')[0:3]) # /subscriptions/<sub_id>
4949
monitor_role_id = resolve_role_id(cmd.cli_ctx, "Monitoring Data Reader", subscription_scope)
50-
principal_types = {"ServicePrincipal"}
5150
# assign the Grafana instance the Monitoring Data Reader role on the Azure Monitor workspace
5251
logger.warning("Azure Monitor workspace of '%s' was linked to the Grafana instance. Now assigning the Grafana"
5352
" instance the Monitoring Data Reader role on the Azure Monitor workspace.", monitor['name'])
54-
_create_role_assignment(cmd.cli_ctx, principal_id, principal_types, monitor_role_id, monitor['id'])
53+
_create_role_assignment(cmd.cli_ctx, principal_id, monitor_role_id, monitor['id'])
5554

5655

5756
def unlink_amw_from_amg(cmd, grafana_name, monitor_name, grafana_resource_group_name, monitor_resource_group_name,

0 commit comments

Comments
 (0)