Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 18 additions & 14 deletions src/azure-cli-core/azure/cli/core/_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,21 +267,9 @@ def _normalize_properties(self, user, subscriptions, is_service_principal, cert_
_TENANT_ID: s.tenant_id,
_ENVIRONMENT_NAME: self.cli_ctx.cloud.name
}
# For subscription account from Subscriptions - List 2019-06-01 and later.

if subscription_dict[_SUBSCRIPTION_NAME] != _TENANT_LEVEL_ACCOUNT_NAME:
if hasattr(s, 'home_tenant_id'):
subscription_dict[_HOME_TENANT_ID] = s.home_tenant_id
if hasattr(s, 'managed_by_tenants'):
if s.managed_by_tenants is None:
# managedByTenants is missing from the response. This is a known service issue:
# https://github.com/Azure/azure-rest-api-specs/issues/9567
# pylint: disable=line-too-long
raise CLIError("Invalid profile is used for cloud '{cloud_name}'. "
"To configure the cloud profile, run `az cloud set --name {cloud_name} --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"
.format(cloud_name=self.cli_ctx.cloud.name))
subscription_dict[_MANAGED_BY_TENANTS] = [{_TENANT_ID: t.tenant_id} for t in s.managed_by_tenants]
_transform_subscription_for_multiapi(s, subscription_dict)

consolidated.append(subscription_dict)

Expand Down Expand Up @@ -1376,3 +1364,19 @@ def _get_authorization_code(resource, authority_url):
if results.get('no_browser'):
raise RuntimeError()
return results


def _transform_subscription_for_multiapi(s, s_dict):
"""
Transforms properties from Subscriptions - List 2019-06-01 and later to the subscription dict.

:param s: subscription object
:param s_dict: subscription dict
"""
if hasattr(s, 'home_tenant_id'):
s_dict[_HOME_TENANT_ID] = s.home_tenant_id
if hasattr(s, 'managed_by_tenants'):
if s.managed_by_tenants is None:
s_dict[_MANAGED_BY_TENANTS] = None
Comment on lines +1379 to +1380
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.

Consider following steps:

  1. ARM REST API response omits managedByTenants (or returns "managedByTenants": null, though never observed)
  2. Python SDK will still have managed_by_tenants: None
  3. As a consequence, Azure CLI output will have "managedByTenants": null

This behavior is a breaking change and doesn't comply with swagger as empty array [] becomes null (null is no longer an array), so users assuming managedByTenants to be an array may also face failure.

https://github.com/Azure/azure-rest-api-specs/blob/78ec1b99699a4bf44869bd13f1b0ed7d92a99c27/specification/resources/resource-manager/Microsoft.Resources/stable/2019-11-01/subscriptions.json#L438-L439

        "managedByTenants": {
          "type": "array",
          "items": {
            "$ref": "#/definitions/ManagedByTenant"
          },
          "description": "An array containing the tenants managing the subscription."
        },

Actually, there is no way to perfectly handle all kinds of REST API changes or behaviors in the gray area. In my humble opinion, making API consistent and comply with the swagger is the best and ultimate solution.

In this case,

  1. managedByTenants property MUST exist
  2. managedByTenants MUST be an array, not null

https://learn.microsoft.com/en-us/rest/api/resources/subscriptions/list?tabs=HTTP

image

else:
s_dict[_MANAGED_BY_TENANTS] = [{_TENANT_ID: t.tenant_id} for t in s.managed_by_tenants]
52 changes: 51 additions & 1 deletion src/azure-cli-core/azure/cli/core/tests/test_profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from adal import AdalError

from azure.cli.core._profile import (Profile, CredsCache, SubscriptionFinder,
ServicePrincipalAuth, _AUTH_CTX_FACTORY, _USE_VENDORED_SUBSCRIPTION_SDK)
ServicePrincipalAuth, _AUTH_CTX_FACTORY, _USE_VENDORED_SUBSCRIPTION_SDK,
_transform_subscription_for_multiapi)
if _USE_VENDORED_SUBSCRIPTION_SDK:
from azure.cli.core.vendored_sdks.subscriptions.models import \
(SubscriptionState, Subscription, SubscriptionPolicies, SpendingLimit, ManagedByTenant)
Expand Down Expand Up @@ -2039,5 +2040,54 @@ def token(self, value):
self._token = value


class TestUtil(unittest.TestCase):

def test_transform_subscription_for_multiapi(self):

class SimpleSubscription:
pass

class SimpleManagedByTenant:
pass

tenant_id = "00000001-0000-0000-0000-000000000000"

# No 2019-06-01 property is set.
s = SimpleSubscription()
d = {}
_transform_subscription_for_multiapi(s, d)
assert d == {}

# home_tenant_id is set.
s = SimpleSubscription()
s.home_tenant_id = tenant_id
d = {}
_transform_subscription_for_multiapi(s, d)
assert d == {'homeTenantId': '00000001-0000-0000-0000-000000000000'}

# managed_by_tenants is set, but is None. It is still preserved.
s = SimpleSubscription()
s.managed_by_tenants = None
d = {}
_transform_subscription_for_multiapi(s, d)
assert d == {'managedByTenants': None}

# managed_by_tenants is set, but is []. It is still preserved.
s = SimpleSubscription()
s.managed_by_tenants = []
d = {}
_transform_subscription_for_multiapi(s, d)
assert d == {'managedByTenants': []}

# managed_by_tenants is set, and has valid items. It is preserved.
s = SimpleSubscription()
t = SimpleManagedByTenant()
t.tenant_id = tenant_id
s.managed_by_tenants = [t]
d = {}
_transform_subscription_for_multiapi(s, d)
assert d == {'managedByTenants': [{"tenantId": tenant_id}]}


if __name__ == '__main__':
unittest.main()