From 8f48eea3b2193298008169e3329065716e5a819d Mon Sep 17 00:00:00 2001 From: hbc Date: Mon, 23 May 2022 11:35:41 +0800 Subject: [PATCH] refactor: remove redundant `--disable-workload-identity` --- src/aks-preview/HISTORY.rst | 2 ++ src/aks-preview/azext_aks_preview/_help.py | 7 ++--- src/aks-preview/azext_aks_preview/_params.py | 3 +- src/aks-preview/azext_aks_preview/custom.py | 3 -- .../azext_aks_preview/decorator.py | 29 +++++++------------ .../tests/latest/test_aks_commands.py | 2 +- .../tests/latest/test_decorator.py | 15 +--------- src/aks-preview/linter_exclusions.yml | 3 -- 8 files changed, 17 insertions(+), 47 deletions(-) diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index 649067381d2..dcfb6c68d0c 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -12,6 +12,8 @@ To release a new version, please select a new version number (usually plus 1 to Pending +++++++ +* Refactor: Removed redundant `--disable-workload-identity` flag. User can disable the workload identity feature by using `--enable-workload-identity False`. + 0.5.71 ++++++ diff --git a/src/aks-preview/azext_aks_preview/_help.py b/src/aks-preview/azext_aks_preview/_help.py index bf7637f9258..77e3dde9ebf 100644 --- a/src/aks-preview/azext_aks_preview/_help.py +++ b/src/aks-preview/azext_aks_preview/_help.py @@ -361,7 +361,7 @@ short-summary: (PREVIEW) Enable pod identity addon for cluster using Kubnet network plugin. - name: --enable-workload-identity type: bool - short-summary: (PREVIEW) Enable workload identity addon. + short-summary: Enable workload identity addon. - name: --disable-disk-driver type: bool short-summary: Disable AzureDisk CSI Driver. @@ -650,10 +650,7 @@ short-summary: (PREVIEW) Disable Pod Identity addon for cluster. - name: --enable-workload-identity type: bool - short-summary: (PREVIEW) Enable Workload Identity addon for cluster. - - name: --disable-workload-identity - type: bool - short-summary: (PREVIEW) Disable Workload Identity addon for cluster. + short-summary: Enable Workload Identity addon for cluster. - name: --enable-secret-rotation type: bool short-summary: Enable secret rotation. Use with azure-keyvault-secrets-provider addon. diff --git a/src/aks-preview/azext_aks_preview/_params.py b/src/aks-preview/azext_aks_preview/_params.py index abd581b3c8d..b9af5deb0e9 100644 --- a/src/aks-preview/azext_aks_preview/_params.py +++ b/src/aks-preview/azext_aks_preview/_params.py @@ -283,7 +283,7 @@ def load_arguments(self, _): c.argument('http_proxy_config') c.argument('enable_pod_security_policy', action='store_true') c.argument('enable_pod_identity', action='store_true') - c.argument('enable_workload_identity', arg_type=get_three_state_flag(), is_preview=True) + c.argument('enable_workload_identity', arg_type=get_three_state_flag()) c.argument('enable_oidc_issuer', action='store_true', is_preview=True) c.argument('enable_azure_keyvault_kms', action='store_true', is_preview=True) c.argument('azure_keyvault_kms_key_id', validator=validate_azure_keyvault_kms_key_id, is_preview=True) @@ -365,7 +365,6 @@ def load_arguments(self, _): c.argument('enable_pod_identity', action='store_true') c.argument('disable_pod_identity', action='store_true') c.argument('enable_workload_identity', arg_type=get_three_state_flag(), is_preview=True) - c.argument('disable_workload_identity', arg_type=get_three_state_flag(), is_preview=True) c.argument('enable_oidc_issuer', action='store_true', is_preview=True) c.argument('enable_azure_keyvault_kms', action='store_true', is_preview=True) c.argument('azure_keyvault_kms_key_id', validator=validate_azure_keyvault_kms_key_id, is_preview=True) diff --git a/src/aks-preview/azext_aks_preview/custom.py b/src/aks-preview/azext_aks_preview/custom.py index 7995a35e8f3..4ba720861a6 100644 --- a/src/aks-preview/azext_aks_preview/custom.py +++ b/src/aks-preview/azext_aks_preview/custom.py @@ -774,7 +774,6 @@ def aks_create(cmd, auto_upgrade_channel=None, enable_pod_identity=False, enable_pod_identity_with_kubenet=False, - # NOTE: for workload identity flags, we need to know if it's set to True/False or not set (None) enable_workload_identity=None, enable_encryption_at_host=False, enable_ultra_ssd=False, @@ -864,9 +863,7 @@ def aks_update(cmd, # pylint: disable=too-many-statements,too-many-branches, enable_pod_identity=False, enable_pod_identity_with_kubenet=False, disable_pod_identity=False, - # NOTE: for workload identity flags, we need to know if it's set to True/False or not set (None) enable_workload_identity=None, - disable_workload_identity=None, enable_secret_rotation=False, disable_secret_rotation=False, rotation_poll_interval=None, diff --git a/src/aks-preview/azext_aks_preview/decorator.py b/src/aks-preview/azext_aks_preview/decorator.py index cfc1d7b198e..8011c40e807 100644 --- a/src/aks-preview/azext_aks_preview/decorator.py +++ b/src/aks-preview/azext_aks_preview/decorator.py @@ -1808,32 +1808,23 @@ def get_workload_identity_profile(self) -> Optional[ManagedClusterSecurityProfil :return: Optional[ManagedClusterSecurityProfileWorkloadIdentity] """ + # NOTE: enable_workload_identity can be one of: + # + # - True: sets by user, to enable the workload identity feature + # - False: sets by user, to disable the workload identity feature + # - None: user unspecified, don't set the profile and let server side to backfill enable_workload_identity = self.raw_param.get("enable_workload_identity") - disable_workload_identity = self.raw_param.get("disable_workload_identity") - if self.decorator_mode == DecoratorMode.CREATE: - # CREATE mode has no --disable-workload-identity flag - disable_workload_identity = None - if enable_workload_identity is None and disable_workload_identity is None: - # no flags have been set, return None; server side will backfill the default/existing value + if enable_workload_identity is None: return None - if enable_workload_identity and disable_workload_identity: - raise MutuallyExclusiveArgumentError( - "Cannot specify --enable-workload-identity and " - "--disable-workload-identity at the same time." - ) - profile = self.models.ManagedClusterSecurityProfileWorkloadIdentity() - if self.decorator_mode == DecoratorMode.CREATE: - profile.enabled = bool(enable_workload_identity) - elif self.decorator_mode == DecoratorMode.UPDATE: + if self.decorator_mode == DecoratorMode.UPDATE: if self.mc.security_profile is not None and self.mc.security_profile.workload_identity is not None: + # reuse previous profile is has been set profile = self.mc.security_profile.workload_identity - if enable_workload_identity: - profile.enabled = True - elif disable_workload_identity: - profile.enabled = False + + profile.enabled = bool(enable_workload_identity) if profile.enabled: # in enable case, we need to check if OIDC issuer has been enabled diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py b/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py index e9b385fafb0..c6d13053023 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py @@ -3816,7 +3816,7 @@ def test_aks_update_with_workload_identity(self, resource_group, resource_group_ disable_cmd = ' '.join([ 'aks', 'update', '--resource-group={resource_group}', '--name={name}', - '--disable-workload-identity', + '--enable-workload-identity', 'False', '--aks-custom-headers AKSHTTPCustomFeatures=Microsoft.ContainerService/EnableWorkloadIdentityPreview,AKSHTTPCustomFeatures=Microsoft.ContainerService/EnableOIDCIssuerPreview', ]) self.cmd(disable_cmd, checks=[ diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_decorator.py index 80eb78e9141..b852b3a3479 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_decorator.py @@ -1970,19 +1970,6 @@ def test_get_workload_identity_profile__update_not_set(self): ctx.attach_mc(self.models.ManagedCluster(location="test_location")) self.assertIsNone(ctx.get_workload_identity_profile()) - def test_get_workload_identity_profile__update_with_enable_and_disable(self): - ctx = AKSPreviewContext( - self.cmd, - { - "enable_workload_identity": True, - "disable_workload_identity": True, - }, - self.models, decorator_mode=DecoratorMode.UPDATE - ) - ctx.attach_mc(self.models.ManagedCluster(location="test_location")) - with self.assertRaises(MutuallyExclusiveArgumentError): - ctx.get_workload_identity_profile() - def test_get_workload_identity_profile__update_with_enable_without_oidc_issuer(self): ctx = AKSPreviewContext( self.cmd, @@ -2031,7 +2018,7 @@ def test_get_workload_identity_profile__update_with_disable(self): ctx = AKSPreviewContext( self.cmd, { - "disable_workload_identity": True, + "enable_workload_identity": False, }, self.models, decorator_mode=DecoratorMode.UPDATE ) diff --git a/src/aks-preview/linter_exclusions.yml b/src/aks-preview/linter_exclusions.yml index acfdc70c07f..ecfc780aea1 100644 --- a/src/aks-preview/linter_exclusions.yml +++ b/src/aks-preview/linter_exclusions.yml @@ -48,9 +48,6 @@ aks update: enable_workload_identity: rule_exclusions: - option_length_too_long - disable_workload_identity: - rule_exclusions: - - option_length_too_long enable_snapshot_controller: rule_exclusions: - option_length_too_long