From 594496f389f3b0af1896e658150ef9603eaed7bd Mon Sep 17 00:00:00 2001
From: Rodrigo Mendoza <43052640+rosanch@users.noreply.github.com>
Date: Mon, 5 Apr 2021 00:47:33 -0700
Subject: [PATCH 1/6] new command to manage connected registry repo permissions
 update

---
 .../azure/cli/command_modules/acr/_help.py    |  12 ++
 .../azure/cli/command_modules/acr/_params.py  |   7 +-
 .../azure/cli/command_modules/acr/_utils.py   |   4 +-
 .../azure/cli/command_modules/acr/commands.py |   1 +
 .../command_modules/acr/connected_registry.py | 168 +++++++++++++++---
 5 files changed, 161 insertions(+), 31 deletions(-)

diff --git a/src/azure-cli/azure/cli/command_modules/acr/_help.py b/src/azure-cli/azure/cli/command_modules/acr/_help.py
index 9ab111d52ce..d4ecec9d9ea 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_help.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_help.py
@@ -1327,6 +1327,18 @@
     text: >
         az acr connected-registry install renew-credentials -r mycloudregistry -n myconnectedregistry
 """
+
+helps['acr connected-registry repo'] = """
+type: command
+short-summary: Updates all the necessary connected registry sync scope maps repository permissions.
+examples:
+  - name: Adds the 'myconnectedregistry' mode repository permissions to it and its ancestors' sync scope map actions.
+    text: >
+        az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --add repo1 repo2
+  - name: removes all 'repo1' and 'repo2' permissions from 'myconnectedregistry' and its sucesors sync scope maps actions.
+    text: >
+        az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --add repo1 repo2
+"""
 # endregion
 
 # region private-endpoint-connection
diff --git a/src/azure-cli/azure/cli/command_modules/acr/_params.py b/src/azure-cli/azure/cli/command_modules/acr/_params.py
index dc2f2390b4d..d429b639606 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_params.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_params.py
@@ -407,7 +407,7 @@ def load_arguments(self, _):  # pylint: disable=too-many-statements
 
     with self.argument_context('acr connected-registry create') as c:
         c.argument('log_level', help='Sets the log level for logging on the instance. Accepted log levels are Debug, Information, Warning, Error, and None.', required=False, default="Information")
-        c.argument('mode', options_list=['--mode', '-m'], help='Can be one of the two operating modes: registry or mirror(pull-only mode).', required=False, default="registry")
+        c.argument('mode', options_list=['--mode', '-m'], help='Can be one of the two operating modes: registry or mirror(pull-only mode).', required=False, default="Registry")
         c.argument('client_token_list', options_list=['--client-tokens'], nargs='+', help='Specifies the client access to the repositories in the connected registry. It can be in the format [TOKEN_NAME01] [TOKEN_NAME02]...', required=False)
         c.argument('sync_window', options_list=['--sync-window', '-w'], help='Required parameter if --sync-schedule is present. Used to determine the schedule duration. Uses ISO 8601 duration format.', required=False)
         c.argument('sync_schedule', options_list=['--sync-schedule', '-s'], help='Optional parameter to define the sync schedule. Uses cron expression to determine the schedule. If not specified, the instance is considered always online and attempts to sync every minute.', required=False, default="* * * * *")
@@ -423,6 +423,11 @@ def load_arguments(self, _):  # pylint: disable=too-many-statements
         c.argument('sync_schedule', options_list=['--sync-schedule', '-s'], help='Optional parameter to define the sync schedule. Uses cron expression to determine the schedule. If not specified, the instance is considered always online and attempts to sync every minute.', required=False)
         c.argument('sync_message_ttl', help='Determines how long the sync messages will be kept in the cloud. Uses ISO 8601 duration format.', required=False)
 
+    with self.argument_context('acr connected-registry repo') as c:
+        c.argument('add_repos', options_list=['--add'], nargs='*', required=False,
+                   help='repository permissions to be added to the targeted connected registry and it\'s ancestors sync scope maps. Use the format "--add [REPO1 REPO2 ...]" per flag. ' + repo_valid_actions)
+        c.argument('remove_repos', options_list=['--remove'], nargs='*', required=False,
+                   help='respsitory permissions to be removed from the targeted connected registry and it\'s succesors sync scope maps. Use the format "--remove [REPO1 REPO2 ...]" per flag. ' + repo_valid_actions)
 
 def _get_helm_default_install_location():
     exe_name = 'helm'
diff --git a/src/azure-cli/azure/cli/command_modules/acr/_utils.py b/src/azure-cli/azure/cli/command_modules/acr/_utils.py
index 95e5176510b..d4fafe7d099 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_utils.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_utils.py
@@ -472,7 +472,7 @@ def parse_repositories_from_actions(actions):
     return list(set(repositories))
 
 
-def parse_scope_map_actions(repository_actions_list, gateway_actions_list):
+def parse_scope_map_actions(repository_actions_list, gateway_actions_list=None):
     from .scope_map import RepoScopeMapActions, GatewayScopeMapActions
     valid_actions = {action.value for action in RepoScopeMapActions}
     actions = _parse_scope_map_actions(repository_actions_list, valid_actions, 'repositories')
@@ -486,7 +486,7 @@ def _parse_scope_map_actions(actions_list, valid_actions, action_prefix):
         return []
     actions = []
     for rule in actions_list:
-        resource = rule[0]
+        resource = rule[0].lower()
         if len(rule) < 2:
             raise CLIError('At least one action must be specified with "{}".'.format(resource))
         for action in rule[1:]:
diff --git a/src/azure-cli/azure/cli/command_modules/acr/commands.py b/src/azure-cli/azure/cli/command_modules/acr/commands.py
index 6f8618a6e3b..e599217b97f 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/commands.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/commands.py
@@ -359,6 +359,7 @@ def _helm_deprecate_message(self):
         g.show_command('show', 'acr_connected_registry_show')
         g.command('deactivate', 'acr_connected_registry_deactivate')
         g.command('update', 'acr_connected_registry_update')
+        g.command('repo', 'acr_connected_registry_repo')
         g.command('install info', 'acr_connected_registry_install_info')
         g.command('install renew-credentials', 'acr_connected_registry_install_renew_credentials')
         g.command('list', 'acr_connected_registry_list',
diff --git a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
index 616e5620ae6..373026f68f5 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
@@ -11,18 +11,20 @@
 from azure.cli.core.commands.client_factory import get_subscription_id
 from ._client_factory import cf_acr_tokens, cf_acr_scope_maps
 from ._utils import (
-    get_registry_by_name,
-    validate_managed_registry,
-    user_confirmation,
+    build_token_id,
     create_default_scope_map,
+    get_registry_by_name,
+    get_scope_map_from_id,
     get_token_from_id,
-    build_token_id
+    parse_scope_map_actions,
+    user_confirmation,
+    validate_managed_registry
 )
 
 
 class ConnectedRegistryModes(Enum):
-    MIRROR = 'mirror'
-    REGISTRY = 'registry'
+    MIRROR = 'Mirror'
+    REGISTRY = 'Registry'
 
 
 DEFAULT_GATEWAY_SCOPE = ['config/read', 'config/write', 'message/read', 'message/write']
@@ -68,6 +70,7 @@ def acr_connected_registry_create(cmd,  # pylint: disable=too-many-locals, too-m
 
     ErrorResponseException = cmd.get_models('ErrorResponseException')
     parent = None
+    mode = mode.capitalize()
     if parent_name:
         try:
             parent = acr_connected_registry_show(cmd, client, parent_name, registry_name, resource_group_name)
@@ -75,7 +78,7 @@ def acr_connected_registry_create(cmd,  # pylint: disable=too-many-locals, too-m
             if ex.response.status_code == 404:
                 raise CLIError("The parent connected registry '{}' could not be found.".format(parent_name))
             raise CLIError(ex)
-        if parent.mode.lower() != ConnectedRegistryModes.REGISTRY.value and parent.mode.lower() != mode.lower():
+        if parent.mode != ConnectedRegistryModes.REGISTRY.value and parent.mode != mode:
             raise CLIError("Can't create the registry '{}' with mode '{}' ".format(connected_registry_name, mode) +
                            "when the connected registry parent '{}' mode is '{}'. ".format(parent_name, parent.mode) +
                            "For more information on connected registries " +
@@ -271,19 +274,10 @@ def acr_connected_registry_list(cmd,
         else:
             result = [registry for registry in connected_registry_list if not registry.parent.id]
     elif parent_name:
-        family_tree = {}
-        for registry in connected_registry_list:
-            family_tree[registry.id] = {
-                "registry": registry,
-                "childs": []
-            }
-            if registry.name == parent_name:
-                root_parent_id = registry.id
-        for registry in connected_registry_list:
-            parent_id = registry.parent.id
-            if parent_id and not parent_id.isspace():
-                family_tree[parent_id]["childs"].append(registry.id)
-        result = _get_descendancy(family_tree, root_parent_id)
+        family_tree, parent = _get_family_tree(connected_registry_list, parent_name)
+        if parent is None:
+            raise CLIError("Parent connected registry '{}' doesn't exist.".format(parent_name))
+        result = _get_lineage(family_tree, parent.id)
     else:
         result = connected_registry_list
     return result
@@ -327,7 +321,7 @@ def _create_sync_token(cmd,
                        mode):
     token_client = cf_acr_tokens(cmd.cli_ctx)
 
-    mode = mode.lower()
+    mode = mode.capitalize()
     if not any(option for option in ConnectedRegistryModes if option.value == mode):
         raise CLIError("usage error: --mode supports only 'registry' and 'mirror' values.")
     repository_actions_list = [[repo] + REPO_SCOPES_BY_MODE[mode] for repo in repositories]
@@ -360,14 +354,34 @@ def _create_sync_token(cmd,
         raise CLIError(e)
 
 
-def _get_descendancy(family_tree, parent_id):
-    childs = family_tree[parent_id]['childs']
+def _get_family_tree(connected_registry_list, target_connected_registry_name):
+    family_tree = {}
+    targetConnectedRegistry = None
+    # Populate the dictionary
+    for ConnectedRegistry in connected_registry_list:
+        family_tree[ConnectedRegistry.id] = {
+            "connectedRegistry": ConnectedRegistry,
+            "children": []
+        }
+        if ConnectedRegistry.name == target_connected_registry_name:
+            targetConnectedRegistry = ConnectedRegistry
+
+    # Populate Children dependencies
+    for ConnectedRegistry in connected_registry_list:
+        parent_id = ConnectedRegistry.parent.id
+        if parent_id and not parent_id.isspace():
+            family_tree[parent_id]["children"].append(ConnectedRegistry.id)
+    return family_tree, targetConnectedRegistry
+
+
+def _get_lineage(family_tree, parent_id):
+    children = family_tree[parent_id]['children']
     result = []
-    for child_id in childs:
-        result = [family_tree[child_id]["registry"]]
-        descendancy = _get_descendancy(family_tree, child_id)
-        if descendancy:
-            result.extend(descendancy)
+    for child_id in children:
+        result = [family_tree[child_id]["connectedRegistry"]]
+        lineage = _get_lineage(family_tree, child_id)
+        if lineage:
+            result.extend(lineage)
     return result
 
 
@@ -440,3 +454,101 @@ def _get_install_info(cmd,
         "ACR_PARENT_PROTOCOL": "https"
     }
 # endregion
+
+# region connected-registry repo update
+def _update_repo_permissions(cmd,
+                             resource_group_name,
+                             registry_name,
+                             connected_registry,
+                             add_actions_set,
+                             remove_actions_set,
+                             description=None):
+    scope_map_client = cf_acr_scope_maps(cmd.cli_ctx)
+    sync_token = get_token_from_id(cmd, connected_registry.parent.sync_properties.token_id)
+    sync_scope_map = get_scope_map_from_id(cmd, sync_token.scope_map_id)
+    sync_scope_map_name = sync_scope_map.name
+    final_actions_set = set(sync_scope_map.actions).union(add_actions_set).difference(remove_actions_set)
+    current_actions = list(final_actions_set)
+    return scope_map_client.update(
+        resource_group_name,
+        registry_name,
+        sync_scope_map_name,
+        description,
+        current_actions
+    )
+
+def _get_scope_map_actions_set(repos, actions):
+    for i, repo_name in enumerate(repos):
+        repos[i] = [repo_name] + actions
+    return set(parse_scope_map_actions(repos))
+
+
+def acr_connected_registry_repo(cmd,
+                                client,
+                                connected_registry_name,
+                                registry_name,
+                                add_repos=None,
+                                remove_repos=None,
+                                resource_group_name=None):
+    if not (add_repos or remove_repos):
+        raise CLIError('No repository permissions to update.')
+    _, resource_group_name = validate_managed_registry(
+        cmd, registry_name, resource_group_name)
+
+    add_repos_set = set(add_repos) if add_repos is not None else set()
+    remove_repos_set = set(remove_repos) if remove_repos is not None else set()
+    duplicate_repos = set.intersection(add_repos_set, remove_repos_set)
+    if duplicate_repos:
+        errors = sorted(map(lambda action: action[action.rfind('/') + 1:], duplicate_repos))
+        raise CLIError(
+            'Update ambiguity. Duplicate repository names were provided with ' +
+            '--add and --remove arguments.\n{}'.format(errors))
+
+    connected_registry_list = list(client.list(resource_group_name, registry_name))
+    family_tree, target_connected_registry = _get_family_tree(connected_registry_list, connected_registry_name)
+    if target_connected_registry is None:
+        raise CLIError("Connected registry '{}' doesn't exist.".format(connected_registry_name))
+
+    # remove repo permissions from connected registry lineage.
+    remove_actions = REPO_SCOPES_BY_MODE[ConnectedRegistryModes.REGISTRY.value]
+    if remove_repos is not None:
+        remove_repos_txt = ", ".join(remove_repos)
+        remove_repos_set = _get_scope_map_actions_set(remove_repos, remove_actions)
+        lineage = _get_lineage(family_tree, target_connected_registry.id)
+        for connected_registry in lineage:
+            msg = "Removing '{}' permissions from {}".format(remove_repos_txt, connected_registry.name)
+            logger.warning(msg)
+            _update_repo_permissions(cmd, resource_group_name, registry_name,
+                                     connected_registry, set(), remove_repos_set)
+    else:
+        remove_repos_set = set()
+
+    # add repo permissions to ancestors.
+    add_actions = REPO_SCOPES_BY_MODE[target_connected_registry.mode]
+    if add_repos is not None:
+        add_repos_txt = ", ".join(add_repos)
+        add_repos_set = _get_scope_map_actions_set(add_repos, add_actions)
+        parent_id = target_connected_registry.parent.id
+        while parent_id and not parent_id.isspace():
+            connected_registry = family_tree[parent_id]["connectedRegistry"]
+            msg = "Adding '{}' permissions from {}".format(add_repos_txt, connected_registry.name)
+            logger.warning(msg)
+            _update_repo_permissions(cmd, resource_group_name, registry_name,
+                                     connected_registry, add_repos_set, set())
+            parent_id = connected_registry.parent.id
+    else:
+        add_repos_set = set()
+
+    # update target connected registry repo permissions.
+    if add_repos and remove_repos:
+        msg = "Adding '{}' and removing '{}' permissions from {}".format(
+            add_repos_txt, remove_repos_txt, target_connected_registry.name)
+    elif add_repos:
+        msg = "Adding '{}' permissions from {}".format(add_repos_txt, target_connected_registry.name)
+    else:
+        msg = "Removing '{}' permissions from {}".format(remove_repos_txt, target_connected_registry.name)
+    logger.warning(msg)
+    _update_repo_permissions(cmd, resource_group_name, registry_name,
+                             target_connected_registry, add_repos_set, remove_repos_set)
+    return
+# endregion

From 96ddc861c84fa576d0119ec2dc39bd35bc4e868d Mon Sep 17 00:00:00 2001
From: Rodrigo Mendoza <43052640+rosanch@users.noreply.github.com>
Date: Mon, 5 Apr 2021 00:53:50 -0700
Subject: [PATCH 2/6] remove ancestry gateway permissions when succesfully
 deleting a connected registry

---
 .../azure/cli/command_modules/acr/_utils.py       |  2 +-
 .../cli/command_modules/acr/connected_registry.py | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/azure-cli/azure/cli/command_modules/acr/_utils.py b/src/azure-cli/azure/cli/command_modules/acr/_utils.py
index d4fafe7d099..0d07040588f 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_utils.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_utils.py
@@ -472,7 +472,7 @@ def parse_repositories_from_actions(actions):
     return list(set(repositories))
 
 
-def parse_scope_map_actions(repository_actions_list, gateway_actions_list=None):
+def parse_scope_map_actions(repository_actions_list=None, gateway_actions_list=None):
     from .scope_map import RepoScopeMapActions, GatewayScopeMapActions
     valid_actions = {action.value for action in RepoScopeMapActions}
     actions = _parse_scope_map_actions(repository_actions_list, valid_actions, 'repositories')
diff --git a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
index 373026f68f5..3073eb1f787 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
@@ -223,8 +223,23 @@ def acr_connected_registry_delete(cmd,
             token_client = cf_acr_tokens(cmd.cli_ctx)
             scope_map_client = cf_acr_scope_maps(cmd.cli_ctx)
 
+            # Delete target sync scope map and token.
             acr_token_delete(cmd, token_client, registry_name, sync_token_name, yes, resource_group_name)
             acr_scope_map_delete(cmd, scope_map_client, registry_name, sync_scope_map_name, yes, resource_group_name)
+            # Cleanup gateway permissions from ancestors
+            connected_registry_list = list(client.list(resource_group_name, registry_name))
+            family_tree, _ = _get_family_tree(connected_registry_list, None)
+            gateway_actions_list = [[connected_registry_name.lower()] + DEFAULT_GATEWAY_SCOPE]
+            gateway_actions_set = set(parse_scope_map_actions(gateway_actions_list=gateway_actions_list))
+
+            parent_id = connected_registry.parent.id
+            while parent_id and not parent_id.isspace():
+                ancestor = family_tree[parent_id]["connectedRegistry"]
+                msg = "Removing '{}' gateway permissions from {}".format(connected_registry_name, ancestor.name)
+                logger.warning(msg)
+                _update_repo_permissions(cmd, resource_group_name, registry_name,
+                                         ancestor, set(), gateway_actions_set)
+                parent_id = ancestor.parent.id
         else:
             msg = "Connected registry successfully deleted. Please cleanup your sync tokens and scope maps. " + \
                 "Run the following commands for cleanup: \n\t" + \

From d6a2ad54833ba29adebb19b202d3bfd5fa587f62 Mon Sep 17 00:00:00 2001
From: Rodrigo Mendoza <43052640+rosanch@users.noreply.github.com>
Date: Mon, 5 Apr 2021 03:10:09 -0700
Subject: [PATCH 3/6] update gateway and repository action permissions from all
 ancestors before creating a connected registry

---
 .../command_modules/acr/connected_registry.py | 90 ++++++++++++++-----
 1 file changed, 67 insertions(+), 23 deletions(-)

diff --git a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
index 3073eb1f787..319d59fb7c8 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
@@ -27,6 +27,11 @@ class ConnectedRegistryModes(Enum):
     REGISTRY = 'Registry'
 
 
+class ConnectedRegistryActivationStatus(Enum):
+    ACTIVE = 'Active'
+    INACTIVE = 'Inactive'
+
+
 DEFAULT_GATEWAY_SCOPE = ['config/read', 'config/write', 'message/read', 'message/write']
 REPO_SCOPES_BY_MODE = {
     ConnectedRegistryModes.MIRROR.value: ['content/read', 'metadata/read'],
@@ -74,6 +79,8 @@ def acr_connected_registry_create(cmd,  # pylint: disable=too-many-locals, too-m
     if parent_name:
         try:
             parent = acr_connected_registry_show(cmd, client, parent_name, registry_name, resource_group_name)
+            connected_registry_list = list(client.list(resource_group_name, registry_name))
+            family_tree, _ = _get_family_tree(connected_registry_list, None)
         except ErrorResponseException as ex:
             if ex.response.status_code == 404:
                 raise CLIError("The parent connected registry '{}' could not be found.".format(parent_name))
@@ -83,6 +90,13 @@ def acr_connected_registry_create(cmd,  # pylint: disable=too-many-locals, too-m
                            "when the connected registry parent '{}' mode is '{}'. ".format(parent_name, parent.mode) +
                            "For more information on connected registries " +
                            "please visit https://aka.ms/acr/connected-registry.")
+        msg = "Can't create the registry '{}'. The ancestor connected ".format(connected_registry_name) +\
+              "registry activation status is not '{}'. ".format(ConnectedRegistryActivationStatus.ACTIVE.value) +\
+              "Please install the parent connected registry and try again. For more information on connected " +\
+              "registries, please visit https://aka.ms/acr/connected-registry."
+        _check_ancestors_are_active(family_tree, parent.id, msg)
+        _update_ancestor_permissions(cmd, family_tree, resource_group_name, registry_name, parent.id,
+                                     connected_registry_name, repositories, mode, False)
 
     if sync_token_name:
         sync_token_id = build_token_id(subscription_id, resource_group_name, registry_name, sync_token_name)
@@ -205,7 +219,6 @@ def acr_connected_registry_delete(cmd,
                                   cleanup=False,
                                   yes=False,
                                   resource_group_name=None):
-
     _, resource_group_name = validate_managed_registry(
         cmd, registry_name, resource_group_name)
     user_confirmation("Are you sure you want to delete the connected registry '{}' in '{}'?".format(
@@ -229,17 +242,8 @@ def acr_connected_registry_delete(cmd,
             # Cleanup gateway permissions from ancestors
             connected_registry_list = list(client.list(resource_group_name, registry_name))
             family_tree, _ = _get_family_tree(connected_registry_list, None)
-            gateway_actions_list = [[connected_registry_name.lower()] + DEFAULT_GATEWAY_SCOPE]
-            gateway_actions_set = set(parse_scope_map_actions(gateway_actions_list=gateway_actions_list))
-
-            parent_id = connected_registry.parent.id
-            while parent_id and not parent_id.isspace():
-                ancestor = family_tree[parent_id]["connectedRegistry"]
-                msg = "Removing '{}' gateway permissions from {}".format(connected_registry_name, ancestor.name)
-                logger.warning(msg)
-                _update_repo_permissions(cmd, resource_group_name, registry_name,
-                                         ancestor, set(), gateway_actions_set)
-                parent_id = ancestor.parent.id
+            _update_ancestor_permissions(cmd, family_tree, resource_group_name, registry_name,
+                                         connected_registry.parent.id, connected_registry_name, remove_access=True)
         else:
             msg = "Connected registry successfully deleted. Please cleanup your sync tokens and scope maps. " + \
                 "Run the following commands for cleanup: \n\t" + \
@@ -470,6 +474,45 @@ def _get_install_info(cmd,
     }
 # endregion
 
+def _check_ancestors_are_active(family_tree, parent_id, msg):
+    while parent_id and not parent_id.isspace():
+        ancestor = family_tree[parent_id]["connectedRegistry"]
+        if ancestor.activation.status != ConnectedRegistryActivationStatus.ACTIVE.value:
+            raise CLIError(msg)
+        parent_id = ancestor.parent.id
+
+
+def _update_ancestor_permissions(cmd,
+                                 family_tree,
+                                 resource_group_name,
+                                 registry_name,
+                                 parent_id,
+                                 gateway,
+                                 repositories=None,
+                                 mode=None,
+                                 remove_access=False):
+    gateway_actions_list = [[gateway.lower()] + DEFAULT_GATEWAY_SCOPE]
+    if repositories is not None:
+        repository_actions_list = [[repo] + REPO_SCOPES_BY_MODE[mode] for repo in repositories]
+        repo_msg = ", ".join(repositories)
+        repo_msg = " and repo(s) '{}' {} permissions".format(repo_msg, mode)
+    if remove_access:
+        action_txt = "Removing"
+        add_actions_set = set()
+        remove_actions_set = set(parse_scope_map_actions(gateway_actions_list=gateway_actions_list))
+    else:
+        action_txt = "Adding"
+        add_actions_set = set(parse_scope_map_actions(repository_actions_list, gateway_actions_list))
+        remove_actions_set = set()
+
+    while parent_id and not parent_id.isspace():
+        ancestor = family_tree[parent_id]["connectedRegistry"]
+        msg = "{} '{}' gateway permissions{} to connected registry '{}' sync scope map.".format(
+            action_txt, gateway, repo_msg, ancestor.name)
+        _update_repo_permissions(cmd, resource_group_name, registry_name,
+                                 ancestor, add_actions_set, remove_actions_set, msg=msg)
+        parent_id = ancestor.parent.id
+
 # region connected-registry repo update
 def _update_repo_permissions(cmd,
                              resource_group_name,
@@ -477,13 +520,18 @@ def _update_repo_permissions(cmd,
                              connected_registry,
                              add_actions_set,
                              remove_actions_set,
+                             msg=None,
                              description=None):
     scope_map_client = cf_acr_scope_maps(cmd.cli_ctx)
     sync_token = get_token_from_id(cmd, connected_registry.parent.sync_properties.token_id)
     sync_scope_map = get_scope_map_from_id(cmd, sync_token.scope_map_id)
     sync_scope_map_name = sync_scope_map.name
-    final_actions_set = set(sync_scope_map.actions).union(add_actions_set).difference(remove_actions_set)
+    current_actions_set = set(sync_scope_map.actions)
+    final_actions_set = current_actions_set.union(add_actions_set).difference(remove_actions_set)
+    if final_actions_set == current_actions_set:
+        return None
     current_actions = list(final_actions_set)
+    logger.warning(msg)
     return scope_map_client.update(
         resource_group_name,
         registry_name,
@@ -532,9 +580,8 @@ def acr_connected_registry_repo(cmd,
         lineage = _get_lineage(family_tree, target_connected_registry.id)
         for connected_registry in lineage:
             msg = "Removing '{}' permissions from {}".format(remove_repos_txt, connected_registry.name)
-            logger.warning(msg)
             _update_repo_permissions(cmd, resource_group_name, registry_name,
-                                     connected_registry, set(), remove_repos_set)
+                                     connected_registry, set(), remove_repos_set, msg=msg)
     else:
         remove_repos_set = set()
 
@@ -546,24 +593,21 @@ def acr_connected_registry_repo(cmd,
         parent_id = target_connected_registry.parent.id
         while parent_id and not parent_id.isspace():
             connected_registry = family_tree[parent_id]["connectedRegistry"]
-            msg = "Adding '{}' permissions from {}".format(add_repos_txt, connected_registry.name)
-            logger.warning(msg)
+            msg = "Adding '{}' permissions to {}".format(add_repos_txt, connected_registry.name)
             _update_repo_permissions(cmd, resource_group_name, registry_name,
-                                     connected_registry, add_repos_set, set())
+                                     connected_registry, add_repos_set, set(), msg=msg)
             parent_id = connected_registry.parent.id
     else:
         add_repos_set = set()
 
     # update target connected registry repo permissions.
     if add_repos and remove_repos:
-        msg = "Adding '{}' and removing '{}' permissions from {}".format(
+        msg = "Adding '{}' and removing '{}' permissions in {}".format(
             add_repos_txt, remove_repos_txt, target_connected_registry.name)
     elif add_repos:
-        msg = "Adding '{}' permissions from {}".format(add_repos_txt, target_connected_registry.name)
+        msg = "Adding '{}' permissions to {}".format(add_repos_txt, target_connected_registry.name)
     else:
         msg = "Removing '{}' permissions from {}".format(remove_repos_txt, target_connected_registry.name)
-    logger.warning(msg)
     _update_repo_permissions(cmd, resource_group_name, registry_name,
-                             target_connected_registry, add_repos_set, remove_repos_set)
-    return
+                             target_connected_registry, add_repos_set, remove_repos_set, msg=msg)
 # endregion

From 9cbf671f143c7453251fdf675aaa01f377aefd81 Mon Sep 17 00:00:00 2001
From: Rodrigo Mendoza <43052640+rosanch@users.noreply.github.com>
Date: Mon, 5 Apr 2021 14:18:23 -0700
Subject: [PATCH 4/6] pr review

---
 src/azure-cli/azure/cli/command_modules/acr/_help.py           | 2 +-
 src/azure-cli/azure/cli/command_modules/acr/_params.py         | 1 +
 .../azure/cli/command_modules/acr/connected_registry.py        | 3 +++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/azure-cli/azure/cli/command_modules/acr/_help.py b/src/azure-cli/azure/cli/command_modules/acr/_help.py
index d4ecec9d9ea..1204b845516 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_help.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_help.py
@@ -1337,7 +1337,7 @@
         az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --add repo1 repo2
   - name: removes all 'repo1' and 'repo2' permissions from 'myconnectedregistry' and its sucesors sync scope maps actions.
     text: >
-        az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --add repo1 repo2
+        az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --remove repo1 repo2
 """
 # endregion
 
