From 478e0d94a90379ba40abd97fd5e4664e061a9ead Mon Sep 17 00:00:00 2001 From: Isaac Lee Date: Tue, 7 Jan 2020 09:21:32 -0800 Subject: [PATCH 1/4] Add notary version compatibility checks --- .../azure/cli/command_modules/acr/_errors.py | 12 ++++ .../cli/command_modules/acr/check_health.py | 70 +++++++++++++++---- .../azure/cli/command_modules/acr/notary.py | 38 ++++++++++ 3 files changed, 107 insertions(+), 13 deletions(-) create mode 100644 src/azure-cli/azure/cli/command_modules/acr/notary.py diff --git a/src/azure-cli/azure/cli/command_modules/acr/_errors.py b/src/azure-cli/azure/cli/command_modules/acr/_errors.py index 47474b357c9..cc509a54162 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/_errors.py +++ b/src/azure-cli/azure/cli/command_modules/acr/_errors.py @@ -67,6 +67,18 @@ def format_error_message(self, *args): ) +# NOTARY ERRORS +NOTARY_COMMAND_ERROR = ErrorClass( + "NOTARY_COMMAND_ERROR", + "Please verify if notary is installed." +) + +NOTARY_VERSION_ERROR = ErrorClass( + "NOTARY_VERSION_ERROR", + "An error occurred while retrieving notary version. Please make sure that you have the latest Azure CLI version, and that you are using the recommended notary version." +) + + # CONNECTIVITY ERRORS CONNECTIVITY_DNS_ERROR = ErrorClass( "CONNECTIVITY_DNS_ERROR", diff --git a/src/azure-cli/azure/cli/command_modules/acr/check_health.py b/src/azure-cli/azure/cli/command_modules/acr/check_health.py index 21a3690150c..61304fb26a5 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/check_health.py +++ b/src/azure-cli/azure/cli/command_modules/acr/check_health.py @@ -23,8 +23,10 @@ FAQ_MESSAGE = "\nPlease refer to https://aka.ms/acr/health-check for more information." ERROR_MSG_DEEP_LINK = "\nPlease refer to https://aka.ms/acr/errors#{} for more information." MIN_HELM_VERSION = "2.11.0" -HELM_VERSION_REGEX = re.compile(r'(SemVer|Version):"v([.\d]+)"') +HELM_VERSION_REGEX = re.compile(r'SemVer:"v([.\d]+)"', re.I) ACR_CHECK_HEALTH_MSG = "Try running 'az acr check-health -n {} --yes' to diagnose this issue." +RECOMMENDED_NOTARY_VERSION = "0.6.0" +NOTARY_VERSION_REGEX = re.compile(r'Version:\s+([.\d]+)', re.I) # Utilities functions @@ -156,7 +158,7 @@ def _get_helm_version(ignore_errors): # Retrieve the helm version if regex pattern is found match_obj = HELM_VERSION_REGEX.search(output) if match_obj: - output = match_obj.group(2) + output = match_obj.group(1) print("Helm version: {}".format(output), file=sys.stderr) @@ -168,6 +170,58 @@ def _get_helm_version(ignore_errors): _handle_error(obsolete_ver_error, ignore_errors) +def _get_notary_version(ignore_errors): + from ._errors import NOTARY_VERSION_ERROR + from .notary import get_notary_command + from distutils.version import LooseVersion # pylint: disable=import-error,no-name-in-module + + # Notary command check + notary_command, error = get_notary_command(is_diagnostics_context=True) + + if error: + _handle_error(error, ignore_errors) + return + + # Notary version check + output, warning, stderr = _subprocess_communicate([notary_command, "version"]) + + if stderr: + _handle_error(NOTARY_VERSION_ERROR.append_error_message(stderr), ignore_errors) + return + + if warning: + logger.warning(warning) + + # Retrieve the notary version if regex pattern is found + match_obj = NOTARY_VERSION_REGEX.search(output) + if match_obj: + output = match_obj.group(1) + + print("Notary version: {}".format(output), file=sys.stderr) + + # Display error if the current version does not match the recommended version + if match_obj and LooseVersion(output) != LooseVersion(RECOMMENDED_NOTARY_VERSION): + version_msg = "upgrade" + if LooseVersion(output) > LooseVersion(RECOMMENDED_NOTARY_VERSION): + version_msg = "downgrade" + obsolete_ver_error = NOTARY_VERSION_ERROR.set_error_message( + "Current notary version is not recommended. Please {} your notary client to version {}." + .format(version_msg, RECOMMENDED_NOTARY_VERSION)) + _handle_error(obsolete_ver_error, ignore_errors) + + +def _check_health_environment(ignore_errors, yes): + from azure.cli.core.util import in_cloud_console + if in_cloud_console(): + logger.warning("Environment checks are not supported in Azure Cloud Shell.") + return + + _get_docker_status_and_version(ignore_errors, yes) + _get_cli_version() + _get_helm_version(ignore_errors) + _get_notary_version(ignore_errors) + + # Checks for the connectivity # Check DNS lookup and access to challenge endpoint def _get_registry_status(login_server, registry_name, ignore_errors): @@ -277,17 +331,7 @@ def acr_check_health(cmd, # pylint: disable useless-return ignore_errors=False, yes=False, registry_name=None): - from azure.cli.core.util import in_cloud_console - in_cloud_console = in_cloud_console() - if in_cloud_console: - logger.warning("Environment checks are not supported in Azure Cloud Shell.") - else: - _get_docker_status_and_version(ignore_errors, yes) - _get_cli_version() + _check_health_environment(ignore_errors, yes) _check_health_connectivity(cmd, registry_name, ignore_errors) - - if not in_cloud_console: - _get_helm_version(ignore_errors) - print(FAQ_MESSAGE, file=sys.stderr) diff --git a/src/azure-cli/azure/cli/command_modules/acr/notary.py b/src/azure-cli/azure/cli/command_modules/acr/notary.py new file mode 100644 index 00000000000..a6dc91fb285 --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/acr/notary.py @@ -0,0 +1,38 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +from knack.util import CLIError +from knack.log import get_logger + +logger = get_logger(__name__) + + +def get_notary_command(is_diagnostics_context=False): + from ._errors import NOTARY_COMMAND_ERROR + notary_command = "notary" + + from subprocess import PIPE, Popen + try: + p = Popen([notary_command, "--help"], stdout=PIPE, stderr=PIPE) + _, stderr = p.communicate() + except OSError as e: + logger.debug("Could not run '%s' command. Exception: %s", notary_command, str(e)) + # The executable may not be discoverable in WSL so retry *.exe once + try: + notary_command = 'notary.exe' + p = Popen([notary_command, "--help"], stdout=PIPE, stderr=PIPE) + _, stderr = p.communicate() + except OSError as inner: + logger.debug("Could not run '%s' command. Exception: %s", notary_command, str(inner)) + if is_diagnostics_context: + return None, NOTARY_COMMAND_ERROR + raise CLIError(NOTARY_COMMAND_ERROR.get_error_message()) + + if stderr: + if is_diagnostics_context: + return None, NOTARY_COMMAND_ERROR.append_error_message(stderr.decode()) + raise CLIError(NOTARY_COMMAND_ERROR.append_error_message(stderr.decode()).get_error_message()) + + return notary_command, None From 54a0a6ca3889e8ac968c6995f2f8542f346fbf2d Mon Sep 17 00:00:00 2001 From: Isaac Lee Date: Wed, 8 Jan 2020 11:59:28 -0800 Subject: [PATCH 2/4] Revise to branch off newest changes --- .../cli/command_modules/acr/check_health.py | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acr/check_health.py b/src/azure-cli/azure/cli/command_modules/acr/check_health.py index 61304fb26a5..e2ffe787aea 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/check_health.py +++ b/src/azure-cli/azure/cli/command_modules/acr/check_health.py @@ -23,10 +23,10 @@ FAQ_MESSAGE = "\nPlease refer to https://aka.ms/acr/health-check for more information." ERROR_MSG_DEEP_LINK = "\nPlease refer to https://aka.ms/acr/errors#{} for more information." MIN_HELM_VERSION = "2.11.0" -HELM_VERSION_REGEX = re.compile(r'SemVer:"v([.\d]+)"', re.I) +HELM_VERSION_REGEX = re.compile(r'(SemVer|Version):"v([.\d]+)"') ACR_CHECK_HEALTH_MSG = "Try running 'az acr check-health -n {} --yes' to diagnose this issue." RECOMMENDED_NOTARY_VERSION = "0.6.0" -NOTARY_VERSION_REGEX = re.compile(r'Version:\s+([.\d]+)', re.I) +NOTARY_VERSION_REGEX = re.compile(r'Version:\s+([.\d]+)') # Utilities functions @@ -158,7 +158,7 @@ def _get_helm_version(ignore_errors): # Retrieve the helm version if regex pattern is found match_obj = HELM_VERSION_REGEX.search(output) if match_obj: - output = match_obj.group(1) + output = match_obj.group(2) print("Helm version: {}".format(output), file=sys.stderr) @@ -210,18 +210,6 @@ def _get_notary_version(ignore_errors): _handle_error(obsolete_ver_error, ignore_errors) -def _check_health_environment(ignore_errors, yes): - from azure.cli.core.util import in_cloud_console - if in_cloud_console(): - logger.warning("Environment checks are not supported in Azure Cloud Shell.") - return - - _get_docker_status_and_version(ignore_errors, yes) - _get_cli_version() - _get_helm_version(ignore_errors) - _get_notary_version(ignore_errors) - - # Checks for the connectivity # Check DNS lookup and access to challenge endpoint def _get_registry_status(login_server, registry_name, ignore_errors): @@ -331,7 +319,18 @@ def acr_check_health(cmd, # pylint: disable useless-return ignore_errors=False, yes=False, registry_name=None): + from azure.cli.core.util import in_cloud_console + in_cloud_console = in_cloud_console() + if in_cloud_console: + logger.warning("Environment checks are not supported in Azure Cloud Shell.") + else: + _get_docker_status_and_version(ignore_errors, yes) + _get_cli_version() - _check_health_environment(ignore_errors, yes) _check_health_connectivity(cmd, registry_name, ignore_errors) + + if not in_cloud_console: + _get_helm_version(ignore_errors) + _get_notary_version(ignore_errors) + print(FAQ_MESSAGE, file=sys.stderr) From bd4c62ce297d34d86875f662057269da2409bb8b Mon Sep 17 00:00:00 2001 From: Isaac Lee Date: Thu, 9 Jan 2020 11:13:52 -0800 Subject: [PATCH 3/4] Use logger.warning and specify .exe for Windows --- .../cli/command_modules/acr/check_health.py | 4 ++-- .../azure/cli/command_modules/acr/notary.py | 22 +++++++++---------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acr/check_health.py b/src/azure-cli/azure/cli/command_modules/acr/check_health.py index e2ffe787aea..c91ecf8ee73 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/check_health.py +++ b/src/azure-cli/azure/cli/command_modules/acr/check_health.py @@ -160,7 +160,7 @@ def _get_helm_version(ignore_errors): if match_obj: output = match_obj.group(2) - print("Helm version: {}".format(output), file=sys.stderr) + logger.warning("Helm version: {}".format(output)) # Display an error message if the current helm version < min required version if match_obj and LooseVersion(output) < LooseVersion(MIN_HELM_VERSION): @@ -197,7 +197,7 @@ def _get_notary_version(ignore_errors): if match_obj: output = match_obj.group(1) - print("Notary version: {}".format(output), file=sys.stderr) + logger.warning("Notary version: {}".format(output)) # Display error if the current version does not match the recommended version if match_obj and LooseVersion(output) != LooseVersion(RECOMMENDED_NOTARY_VERSION): diff --git a/src/azure-cli/azure/cli/command_modules/acr/notary.py b/src/azure-cli/azure/cli/command_modules/acr/notary.py index a6dc91fb285..e320f120ccd 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/notary.py +++ b/src/azure-cli/azure/cli/command_modules/acr/notary.py @@ -11,24 +11,22 @@ def get_notary_command(is_diagnostics_context=False): from ._errors import NOTARY_COMMAND_ERROR - notary_command = "notary" - from subprocess import PIPE, Popen + from platform import system + + if system() == "Windows": + notary_command = "notary.exe" + else: + notary_command = "notary" + try: p = Popen([notary_command, "--help"], stdout=PIPE, stderr=PIPE) _, stderr = p.communicate() except OSError as e: logger.debug("Could not run '%s' command. Exception: %s", notary_command, str(e)) - # The executable may not be discoverable in WSL so retry *.exe once - try: - notary_command = 'notary.exe' - p = Popen([notary_command, "--help"], stdout=PIPE, stderr=PIPE) - _, stderr = p.communicate() - except OSError as inner: - logger.debug("Could not run '%s' command. Exception: %s", notary_command, str(inner)) - if is_diagnostics_context: - return None, NOTARY_COMMAND_ERROR - raise CLIError(NOTARY_COMMAND_ERROR.get_error_message()) + if is_diagnostics_context: + return None, NOTARY_COMMAND_ERROR + raise CLIError(NOTARY_COMMAND_ERROR.get_error_message()) if stderr: if is_diagnostics_context: From b1651d4ab54015f97253f860b7612df95e4a0af7 Mon Sep 17 00:00:00 2001 From: Isaac Lee Date: Fri, 17 Jan 2020 10:37:15 -0800 Subject: [PATCH 4/4] Update logging to use % --- src/azure-cli/azure/cli/command_modules/acr/check_health.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acr/check_health.py b/src/azure-cli/azure/cli/command_modules/acr/check_health.py index c91ecf8ee73..d7803a2b68a 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/check_health.py +++ b/src/azure-cli/azure/cli/command_modules/acr/check_health.py @@ -160,7 +160,7 @@ def _get_helm_version(ignore_errors): if match_obj: output = match_obj.group(2) - logger.warning("Helm version: {}".format(output)) + logger.warning("Helm version: %s", output) # Display an error message if the current helm version < min required version if match_obj and LooseVersion(output) < LooseVersion(MIN_HELM_VERSION): @@ -197,7 +197,7 @@ def _get_notary_version(ignore_errors): if match_obj: output = match_obj.group(1) - logger.warning("Notary version: {}".format(output)) + logger.warning("Notary version: %s", output) # Display error if the current version does not match the recommended version if match_obj and LooseVersion(output) != LooseVersion(RECOMMENDED_NOTARY_VERSION):