From 44a6b733926827d706368f7ff483a0663c65204a Mon Sep 17 00:00:00 2001 From: Bavneet Singh Date: Tue, 23 Jul 2024 20:53:46 -0700 Subject: [PATCH] add validation for cli params --- .../azext_connectedk8s/_constants.py | 1 + .../azext_connectedk8s/_params.py | 18 +++++++++------ .../azext_connectedk8s/_validators.py | 23 +++++++++++++++++++ src/connectedk8s/azext_connectedk8s/custom.py | 19 ++++----------- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/connectedk8s/azext_connectedk8s/_constants.py b/src/connectedk8s/azext_connectedk8s/_constants.py index a79c209805d..e88cb790151 100644 --- a/src/connectedk8s/azext_connectedk8s/_constants.py +++ b/src/connectedk8s/azext_connectedk8s/_constants.py @@ -102,6 +102,7 @@ Proxy_Cert_Path_Does_Not_Exist_Error = 'Proxy cert path {} does not exist. Please check the path provided' Get_Kubernetes_Infra_Fault_Type = 'kubernetes-get-infrastructure-error' No_Param_Error = 'No parmeters were specified with update command. Please run az connectedk8s update --help to check parameters available for update' +Gateway_ArmId_Is_Invalid = "The provided Gateway ArmID in --gateway-resource-id {} is invalid. Please provide a valid Gateway ArmID." EnableProxy_Conflict_Error = 'Conflict detected: --disable-proxy can not be set with --https-proxy, --http-proxy, --proxy-skip-range and --proxy-cert at the same time. Please run az connectedk8s update --help for more information about the parameters' Manual_Upgrade_Called_In_Auto_Update_Enabled = 'Manual Upgrade was called while in auto_Update enabled mode' Upgrade_Agent_Success = 'Agents for Connected Cluster {} have been upgraded successfully' diff --git a/src/connectedk8s/azext_connectedk8s/_params.py b/src/connectedk8s/azext_connectedk8s/_params.py index 4c7da70f78f..349da56fd74 100644 --- a/src/connectedk8s/azext_connectedk8s/_params.py +++ b/src/connectedk8s/azext_connectedk8s/_params.py @@ -5,14 +5,18 @@ # pylint: disable=line-too-long import os.path -from ._validators import override_client_request_id_header from argcomplete.completers import FilesCompleter from azure.cli.core.commands.parameters import get_location_type, get_enum_type, file_type, tags_type, get_three_state_flag from azure.cli.core.commands.validators import get_default_location_from_resource_group from azext_connectedk8s._constants import Distribution_Enum_Values, Infrastructure_Enum_Values, Feature_Values, AHB_Enum_Values from knack.arguments import (CLIArgumentType, CaseInsensitiveList) -from ._validators import validate_private_link_properties +from ._validators import ( + validate_private_link_properties, + override_client_request_id_header, + validate_gateway_properties, + validate_gateway_updates +) from .action import ( AddConfigurationSettings, AddConfigurationProtectedSettings, @@ -58,8 +62,8 @@ def load_arguments(self, _): c.argument('enable_oidc_issuer', arg_type=get_three_state_flag(), help="Enable creation of OIDC issuer url used for workload identity", is_preview=True) c.argument('self_hosted_issuer', options_list=['--self-hosted-issuer'], help="Self hosted issuer url for public cloud clusters - AKS, GKE, EKS", is_preview=True) c.argument('enable_workload_identity', options_list=["--enable-workload-identity", "--enable-wi"], arg_type=get_three_state_flag(), help="Enable workload identity webhook", is_preview=True) - c.argument('enable_gateway', options_list=['--enable-gateway'], help='Pass this value to enable Arc Gateway.', is_preview=True) - c.argument('gateway_resource_id', options_list=['--gateway-resource-id'], help='ArmID of the Arc Gateway resource.', is_preview=True) + c.argument('enable_gateway', options_list=['--enable-gateway'], arg_group='Gateway', help='Pass this value to enable Arc Gateway.', validator=validate_gateway_properties) + c.argument('gateway_resource_id', options_list=['--gateway-resource-id'], arg_group='Gateway', help='ArmID of the Arc Gateway resource.', validator=validate_gateway_properties) c.argument('configuration_settings', options_list=['--configuration-settings', '--config'], action=AddConfigurationSettings, nargs='+', help='Configuration Settings as key=value pair. Repeat parameter for each setting. Do not use this for secrets, as this value is returned in response.', is_preview=True) c.argument('configuration_protected_settings', options_list=['--config-protected-settings', '--config-protected'], action=AddConfigurationProtectedSettings, nargs='+', help='Configuration Protected Settings as key=value pair. Repeat parameter for each setting. Only the key is returned in response, the value is not.', is_preview=True) @@ -84,9 +88,9 @@ def load_arguments(self, _): c.argument('self_hosted_issuer', options_list=["--self-hosted-issuer"], help="Self hosted issuer url for public cloud clusters - AKS, GKE, EKS", is_preview=True) c.argument('enable_workload_identity', options_list=["--enable-workload-identity", "--enable-wi"], arg_type=get_three_state_flag(), help="Enable workload identity webhook", is_preview=True) c.argument('disable_workload_identity', options_list=["--disable-workload-identity", "--disable-wi"], arg_type=get_three_state_flag(), help="Disable workload identity webhook", is_preview=True) - c.argument('enable_gateway', options_list=['--enable-gateway'], help='Flag to enable Arc Gateway.', is_preview=True) - c.argument('gateway_resource_id', options_list=['--gateway-resource-id'], help='ArmID of the Arc Gateway resource.', is_preview=True) - c.argument('disable_gateway', options_list=['--disable-gateway'], help='Flag to disable Arc Gateway', is_preview=True) + c.argument('enable_gateway', options_list=['--enable-gateway'], arg_group='Gateway', help='Flag to enable Arc Gateway.', validator=validate_gateway_updates) + c.argument('gateway_resource_id', options_list=['--gateway-resource-id'], arg_group='Gateway', help='ArmID of the Arc Gateway resource.', validator=validate_gateway_properties) + c.argument('disable_gateway', options_list=['--disable-gateway'], arg_group='Gateway', help='Flag to disable Arc Gateway', validator=validate_gateway_updates) c.argument('configuration_settings', options_list=['--configuration-settings', '--config'], action=AddConfigurationSettings, nargs='+', help='Configuration Settings as key=value pair. Repeat parameter for each setting. Do not use this for secrets, as this value is returned in response.', is_preview=True) c.argument('configuration_protected_settings', options_list=['--config-protected-settings', '--config-protected'], action=AddConfigurationProtectedSettings, nargs='+', help='Configuration Protected Settings as key=value pair. Repeat parameter for each setting. Only the key is returned in response, the value is not.', is_preview=True) diff --git a/src/connectedk8s/azext_connectedk8s/_validators.py b/src/connectedk8s/azext_connectedk8s/_validators.py index c0c0bb114da..1404192c4c0 100644 --- a/src/connectedk8s/azext_connectedk8s/_validators.py +++ b/src/connectedk8s/azext_connectedk8s/_validators.py @@ -7,6 +7,11 @@ from os import name from azure.cli.core.azclierror import ArgumentUsageError +from azure.cli.core.azclierror import ( + InvalidArgumentValueError, + ArgumentUsageError +) +import re def example_name_or_id_validator(cmd, namespace): @@ -39,3 +44,21 @@ def override_client_request_id_header(cmd, namespace): cmd.cli_ctx.data['headers'][consts.Client_Request_Id_Header] = namespace.correlation_id else: cmd.cli_ctx.data['headers'][consts.Client_Request_Id_Header] = consts.Default_Onboarding_Source_Tracking_Guid + + +def validate_gateway_properties(namespace): + if namespace.enable_gateway is False and namespace.gateway_resource_id != "": + raise ArgumentUsageError("Conflicting gateway parameters received. The parameter '--gateway-resource-id' should not be set only if '--enable-gateway' is set to true.") + if namespace.enable_gateway is True and namespace.gateway_resource_id != "": + gateway_armid_pattern = r"^/subscriptions/[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/resourceGroups/[a-zA-Z0-9_-]+/providers/Microsoft\.HybridCompute/gateways/[a-zA-Z0-9_-]+$" + if not re.match(gateway_armid_pattern, namespace.gateway_resource_id): + raise InvalidArgumentValueError(str.format(consts.Gateway_ArmId_Is_Invalid, namespace.gateway_resource_id)) + if namespace.enable_gateway is True and namespace.gateway_resource_id == "": + raise ArgumentUsageError("The parameter '--gateway-resource-id' was not provided. It is mandatory to pass this parameter for enabling gateway on the connected cluster resource.") + + +def validate_gateway_updates(namespace): + if namespace.enable_gateway and namespace.disable_gateway: + raise ArgumentUsageError("Cannot specify both --enable-gateway and --disable-gateway simultaneously.") + if namespace.enable_gateway and namespace.gateway_resource_id == "": + raise ArgumentUsageError("The parameter '--gateway-resource-id' was not provided. It is mandatory to pass this parameter for enabling gateway on the connected cluster resource.") \ No newline at end of file diff --git a/src/connectedk8s/azext_connectedk8s/custom.py b/src/connectedk8s/azext_connectedk8s/custom.py index 2e1b3263981..7cfa9505a24 100644 --- a/src/connectedk8s/azext_connectedk8s/custom.py +++ b/src/connectedk8s/azext_connectedk8s/custom.py @@ -152,15 +152,10 @@ def create_connectedk8s(cmd, client, resource_group_name, cluster_name, correlat gateway = None if enable_gateway: - gateway_armid_pattern = r"^/subscriptions/[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}/resourceGroups/[a-zA-Z0-9_-]+/providers/Microsoft\.HybridCompute/gateways/[a-zA-Z0-9_-]+$" - if re.match(gateway_armid_pattern, gateway_resource_id): - logger.warning("The provided Gateway ArmID is valid.") - gateway = Gateway( - enabled=True, - resource_id=gateway_resource_id - ) - else: - raise InvalidArgumentValueError(str.format(consts.Gateway_ArmId_Is_Invalid, gateway_resource_id)) + gateway = Gateway( + enabled=True, + resource_id=gateway_resource_id + ) # Checking whether optional extra values file has been provided. values_file = utils.get_values_file() @@ -1246,12 +1241,6 @@ def update_connected_cluster(cmd, client, resource_group_name, cluster_name, htt if disable_workload_identity is True: enable_workload_identity = False - # Validation for the gateway parameter - if enable_gateway and disable_gateway: - raise InvalidArgumentValueError("Do not specify both enable-gateway and disable-gateway at the same time.") - if enable_gateway and gateway_resource_id == "": - raise RequiredArgumentMissingError("gateway-resource-id is required when enable-gateway is specified.") - # Send cloud information to telemetry send_cloud_telemetry(cmd)