Fix cnf image take 2 (#101) (#102)

* Fix cnf image take 2 (#101)

* Fix CNF image copy to work cross subscription

* Cross subscription works for image copy. Still test same subscription

* lint

* Error message

* oops, code paste error

* markups

* appease mypy

* history

src/aosm/HISTORY.rst

@@ -36,6 +36,7 @@ upcoming
   - Change from using ContainerRegistryManagementClient to `az acr import` subprocess call, so that we don't need to know the Resource Group.
 * NB CHANGE TO PREVIOUS CONFIG FILE FORMAT FOR NSDs
   - Added publisher scope and removed publisher resource group from network function object, as now using proxy references
+* Fix CNF image import from cross-subscription ACR
 0.2.0
 ++++++
 Breaking change to commands - now use `nfd` instead of `definition`. Publish option removed from build.

src/aosm/azext_aosm/deploy/artifact.py

@@ -5,6 +5,7 @@
 
 # pylint: disable=unidiomatic-typecheck
 """A module to handle interacting with artifacts."""
+import json
 import math
 import shutil
 import subprocess
@@ -126,10 +127,10 @@ def _upload_helm_to_acr(
         Requires helm to be installed.
 
         :param artifact_config: configuration for the artifact being uploaded
-        :param use_manifest_permissions: whether to use the manifest credentials
-               for the upload. If False, the CLI user credentials will be used, which
-               does not require Docker to be installed. If True, the manifest creds will
-               be used, which requires Docker.
+        :param use_manifest_permissions: whether to use the manifest credentials for the
+            upload. If False, the CLI user credentials will be used, which does not
+            require Docker to be installed. If True, the manifest creds will be used,
+            which requires Docker.
         """
         self._check_tool_installed("helm")
         assert isinstance(self.artifact_client, OrasClient)
@@ -301,7 +302,7 @@ def _upload_to_storage_account(self, artifact_config: ArtifactConfig) -> None:
 
     def _get_acr(self) -> str:
         """
-        Get the name of the ACR
+        Get the name of the ACR.
 
         :return: The name of the ACR
         """
@@ -392,7 +393,7 @@ def _push_image_from_local_registry(
         Push image to target registry using docker push. Requires docker.
 
         :param local_docker_image: name and tag of the source image on local registry
-                                    e.g. uploadacr.azurecr.io/samples/nginx:stable
+            e.g. uploadacr.azurecr.io/samples/nginx:stable
         :type local_docker_image: str
         :param target_username: The username to use for the az acr login attempt
         :type target_username: str
@@ -466,8 +467,8 @@ def _pull_image_to_local_registry(
         Uses the CLI user's context to log in to the source registry.
 
         :param: source_registry_login_server: e.g. uploadacr.azurecr.io
-        :param: source_image: source docker image name
-                              e.g. uploadacr.azurecr.io/samples/nginx:stable
+        :param: source_image: source docker image name e.g.
+            uploadacr.azurecr.io/samples/nginx:stable
         """
         try:
             # Login to the source registry with the CLI user credentials. This requires
@@ -541,6 +542,41 @@ def _copy_image(
         target_acr = self._get_acr()
         try:
             print("Copying artifact from source registry")
+            # In order to use az acr import cross subscription, we need to use a token
+            # to authenticate to the source registry. This is documented as the way to
+            # us az acr import cross-tenant, not cross-sub, but it also works
+            # cross-subscription, and meant we didn't have to make a breaking change to
+            # the format of input.json. Our usage here won't work cross-tenant since
+            # we're attempting to get the token (source) with the same context as that
+            # in which we are creating the ACR (i.e. the target tenant)
+            get_token_cmd = [str(shutil.which("az")), "account", "get-access-token"]
+            # Dont use _call_subprocess_raise_output here as we don't want to log the
+            # output
+            called_process = subprocess.run(  # noqa: S603
+                get_token_cmd,
+                encoding="utf-8",
+                capture_output=True,
+                text=True,
+                check=True,
+            )
+            access_token_json = json.loads(called_process.stdout)
+            access_token = access_token_json["accessToken"]
+        except subprocess.CalledProcessError as get_token_err:
+            # This error is thrown from the az account get-access-token command
+            # If it errored we can log the output as it doesn't contain the token
+            logger.debug(get_token_err, exc_info=True)
+            raise CLIError(  # pylint: disable=raise-missing-from
+                "Failed to import image: could not get an access token from your"
+                " Azure account. Try logging in again with `az login` and then re-run"
+                " the command. If it fails again, please raise an issue and try"
+                " repeating the command using the --no-subscription-permissions"
+                " flag to pull the image to your local machine and then"
+                " push it to the Artifact Store using manifest credentials scoped"
+                " only to the store. This requires Docker to be installed"
+                " locally."
+            )
+
+        try:
             source = f"{self._clean_name(source_registry_login_server)}/{source_image}"
             acr_import_image_cmd = [
                 str(shutil.which("az")),
@@ -552,6 +588,8 @@ def _copy_image(
                 source,
                 "--image",
                 self._get_acr_target_image(include_hostname=False),
+                "--password",
+                access_token,
             ]
             self._call_subprocess_raise_output(acr_import_image_cmd)
         except CLIError as error:

src/aosm/azext_aosm/deploy/deploy_with_arm.py

@@ -226,7 +226,7 @@ def _cnfd_artifact_upload(self) -> None:
 
         for helm_package in self.config.helm_packages:
             # Go through the helm packages in the config that the user has provided
-            helm_package_name = helm_package.name
+            helm_package_name = helm_package.name  # type: ignore
 
             if helm_package_name not in artifact_dictionary:
                 # Helm package in the config file but not in the artifact manifest
@@ -240,7 +240,7 @@ def _cnfd_artifact_upload(self) -> None:
 
             # The artifact object will use the correct client (ORAS) to upload the
             # artifact
-            manifest_artifact.upload(helm_package, self.use_manifest_permissions)
+            manifest_artifact.upload(helm_package, self.use_manifest_permissions)  # type: ignore
 
             print(f"Finished uploading Helm package: {helm_package_name}")
 
@@ -258,7 +258,7 @@ def _cnfd_artifact_upload(self) -> None:
         # so we validate the config file here.
         if (
             len(artifact_dictionary.values()) > 1
-            and self.config.images.source_local_docker_image
+            and self.config.images.source_local_docker_image  # type: ignore
         ):
             raise ValidationError(
                 "Multiple image artifacts found to upload and a local docker image"
@@ -267,7 +267,7 @@ def _cnfd_artifact_upload(self) -> None:
             )
         for artifact in artifact_dictionary.values():
             assert isinstance(artifact, Artifact)
-            artifact.upload(self.config.images, self.use_manifest_permissions)
+            artifact.upload(self.config.images, self.use_manifest_permissions)  # type: ignore
 
     def nfd_predeploy(self) -> bool:
         """