Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{Core} Quick patch for managed_by_tenants missing from response #17509

Closed
wants to merge 2 commits into from

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Mar 30, 2021

Update: The rollback of ARM has completed at 2021-03-31 08:00 UTC. This quick patch is not longer needed.

⚠ WARNING

This PR is a workaround for an ongoing ARM incident. The ARM service team is investigating with highest priority. We will close this PR once ARM service is fixed.

Symptom

az login fails with

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

or (Azure CLI versions smaller than 2.15.0)

azure/cli/core/_profile.py, ln 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

Root Cause Analysis

  1. Azure CLI started using Subscriptions - List API /subscriptions?api-version=2019-06-01 since Feb 2020 for public cloud AzureCloud ([Core][Profile] Support lighthouse multi-tenant subscription #11886). This call is made during az login to fetch the subscriptions.

  2. Azure Stack returns the response for /subscriptions?api-version=2019-06-01 with older API version 2016-06-01 format without managedByTenants property ([Resources] Response from Azure Stack doesn't comply with the spec subscriptions.json azure-rest-api-specs#9567).

  3. Az CLI checks this explicitly and shows the above error message to the user implying "Invalid profile is used for Azure Stack cloud and the profile is set to latest, you need to change the API profile to something like 2019-03-01-hybrid" ({Core} az login: Capture managed_by_tenants error #15550).

  4. Due to the ARM incident, now in public cloud AzureCloud, the /subscriptions API's response omits managedByTenants in some regions. As a consequence, the lack of managedByTenants

    1. breaks Azure Lighthouse functionality
    2. makes Azure CLI think the Azure Stack issue ([Resources] Response from Azure Stack doesn't comply with the spec subscriptions.json azure-rest-api-specs#9567) is hit, so the user is presented with the above error message implying you are logged in to Azure Stack. This causes az login to fail.

Changes in this PR

This PR provides a quick patch which bypasses the check on managedByTenants and thus bypasses the ARM service incident.

Apply the quick patch

The following commands replaces the _profile.py file of Azure CLI installed on local machine or Azure DevOps agent:

# Ubuntu, `sudo` may be needed
curl https://raw.githubusercontent.com/Azure/azure-cli/fix-managed_by_tenants/src/azure-cli-core/azure/cli/core/_profile.py --output /opt/az/lib/python3.6/site-packages/azure/cli/core/_profile.py

# CentOS, `sudo` may be needed
curl https://raw.githubusercontent.com/Azure/azure-cli/fix-managed_by_tenants/src/azure-cli-core/azure/cli/core/_profile.py --output /usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/_profile.py

# Windows, "Run as administrator" may be needed
curl https://raw.githubusercontent.com/Azure/azure-cli/fix-managed_by_tenants/src/azure-cli-core/azure/cli/core/_profile.py --output "C:\Program Files (x86)\Microsoft SDKs\Azure\CLI2\Lib\site-packages\azure\cli\core\_profile.py"

Remove the quick patch

After the ARM incident is fixed, the patch needs to be removed.

  • Azure DevOps agent: Remove the above line from the script.
  • Linux: The patch will be overwritten automatically when the next Azure CLI version is installed. You may also reinstall the current Azure CLI to get the patch removed.
  • Windows ("Run as administrator" may be needed):
    del "C:\Program Files (x86)\Microsoft SDKs\Azure\CLI2\Lib\site-packages\azure\cli\core\_profile.py"
    

Additional information

Microsoft internal tracking ticket for the ARM incident: IcM 234164333

@yonzhan yonzhan self-requested a review March 30, 2021 06:49
@yonzhan yonzhan added this to the S185 milestone Mar 30, 2021
Copy link
Collaborator

@yonzhan yonzhan left a comment

Choose a reason for hiding this comment

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

LGTM

"For more information about using Azure CLI with Azure Stack, see "
"https://docs.microsoft.com/azure-stack/user/azure-stack-version-profiles-azurecli2"
.format(cloud_name=self.cli_ctx.cloud.name))
if getattr(s, 'managed_by_tenants', None) is not None:
Copy link
Member Author

Choose a reason for hiding this comment

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

  • If managed_by_tenants doesn't exist, getattr returns None and this if evaluates to False
  • If managed_by_tenants is [], getattr returns [] and this if evaluates to True, so [] is preserved

Copy link
Member Author

@jiasli jiasli Jan 30, 2023

Choose a reason for hiding this comment

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

2 years later, I feel it was lucky that we didn't merge this PR.

There are 3 conditions:

# Condition 1: managedByTenants missing
{

}

# Condition 2: managedByTenants is null
{
    "managedByTenants": null
}

# Condition 3: managedByTenants is empty list []
{
    "managedByTenants": []
}

This statement doesn't take condition 2 "managedByTenants": null into consideration. Even though this has never been observed, if ARM returns "managedByTenants": null, Azure CLI output will omit managedByTenants.

Also, for condition 1, the absence of managedByTenants from ARM's Subscriptions - List REST API will propagate all the way to Azure CLI output, causing breaking change.

@yufeih
Copy link

yufeih commented Mar 30, 2021

When will this fix be patched to azure pipeline? We have a bunch of AzureCLI@2 task failures blocking our release pipeline.

@pahlawanUtara
Copy link

Thank you so much to the product group for this patch!

@egodigitus egodigitus mentioned this pull request Mar 30, 2021
@giacris82
Copy link

Sorry but if I apply the work around on windows it doesn't work. Which version of az cli i need for this workaround to work?

@jiasli
Copy link
Member Author

jiasli commented Mar 30, 2021

@giacris82, the patch is for Azure CLI installed with MSI.

How did you install Azure CLI? Could you check the installation location?

# PowerShell
Get-Command az

# CMD
where az

@kuzhao
Copy link

kuzhao commented Mar 31, 2021

Are we sure this one works out 100%? Below is what we saw on Ubuntu after the quick patch:

az login --service-principal --username **** --password **** --tenant ****
The command failed with an unexpected error. Here is the traceback:

cannot import name 'BasicTokenCredential'
Traceback (most recent call last):
  File "/usr/lib64/az/lib/python3.6/site-packages/knack/cli.py", line 215, in invoke
    cmd_result = self.invocation.execute(args)
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 654, in execute
    raise ex
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 718, in _run_jobs_serially
    results.append(self._run_job(expanded_arg, cmd_copy))
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 711, in _run_job
    six.reraise(*sys.exc_info())
  File "/usr/lib64/az/lib/python3.6/site-packages/six.py", line 703, in reraise
    raise value
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 688, in _run_job
    result = cmd_copy(params)
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/commands/__init__.py", line 325, in __call__
    return self.handler(*args, **kwargs)
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/__init__.py", line 545, in default_command_handler
    return op(**command_args)
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/command_modules/profile/custom.py", line 150, in login
    use_cert_sn_issuer=use_cert_sn_issuer)
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/_profile.py", line 209, in find_subscriptions_on_login
    username, sp_auth, tenant, self._ad_resource_uri)
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/_profile.py", line 923, in find_from_service_principal_id
    result = self._find_using_specific_tenant(tenant, token_entry[_ACCESS_TOKEN])
  File "/usr/lib64/az/lib/python3.6/site-packages/azure/cli/core/_profile.py", line 1014, in _find_using_specific_tenant
    from azure.cli.core.adal_authentication import BasicTokenCredential
ImportError: cannot import name 'BasicTokenCredential'

@jiasli
Copy link
Member Author

jiasli commented Mar 31, 2021

@kuzhao, BasicTokenCredential is introduced in Azure CLI 2.16.0 (#15711), so you should be running an Azure CLI older than that.

The patch is only guaranteed to work with the latest Azure CLI 2.21.0. Please update to the latest Azure CLI and apply the patch.

@jiasli jiasli marked this pull request as ready for review March 31, 2021 07:42
@giacris82
Copy link

@kuzhao, BasicTokenCredential is introduced in Azure CLI 2.16.0 (#15711), so you should be running an Azure CLI older than that.

The patch is only guaranteed to work with the latest Azure CLI 2.21.0. Please update to the latest Azure CLI and apply the patch.

It wass not clear

@jiasli jiasli marked this pull request as draft March 31, 2021 09:31
@jiasli jiasli closed this Mar 31, 2021
@jiasli jiasli deleted the fix-managed_by_tenants branch April 8, 2021 10:13
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.

7 participants