From 5fb0ecd059fa3c040b1a7506e87d881a6fce0f40 Mon Sep 17 00:00:00 2001 From: harrli Date: Mon, 15 May 2023 14:21:57 -0700 Subject: [PATCH 1/5] Linting --- .../azext_containerapp/_up_utils.py | 66 +++++++++---------- src/containerapp/azext_containerapp/_utils.py | 49 +++++++------- src/containerapp/azext_containerapp/custom.py | 28 ++++---- 3 files changed, 70 insertions(+), 73 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index 1229d726aeb..f32b9c206a5 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -7,8 +7,8 @@ from tempfile import NamedTemporaryFile from urllib.parse import urlparse -import requests import subprocess +import requests from azure.cli.core.azclierror import ( RequiredArgumentMissingError, @@ -359,7 +359,7 @@ def create_acr(self): self.cmd.cli_ctx, registry_name ) - def build_container_from_source_with_buildpack(self, image_name, source): + def build_container_from_source_with_buildpack(self, image_name, source): # pylint: disable=too-many-statements # Ensure that Docker is running if not is_docker_running(): raise ValidationError("Docker is not running. Please start Docker to use buildpacks.") @@ -379,13 +379,13 @@ def build_container_from_source_with_buildpack(self, image_name, source): command = [pack_exec_path, 'config', 'default-builder', builder_image_name] logger.debug(f"Calling '{' '.join(command)}'") try: - process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = process.communicate() - if process.returncode != 0: - raise CLIError(f"Error thrown when running 'pack config': {stderr.decode('utf-8')}") - logger.debug(f"Successfully set the default builder to {builder_image_name}.") + with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: + stdout, stderr = process.communicate() # pylint: disable=unused-variable + if process.returncode != 0: + raise CLIError(f"Error thrown when running 'pack config': {stderr.decode('utf-8')}") + logger.debug(f"Successfully set the default builder to {builder_image_name}.") except Exception as ex: - raise ValidationError(f"Unable to run 'pack config' command to set default builder: {ex}") + raise ValidationError(f"Unable to run 'pack config' command to set default builder: {ex}") from ex # Run 'pack build' to produce a runnable application image for the Container App command = [pack_exec_path, 'build', image_name, '--builder', builder_image_name, '--path', source] @@ -398,45 +398,45 @@ def build_container_from_source_with_buildpack(self, image_name, source): logger.debug(f"Calling '{' '.join(command)}'") try: is_non_supported_platform = False - process = subprocess.Popen(command, stdout=subprocess.PIPE) + with subprocess.Popen(command, stdout=subprocess.PIPE) as process: - # Stream output of 'pack build' to warning stream - while process.stdout.readable(): - line = process.stdout.readline() - if not line: - break + # Stream output of 'pack build' to warning stream + while process.stdout.readable(): + line = process.stdout.readline() + if not line: + break - stdout_line = str(line.strip(), 'utf-8') - logger.warning(stdout_line) - if not is_non_supported_platform and "No buildpack groups passed detection" in stdout_line: - is_non_supported_platform = True + stdout_line = str(line.strip(), 'utf-8') + logger.warning(stdout_line) + if not is_non_supported_platform and "No buildpack groups passed detection" in stdout_line: + is_non_supported_platform = True - # Update the result of process.returncode - process.communicate() - if is_non_supported_platform: - raise ValidationError("Current buildpacks do not support the platform targeted in the provided source code.") + # Update the result of process.returncode + process.communicate() + if is_non_supported_platform: + raise ValidationError("Current buildpacks do not support the platform targeted in the provided source code.") - if process.returncode != 0: - raise CLIError(f"Non-zero exit code returned from 'pack build'; please check the above output for more details.") + if process.returncode != 0: + raise CLIError("Non-zero exit code returned from 'pack build'; please check the above output for more details.") - logger.debug(f"Successfully built image {image_name} using buildpacks.") + logger.debug(f"Successfully built image {image_name} using buildpacks.") except ValidationError as ex: raise ex except Exception as ex: - raise CLIError(f"Unable to run 'pack build' command to produce runnable application image: {ex}") + raise CLIError(f"Unable to run 'pack build' command to produce runnable application image: {ex}") from ex # Run 'docker push' to push the image to the ACR command = ['docker', 'push', image_name] logger.debug(f"Calling '{' '.join(command)}'") logger.warning(f"Built image {image_name} locally using buildpacks, attempting to push to registry...") try: - process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = process.communicate() - if process.returncode != 0: - raise CLIError(f"Error thrown when running 'docker push': {stderr.decode('utf-8')}") - logger.debug(f"Successfully pushed image {image_name} to ACR.") + with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: + stdout, stderr = process.communicate() + if process.returncode != 0: + raise CLIError(f"Error thrown when running 'docker push': {stderr.decode('utf-8')}") + logger.debug(f"Successfully pushed image {image_name} to ACR.") except Exception as ex: - raise CLIError(f"Unable to run 'docker push' command to push image to ACR: {ex}") + raise CLIError(f"Unable to run 'docker push' command to push image to ACR: {ex}") from ex def build_container_from_source_with_acr_task(self, image_name, source): from azure.cli.command_modules.acr.task import acr_task_create, acr_task_run @@ -509,7 +509,7 @@ def run_acr_build(self, dockerfile, source, quiet=False, build_from_source=False except ValidationError as e: logger.warning(f"Unable to use buildpacks to build image from source: {e}\nFalling back to ACR Task...") except CLIError as e: - logger.error(f"Failed to use buildpacks to build image from source.") + logger.error("Failed to use buildpacks to build image from source.") raise e # If we're unable to use the buildpack, build source using an ACR Task diff --git a/src/containerapp/azext_containerapp/_utils.py b/src/containerapp/azext_containerapp/_utils.py index f535996c908..819e39fc0af 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -7,16 +7,15 @@ import time import json import platform -import docker import io import os -import requests -import hashlib -import packaging.version as SemVer -import re import tarfile import zipfile - +import hashlib +import re +import requests +import docker +import packaging.version as SemVer from urllib.parse import urlparse from urllib.request import urlopen @@ -42,7 +41,7 @@ LOG_ANALYTICS_RP, CONTAINER_APPS_RP, CHECK_CERTIFICATE_NAME_AVAILABILITY_TYPE, ACR_IMAGE_SUFFIX, LOGS_STRING, PENDING_STATUS, SUCCEEDED_STATUS, UPDATING_STATUS) from ._models import (ContainerAppCustomDomainEnvelope as ContainerAppCustomDomainEnvelopeModel, ManagedCertificateEnvelop as ManagedCertificateEnvelopModel) -from ._models import ImagePatchableCheck, OryxMarinerRunImgTagProperty +from ._models import OryxMarinerRunImgTagProperty logger = get_logger(__name__) @@ -1772,20 +1771,20 @@ def get_pack_exec_path(): # Attempt to install the pack CLI url = f"https://github.com/buildpacks/pack/releases/download/{pack_cli_version}/{compressed_download_file_name}" - req = urlopen(url) - compressed_file = io.BytesIO(req.read()) - if host_os == "Windows": - zip_file = zipfile.ZipFile(compressed_file) - for file in zip_file.namelist(): - if file.endswith(exec_name): - with open(exec_path, "wb") as f: - f.write(zip_file.read(file)) - else: - with tarfile.open(fileobj=compressed_file, mode="r:gz") as tar: - for tar_info in tar: - if tar_info.isfile() and tar_info.name.endswith(exec_name): - with open(exec_path, "wb") as f: - f.write(tar.extractfile(tar_info).read()) + with urlopen(url) as req: + compressed_file = io.BytesIO(req.read()) + if host_os == "Windows": + with zipfile.ZipFile(compressed_file) as zip_file: + for file in zip_file.namelist(): + if file.endswith(exec_name): + with open(exec_path, "wb") as f: + f.write(zip_file.read(file)) + else: + with tarfile.open(fileobj=compressed_file, mode="r:gz") as tar: + for tar_info in tar: + if tar_info.isfile() and tar_info.name.endswith(exec_name): + with open(exec_path, "wb") as f: + f.write(tar.extractfile(tar_info).read()) return exec_path except Exception as e: @@ -1862,19 +1861,19 @@ def patchable_check(repo_tag_split: str, oryx_builder_run_img_tags, inspect_resu def get_current_mariner_tags() -> list(OryxMarinerRunImgTagProperty): - r = requests.get("https://mcr.microsoft.com/v2/oryx/builder/tags/list") + r = requests.get("https://mcr.microsoft.com/v2/oryx/builder/tags/list", timeout=30) tags = r.json() tag_list = {} # only keep entries that container keyword "mariner" tags = [tag for tag in tags["tags"] if "mariner" in tag] - for tag in tags: + for tag in tags: # pylint: disable=too-many-nested-blocks tag_obj = parse_oryx_mariner_tag(tag) if tag_obj: major_minor_ver = str(tag_obj["version"].major) + "." + str(tag_obj["version"].minor) support = tag_obj["support"] framework = tag_obj["framework"] mariner_ver = tag_obj["marinerVersion"] - if framework in tag_list.keys(): + if framework in tag_list: if major_minor_ver in tag_list[framework].keys(): if support in tag_list[framework][major_minor_ver].keys(): if mariner_ver in tag_list[framework][major_minor_ver][support].keys(): @@ -1891,7 +1890,7 @@ def get_current_mariner_tags() -> list(OryxMarinerRunImgTagProperty): return tag_list -def get_latest_buildpack_run_tag(framework, version, support = "lts", mariner_version = "cbl-mariner2.0"): +def get_latest_buildpack_run_tag(framework, version, support="lts", mariner_version="cbl-mariner2.0"): tags = get_current_mariner_tags() try: return tags[framework][version][support][mariner_version][0]["fullTag"] diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index 54927e1e665..ce12a94e356 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -13,8 +13,6 @@ import subprocess from concurrent.futures import ThreadPoolExecutor import requests -import json -import subprocess from azure.cli.core import telemetry as telemetry_core @@ -4376,13 +4374,13 @@ def patch_get_image_inspection(pack_exec_path, img, info_list): if (img["imageName"].find("run-dotnet") != -1) and (img["imageName"].find("cbl-mariner") != -1): inspect_result = {"remote_info": {"run_images": [{"name": "mcr.microsoft.com/oryx/builder:" + img["imageName"].split(":")[-1]}]}, "image_name": img["imageName"], "targetContainerName": img["targetContainerName"], "targetContainerAppName": img["targetContainerAppName"], "targetContainerAppEnvironmentName": img["targetContainerAppEnvironmentName"], "targetResourceGroup": img["targetResourceGroup"]} else: - img_info = subprocess.Popen(pack_exec_path + " inspect-image " + img["imageName"] + " --output json", shell=True, stderr=subprocess.PIPE, stdout=subprocess.PIPE) - img_info_out, img_info_err = img_info.communicate() - if img_info_err.find(b"status code 401 Unauthorized") != -1 or img_info_err.find(b"unable to find image") != -1: - inspect_result = dict(remote_info=401, image_name=img["imageName"]) - else: - inspect_result = json.loads(img_info_out) - inspect_result.update({"targetContainerName": img["targetContainerName"], "targetContainerAppName": img["targetContainerAppName"], "targetContainerAppEnvironmentName": img["targetContainerAppEnvironmentName"], "targetResourceGroup": img["targetResourceGroup"]}) + with subprocess.Popen(pack_exec_path + " inspect-image " + img["imageName"] + " --output json", shell=True, stderr=subprocess.PIPE, stdout=subprocess.PIPE) as img_info: + img_info_out, img_info_err = img_info.communicate() + if img_info_err.find(b"status code 401 Unauthorized") != -1 or img_info_err.find(b"unable to find image") != -1: + inspect_result = dict(remote_info=401, image_name=img["imageName"]) + else: + inspect_result = json.loads(img_info_out) + inspect_result.update({"targetContainerName": img["targetContainerName"], "targetContainerAppName": img["targetContainerAppName"], "targetContainerAppEnvironmentName": img["targetContainerAppEnvironmentName"], "targetResourceGroup": img["targetResourceGroup"]}) info_list.append(inspect_result) @@ -4403,7 +4401,7 @@ def patch_interactive(cmd, resource_group_name=None, managed_env=None, show_all= if without_unpatchable_results == []: return user_input = input("Do you want to apply all the patches or specify by id? (y/n/id)\n") - patch_apply(cmd, patchable_check_results, user_input, pack_exec_path) + patch_apply_handle_input(cmd, patchable_check_results, user_input, pack_exec_path) def patch_apply(cmd, resource_group_name=None, managed_env=None, show_all=False): @@ -4422,10 +4420,10 @@ def patch_apply(cmd, resource_group_name=None, managed_env=None, show_all=False) print(patchable_check_results_json) if without_unpatchable_results == []: return - patch_apply(cmd, patchable_check_results, "y", pack_exec_path) + patch_apply_handle_input(cmd, patchable_check_results, "y", pack_exec_path) -def patch_apply(cmd, patch_check_list, method, pack_exec_path): +def patch_apply_handle_input(cmd, patch_check_list, method, pack_exec_path): m = method.strip().lower() # Track number of times patches were applied successfully. patch_apply_count = 0 @@ -4475,11 +4473,11 @@ def patch_apply(cmd, patch_check_list, method, pack_exec_path): def patch_cli_call(cmd, resource_group, container_app_name, container_name, target_image_name, new_run_image, pack_exec_path): try: print("Applying patch for container app: " + container_app_name + " container: " + container_name) - subprocess.run(f"{pack_exec_path} rebase -q {target_image_name} --run-image {new_run_image}", shell=True) + subprocess.run(f"{pack_exec_path} rebase -q {target_image_name} --run-image {new_run_image}", shell=True, check=True) new_target_image_name = target_image_name.split(":")[0] + ":" + new_run_image.split(":")[1] - subprocess.run(f"docker tag {target_image_name} {new_target_image_name}", shell=True) + subprocess.run(f"docker tag {target_image_name} {new_target_image_name}", shell=True, check=True) print(f"Publishing {new_target_image_name} to registry...") - subprocess.run(f"docker push -q {new_target_image_name}", shell=True) + subprocess.run(f"docker push -q {new_target_image_name}", shell=True, check=True) print("Patch applied and published successfully.\nNew image: " + new_target_image_name) except Exception: print("Error: Failed to apply patch and publish. Check if registry is logged in and has write access.") From f3cb3ed69a3fc8b8c984cb20c450f170e40972db Mon Sep 17 00:00:00 2001 From: Cormac McCarthy Date: Mon, 15 May 2023 13:19:11 -0700 Subject: [PATCH 2/5] - Fix pack CLI not having proper permission - Add requirements.txt file to snap to install specific version of docker --- src/containerapp/azext_containerapp/_utils.py | 7 ++++++- src/containerapp/requirements.txt | 1 + src/containerapp/setup.py | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 src/containerapp/requirements.txt diff --git a/src/containerapp/azext_containerapp/_utils.py b/src/containerapp/azext_containerapp/_utils.py index 819e39fc0af..9fb5723d702 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -7,6 +7,7 @@ import time import json import platform +import stat import io import os import tarfile @@ -1745,7 +1746,6 @@ def is_docker_running(): def get_pack_exec_path(): try: - dir_path = os.path.dirname(os.path.realpath(__file__)) bin_folder = os.path.join(dir_path, "bin") if not os.path.exists(bin_folder): @@ -1786,6 +1786,11 @@ def get_pack_exec_path(): with open(exec_path, "wb") as f: f.write(tar.extractfile(tar_info).read()) + # Add executable permissions for the current user if they don't exist + if not os.access(exec_path, os.X_OK): + st = os.stat(exec_path) + os.chmod(exec_path, st.st_mode | stat.S_IXUSR) + return exec_path except Exception as e: # Swallow any exceptions thrown when attempting to install pack CLI diff --git a/src/containerapp/requirements.txt b/src/containerapp/requirements.txt new file mode 100644 index 00000000000..db58120b5ea --- /dev/null +++ b/src/containerapp/requirements.txt @@ -0,0 +1 @@ +docker>=6.1.2 \ No newline at end of file diff --git a/src/containerapp/setup.py b/src/containerapp/setup.py index 30c389c2379..4311722c43e 100644 --- a/src/containerapp/setup.py +++ b/src/containerapp/setup.py @@ -49,7 +49,7 @@ # TODO: Add any additional SDK dependencies here DEPENDENCIES = [ 'pycomposefile>=0.0.29', - 'docker' + 'docker>=6.1.2' ] # Install pack CLI to build runnable application images from source From 11ea2b918e8557ccbca7c9032d0a6d9ad66151b6 Mon Sep 17 00:00:00 2001 From: harrli Date: Mon, 15 May 2023 15:02:23 -0700 Subject: [PATCH 3/5] Addressed comments --- src/containerapp/azext_containerapp/_up_utils.py | 8 ++++---- src/containerapp/azext_containerapp/_utils.py | 5 ----- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index f32b9c206a5..f5a8a38633d 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -380,9 +380,9 @@ def build_container_from_source_with_buildpack(self, image_name, source): # pyl logger.debug(f"Calling '{' '.join(command)}'") try: with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: - stdout, stderr = process.communicate() # pylint: disable=unused-variable + outputs = process.communicate() if process.returncode != 0: - raise CLIError(f"Error thrown when running 'pack config': {stderr.decode('utf-8')}") + raise CLIError(f"Error thrown when running 'pack config': {outputs[1].decode('utf-8')}") # outputs[1] is stderr, outputs[0] is stdout logger.debug(f"Successfully set the default builder to {builder_image_name}.") except Exception as ex: raise ValidationError(f"Unable to run 'pack config' command to set default builder: {ex}") from ex @@ -431,9 +431,9 @@ def build_container_from_source_with_buildpack(self, image_name, source): # pyl logger.warning(f"Built image {image_name} locally using buildpacks, attempting to push to registry...") try: with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: - stdout, stderr = process.communicate() + outputs = process.communicate() if process.returncode != 0: - raise CLIError(f"Error thrown when running 'docker push': {stderr.decode('utf-8')}") + raise CLIError(f"Error thrown when running 'docker push': {outputs[1].decode('utf-8')}") # outputs[1] is stderr, outputs[0] is stdout logger.debug(f"Successfully pushed image {image_name} to ACR.") except Exception as ex: raise CLIError(f"Unable to run 'docker push' command to push image to ACR: {ex}") from ex diff --git a/src/containerapp/azext_containerapp/_utils.py b/src/containerapp/azext_containerapp/_utils.py index cf001fd03cc..9fb5723d702 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -1791,11 +1791,6 @@ def get_pack_exec_path(): st = os.stat(exec_path) os.chmod(exec_path, st.st_mode | stat.S_IXUSR) - # Add executable permissions for the current user if they don't exist - if not os.access(exec_path, os.X_OK): - st = os.stat(exec_path) - os.chmod(exec_path, st.st_mode | stat.S_IXUSR) - return exec_path except Exception as e: # Swallow any exceptions thrown when attempting to install pack CLI From 084f3105daba1c51fc11666caaa4d18b7daf872b Mon Sep 17 00:00:00 2001 From: harrli Date: Mon, 15 May 2023 15:05:40 -0700 Subject: [PATCH 4/5] Updated subprocess outputs --- src/containerapp/azext_containerapp/_up_utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index f5a8a38633d..f28fa645fd6 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -380,9 +380,9 @@ def build_container_from_source_with_buildpack(self, image_name, source): # pyl logger.debug(f"Calling '{' '.join(command)}'") try: with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: - outputs = process.communicate() + _, stderr = process.communicate() if process.returncode != 0: - raise CLIError(f"Error thrown when running 'pack config': {outputs[1].decode('utf-8')}") # outputs[1] is stderr, outputs[0] is stdout + raise CLIError(f"Error thrown when running 'pack config': {stderr.decode('utf-8')}") logger.debug(f"Successfully set the default builder to {builder_image_name}.") except Exception as ex: raise ValidationError(f"Unable to run 'pack config' command to set default builder: {ex}") from ex @@ -431,9 +431,9 @@ def build_container_from_source_with_buildpack(self, image_name, source): # pyl logger.warning(f"Built image {image_name} locally using buildpacks, attempting to push to registry...") try: with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process: - outputs = process.communicate() + _, stderr = process.communicate() if process.returncode != 0: - raise CLIError(f"Error thrown when running 'docker push': {outputs[1].decode('utf-8')}") # outputs[1] is stderr, outputs[0] is stdout + raise CLIError(f"Error thrown when running 'docker push': {stderr.decode('utf-8')}") logger.debug(f"Successfully pushed image {image_name} to ACR.") except Exception as ex: raise CLIError(f"Unable to run 'docker push' command to push image to ACR: {ex}") from ex From a470a785cf1df466ed2fbace1037b28157145b67 Mon Sep 17 00:00:00 2001 From: harrli Date: Mon, 15 May 2023 15:26:58 -0700 Subject: [PATCH 5/5] Fixed wrong github issue link --- src/containerapp/azext_containerapp/_up_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/containerapp/azext_containerapp/_up_utils.py b/src/containerapp/azext_containerapp/_up_utils.py index f28fa645fd6..46f84d6d717 100644 --- a/src/containerapp/azext_containerapp/_up_utils.py +++ b/src/containerapp/azext_containerapp/_up_utils.py @@ -498,7 +498,7 @@ def run_acr_build(self, dockerfile, source, quiet=False, build_from_source=False try: # First try to build source using buildpacks # Temporary fix: using run time tag as customer image tag - # Waiting for buildpacks side to fix this issue: https://github.com/buildpacks/pack/issues/1753 + # Waiting for buildpacks side to fix this issue: https://github.com/buildpacks/pack/issues/1750 logger.warning("Attempting to build image using buildpacks...") run_image_tag = get_latest_buildpack_run_tag("aspnet", "7.0") if run_image_tag is not None: