Skip to content

Conversation

@jiasli
Copy link
Member

@jiasli jiasli commented Oct 16, 2020

Fix #14969

Description

If profile latest is used for Azure Stack cloud, az login will fail:

> az cloud register -n redmond --endpoint-resource-manager "https://management.redmond.azurestack.corp.microsoft.com/" 
> az cloud set -n redmond
> az login
...
  File "d:\cli\azure-cli\src\azure-cli-core\azure\cli\core\_profile.py", line 276, in _normalize_properties
    subscription_dict[_MANAGED_BY_TENANTS] = [{_TENANT_ID: t.tenant_id} for t in s.managed_by_tenants]
TypeError: 'NoneType' object is not iterable

This is because latest should not be accepted by the service, and the service should return Not Supported. On the contrary, the service does accept the request and returns an invalid JSON which lacks managedByTenants. This behavior doesn't comply with the spec (Azure/azure-rest-api-specs#9567), causing the parsing logic of CLI to fail.

Ideally, the user should run az cloud set/update -n redmond --profile 2019-03-01-hybrid to set the profile for the cloud before az login.

This PR wraps the raw exception with better instructions:

UserFault: Invalid profile is used for cloud 'redmond'. To configure the cloud profile, 
run `az cloud set --name redmond --profile <profile>(e.g. 2019-03-01-hybrid)`. For more 
information about using Azure CLI with Azure Stack, see https://docs.microsoft.com/en-us
/azure/azure-stack/user/azure-stack-version-profiles-azurecli2#connect-to-azure-stack

@jiasli jiasli requested a review from houk-ms October 16, 2020 03:15
Comment on lines 280 to 281
from azure.cli.core.azclierror import UserFault
raise UserFault("Invalid profile is used for cloud '{cloud_name}'. "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define an error sub-class may be InvalidProfileError(UserFault) and raise the specific error? UserFault is designed as an abstract class and is not recommended to be raised directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a temporary mitigation to the service bug, it should actually be an Unsupported API version error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@houk-ms , you can use ABC and abstractmethod (from abc import ABC, abstractmethod) to define your class and make the parent class can't be instantiated.

Copy link
Contributor

@houk-ms houk-ms Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arrownj sure, we can do that. Maybe in next CLI release

Copy link
Contributor

@houk-ms houk-ms Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jiasli In this case, cosider using from azure.cli.core.azclierror import UnknownError to replace UserFault if you don't define a new error type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use CLIError for now until the error can be classified as UnsupportedAPIError after the service is fixed.

@yonzhan yonzhan added this to the S177 milestone Oct 16, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Oct 16, 2020

Core

@jiasli jiasli changed the title {Core} az login: Wrap managed_by_tenants error {Core} az login: Capture managed_by_tenants error Oct 23, 2020
@yonzhan yonzhan modified the milestones: S177, S178 Oct 24, 2020
@yonzhan yonzhan requested a review from evelyn-ys October 24, 2020 15:06
@jiasli
Copy link
Member Author

jiasli commented Oct 27, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jiasli
Copy link
Member Author

jiasli commented Jan 31, 2023

This PR has been shipped in Azure CLI 2.15.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

az login into the AzureStackUser cloud fails

6 participants