From a4199af61d3751984d63b0887ef00720a437c80f Mon Sep 17 00:00:00 2001 From: Linlu Liu <43486346+lil131@users.noreply.github.com> Date: Mon, 6 Jun 2022 11:32:11 -0400 Subject: [PATCH] show api error message for invalid cert name (#115) * show api error message for invalid cert name * hide prompt as default & surface api response * rename --prompt * added BREAKING CHANGE bullet to history.rst Co-authored-by: Haroon Feisal <38823870+haroonf@users.noreply.github.com> --- src/containerapp/HISTORY.rst | 2 ++ .../azext_containerapp/_constants.py | 3 ++ .../azext_containerapp/_params.py | 1 + src/containerapp/azext_containerapp/_utils.py | 4 +-- src/containerapp/azext_containerapp/custom.py | 28 +++++++++++++------ 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/containerapp/HISTORY.rst b/src/containerapp/HISTORY.rst index 656eff70161..ab5b286e7db 100644 --- a/src/containerapp/HISTORY.rst +++ b/src/containerapp/HISTORY.rst @@ -10,6 +10,8 @@ Release History * BREAKING CHANGE: 'az containerapp revision list' now shows only active revisions by default, added flag --all to show all revisions * Fixed bug with 'az containerapp up' where custom domains would be removed when updating existing containerapp * Fixed bug with 'az containerapp auth update' when using --unauthenticated-client-action +* BREAKING CHANGE: 'az containerapp env certificate upload' now overwrites the existing certificate by default. Added flag --overwrite to show the prompt for confirmation of overwriting the existing certificate. +* Fixed bug with 'az containerapp env certificate upload' where it shows a misleading message for invalid certificate name 0.3.5 ++++++ diff --git a/src/containerapp/azext_containerapp/_constants.py b/src/containerapp/azext_containerapp/_constants.py index 0b14b16d8f4..3840f24a6d4 100644 --- a/src/containerapp/azext_containerapp/_constants.py +++ b/src/containerapp/azext_containerapp/_constants.py @@ -24,3 +24,6 @@ UNAUTHENTICATED_CLIENT_ACTION = ['RedirectToLoginPage', 'AllowAnonymous', 'Return401', 'Return403'] FORWARD_PROXY_CONVENTION = ['NoProxy', 'Standard', 'Custom'] CHECK_CERTIFICATE_NAME_AVAILABILITY_TYPE = "Microsoft.App/managedEnvironments/certificates" + +NAME_INVALID = "Invalid" +NAME_ALREADY_EXISTS = "AlreadyExists" diff --git a/src/containerapp/azext_containerapp/_params.py b/src/containerapp/azext_containerapp/_params.py index fb71dc93ccd..46fd18ab7fc 100644 --- a/src/containerapp/azext_containerapp/_params.py +++ b/src/containerapp/azext_containerapp/_params.py @@ -155,6 +155,7 @@ def load_arguments(self, _): c.argument('certificate_file', options_list=['--certificate-file', '-f'], help='The filepath of the .pfx or .pem file') c.argument('certificate_name', options_list=['--certificate-name', '-c'], help='Name of the certificate which should be unique within the Container Apps environment.') c.argument('certificate_password', options_list=['--password', '-p'], help='The certificate file password') + c.argument('prompt', options_list=['--overwrite'], help='Boolean indicating whether to show a prompt for confirmation of overwriting the existing certificate.') with self.argument_context('containerapp env certificate list') as c: c.argument('name', id_part=None) diff --git a/src/containerapp/azext_containerapp/_utils.py b/src/containerapp/azext_containerapp/_utils.py index 829cde920f7..fe870233ce5 100644 --- a/src/containerapp/azext_containerapp/_utils.py +++ b/src/containerapp/azext_containerapp/_utils.py @@ -12,7 +12,7 @@ from datetime import datetime from dateutil.relativedelta import relativedelta from azure.cli.core.azclierror import (ValidationError, RequiredArgumentMissingError, CLIInternalError, - ResourceNotFoundError, ArgumentUsageError, FileOperationError, CLIError) + ResourceNotFoundError, FileOperationError, CLIError) from azure.cli.core.commands.client_factory import get_subscription_id from azure.cli.command_modules.appservice.utils import _normalize_location from azure.cli.command_modules.network._client_factory import network_client_factory @@ -1293,7 +1293,7 @@ def check_cert_name_availability(cmd, resource_group_name, name, cert_name): r = ManagedEnvironmentClient.check_name_availability(cmd, resource_group_name, name, name_availability_request) except CLIError as e: handle_raw_exception(e) - return r["nameAvailable"] + return r def validate_hostname(cmd, resource_group_name, name, hostname): diff --git a/src/containerapp/azext_containerapp/custom.py b/src/containerapp/azext_containerapp/custom.py index 9f638f3238a..7d9d78fcfe0 100644 --- a/src/containerapp/azext_containerapp/custom.py +++ b/src/containerapp/azext_containerapp/custom.py @@ -68,7 +68,8 @@ from ._ssh_utils import (SSH_DEFAULT_ENCODING, WebSocketConnection, read_ssh, get_stdin_writer, SSH_CTRL_C_MSG, SSH_BACKUP_ENCODING) from ._constants import (MAXIMUM_SECRET_LENGTH, MICROSOFT_SECRET_SETTING_NAME, FACEBOOK_SECRET_SETTING_NAME, GITHUB_SECRET_SETTING_NAME, - GOOGLE_SECRET_SETTING_NAME, TWITTER_SECRET_SETTING_NAME, APPLE_SECRET_SETTING_NAME, CONTAINER_APPS_RP) + GOOGLE_SECRET_SETTING_NAME, TWITTER_SECRET_SETTING_NAME, APPLE_SECRET_SETTING_NAME, CONTAINER_APPS_RP, + NAME_INVALID, NAME_ALREADY_EXISTS) logger = get_logger(__name__) @@ -2462,25 +2463,36 @@ def both_match(c): handle_raw_exception(e) -def upload_certificate(cmd, name, resource_group_name, certificate_file, certificate_name=None, certificate_password=None, location=None): +def upload_certificate(cmd, name, resource_group_name, certificate_file, certificate_name=None, certificate_password=None, location=None, prompt=False): _validate_subscription_registered(cmd, CONTAINER_APPS_RP) blob, thumbprint = load_cert_file(certificate_file, certificate_password) cert_name = None if certificate_name: - if not check_cert_name_availability(cmd, resource_group_name, name, certificate_name): - msg = 'A certificate with the name {} already exists in {}. If continue with this name, it will be overwritten by the new certificate file.\nOverwrite?' - overwrite = prompt_y_n(msg.format(certificate_name, name)) - if overwrite: - cert_name = certificate_name + name_availability = check_cert_name_availability(cmd, resource_group_name, name, certificate_name) + if not name_availability["nameAvailable"]: + if name_availability["reason"] == NAME_ALREADY_EXISTS: + msg = '{}. If continue with this name, it will be overwritten by the new certificate file.\nOverwrite?' + overwrite = True + if prompt: + overwrite = prompt_y_n(msg.format(name_availability["message"])) + else: + logger.warning('{}. It will be overwritten by the new certificate file.'.format(name_availability["message"])) + if overwrite: + cert_name = certificate_name + else: + raise ValidationError(name_availability["message"]) else: cert_name = certificate_name while not cert_name: random_name = generate_randomized_cert_name(thumbprint, name, resource_group_name) - if check_cert_name_availability(cmd, resource_group_name, name, random_name): + check_result = check_cert_name_availability(cmd, resource_group_name, name, random_name) + if check_result["nameAvailable"]: cert_name = random_name + elif not check_result["nameAvailable"] and (check_result["reason"] == NAME_INVALID): + raise ValidationError(check_result["message"]) certificate = ContainerAppCertificateEnvelopeModel certificate["properties"]["password"] = certificate_password