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
Show file tree
Hide file tree
Changes from all 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
54 changes: 46 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,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")),
Expand All @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions src/aosm/azext_aosm/deploy/deploy_with_arm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}")

Expand All @@ -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"
Expand All @@ -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:
"""
Expand Down