From fc586c6fc98059ef05f470b52f8c2c9556647673 Mon Sep 17 00:00:00 2001 From: songlu Date: Fri, 9 Apr 2021 17:45:21 +0800 Subject: [PATCH 1/8] fix issue 16579 --- src/azure-cli-core/azure/cli/core/_profile.py | 47 +++++++++++-------- .../azure/cli/core/decorators.py | 2 +- .../azure/cli/core/telemetry.py | 5 +- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 71411aa430b..1db0887d567 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -494,32 +494,39 @@ def load_cached_subscriptions(self, all_clouds=False): def get_current_account_user(self): try: active_account = self.get_subscription() + if not active_account: + raise CLIError('There are no active accounts.') + return active_account[_USER_ENTITY][_USER_NAME] except CLIError: - raise CLIError('There are no active accounts.') + return '' - return active_account[_USER_ENTITY][_USER_NAME] def get_subscription(self, subscription=None): # take id or name - subscriptions = self.load_cached_subscriptions() - if not subscriptions: - raise CLIError(_AZ_LOGIN_MESSAGE) - - result = [x for x in subscriptions if ( - not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or - subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ - _SUBSCRIPTION_NAME].lower()])] - if not result and subscription: - raise CLIError("Subscription '{}' not found. " - "Check the spelling and casing and try again.".format(subscription)) - if not result and not subscription: - raise CLIError("No subscription found. Run 'az account set' to select a subscription.") - if len(result) > 1: - raise CLIError("Multiple subscriptions with the name '{}' found. " - "Specify the subscription ID.".format(subscription)) - return result[0] + try: + subscriptions = self.load_cached_subscriptions() + if not subscriptions: + raise CLIError(_AZ_LOGIN_MESSAGE) + result = [x for x in subscriptions if ( + not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or + subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ + _SUBSCRIPTION_NAME].lower()])] + if not result and subscription: + raise CLIError("Subscription '{}' not found. " + "Check the spelling and casing and try again.".format(subscription)) + if not result and not subscription: + raise CLIError("No subscription found. Run 'az account set' to select a subscription.") + if len(result) > 1: + raise CLIError("Multiple subscriptions with the name '{}' found. " + "Specify the subscription ID.".format(subscription)) + return result[0] + except: + return {} def get_subscription_id(self, subscription=None): # take id or name - return self.get_subscription(subscription)[_SUBSCRIPTION_ID] + try: + return self.get_subscription(subscription)[_SUBSCRIPTION_ID] + except : + return '' def get_access_token_for_resource(self, username, tenant, resource): tenant = tenant or 'common' diff --git a/src/azure-cli-core/azure/cli/core/decorators.py b/src/azure-cli-core/azure/cli/core/decorators.py index 0066fef71ea..8d5072c530d 100644 --- a/src/azure-cli-core/azure/cli/core/decorators.py +++ b/src/azure-cli-core/azure/cli/core/decorators.py @@ -55,7 +55,7 @@ def hash256_result(func): @wraps(func) def _decorator(*args, **kwargs): val = func(*args, **kwargs) - if not val: + if val is None: raise ValueError('Return value is None') if not isinstance(val, str): raise ValueError('Return value is not string') diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index dda2625ddd4..7527adfbb84 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -537,7 +537,10 @@ def _get_env_string(): @decorators.suppress_all_exceptions(fallback_return=None) def _get_azure_subscription_id(): - return _get_profile().get_subscription_id() + try: + return _get_profile().get_subscription_id() + except: + return '{}' def _get_shell_type(): From 9e8ffbcd8d8a1f5865357ad58eaf5129df14a1b3 Mon Sep 17 00:00:00 2001 From: songlu Date: Mon, 12 Apr 2021 13:12:41 +0800 Subject: [PATCH 2/8] #17631 move the except logic to telemetry. --- src/azure-cli-core/azure/cli/core/_profile.py | 48 ++++++++----------- .../azure/cli/core/telemetry.py | 8 ++-- 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 1db0887d567..34359ca83ba 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -494,39 +494,31 @@ def load_cached_subscriptions(self, all_clouds=False): def get_current_account_user(self): try: active_account = self.get_subscription() - if not active_account: - raise CLIError('There are no active accounts.') - return active_account[_USER_ENTITY][_USER_NAME] except CLIError: - return '' - + raise CLIError('There are no active accounts.') + return active_account[_USER_ENTITY][_USER_NAME] def get_subscription(self, subscription=None): # take id or name - try: - subscriptions = self.load_cached_subscriptions() - if not subscriptions: - raise CLIError(_AZ_LOGIN_MESSAGE) - result = [x for x in subscriptions if ( - not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or - subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ - _SUBSCRIPTION_NAME].lower()])] - if not result and subscription: - raise CLIError("Subscription '{}' not found. " - "Check the spelling and casing and try again.".format(subscription)) - if not result and not subscription: - raise CLIError("No subscription found. Run 'az account set' to select a subscription.") - if len(result) > 1: - raise CLIError("Multiple subscriptions with the name '{}' found. " - "Specify the subscription ID.".format(subscription)) - return result[0] - except: - return {} + subscriptions = self.load_cached_subscriptions() + if not subscriptions: + raise CLIError(_AZ_LOGIN_MESSAGE) + + result = [x for x in subscriptions if ( + not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or + subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ + _SUBSCRIPTION_NAME].lower()])] + if not result and subscription: + raise CLIError("Subscription '{}' not found. " + "Check the spelling and casing and try again.".format(subscription)) + if not result and not subscription: + raise CLIError("No subscription found. Run 'az account set' to select a subscription.") + if len(result) > 1: + raise CLIError("Multiple subscriptions with the name '{}' found. " + "Specify the subscription ID.".format(subscription)) + return result[0] def get_subscription_id(self, subscription=None): # take id or name - try: - return self.get_subscription(subscription)[_SUBSCRIPTION_ID] - except : - return '' + return self.get_subscription(subscription)[_SUBSCRIPTION_ID] def get_access_token_for_resource(self, username, tenant, resource): tenant = tenant or 'common' diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index 7527adfbb84..d8dfdcff993 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -527,8 +527,10 @@ def _get_hash_machine_id(): @decorators.suppress_all_exceptions(fallback_return='') @decorators.hash256_result def _get_user_azure_id(): - return _get_profile().get_current_account_user() - + try: + return _get_profile().get_current_account_user() + except: + return '' def _get_env_string(): return _remove_cmd_chars(_remove_symbols(str([v for v in os.environ @@ -540,7 +542,7 @@ def _get_azure_subscription_id(): try: return _get_profile().get_subscription_id() except: - return '{}' + return '' def _get_shell_type(): From 42fc2c7744358ac6573a58bed956fa6b2caeddf6 Mon Sep 17 00:00:00 2001 From: songlu Date: Mon, 12 Apr 2021 13:16:47 +0800 Subject: [PATCH 3/8] Revert "#17631" This reverts commit 9e8ffbcd8d8a1f5865357ad58eaf5129df14a1b3. --- src/azure-cli-core/azure/cli/core/_profile.py | 48 +++++++++++-------- .../azure/cli/core/telemetry.py | 8 ++-- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 34359ca83ba..1db0887d567 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -494,31 +494,39 @@ def load_cached_subscriptions(self, all_clouds=False): def get_current_account_user(self): try: active_account = self.get_subscription() + if not active_account: + raise CLIError('There are no active accounts.') + return active_account[_USER_ENTITY][_USER_NAME] except CLIError: - raise CLIError('There are no active accounts.') - return active_account[_USER_ENTITY][_USER_NAME] + return '' + def get_subscription(self, subscription=None): # take id or name - subscriptions = self.load_cached_subscriptions() - if not subscriptions: - raise CLIError(_AZ_LOGIN_MESSAGE) - - result = [x for x in subscriptions if ( - not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or - subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ - _SUBSCRIPTION_NAME].lower()])] - if not result and subscription: - raise CLIError("Subscription '{}' not found. " - "Check the spelling and casing and try again.".format(subscription)) - if not result and not subscription: - raise CLIError("No subscription found. Run 'az account set' to select a subscription.") - if len(result) > 1: - raise CLIError("Multiple subscriptions with the name '{}' found. " - "Specify the subscription ID.".format(subscription)) - return result[0] + try: + subscriptions = self.load_cached_subscriptions() + if not subscriptions: + raise CLIError(_AZ_LOGIN_MESSAGE) + result = [x for x in subscriptions if ( + not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or + subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ + _SUBSCRIPTION_NAME].lower()])] + if not result and subscription: + raise CLIError("Subscription '{}' not found. " + "Check the spelling and casing and try again.".format(subscription)) + if not result and not subscription: + raise CLIError("No subscription found. Run 'az account set' to select a subscription.") + if len(result) > 1: + raise CLIError("Multiple subscriptions with the name '{}' found. " + "Specify the subscription ID.".format(subscription)) + return result[0] + except: + return {} def get_subscription_id(self, subscription=None): # take id or name - return self.get_subscription(subscription)[_SUBSCRIPTION_ID] + try: + return self.get_subscription(subscription)[_SUBSCRIPTION_ID] + except : + return '' def get_access_token_for_resource(self, username, tenant, resource): tenant = tenant or 'common' diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index d8dfdcff993..7527adfbb84 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -527,10 +527,8 @@ def _get_hash_machine_id(): @decorators.suppress_all_exceptions(fallback_return='') @decorators.hash256_result def _get_user_azure_id(): - try: - return _get_profile().get_current_account_user() - except: - return '' + return _get_profile().get_current_account_user() + def _get_env_string(): return _remove_cmd_chars(_remove_symbols(str([v for v in os.environ @@ -542,7 +540,7 @@ def _get_azure_subscription_id(): try: return _get_profile().get_subscription_id() except: - return '' + return '{}' def _get_shell_type(): From 570b9149cb78501902aaf40cc87b3f098566663e Mon Sep 17 00:00:00 2001 From: songlu Date: Mon, 12 Apr 2021 13:18:27 +0800 Subject: [PATCH 4/8] Revert "Revert "#17631"" This reverts commit 42fc2c7744358ac6573a58bed956fa6b2caeddf6. --- src/azure-cli-core/azure/cli/core/_profile.py | 48 ++++++++----------- .../azure/cli/core/telemetry.py | 8 ++-- 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 1db0887d567..34359ca83ba 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -494,39 +494,31 @@ def load_cached_subscriptions(self, all_clouds=False): def get_current_account_user(self): try: active_account = self.get_subscription() - if not active_account: - raise CLIError('There are no active accounts.') - return active_account[_USER_ENTITY][_USER_NAME] except CLIError: - return '' - + raise CLIError('There are no active accounts.') + return active_account[_USER_ENTITY][_USER_NAME] def get_subscription(self, subscription=None): # take id or name - try: - subscriptions = self.load_cached_subscriptions() - if not subscriptions: - raise CLIError(_AZ_LOGIN_MESSAGE) - result = [x for x in subscriptions if ( - not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or - subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ - _SUBSCRIPTION_NAME].lower()])] - if not result and subscription: - raise CLIError("Subscription '{}' not found. " - "Check the spelling and casing and try again.".format(subscription)) - if not result and not subscription: - raise CLIError("No subscription found. Run 'az account set' to select a subscription.") - if len(result) > 1: - raise CLIError("Multiple subscriptions with the name '{}' found. " - "Specify the subscription ID.".format(subscription)) - return result[0] - except: - return {} + subscriptions = self.load_cached_subscriptions() + if not subscriptions: + raise CLIError(_AZ_LOGIN_MESSAGE) + + result = [x for x in subscriptions if ( + not subscription and x.get(_IS_DEFAULT_SUBSCRIPTION) or + subscription and subscription.lower() in [x[_SUBSCRIPTION_ID].lower(), x[ + _SUBSCRIPTION_NAME].lower()])] + if not result and subscription: + raise CLIError("Subscription '{}' not found. " + "Check the spelling and casing and try again.".format(subscription)) + if not result and not subscription: + raise CLIError("No subscription found. Run 'az account set' to select a subscription.") + if len(result) > 1: + raise CLIError("Multiple subscriptions with the name '{}' found. " + "Specify the subscription ID.".format(subscription)) + return result[0] def get_subscription_id(self, subscription=None): # take id or name - try: - return self.get_subscription(subscription)[_SUBSCRIPTION_ID] - except : - return '' + return self.get_subscription(subscription)[_SUBSCRIPTION_ID] def get_access_token_for_resource(self, username, tenant, resource): tenant = tenant or 'common' diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index 7527adfbb84..d8dfdcff993 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -527,8 +527,10 @@ def _get_hash_machine_id(): @decorators.suppress_all_exceptions(fallback_return='') @decorators.hash256_result def _get_user_azure_id(): - return _get_profile().get_current_account_user() - + try: + return _get_profile().get_current_account_user() + except: + return '' def _get_env_string(): return _remove_cmd_chars(_remove_symbols(str([v for v in os.environ @@ -540,7 +542,7 @@ def _get_azure_subscription_id(): try: return _get_profile().get_subscription_id() except: - return '{}' + return '' def _get_shell_type(): From 2028e2c6b4f9bc7356a54d94d731c4043e4db42a Mon Sep 17 00:00:00 2001 From: songlu Date: Mon, 12 Apr 2021 14:10:27 +0800 Subject: [PATCH 5/8] #17631 Modify the code style and make it conforms to the rules --- src/azure-cli-core/azure/cli/core/telemetry.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index d8dfdcff993..45f60fc7389 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -14,7 +14,7 @@ import uuid from collections import defaultdict from functools import wraps - +from knack.util import CLIError import azure.cli.core.decorators as decorators PRODUCT_NAME = 'azurecli' @@ -529,9 +529,10 @@ def _get_hash_machine_id(): def _get_user_azure_id(): try: return _get_profile().get_current_account_user() - except: + except CLIError: return '' + def _get_env_string(): return _remove_cmd_chars(_remove_symbols(str([v for v in os.environ if v.startswith('AZURE_CLI')]))) @@ -541,7 +542,7 @@ def _get_env_string(): def _get_azure_subscription_id(): try: return _get_profile().get_subscription_id() - except: + except CLIError: return '' From 8e8db8d438fdf1148708b7f10834b3d6e08d998a Mon Sep 17 00:00:00 2001 From: songlu Date: Mon, 12 Apr 2021 17:04:15 +0800 Subject: [PATCH 6/8] Update _profile.py --- src/azure-cli-core/azure/cli/core/_profile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 34359ca83ba..450f18414f4 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -496,6 +496,7 @@ def get_current_account_user(self): active_account = self.get_subscription() except CLIError: raise CLIError('There are no active accounts.') + return active_account[_USER_ENTITY][_USER_NAME] def get_subscription(self, subscription=None): # take id or name From 0505b2cd16af1d48222303e0d5c17069ec8494dd Mon Sep 17 00:00:00 2001 From: songlu Date: Mon, 12 Apr 2021 18:25:41 +0800 Subject: [PATCH 7/8] #17631 Modify the code style and make it conforms to the rules --- src/azure-cli-core/azure/cli/core/_profile.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/_profile.py b/src/azure-cli-core/azure/cli/core/_profile.py index 450f18414f4..71411aa430b 100644 --- a/src/azure-cli-core/azure/cli/core/_profile.py +++ b/src/azure-cli-core/azure/cli/core/_profile.py @@ -496,7 +496,7 @@ def get_current_account_user(self): active_account = self.get_subscription() except CLIError: raise CLIError('There are no active accounts.') - + return active_account[_USER_ENTITY][_USER_NAME] def get_subscription(self, subscription=None): # take id or name From 88256a2585933fb687b0d9ea1e04f988ecb7c296 Mon Sep 17 00:00:00 2001 From: songlu <442586197@QQ.COM> Date: Tue, 13 Apr 2021 12:17:59 +0800 Subject: [PATCH 8/8] #17631 --- src/azure-cli-core/azure/cli/core/decorators.py | 2 ++ src/azure-cli-core/azure/cli/core/telemetry.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/azure-cli-core/azure/cli/core/decorators.py b/src/azure-cli-core/azure/cli/core/decorators.py index 8d5072c530d..55a1d88e11e 100644 --- a/src/azure-cli-core/azure/cli/core/decorators.py +++ b/src/azure-cli-core/azure/cli/core/decorators.py @@ -59,6 +59,8 @@ def _decorator(*args, **kwargs): raise ValueError('Return value is None') if not isinstance(val, str): raise ValueError('Return value is not string') + if not val: + return val hash_object = hashlib.sha256(val.encode('utf-8')) return str(hash_object.hexdigest()) diff --git a/src/azure-cli-core/azure/cli/core/telemetry.py b/src/azure-cli-core/azure/cli/core/telemetry.py index 45f60fc7389..e6b8e88bf97 100644 --- a/src/azure-cli-core/azure/cli/core/telemetry.py +++ b/src/azure-cli-core/azure/cli/core/telemetry.py @@ -543,7 +543,7 @@ def _get_azure_subscription_id(): try: return _get_profile().get_subscription_id() except CLIError: - return '' + return None def _get_shell_type():