From 538bbfa3ecabf2467a732012ae8e396d61a9329a Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Mon, 16 Sep 2024 12:56:44 -0700 Subject: [PATCH 1/8] CLI Phase 1 - add UpgradeableTo field to update functionality --- python/az/aro/azext_aro/_params.py | 3 +++ python/az/aro/azext_aro/_validators.py | 7 +++++++ python/az/aro/azext_aro/custom.py | 27 ++++++++++++++------------ 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/python/az/aro/azext_aro/_params.py b/python/az/aro/azext_aro/_params.py index c532cf13abb..4f74e3399f2 100644 --- a/python/az/aro/azext_aro/_params.py +++ b/python/az/aro/azext_aro/_params.py @@ -160,6 +160,9 @@ def load_arguments(self, _): options_list=['--assign-platform-workload-identity', '--assign-platform-wi'], validator=validate_platform_workload_identities(isCreate=False), action=AROPlatformWorkloadIdentityAddAction, nargs='+') + c.argument('upgradeable_to', arg_group='Identity', options_list=['--upgradeable_to'], + help='OpenShift version to upgrade/update to.', is_preview=True, + validator=validate_version_format) with self.argument_context('aro get-admin-kubeconfig') as c: c.argument('file', diff --git a/python/az/aro/azext_aro/_validators.py b/python/az/aro/azext_aro/_validators.py index 9fdee64cf32..92a19afa9c4 100644 --- a/python/az/aro/azext_aro/_validators.py +++ b/python/az/aro/azext_aro/_validators.py @@ -287,6 +287,13 @@ def validate_version_format(namespace): if namespace.version is not None and not re.match(r'^[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}$', namespace.version): raise InvalidArgumentValueError('--version is invalid') +def validate_upgradeable_to_format(namespace): + if not namespace.upgradeable_to: + return + if not re.match(r'^[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}$', namespace.upgradeable_to): + raise InvalidArgumentValueError('--upgradeable-to is invalid') + if namespace.client_secret is not None or namespace.client_id is not None: + raise RequiredArgumentMissingError('--client-id and --client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long def validate_load_balancer_managed_outbound_ip_count(namespace): if namespace.load_balancer_managed_outbound_ip_count is None: diff --git a/python/az/aro/azext_aro/custom.py b/python/az/aro/azext_aro/custom.py index 056661f596a..486fb726e97 100644 --- a/python/az/aro/azext_aro/custom.py +++ b/python/az/aro/azext_aro/custom.py @@ -455,6 +455,7 @@ def aro_update(cmd, client_secret=None, platform_workload_identities=None, load_balancer_managed_outbound_ip_count=None, + upgradeable_to=None, no_wait=False): # if we can't read cluster spec, we will not be able to do much. Fail. oc = client.open_shift_clusters.get(resource_group_name, resource_name) @@ -479,20 +480,22 @@ def aro_update(cmd, if client_id is not None: oc_update.service_principal_profile.client_id = client_id - if platform_workload_identities is not None: - pwis = {} - for i in oc.platform_workload_identity_profile.platform_workload_identities: - pwis[i.operator_name] = openshiftcluster.PlatformWorkloadIdentity( - operator_name=i.operator_name, - resource_id=i.resource_id - ) + if oc.platform_workload_identity_profile is not None: + if platform_workload_identities is not None: + pwis = {} + for i in oc.platform_workload_identity_profile.platform_workload_identities: + pwis[i.operator_name] = openshiftcluster.PlatformWorkloadIdentity( + operator_name=i.operator_name, + resource_id=i.resource_id + ) - for i in platform_workload_identities: - pwis[i.operator_name] = i + for i in platform_workload_identities: + pwis[i.operator_name] = i - oc_update.platform_workload_identity_profile = openshiftcluster.PlatformWorkloadIdentityProfile( - platform_workload_identities=list(pwis.values()) - ) + oc_update.platform_workload_identity_profile = openshiftcluster.PlatformWorkloadIdentityProfile( + platform_workload_identities=list(pwis.values()) + ) + oc.platform_workload_identity_profile.upgradeable_to = upgradeable_to if load_balancer_managed_outbound_ip_count is not None: oc_update.network_profile = openshiftcluster.NetworkProfile() From 27bc4743bab6469c4b23e91801dba8969863471e Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Mon, 16 Sep 2024 14:14:51 -0700 Subject: [PATCH 2/8] fix upgradeableTo parameter --- python/az/aro/azext_aro/_params.py | 5 +++-- python/az/aro/azext_aro/custom.py | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/python/az/aro/azext_aro/_params.py b/python/az/aro/azext_aro/_params.py index 4f74e3399f2..57e80572f1c 100644 --- a/python/az/aro/azext_aro/_params.py +++ b/python/az/aro/azext_aro/_params.py @@ -23,6 +23,7 @@ validate_enable_managed_identity, validate_platform_workload_identities, validate_cluster_identity, + validate_upgradeable_to_format, ) from azure.cli.core.commands.parameters import ( name_type, @@ -160,9 +161,9 @@ def load_arguments(self, _): options_list=['--assign-platform-workload-identity', '--assign-platform-wi'], validator=validate_platform_workload_identities(isCreate=False), action=AROPlatformWorkloadIdentityAddAction, nargs='+') - c.argument('upgradeable_to', arg_group='Identity', options_list=['--upgradeable_to'], + c.argument('upgradeable_to', arg_group='Identity', options_list=['--upgradeable-to'], help='OpenShift version to upgrade/update to.', is_preview=True, - validator=validate_version_format) + validator=validate_upgradeable_to_format) with self.argument_context('aro get-admin-kubeconfig') as c: c.argument('file', diff --git a/python/az/aro/azext_aro/custom.py b/python/az/aro/azext_aro/custom.py index 486fb726e97..9436cb31f62 100644 --- a/python/az/aro/azext_aro/custom.py +++ b/python/az/aro/azext_aro/custom.py @@ -481,6 +481,9 @@ def aro_update(cmd, oc_update.service_principal_profile.client_id = client_id if oc.platform_workload_identity_profile is not None: + if platform_workload_identities is not None or upgradeable_to is not None: + oc_update.platform_workload_identity_profile = openshiftcluster.PlatformWorkloadIdentityProfile() + if platform_workload_identities is not None: pwis = {} for i in oc.platform_workload_identity_profile.platform_workload_identities: @@ -492,10 +495,9 @@ def aro_update(cmd, for i in platform_workload_identities: pwis[i.operator_name] = i - oc_update.platform_workload_identity_profile = openshiftcluster.PlatformWorkloadIdentityProfile( - platform_workload_identities=list(pwis.values()) - ) - oc.platform_workload_identity_profile.upgradeable_to = upgradeable_to + oc_update.platform_workload_identity_profile.platform_workload_identities=list(pwis.values()) + + oc_update.platform_workload_identity_profile.upgradeable_to = upgradeable_to if load_balancer_managed_outbound_ip_count is not None: oc_update.network_profile = openshiftcluster.NetworkProfile() From b161f58dedb4f54d188e2dba6a12185af24afac4 Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Wed, 18 Sep 2024 11:35:47 -0700 Subject: [PATCH 3/8] apply suggestions from code review --- python/az/aro/azext_aro/_params.py | 4 ++-- python/az/aro/azext_aro/_validators.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/az/aro/azext_aro/_params.py b/python/az/aro/azext_aro/_params.py index 57e80572f1c..30652368e04 100644 --- a/python/az/aro/azext_aro/_params.py +++ b/python/az/aro/azext_aro/_params.py @@ -162,8 +162,8 @@ def load_arguments(self, _): validator=validate_platform_workload_identities(isCreate=False), action=AROPlatformWorkloadIdentityAddAction, nargs='+') c.argument('upgradeable_to', arg_group='Identity', options_list=['--upgradeable-to'], - help='OpenShift version to upgrade/update to.', is_preview=True, - validator=validate_upgradeable_to_format) + help='OpenShift version to upgrade to.', is_preview=True, + validator=validate_upgradeable_to_format) with self.argument_context('aro get-admin-kubeconfig') as c: c.argument('file', diff --git a/python/az/aro/azext_aro/_validators.py b/python/az/aro/azext_aro/_validators.py index 92a19afa9c4..883ef2f2c48 100644 --- a/python/az/aro/azext_aro/_validators.py +++ b/python/az/aro/azext_aro/_validators.py @@ -41,6 +41,7 @@ def _validate_cidr(namespace): def validate_client_id(namespace): if namespace.client_id is None: return + raise RequiredArgumentMissingError('--client-id must be not set with --upgradeable-to.') # pylint: disable=line-too-long if namespace.enable_managed_identity is True: raise MutuallyExclusiveArgumentError('Must not specify --client-id when --enable-managed-identity is True') # pylint: disable=line-too-long if namespace.platform_workload_identities is not None: @@ -59,6 +60,7 @@ def validate_client_secret(isCreate): def _validate_client_secret(namespace): if namespace.client_secret is None: return + raise RequiredArgumentMissingError('--client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long if namespace.enable_managed_identity is True: raise MutuallyExclusiveArgumentError('Must not specify --client-secret when --enable-managed-identity is True') # pylint: disable=line-too-long if namespace.platform_workload_identities is not None: @@ -292,8 +294,6 @@ def validate_upgradeable_to_format(namespace): return if not re.match(r'^[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}$', namespace.upgradeable_to): raise InvalidArgumentValueError('--upgradeable-to is invalid') - if namespace.client_secret is not None or namespace.client_id is not None: - raise RequiredArgumentMissingError('--client-id and --client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long def validate_load_balancer_managed_outbound_ip_count(namespace): if namespace.load_balancer_managed_outbound_ip_count is None: From 4efa3b56b02e1ad9c5a681482d09b77379cdd49e Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Thu, 19 Sep 2024 12:59:37 -0700 Subject: [PATCH 4/8] add unit tests for upgradeableTo --- python/az/aro/azext_aro/_validators.py | 9 ++-- python/az/aro/azext_aro/custom.py | 6 +-- .../tests/latest/unit/test_validators.py | 51 ++++++++++++++++++- 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/python/az/aro/azext_aro/_validators.py b/python/az/aro/azext_aro/_validators.py index 883ef2f2c48..ae6cb3dd19c 100644 --- a/python/az/aro/azext_aro/_validators.py +++ b/python/az/aro/azext_aro/_validators.py @@ -41,12 +41,10 @@ def _validate_cidr(namespace): def validate_client_id(namespace): if namespace.client_id is None: return - raise RequiredArgumentMissingError('--client-id must be not set with --upgradeable-to.') # pylint: disable=line-too-long if namespace.enable_managed_identity is True: raise MutuallyExclusiveArgumentError('Must not specify --client-id when --enable-managed-identity is True') # pylint: disable=line-too-long if namespace.platform_workload_identities is not None: raise MutuallyExclusiveArgumentError('Must not specify --client-id when --assign-platform-workload-identity is used') # pylint: disable=line-too-long - try: uuid.UUID(namespace.client_id) except ValueError as e: @@ -54,19 +52,20 @@ def validate_client_id(namespace): if namespace.client_secret is None or not str(namespace.client_secret): raise RequiredArgumentMissingError('Must specify --client-secret with --client-id.') # pylint: disable=line-too-long + raise RequiredArgumentMissingError('--client-id must be not set with --upgradeable-to.') # pylint: disable=line-too-long def validate_client_secret(isCreate): def _validate_client_secret(namespace): if namespace.client_secret is None: return - raise RequiredArgumentMissingError('--client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long if namespace.enable_managed_identity is True: raise MutuallyExclusiveArgumentError('Must not specify --client-secret when --enable-managed-identity is True') # pylint: disable=line-too-long if namespace.platform_workload_identities is not None: raise MutuallyExclusiveArgumentError('Must not specify --client-secret when --assign-platform-workload-identity is used') # pylint: disable=line-too-long if isCreate and (namespace.client_id is None or not str(namespace.client_id)): raise RequiredArgumentMissingError('Must specify --client-id with --client-secret.') + raise RequiredArgumentMissingError('--client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long return _validate_client_secret @@ -289,11 +288,13 @@ def validate_version_format(namespace): if namespace.version is not None and not re.match(r'^[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}$', namespace.version): raise InvalidArgumentValueError('--version is invalid') + def validate_upgradeable_to_format(namespace): if not namespace.upgradeable_to: return if not re.match(r'^[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}$', namespace.upgradeable_to): - raise InvalidArgumentValueError('--upgradeable-to is invalid') + raise InvalidArgumentValueError('--upgradeable-to format is invalid') + def validate_load_balancer_managed_outbound_ip_count(namespace): if namespace.load_balancer_managed_outbound_ip_count is None: diff --git a/python/az/aro/azext_aro/custom.py b/python/az/aro/azext_aro/custom.py index 9436cb31f62..c8fe7f24531 100644 --- a/python/az/aro/azext_aro/custom.py +++ b/python/az/aro/azext_aro/custom.py @@ -495,9 +495,9 @@ def aro_update(cmd, for i in platform_workload_identities: pwis[i.operator_name] = i - oc_update.platform_workload_identity_profile.platform_workload_identities=list(pwis.values()) - - oc_update.platform_workload_identity_profile.upgradeable_to = upgradeable_to + oc_update.platform_workload_identity_profile.platform_workload_identities = list(pwis.values()) + + oc_update.platform_workload_identity_profile.upgradeable_to = upgradeable_to if load_balancer_managed_outbound_ip_count is not None: oc_update.network_profile = openshiftcluster.NetworkProfile() diff --git a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py index 4fa9018be51..5c3e4c20386 100644 --- a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py +++ b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py @@ -21,7 +21,8 @@ validate_load_balancer_managed_outbound_ip_count, validate_enable_managed_identity, validate_platform_workload_identities, - validate_cluster_identity + validate_cluster_identity, + validate_upgradeable_to_format ) from azure.cli.core.azclierror import ( InvalidArgumentValueError, RequiredArgumentMissingError, @@ -1276,3 +1277,51 @@ def test_validate_cluster_identity(test_description, namespace, expected_excepti if expected_identity is not None: assert (expected_identity == namespace.mi_user_assigned) + + +test_validate_upgradeable_to_data = [ + ( + "should not raise any Exception because namespace.upgradeable_to is empty", + Mock(upgradeable_to="", client_id=None, client_secret=None), + None, None + ), + + ( + "should raise RequiredArgumentMissingError Exception because namespace.client_id is present", + Mock(upgradeable_to="4.14.5", client_id="client_id_456", client_secret=None), + RequiredArgumentMissingError, '--client-id must be not set with --upgradeable-to.' + ), + + ( + "should raise RequiredArgumentMissingError Exception because namespace.client_secret is present", + Mock(upgradeable_to="4.14.5", client_id=None, client_secret="secret_123"), + RequiredArgumentMissingError, '--client-secret must be not set with --upgradeable-to.' + ), + + ( + "should raise InvalidArgumentValueError Exception because upgradeable_to format is invalid", + Mock(upgradeable_to="a", client_id=None, client_secret=None), + InvalidArgumentValueError, "--upgradeable-to is invalid" + ), + + ( + "Should raise InvalidArgumentValueError when --upgradeable-to < 4.14.0", + Mock(upgradeable_to="4.0.4", + client_id=None, client_secret=None), + InvalidArgumentValueError, 'Enabling managed identity requires --upgradeable-to >= 4.14.z' + ), + +] + + +@pytest.mark.parametrize( + "test_description, namespace, expected_exception, expected_exception_message", + test_validate_upgradeable_to_data, + ids=[i[0] for i in test_validate_upgradeable_to_data] +) +def test_validate_upgradeable_to_data(test_description, namespace, expected_exception, expected_exception_message): + if expected_exception is None: + validate_upgradeable_to_format(namespace) + else: + with pytest.raises(expected_exception): + validate_upgradeable_to_format(namespace) From dcf1c96296e7ccc06327dbfaf3e30a1bac191707 Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Thu, 26 Sep 2024 10:21:02 -0700 Subject: [PATCH 5/8] fix unit tests --- python/az/aro/azext_aro/_validators.py | 8 +++-- .../tests/latest/unit/test_validators.py | 32 +++++++++---------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/python/az/aro/azext_aro/_validators.py b/python/az/aro/azext_aro/_validators.py index ae6cb3dd19c..e6c9a897dec 100644 --- a/python/az/aro/azext_aro/_validators.py +++ b/python/az/aro/azext_aro/_validators.py @@ -52,7 +52,8 @@ def validate_client_id(namespace): if namespace.client_secret is None or not str(namespace.client_secret): raise RequiredArgumentMissingError('Must specify --client-secret with --client-id.') # pylint: disable=line-too-long - raise RequiredArgumentMissingError('--client-id must be not set with --upgradeable-to.') # pylint: disable=line-too-long + if namespace.upgradeable_to is not None: + raise RequiredArgumentMissingError('--client-id must be not set with --upgradeable-to.') # pylint: disable=line-too-long def validate_client_secret(isCreate): @@ -65,7 +66,8 @@ def _validate_client_secret(namespace): raise MutuallyExclusiveArgumentError('Must not specify --client-secret when --assign-platform-workload-identity is used') # pylint: disable=line-too-long if isCreate and (namespace.client_id is None or not str(namespace.client_id)): raise RequiredArgumentMissingError('Must specify --client-id with --client-secret.') - raise RequiredArgumentMissingError('--client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long + if namespace.upgradeable_to is not None: + raise RequiredArgumentMissingError('--client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long return _validate_client_secret @@ -292,7 +294,7 @@ def validate_version_format(namespace): def validate_upgradeable_to_format(namespace): if not namespace.upgradeable_to: return - if not re.match(r'^[4-9]{1}\.[0-9]{1,2}\.[0-9]{1,2}$', namespace.upgradeable_to): + if not re.match(r'^[4-9]{1}\.(1[4-9]|[1-9][0-9])\.[0-9]{1,2}$', namespace.upgradeable_to): raise InvalidArgumentValueError('--upgradeable-to format is invalid') diff --git a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py index 5c3e4c20386..545ab188158 100644 --- a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py +++ b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py @@ -113,7 +113,7 @@ def test_validate_cidr(test_description, dummyclass, attribute_to_get_from_objec ), ( "should not raise any exception when namespace.client_id is a valid input for creating a UUID and namespace.client_secret has a valid str representation", - Mock(client_id="12345678123456781234567812345678", platform_workload_identities=None, client_secret="12345"), + Mock(upgradeable_to=None, client_id="12345678123456781234567812345678", platform_workload_identities=None, client_secret="12345"), None ) ] @@ -172,15 +172,27 @@ def test_validate_client_id(test_description, namespace, expected_exception): ( "should not raise any exception when isCreate is true and all arguments valid", True, - Mock(client_id="12345678123456781234567812345678", client_secret="123", platform_workload_identities=None), + Mock(upgradeable_to=None, client_id="12345678123456781234567812345678", client_secret="123", platform_workload_identities=None), None ), ( "should not raise any exception when isCreate is false and all arguments valid", False, - Mock(client_secret="123", platform_workload_identities=None), + Mock(upgradeable_to=None, client_secret="123", platform_workload_identities=None), None ), + ( + "should raise any exception when isCreate is true and upgradeable_to, client_id and client_secret are present", + True, + Mock(upgradeable_to="4.14.2", client_id="12345678123456781234567812345678", client_secret="123", platform_workload_identities=None), + RequiredArgumentMissingError + ), + ( + "should raise any exception when isCreate is false and upgradeable_to, client_id and client_secret are present", + False, + Mock(upgradeable_to="4.14.2", client_id="12345678123456781234567812345678", client_secret="123", platform_workload_identities=None), + RequiredArgumentMissingError + ), ] @@ -1286,18 +1298,6 @@ def test_validate_cluster_identity(test_description, namespace, expected_excepti None, None ), - ( - "should raise RequiredArgumentMissingError Exception because namespace.client_id is present", - Mock(upgradeable_to="4.14.5", client_id="client_id_456", client_secret=None), - RequiredArgumentMissingError, '--client-id must be not set with --upgradeable-to.' - ), - - ( - "should raise RequiredArgumentMissingError Exception because namespace.client_secret is present", - Mock(upgradeable_to="4.14.5", client_id=None, client_secret="secret_123"), - RequiredArgumentMissingError, '--client-secret must be not set with --upgradeable-to.' - ), - ( "should raise InvalidArgumentValueError Exception because upgradeable_to format is invalid", Mock(upgradeable_to="a", client_id=None, client_secret=None), @@ -1308,7 +1308,7 @@ def test_validate_cluster_identity(test_description, namespace, expected_excepti "Should raise InvalidArgumentValueError when --upgradeable-to < 4.14.0", Mock(upgradeable_to="4.0.4", client_id=None, client_secret=None), - InvalidArgumentValueError, 'Enabling managed identity requires --upgradeable-to >= 4.14.z' + InvalidArgumentValueError, 'Enabling managed identity requires --upgradeable-to >= 4.14.0' ), ] From 061f7a28c71e7ab717a6afcdf2ee385a6bb1f99f Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Thu, 26 Sep 2024 16:51:06 -0700 Subject: [PATCH 6/8] Disallow refresh_cluster_credentials together with upgradeable_to --- python/az/aro/azext_aro/_validators.py | 6 ++++-- .../tests/latest/unit/test_validators.py | 18 ++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/python/az/aro/azext_aro/_validators.py b/python/az/aro/azext_aro/_validators.py index e6c9a897dec..bdc2cc75d78 100644 --- a/python/az/aro/azext_aro/_validators.py +++ b/python/az/aro/azext_aro/_validators.py @@ -53,7 +53,7 @@ def validate_client_id(namespace): if namespace.client_secret is None or not str(namespace.client_secret): raise RequiredArgumentMissingError('Must specify --client-secret with --client-id.') # pylint: disable=line-too-long if namespace.upgradeable_to is not None: - raise RequiredArgumentMissingError('--client-id must be not set with --upgradeable-to.') # pylint: disable=line-too-long + raise MutuallyExclusiveArgumentError('Must not specify --client-id when --upgradeable-to is used.') # pylint: disable=line-too-long def validate_client_secret(isCreate): @@ -67,7 +67,7 @@ def _validate_client_secret(namespace): if isCreate and (namespace.client_id is None or not str(namespace.client_id)): raise RequiredArgumentMissingError('Must specify --client-id with --client-secret.') if namespace.upgradeable_to is not None: - raise RequiredArgumentMissingError('--client-secret must be not set with --upgradeable-to.') # pylint: disable=line-too-long + raise MutuallyExclusiveArgumentError('Must not specify --client-secret when --upgradeable-to is used.') # pylint: disable=line-too-long return _validate_client_secret @@ -284,6 +284,8 @@ def validate_refresh_cluster_credentials(namespace): return if namespace.client_secret is not None or namespace.client_id is not None: raise RequiredArgumentMissingError('--client-id and --client-secret must be not set with --refresh-credentials.') # pylint: disable=line-too-long + if namespace.upgradeable_to is not None: + raise MutuallyExclusiveArgumentError('Must not specify --refresh-credentials when --upgradeable-to is used.') # pylint: disable=line-too-long def validate_version_format(namespace): diff --git a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py index 545ab188158..d02a823e14d 100644 --- a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py +++ b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py @@ -182,16 +182,16 @@ def test_validate_client_id(test_description, namespace, expected_exception): None ), ( - "should raise any exception when isCreate is true and upgradeable_to, client_id and client_secret are present", + "should raise MutuallyExclusiveArgumentError exception when isCreate is true and upgradeable_to, client_id and client_secret are present", True, Mock(upgradeable_to="4.14.2", client_id="12345678123456781234567812345678", client_secret="123", platform_workload_identities=None), - RequiredArgumentMissingError + MutuallyExclusiveArgumentError ), ( - "should raise any exception when isCreate is false and upgradeable_to, client_id and client_secret are present", + "should raise MutuallyExclusiveArgumentError exception when isCreate is false and upgradeable_to, client_id and client_secret are present", False, Mock(upgradeable_to="4.14.2", client_id="12345678123456781234567812345678", client_secret="123", platform_workload_identities=None), - RequiredArgumentMissingError + MutuallyExclusiveArgumentError ), ] @@ -817,9 +817,15 @@ def test_validate_worker_vm_disk_size_gb(test_description, namespace, expected_e ), ( "should not raise any Exception because namespace.client_secret is None and namespace.client_id is None", - Mock(client_secret=None, client_id=None), + Mock(upgradeable_to=None, client_secret=None, client_id=None), None - ) + ), + ( + "should raise RequiredArgumentMissingError Exception because namespace.upgradeable_to is not None", + Mock(upgradeable_to="4.14.2", client_id=None, client_secret=None), + MutuallyExclusiveArgumentError + ), + ] From 2fa27212a079651bc36f2f9d330c9ae8f08373f5 Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Fri, 27 Sep 2024 11:42:49 -0700 Subject: [PATCH 7/8] apply code review suggestions --- python/az/aro/azext_aro/_validators.py | 2 +- python/az/aro/azext_aro/tests/latest/unit/test_validators.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/python/az/aro/azext_aro/_validators.py b/python/az/aro/azext_aro/_validators.py index bdc2cc75d78..96b8941323a 100644 --- a/python/az/aro/azext_aro/_validators.py +++ b/python/az/aro/azext_aro/_validators.py @@ -285,7 +285,7 @@ def validate_refresh_cluster_credentials(namespace): if namespace.client_secret is not None or namespace.client_id is not None: raise RequiredArgumentMissingError('--client-id and --client-secret must be not set with --refresh-credentials.') # pylint: disable=line-too-long if namespace.upgradeable_to is not None: - raise MutuallyExclusiveArgumentError('Must not specify --refresh-credentials when --upgradeable-to is used.') # pylint: disable=line-too-long + raise MutuallyExclusiveArgumentError('Must not specify --refresh-credentials when --upgradeable-to is used.') # pylint: disable=line-too-long def validate_version_format(namespace): diff --git a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py index d02a823e14d..8a6390f4d72 100644 --- a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py +++ b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py @@ -821,7 +821,7 @@ def test_validate_worker_vm_disk_size_gb(test_description, namespace, expected_e None ), ( - "should raise RequiredArgumentMissingError Exception because namespace.upgradeable_to is not None", + "should raise MutuallyExclusiveArgumentError exception because namespace.upgradeable_to is not None", Mock(upgradeable_to="4.14.2", client_id=None, client_secret=None), MutuallyExclusiveArgumentError ), From 6ac3093d6f1daa6fb49da5d5edd7935fa18eee0d Mon Sep 17 00:00:00 2001 From: Sanjana Lawande Date: Tue, 1 Oct 2024 10:40:28 -0700 Subject: [PATCH 8/8] fix python lint issue --- python/az/aro/azext_aro/tests/latest/unit/test_validators.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py index 8a6390f4d72..0df05cf9c94 100644 --- a/python/az/aro/azext_aro/tests/latest/unit/test_validators.py +++ b/python/az/aro/azext_aro/tests/latest/unit/test_validators.py @@ -819,12 +819,12 @@ def test_validate_worker_vm_disk_size_gb(test_description, namespace, expected_e "should not raise any Exception because namespace.client_secret is None and namespace.client_id is None", Mock(upgradeable_to=None, client_secret=None, client_id=None), None - ), + ), ( "should raise MutuallyExclusiveArgumentError exception because namespace.upgradeable_to is not None", Mock(upgradeable_to="4.14.2", client_id=None, client_secret=None), MutuallyExclusiveArgumentError - ), + ), ]