diff --git a/src/azure-cli/azure/cli/command_modules/acr/_params.py b/src/azure-cli/azure/cli/command_modules/acr/_params.py
index d429b639606..02c6682c3eb 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_params.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_params.py
@@ -429,6 +429,7 @@ def load_arguments(self, _):  # pylint: disable=too-many-statements
         c.argument('remove_repos', options_list=['--remove'], nargs='*', required=False,
                    help='respsitory permissions to be removed from the targeted connected registry and it\'s succesors sync scope maps. Use the format "--remove [REPO1 REPO2 ...]" per flag. ' + repo_valid_actions)
 
+
 def _get_helm_default_install_location():
     exe_name = 'helm'
     system = platform.system()
diff --git a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
index 319d59fb7c8..4f23a63f76a 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
@@ -474,6 +474,7 @@ def _get_install_info(cmd,
     }
 # endregion
 
+
 def _check_ancestors_are_active(family_tree, parent_id, msg):
     while parent_id and not parent_id.isspace():
         ancestor = family_tree[parent_id]["connectedRegistry"]
@@ -513,6 +514,7 @@ def _update_ancestor_permissions(cmd,
                                  ancestor, add_actions_set, remove_actions_set, msg=msg)
         parent_id = ancestor.parent.id
 
+
 # region connected-registry repo update
 def _update_repo_permissions(cmd,
                              resource_group_name,
@@ -540,6 +542,7 @@ def _update_repo_permissions(cmd,
         current_actions
     )
 
+
 def _get_scope_map_actions_set(repos, actions):
     for i, repo_name in enumerate(repos):
         repos[i] = [repo_name] + actions

From 708d80699b57df9942b2ce4a9560c07da2ce7d33 Mon Sep 17 00:00:00 2001
From: Rodrigo Mendoza <43052640+rosanch@users.noreply.github.com>
Date: Tue, 6 Apr 2021 02:41:25 -0700
Subject: [PATCH 5/6] pr review 2

---
 .../azure/cli/command_modules/acr/_help.py       |  4 ++--
 .../command_modules/acr/connected_registry.py    | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/azure-cli/azure/cli/command_modules/acr/_help.py b/src/azure-cli/azure/cli/command_modules/acr/_help.py
index 1204b845516..75d0ecd7944 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_help.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_help.py
@@ -1332,10 +1332,10 @@
 type: command
 short-summary: Updates all the necessary connected registry sync scope maps repository permissions.
 examples:
-  - name: Adds the 'myconnectedregistry' mode repository permissions to it and its ancestors' sync scope map actions.
+  - name: Adds the 'myconnectedregistry' mode minimum repository permissions to it and its ancestors' sync scope map actions. Meaning that if the if 'myconnectedregistry' is a mirror, it will only add the mirror permissions to all its ancestors regardless of their mode. 
     text: >
         az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --add repo1 repo2
-  - name: removes all 'repo1' and 'repo2' permissions from 'myconnectedregistry' and its sucesors sync scope maps actions.
+  - name: removes all 'repo1' and 'repo2' existing permissions from 'myconnectedregistry' and its sucesors sync scope maps actions.
     text: >
         az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --remove repo1 repo2
 """
diff --git a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
index 4f23a63f76a..a0146c215c3 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py
@@ -296,7 +296,7 @@ def acr_connected_registry_list(cmd,
         family_tree, parent = _get_family_tree(connected_registry_list, parent_name)
         if parent is None:
             raise CLIError("Parent connected registry '{}' doesn't exist.".format(parent_name))
-        result = _get_lineage(family_tree, parent.id)
+        result = _get_descendants(family_tree, parent.id)
     else:
         result = connected_registry_list
     return result
@@ -393,14 +393,14 @@ def _get_family_tree(connected_registry_list, target_connected_registry_name):
     return family_tree, targetConnectedRegistry
 
 
-def _get_lineage(family_tree, parent_id):
+def _get_descendants(family_tree, parent_id):
     children = family_tree[parent_id]['children']
     result = []
     for child_id in children:
         result = [family_tree[child_id]["connectedRegistry"]]
-        lineage = _get_lineage(family_tree, child_id)
-        if lineage:
-            result.extend(lineage)
+        descendants = _get_descendants(family_tree, child_id)
+        if descendants:
+            result.extend(descendants)
     return result
 
 
@@ -575,13 +575,13 @@ def acr_connected_registry_repo(cmd,
     if target_connected_registry is None:
         raise CLIError("Connected registry '{}' doesn't exist.".format(connected_registry_name))
 
-    # remove repo permissions from connected registry lineage.
+    # remove repo permissions from connected registry descendants.
     remove_actions = REPO_SCOPES_BY_MODE[ConnectedRegistryModes.REGISTRY.value]
     if remove_repos is not None:
         remove_repos_txt = ", ".join(remove_repos)
         remove_repos_set = _get_scope_map_actions_set(remove_repos, remove_actions)
-        lineage = _get_lineage(family_tree, target_connected_registry.id)
-        for connected_registry in lineage:
+        descendants = _get_descendants(family_tree, target_connected_registry.id)
+        for connected_registry in descendants:
             msg = "Removing '{}' permissions from {}".format(remove_repos_txt, connected_registry.name)
             _update_repo_permissions(cmd, resource_group_name, registry_name,
                                      connected_registry, set(), remove_repos_set, msg=msg)

From d57e3b8b316c546c2abb6773cc6965aff8c41e01 Mon Sep 17 00:00:00 2001
From: Rodrigo Mendoza <43052640+rosanch@users.noreply.github.com>
Date: Thu, 8 Apr 2021 13:27:07 -0700
Subject: [PATCH 6/6] Update help info

---
 src/azure-cli/azure/cli/command_modules/acr/_help.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/azure-cli/azure/cli/command_modules/acr/_help.py b/src/azure-cli/azure/cli/command_modules/acr/_help.py
index 75d0ecd7944..1d59680266c 100644
--- a/src/azure-cli/azure/cli/command_modules/acr/_help.py
+++ b/src/azure-cli/azure/cli/command_modules/acr/_help.py
@@ -1332,12 +1332,15 @@
 type: command
 short-summary: Updates all the necessary connected registry sync scope maps repository permissions.
 examples:
-  - name: Adds the 'myconnectedregistry' mode minimum repository permissions to it and its ancestors' sync scope map actions. Meaning that if the if 'myconnectedregistry' is a mirror, it will only add the mirror permissions to all its ancestors regardless of their mode. 
+  - name: Adds permissions to synchronize images from 'repo1' and 'repo2' to the connected registry 'myconnectedregistry' and its ancestors.
     text: >
         az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --add repo1 repo2
-  - name: removes all 'repo1' and 'repo2' existing permissions from 'myconnectedregistry' and its sucesors sync scope maps actions.
+  - name: Removes permissions to synchronize images from 'repo1' and 'repo2' to the connected registry 'myconnectedregistry' and its descendants.
     text: >
         az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --remove repo1 repo2
+  - name: Removes permissions to synchronize 'repo1' images and adds permissions for 'repo2' images.
+    text: >
+        az acr connected-registry repo -r mycloudregistry -n myconnectedregistry --remove repo1 --add repo2
 """
 # endregion