Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cnf image take 2 #101

Merged
merged 8 commits into from
Oct 3, 2023
Merged
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 45 additions & 8 deletions src/aosm/azext_aosm/deploy/artifact.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

# pylint: disable=unidiomatic-typecheck
"""A module to handle interacting with artifacts."""
import json
import math
import shutil
import subprocess
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -541,6 +542,26 @@ def _copy_image(
target_acr = self._get_acr()
try:
print("Copying artifact from source registry")
# In order to use az acr import cross subscription(or cross tenant?), we
# need to use a token to authenticate to the source registry. This is not
# exactly as docced, but seems to work (it is documented to work cross
# tenant, but not cross subscription, but it works cross susbscription, at
# least for public ACRs). It probably won't work cross-tenant as the token
# is retrieved from the CLI user's context which will be in 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(
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"]

source = f"{self._clean_name(source_registry_login_server)}/{source_image}"
acr_import_image_cmd = [
str(shutil.which("az")),
Expand All @@ -552,6 +573,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:
Expand Down Expand Up @@ -588,3 +611,17 @@ def _copy_image(
target_acr,
error,
)
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."
)