diff --git a/src/containerapp/HISTORY.rst b/src/containerapp/HISTORY.rst index d8a16d4737d..db79e5b2b5a 100644 --- a/src/containerapp/HISTORY.rst +++ b/src/containerapp/HISTORY.rst @@ -4,6 +4,7 @@ Release History =============== upcoming ++++++ +* `az containerapp create`, `az containerapp update`, `az containerapp job create`: Fix managed identity registry authentication for Azure Container Registry sovereign cloud domains (`*.azurecr.us`, `*.azurecr.cn`) 1.3.0b4 ++++++ diff --git a/src/containerapp/azext_containerapp/_constants.py b/src/containerapp/azext_containerapp/_constants.py index 55e31b6899e..6d04fb46fd6 100644 --- a/src/containerapp/azext_containerapp/_constants.py +++ b/src/containerapp/azext_containerapp/_constants.py @@ -37,6 +37,7 @@ LONG_POLLING_INTERVAL_SECS = 10 ACR_IMAGE_SUFFIX = ".azurecr.io" +ACR_IMAGE_SUFFIXES = [".azurecr.io", ".azurecr.us", ".azurecr.cn"] CONTAINER_APPS_SDK_MODELS = "azext_containerapp._sdk_models" diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 4ff360213dc..d37ba70d826 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -59,7 +59,7 @@ get_pack_exec_path, _validate_custom_loc_and_location, _validate_connected_k8s_exists, get_custom_location, create_extension, create_custom_location, get_cluster_extension, validate_environment_location, list_environment_locations, get_randomized_name_with_dash, get_randomized_name, get_connected_k8s, - list_cluster_extensions, list_custom_location, is_cloud_supported_by_connected_env + list_cluster_extensions, list_custom_location, is_cloud_supported_by_connected_env, is_acr_registry ) from ._constants import (MAXIMUM_SECRET_LENGTH, @@ -514,7 +514,8 @@ def create_acr_if_needed(self): def create_acr(self): registry_rg = self.resource_group url = self.registry_server - registry_name = url[: url.rindex(ACR_IMAGE_SUFFIX)] + parsed = urlparse(url) + registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0] location = "eastus" if self.env.location and self.env.location.lower() != "northcentralusstage": location = self.env.location @@ -542,7 +543,7 @@ def _docker_push_to_container_registry(self, image_name, forced_acr_login=False) _, stderr = process.communicate() if process.returncode != 0: docker_push_error = stderr.decode('utf-8') - if not forced_acr_login and ".azurecr.io/" in image_name and "unauthorized" in docker_push_error: + if not forced_acr_login and is_acr_registry(image_name) and "unauthorized" in docker_push_error: # Couldn't push to ACR because the user isn't authenticated. Let's try to login to ACR and retrigger the docker push logger.warning(f"The current user isn't authenticated to the {self.acr.name} ACR instance. Triggering an ACR login and retrying to push the image...") # Logic to login to ACR @@ -695,7 +696,8 @@ def build_container_from_source_with_acr_task(self, image_name, source): raise ValidationError(f"Impossible to build the artifact file {source} with ACR Task. Please make sure that you use --source and target a directory, or if you want to build your artifact locally, please make sure Docker is running on your machine.") task_name = "cli_build_containerapp" - registry_name = (self.registry_server[: self.registry_server.rindex(ACR_IMAGE_SUFFIX)]).lower() + parsed = urlparse(self.registry_server) + registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0].lower() if not self.target_port: self.target_port = DEFAULT_PORT task_content = ACR_TASK_TEMPLATE.replace("{{image_name}}", image_name).replace("{{target_port}}", str(self.target_port)) @@ -751,7 +753,7 @@ def run_source_to_cloud_flow(self, source, dockerfile, build_env_vars, can_creat self.create_acr_if_needed() elif not registry_server: raise RequiredArgumentMissingError("Usage error: --registry-server is required while using --source with a Dockerfile") - elif ACR_IMAGE_SUFFIX not in registry_server: + elif not is_acr_registry(registry_server): raise InvalidArgumentValueError("Usage error: --registry-server: expected an ACR registry (*.azurecr.io) for --source with a Dockerfile") self.image = self.registry_server + "/" + image_name_with_tag queue_acr_build( @@ -777,7 +779,7 @@ def run_source_to_cloud_flow(self, source, dockerfile, build_env_vars, can_creat self.create_acr_if_needed() elif not registry_server: raise RequiredArgumentMissingError("Usage error: --registry-server is required while using --source or --artifact in this context") - elif ACR_IMAGE_SUFFIX not in registry_server: + elif not is_acr_registry(registry_server): raise InvalidArgumentValueError("Usage error: --registry-server: expected an ACR registry (*.azurecr.io) for --source or --artifact in this context") # At this point in the logic, we know that the customer doesn't have a Dockerfile but has a container registry. @@ -1083,7 +1085,7 @@ def _validate_up_args(cmd, source, artifact, build_env_vars, image, repo, regist "--build_env_vars must be used with --source, --artifact, or --repo together" ) _validate_source_artifact_args(source, artifact) - if repo and registry_server and "azurecr.io" in registry_server: + if repo and registry_server and is_acr_registry(registry_server): parsed = urlparse(registry_server) registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0] if registry_name and len(registry_name) > MAXIMUM_SECRET_LENGTH: @@ -1485,7 +1487,7 @@ def _get_env_and_group_from_log_analytics( def _get_acr_from_image(cmd, app): - if app.image is not None and "azurecr.io" in app.image: + if app.image is not None and is_acr_registry(app.image): app.registry_server = app.image.split("/")[ 0 ] # TODO what if this conflicts with registry_server param? @@ -1503,7 +1505,7 @@ def _get_registry_from_app(app, source): containerapp_def = app.get() existing_registries = safe_get(containerapp_def, "properties", "configuration", "registries", default=[]) if source: - existing_registries = [r for r in existing_registries if ACR_IMAGE_SUFFIX in r["server"]] + existing_registries = [r for r in existing_registries if is_acr_registry(r["server"])] if containerapp_def: if len(existing_registries) == 1: app.registry_server = existing_registries[0]["server"] @@ -1515,7 +1517,8 @@ def _get_registry_from_app(app, source): def _get_acr_rg(app): - registry_name = app.registry_server[: app.registry_server.rindex(ACR_IMAGE_SUFFIX)] + parsed = urlparse(app.registry_server) + registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0] client = get_mgmt_service_client( app.cmd.cli_ctx, ContainerRegistryManagementClient ).registries @@ -1551,7 +1554,7 @@ def _get_registry_details(cmd, app: "ContainerApp", source): registry_rg = None registry_name = None if app.registry_server: - if "azurecr.io" not in app.registry_server and source: + if not is_acr_registry(app.registry_server) and source: raise ValidationError( "Cannot supply non-Azure registry when using --source." ) @@ -1579,7 +1582,7 @@ def _get_registry_details(cmd, app: "ContainerApp", source): def _get_registry_details_without_get_creds(cmd, app: "ContainerApp", source): if app.registry_server: - if "azurecr.io" not in app.registry_server and source: + if not is_acr_registry(app.registry_server) and source: raise ValidationError( "Cannot supply non-Azure registry when using --source." ) diff --git a/src/containerapp/azext_containerapp/_utils.py b/src/containerapp/azext_containerapp/_utils.py index b7015e9d4ac..a4065b1b008 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -30,7 +30,7 @@ from azure.cli.core.commands.client_factory import get_mgmt_service_client, get_subscription_id from azure.cli.command_modules.containerapp._utils import is_registry_msi_system from azure.mgmt.core.tools import parse_resource_id, is_valid_resource_id -from ._utils_validation import validate_image_name # noqa: F401 +from ._utils_validation import validate_image_name, is_acr_registry # noqa: F401 from azure.mgmt.resource import ResourceManagementClient from azure.mgmt.servicelinker import ServiceLinkerManagementClient diff --git a/src/containerapp/azext_containerapp/_utils_validation.py b/src/containerapp/azext_containerapp/_utils_validation.py index 939b059f97c..125c505979d 100644 --- a/src/containerapp/azext_containerapp/_utils_validation.py +++ b/src/containerapp/azext_containerapp/_utils_validation.py @@ -6,6 +6,7 @@ import re from azure.cli.core.azclierror import ValidationError +from ._constants import ACR_IMAGE_SUFFIXES _VALID_IMAGE_NAME_RE = re.compile(r'^[a-zA-Z0-9._:/@-]+$') @@ -17,3 +18,10 @@ def validate_image_name(name): f"Invalid container image name: {name!r}. " "Image names may only contain alphanumeric characters, '.', '_', ':', '/', '@', and '-'." ) + + +def is_acr_registry(server): + """Return True if the registry server belongs to any Azure Container Registry domain (all sovereign clouds).""" + if not server: + return False + return any(suffix in server for suffix in ACR_IMAGE_SUFFIXES) diff --git a/src/containerapp/azext_containerapp/_validators.py b/src/containerapp/azext_containerapp/_validators.py index 22b88f40266..3c617831384 100644 --- a/src/containerapp/azext_containerapp/_validators.py +++ b/src/containerapp/azext_containerapp/_validators.py @@ -20,6 +20,7 @@ EXTENDED_LOCATION_RP, CUSTOM_LOCATION_RESOURCE_TYPE, MAXIMUM_SECRET_LENGTH, CONTAINER_APPS_RP, \ CONNECTED_ENVIRONMENT_RESOURCE_TYPE, MANAGED_ENVIRONMENT_RESOURCE_TYPE, MANAGED_ENVIRONMENT_TYPE, \ RUNTIME_GENERIC +from ._utils import is_acr_registry logger = get_logger(__name__) @@ -37,9 +38,9 @@ def validate_create(registry_identity, registry_pass, registry_user, registry_se if repo: if not registry_server: raise RequiredArgumentMissingError('Usage error: --registry-server is required while using --repo') - if ACR_IMAGE_SUFFIX not in registry_server: + if not is_acr_registry(registry_server): raise InvalidArgumentValueError("Usage error: --registry-server: expected an ACR registry (*.azurecr.io) for --repo") - if repo and registry_server and "azurecr.io" in registry_server: + if repo and registry_server and is_acr_registry(registry_server): parsed = urlparse(registry_server) registry_name = (parsed.netloc if parsed.scheme else parsed.path).split(".")[0] if registry_name and len(registry_name) > MAXIMUM_SECRET_LENGTH: @@ -51,7 +52,7 @@ def validate_create(registry_identity, registry_pass, registry_user, registry_se raise MutuallyExclusiveArgumentError("--no-wait is not supported with system registry identity") if registry_identity and not is_valid_resource_id(registry_identity) and not is_registry_msi_system(registry_identity) and not is_registry_msi_system_environment(registry_identity): raise InvalidArgumentValueError("--registry-identity must be an identity resource ID or 'system' or 'system-environment'") - if registry_identity and ACR_IMAGE_SUFFIX not in (registry_server or ""): + if registry_identity and not is_acr_registry(registry_server): raise InvalidArgumentValueError("--registry-identity: expected an ACR registry (*.azurecr.io) for --registry-server") if target_label and (not revisions_mode or revisions_mode.lower() != 'labels'): raise InvalidArgumentValueError("--target-label must only be specified with --revisions-mode labels.") diff --git a/src/containerapp/azext_containerapp/containerapp_decorator.py b/src/containerapp/azext_containerapp/containerapp_decorator.py index 9dfa3f4ceb5..94747ba158d 100644 --- a/src/containerapp/azext_containerapp/containerapp_decorator.py +++ b/src/containerapp/azext_containerapp/containerapp_decorator.py @@ -69,7 +69,8 @@ infer_runtime_option ) from ._utils import parse_service_bindings, check_unique_bindings, is_registry_msi_system_environment, \ - env_has_managed_identity, create_acrpull_role_assignment_if_needed, is_cloud_supported_by_service_connector + env_has_managed_identity, create_acrpull_role_assignment_if_needed, is_cloud_supported_by_service_connector, \ + is_acr_registry from ._validators import validate_create, validate_runtime from ._constants import (HELLO_WORLD_IMAGE, CONNECTED_ENVIRONMENT_TYPE, @@ -400,7 +401,7 @@ def construct_payload(self): if self.get_argument_registry_server(): if (not self.get_argument_registry_pass() or not self.get_argument_registry_user()) and not self.get_argument_registry_identity(): - if ACR_IMAGE_SUFFIX not in self.get_argument_registry_server(): + if not is_acr_registry(self.get_argument_registry_server()): raise RequiredArgumentMissingError( 'Registry url is required if using Azure Container Registry, otherwise Registry username and password are required if using Dockerhub') logger.warning( @@ -737,7 +738,7 @@ def set_up_managed_identity(self): def set_up_system_assigned_identity_as_default_if_using_acr(self): registry_server = self.get_argument_registry_server() if registry_server is not None: - if ACR_IMAGE_SUFFIX not in registry_server: + if not is_acr_registry(registry_server): return if self.get_argument_registry_identity() is None and self.get_argument_registry_user() is None and self.get_argument_registry_pass() is None: @@ -950,7 +951,7 @@ def validate_arguments(self): raise InvalidArgumentValueError("--scale-rule-identity cannot be set when --scale-rule-type is 'http' or 'tcp'") if self.get_argument_registry_server() is not None and \ - ACR_IMAGE_SUFFIX in self.get_argument_registry_server() and \ + is_acr_registry(self.get_argument_registry_server()) and \ self.get_argument_registry_user() is None and \ self.get_argument_registry_pass() is None and \ self.get_argument_no_wait(): @@ -1500,7 +1501,7 @@ def set_up_managed_identity(self): def set_up_system_assigned_identity_as_default_if_using_acr(self): registry_server = self.get_argument_registry_server() if registry_server is not None: - if ACR_IMAGE_SUFFIX not in registry_server: + if not is_acr_registry(registry_server): return if self.get_argument_registry_identity() is None and self.get_argument_registry_user() is None and self.get_argument_registry_pass() is None: @@ -1563,7 +1564,7 @@ def set_up_source(self): raise MutuallyExclusiveArgumentError("Cannot use --source with --yaml together. Can either deploy from a local directory or provide a yaml file") # Check if an ACR registry is associated with the container app existing_registries = safe_get(self.containerapp_def, "properties", "configuration", "registries", default=[]) - existing_registries = [r for r in existing_registries if ACR_IMAGE_SUFFIX in r["server"]] + existing_registries = [r for r in existing_registries if is_acr_registry(r["server"])] if existing_registries and len(existing_registries) > 0: registry_server = existing_registries[0]["server"] self._update_container_app_source(cmd=self.cmd, source=source, build_env_vars=build_env_vars, registry_server=registry_server) diff --git a/src/containerapp/azext_containerapp/containerapp_job_decorator.py b/src/containerapp/azext_containerapp/containerapp_job_decorator.py index db80dadfdb8..382879d2578 100644 --- a/src/containerapp/azext_containerapp/containerapp_job_decorator.py +++ b/src/containerapp/azext_containerapp/containerapp_job_decorator.py @@ -47,7 +47,7 @@ ContainerResources as ContainerResourcesModel, Container as ContainerModel, ScaleRule as ScaleRuleModel) -from ._utils import is_registry_msi_system_environment, env_has_managed_identity +from ._utils import is_registry_msi_system_environment, env_has_managed_identity, is_acr_registry from ._validators import validate_create logger = get_logger(__name__) @@ -391,7 +391,7 @@ def set_up_registry(self): if self.get_argument_registry_server(): if not self.get_argument_registry_pass or not self.get_argument_registry_user(): - if ACR_IMAGE_SUFFIX not in self.get_argument_registry_server(): + if not is_acr_registry(self.get_argument_registry_server()): raise RequiredArgumentMissingError( 'Registry url is required if using Azure Container Registry, otherwise Registry username and password are required if using Dockerhub') logger.warning( diff --git a/src/containerapp/azext_containerapp/containerapp_job_registry_decorator.py b/src/containerapp/azext_containerapp/containerapp_job_registry_decorator.py index ff0b8b4f99f..dc5f415e7c1 100644 --- a/src/containerapp/azext_containerapp/containerapp_job_registry_decorator.py +++ b/src/containerapp/azext_containerapp/containerapp_job_registry_decorator.py @@ -19,7 +19,7 @@ from ._constants import ACR_IMAGE_SUFFIX from ._models import (RegistryCredentials as RegistryCredentialsModel) -from ._utils import env_has_managed_identity +from ._utils import env_has_managed_identity, is_acr_registry from ._validators import validate_create logger = get_logger(__name__) @@ -39,7 +39,7 @@ def parent_construct_payload(self): if (not self.get_argument_username() or not self.get_argument_password()) and not self.get_argument_identity(): # If registry is Azure Container Registry, we can try inferring credentials - if ACR_IMAGE_SUFFIX not in self.get_argument_server(): + if not is_acr_registry(self.get_argument_server()): raise RequiredArgumentMissingError( 'Registry username and password are required if you are not using Azure Container Registry.') if not self.get_argument_disable_warnings(): diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index 6054e0b8f45..3e6c6e949bd 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -143,7 +143,8 @@ from ._ssh_utils import (SSH_DEFAULT_ENCODING, DebugWebSocketConnection, read_debug_ssh) from ._utils import (connected_env_check_cert_name_availability, get_oryx_run_image_tags, patchable_check, - get_pack_exec_path, is_docker_running, parse_build_env_vars, env_has_managed_identity) + get_pack_exec_path, is_docker_running, parse_build_env_vars, env_has_managed_identity, + is_acr_registry) from ._utils_validation import validate_image_name from ._arc_utils import (extract_domain_from_configmap, get_core_dns_deployment, get_core_dns_configmap, backup_custom_core_dns_configmap, @@ -1204,7 +1205,7 @@ def create_or_update_github_action(cmd, # Registry if registry_username is None or registry_password is None: # If registry is Azure Container Registry, we can try inferring credentials - if not registry_url or ACR_IMAGE_SUFFIX not in registry_url: + if not registry_url or not is_acr_registry(registry_url): raise RequiredArgumentMissingError('Registry url is required if using Azure Container Registry, otherwise Registry username and password are required if using Dockerhub') logger.warning('No credential was provided to access Azure Container Registry. Trying to look up...') parsed = urlparse(registry_url) @@ -3482,7 +3483,7 @@ def set_registry(cmd, name, resource_group_name, server, username=None, password if (not username or not password) and not identity: # If registry is Azure Container Registry, we can try inferring credentials - if ACR_IMAGE_SUFFIX not in server: + if not is_acr_registry(server): raise RequiredArgumentMissingError('Registry username and password are required if you are not using Azure Container Registry.') not disable_warnings and logger.warning('No credential was provided to access Azure Container Registry. Trying to look up...') parsed = urlparse(server) diff --git a/src/containerapp/azext_containerapp/tests/latest/test_containerapp_validator_unit.py b/src/containerapp/azext_containerapp/tests/latest/test_containerapp_validator_unit.py new file mode 100644 index 00000000000..228a5fab0ef --- /dev/null +++ b/src/containerapp/azext_containerapp/tests/latest/test_containerapp_validator_unit.py @@ -0,0 +1,95 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import os +import ast +import textwrap +import unittest + + +def _extract_constants(): + """Load ACR_IMAGE_SUFFIXES from _constants.py without importing the full package.""" + constants_path = os.path.normpath( + os.path.join(os.path.dirname(__file__), "..", "..", "_constants.py") + ) + ns = {} + with open(constants_path, "r", encoding="utf-8") as f: + exec(compile(f.read(), constants_path, "exec"), ns) # pylint: disable=exec-used + return ns + + +def _extract_function(func_name): + """Extract a function from _utils_validation.py source using ast, avoiding import of azure.cli deps.""" + src_path = os.path.normpath( + os.path.join(os.path.dirname(__file__), "..", "..", "_utils_validation.py") + ) + with open(src_path, "r", encoding="utf-8") as f: + source = f.read() + tree = ast.parse(source, filename=src_path) + for node in ast.walk(tree): + if isinstance(node, ast.FunctionDef) and node.name == func_name: + func_source = ast.get_source_segment(source, node) + ns = {**_extract_constants()} + exec(compile(textwrap.dedent(func_source), src_path, "exec"), ns) # pylint: disable=exec-used + return ns[func_name] + raise ValueError(f"Function {func_name!r} not found in {src_path}") + + +class TestIsAcrRegistry(unittest.TestCase): + """Unit tests for is_acr_registry() — verifies all ACR sovereign cloud domains are recognized.""" + + def setUp(self): + self.is_acr_registry = _extract_function("is_acr_registry") + + # Standard Azure Public cloud + def test_azurecr_io_is_acr(self): + self.assertTrue(self.is_acr_registry("myregistry.azurecr.io")) + + def test_azurecr_io_with_path_is_acr(self): + self.assertTrue(self.is_acr_registry("myregistry.azurecr.io/myimage:latest")) + + # US Government cloud (the original bug report) + def test_azurecr_us_is_acr(self): + self.assertTrue(self.is_acr_registry("myregistry.azurecr.us")) + + def test_azurecr_us_with_path_is_acr(self): + self.assertTrue(self.is_acr_registry("myregistry.azurecr.us/myimage:latest")) + + # Azure China cloud + def test_azurecr_cn_is_acr(self): + self.assertTrue(self.is_acr_registry("myregistry.azurecr.cn")) + + def test_azurecr_cn_with_path_is_acr(self): + self.assertTrue(self.is_acr_registry("myregistry.azurecr.cn/myimage:latest")) + + # Non-ACR registries should return False + def test_dockerhub_is_not_acr(self): + self.assertFalse(self.is_acr_registry("docker.io")) + + def test_dockerhub_image_is_not_acr(self): + self.assertFalse(self.is_acr_registry("docker.io/library/nginx:latest")) + + def test_ghcr_is_not_acr(self): + self.assertFalse(self.is_acr_registry("ghcr.io/myorg/myimage:latest")) + + def test_mcr_is_not_acr(self): + self.assertFalse(self.is_acr_registry("mcr.microsoft.com/azure-cli:latest")) + + def test_custom_registry_is_not_acr(self): + self.assertFalse(self.is_acr_registry("my-private-registry.example.com")) + + # Edge cases + def test_none_is_not_acr(self): + self.assertFalse(self.is_acr_registry(None)) + + def test_empty_string_is_not_acr(self): + self.assertFalse(self.is_acr_registry("")) + + def test_totally_fake_domain_is_not_acr(self): + self.assertFalse(self.is_acr_registry("totally-fake-registry.com")) + + +if __name__ == '__main__': + unittest.main()