Skip to content

Commit 16afcbd

Browse files
committed
Remove unnecessary network call in az acr cache subcommand
`az acr cache create` and `az acr cache update` are used to create and update cache rules under Azure Container Registry. A _cache rule_ is a sub resource of a container registry. Currently, when the above two operations are performed, the code first makes a GET request to _registry_ resource, only to get the id of the resource by its name. It needs the registry id in order to create credential set id which is needed when creating and updating a cache rule. The credential set id can be determined without making the above GET request. This GET request is made on the registry resource, which implies the executing user must have 'Reader' role of the registry. This could lead to user overprivileging roles, e.g. now they have to give executing user registry read permissions, in additoinal to cache rule CRUD permissions. This commit removed the GET registry request in cache create and cache update workflow. As a result, a role with only cache rule permissions is able to execute cache rule create and update. This makes our security model clearer, and also makes the operation more efficient.
1 parent 89c41db commit 16afcbd

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

src/azure-cli/azure/cli/command_modules/acr/_constants.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
WEBHOOK_RESOURCE_TYPE = REGISTRY_RESOURCE_TYPE + '/webhooks'
1212
REPLICATION_RESOURCE_TYPE = REGISTRY_RESOURCE_TYPE + '/replications'
1313

14+
CREDENTIAL_SET_RESOURCE_ID_TEMPLATE = '/subscriptions/{sub_id}/resourceGroups/{rg}/providers/Microsoft.ContainerRegistry/registries/{reg_name}/credentialSets/{cred_set_name}'
15+
1416
TASK_RESOURCE_TYPE = REGISTRY_RESOURCE_TYPE + '/tasks'
1517
TASK_VALID_VSTS_URLS = ['visualstudio.com', 'dev.azure.com']
1618
TASK_RESOURCE_ID_TEMPLATE = '/subscriptions/{sub_id}/resourceGroups/{rg}/providers/Microsoft.ContainerRegistry/registries/{reg}/tasks/{name}'

src/azure-cli/azure/cli/command_modules/acr/cache.py

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
# Licensed under the MIT License. See License.txt in the project root for license information.
44
# --------------------------------------------------------------------------------------------
55

6+
from ._constants import CREDENTIAL_SET_RESOURCE_ID_TEMPLATE
67
from ._utils import get_registry_by_name, get_resource_group_name_by_registry_name
78
from azure.cli.core.azclierror import InvalidArgumentValueError
9+
from azure.cli.core.commands.client_factory import get_subscription_id
810
from azure.core.serialization import NULL as AzureCoreNull
911

10-
1112
def acr_cache_show(cmd,
1213
client,
1314
registry_name,
@@ -54,9 +55,18 @@ def acr_cache_create(cmd,
5455
resource_group_name=None,
5556
cred_set=None):
5657

57-
registry, rg = get_registry_by_name(cmd.cli_ctx, registry_name, resource_group_name)
58-
59-
cred_set_id = AzureCoreNull if not cred_set else f'{registry.id}/credentialSets/{cred_set}'
58+
if cred_set:
59+
sub_id = get_subscription_id(cmd.cli_ctx)
60+
rg = get_resource_group_name_by_registry_name(cmd.cli_ctx, registry_name, resource_group_name)
61+
# Format the credential set ID using subscription ID, resource group, registry name, and credential set name
62+
cred_set_id = CREDENTIAL_SET_RESOURCE_ID_TEMPLATE.format(
63+
sub_id=sub_id,
64+
rg=rg,
65+
reg_name=registry_name,
66+
cred_set_name=cred_set
67+
)
68+
else:
69+
cred_set_id = AzureCoreNull
6070

6171
CacheRuleCreateParameters = cmd.get_models('CacheRule', operation_group='cache_rules')
6272

@@ -82,9 +92,18 @@ def acr_cache_update_custom(cmd,
8292
if cred_set is None and remove_cred_set is False:
8393
raise InvalidArgumentValueError("You must either update the credential set ID or remove it.")
8494

85-
registry, _ = get_registry_by_name(cmd.cli_ctx, registry_name, resource_group_name)
86-
87-
cred_set_id = AzureCoreNull if remove_cred_set else f'{registry.id}/credentialSets/{cred_set}'
95+
if remove_cred_set:
96+
cred_set_id = AzureCoreNull
97+
else:
98+
sub_id = get_subscription_id(cmd.cli_ctx)
99+
rg = get_resource_group_name_by_registry_name(cmd.cli_ctx, registry_name, resource_group_name)
100+
# Format the credential set ID using subscription ID, resource group, registry name, and credential set name
101+
cred_set_id = CREDENTIAL_SET_RESOURCE_ID_TEMPLATE.format(
102+
sub_id=sub_id,
103+
rg=rg,
104+
reg_name=registry_name,
105+
cred_set_name=cred_set
106+
)
88107

89108
instance.credential_set_resource_id = cred_set_id
90109

0 commit comments

Comments
 (0)