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

[Subscription RP] create subscription command #119

Merged
merged 9 commits into from
Apr 2, 2018
Merged
Show file tree
Hide file tree
Changes from 5 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
4 changes: 2 additions & 2 deletions src/subscription/azext_subscription/_client_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ def cf_subscription(cli_ctx, **_):
return client


def subscription_definitions_mgmt_client_factory(cli_ctx, kwargs):
return cf_subscription(cli_ctx, **kwargs).subscription_definitions
def subscription_factory(cli_ctx, kwargs):
return cf_subscription(cli_ctx, **kwargs).subscription_factory
9 changes: 2 additions & 7 deletions src/subscription/azext_subscription/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,7 @@
from knack.help_files import helps


helps['account subscription-definition'] = """
type: group
short-summary: Manage Azure Subscription Definitions.
"""

helps['account subscription-definition create'] = """
helps['account create'] = """
type: command
short-summary: Create a subscription definition.
short-summary: Create a subscription.
"""
11 changes: 5 additions & 6 deletions src/subscription/azext_subscription/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@

# pylint: disable=line-too-long
def load_arguments(self, _):
with self.argument_context('account subscription-definition create') as c:
with self.argument_context('account create') as c:
c.argument('offer_type', required=True, help='The offer type of the subscription. For example, MS-AZR-0017P (EnterpriseAgreement) and MS-AZR-0148P (EnterpriseAgreement devTest) are available.', arg_type=get_enum_type(['MS-AZR-0017P', 'MS-AZR-0148P']))
c.argument('subscription_display_name', help='The subscription display name of the subscription definition.')

for scope in ['account subscription-definition create', 'account subscription-definition show']:
with self.argument_context(scope) as c:
c.argument('subscription_definition_name', options_list=['--name', '-n'], help='Name of the subscription definition.')
c.argument('display_name', help='The display name of the subscription.')
c.argument('object_id', help='The object id(s) of owners which should be granted access to the new subscription.')
c.argument('spn', help='The service principal names of owners which should be granted access to the new subscription.')
c.argument('upn', help='The user principal names of owners who should be granted access to the new subscription.')
Copy link
Member

Choose a reason for hiding this comment

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

There is no help text for --enrollment-account-name

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

It is fine to be like this since it is an extension command, but if you plan to convert the extension to a core command module, I would suggest fold all 3 into a single --owners argument and your command do the hard work by making the graph call to figure out what the real type it is. Per usage feedback, lots of users have a hard time to figure out whose graph objects, nor they have any passions to learn those terms. By just seeing the spn, upn, they might already get lost.

14 changes: 6 additions & 8 deletions src/subscription/azext_subscription/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,17 @@
# pylint: disable=line-too-long

from azure.cli.core.commands import CliCommandType
from ._client_factory import subscription_definitions_mgmt_client_factory
from ._client_factory import subscription_factory
from ._exception_handler import subscription_exception_handler


def load_command_table(self, _):
subscription_definition_util = CliCommandType(
operations_tmpl='azext_subscription.subscription.operations.subscription_definitions_operations#SubscriptionDefinitionsOperations.{}',
client_factory=subscription_definitions_mgmt_client_factory,
subscription_util = CliCommandType(
operations_tmpl='azext_subscription.subscription.operations.subscription_factory_operations#SubscriptionFactoryOperations.{}',
client_factory=subscription_factory,
client_arg_name='self',
exception_handler=subscription_exception_handler
)

with self.command_group('account subscription-definition', subscription_definition_util, client_factory=subscription_definitions_mgmt_client_factory) as g:
g.command('list', 'list')
g.command('show', 'get')
g.custom_command('create', 'cli_subscription_create_subscription_definition')
with self.command_group('account', subscription_util, client_factory=subscription_factory) as g:
g.custom_command('create', 'cli_subscription_create')
88 changes: 79 additions & 9 deletions src/subscription/azext_subscription/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,86 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

from azext_subscription.subscription.models import SubscriptionDefinition
from knack.log import get_logger
from knack.util import CLIError
from azext_subscription.subscription.models import (SubscriptionCreationParameters, AdPrincipal)

logger = get_logger(__name__)

def cli_subscription_create_subscription_definition(client, subscription_definition_name,
offer_type, subscription_display_name=None):
if subscription_display_name is None:
subscription_display_name = subscription_definition_name

new_def = SubscriptionDefinition(
subscription_display_name=subscription_display_name,
offer_type=offer_type)
def _get_object_id_by_spn(graph_client, spn):
accounts = list(graph_client.service_principals.list(
filter="servicePrincipalNames/any(c:c eq '{}')".format(spn)))
if not accounts:
logger.warning("Unable to find user with spn '%s'", spn)
return None
if len(accounts) > 1:
logger.warning("Multiple service principals found with spn '%s'. "
Copy link
Contributor

Choose a reason for hiding this comment

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

spn/upn is supposed to be unique, I would suggest you just error out here as such situation should not occur

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 code is copied from KeyVault's policy handling. The parameter names (object_id/spn/upn) also reflect what both the KeyVault and AuthZ modules are doing. Ideally there'd be a separate change to address this feedback across all modules. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me

"You can avoid this by specifying object id.", spn)
return None
return accounts[0].object_id

return client.create(subscription_definition_name, new_def)

def _get_object_id_by_upn(graph_client, upn):
accounts = list(graph_client.users.list(filter="userPrincipalName eq '{}'".format(upn)))
if not accounts:
logger.warning("Unable to find user with upn '%s'", upn)
return None
if len(accounts) > 1:
logger.warning("Multiple users principals found with upn '%s'. "
"You can avoid this by specifying object id.", upn)
return None
return accounts[0].object_id


def _get_object_id_from_subscription(graph_client, subscription):
if subscription['user']:
if subscription['user']['type'] == 'user':
return _get_object_id_by_upn(graph_client, subscription['user']['name'])
elif subscription['user']['type'] == 'servicePrincipal':
return _get_object_id_by_spn(graph_client, subscription['user']['name'])
else:
logger.warning("Unknown user type '%s'", subscription['user']['type'])
else:
logger.warning('Current credentials are not from a user or service principal. '
'Azure Key Vault does not work with certificate credentials.')
return None


def _get_object_id(graph_client, subscription=None, spn=None, upn=None):
if spn:
return _get_object_id_by_spn(graph_client, spn)
if upn:
return _get_object_id_by_upn(graph_client, upn)
return _get_object_id_from_subscription(graph_client, subscription)


def _object_id_args_helper(cli_ctx, object_id=None, spn=None, upn=None):
if not object_id:
from azure.cli.core._profile import Profile
from azure.graphrbac import GraphRbacManagementClient

profile = Profile(cli_ctx=cli_ctx)
cred, _, tenant_id = profile.get_login_credentials(
resource=cli_ctx.cloud.endpoints.active_directory_graph_resource_id)
graph_client = GraphRbacManagementClient(cred,
tenant_id,
base_url=cli_ctx.cloud.endpoints.active_directory_graph_resource_id)
object_id = _get_object_id(graph_client, spn=spn, upn=upn)
if not object_id:
raise CLIError('Unable to get object id from principal name.')
return object_id


def cli_subscription_create(cmd, client, enrollment_account_name, offer_type,
display_name=None, object_id="", spn="", upn=""):
owners = [_object_id_args_helper(cmd.cli_ctx, object_id=x) for x
in object_id.split(',') if object_id] + \
[_object_id_args_helper(cmd.cli_ctx, spn=x) for x in spn.split(',') if spn] + \
[_object_id_args_helper(cmd.cli_ctx, upn=x) for x in upn.split(',') if upn]
creation_parameters = SubscriptionCreationParameters(
display_name=display_name,
offer_type=offer_type,
owners=[AdPrincipal(object_id=x) for x in owners])

return client.create_subscription_in_enrollment_account(enrollment_account_name, creation_parameters)
51 changes: 37 additions & 14 deletions src/subscription/azext_subscription/subscription/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,38 +9,61 @@
# regenerated.
# --------------------------------------------------------------------------

from .subscription_definition import SubscriptionDefinition
from .operation_display import OperationDisplay
from .operation import Operation
from .error_response import ErrorResponse, ErrorResponseException
from .location import Location
from .subscription_policies import SubscriptionPolicies
from .subscription import Subscription
from .tenant_id_description import TenantIdDescription
from .operation_paged import OperationPaged
from .subscription_definition_paged import SubscriptionDefinitionPaged
try:
from .subscription_creation_result_py3 import SubscriptionCreationResult
from .ad_principal_py3 import AdPrincipal
from .subscription_creation_parameters_py3 import SubscriptionCreationParameters
from .error_response_py3 import ErrorResponse, ErrorResponseException
from .subscription_operation_py3 import SubscriptionOperation
from .subscription_operation_list_result_py3 import SubscriptionOperationListResult
from .operation_display_py3 import OperationDisplay
from .operation_py3 import Operation
from .operation_list_result_py3 import OperationListResult
from .location_py3 import Location
from .subscription_policies_py3 import SubscriptionPolicies
from .subscription_py3 import Subscription
from .tenant_id_description_py3 import TenantIdDescription
except (SyntaxError, ImportError):
from .subscription_creation_result import SubscriptionCreationResult
from .ad_principal import AdPrincipal
from .subscription_creation_parameters import SubscriptionCreationParameters
from .error_response import ErrorResponse, ErrorResponseException
from .subscription_operation import SubscriptionOperation
from .subscription_operation_list_result import SubscriptionOperationListResult
from .operation_display import OperationDisplay
from .operation import Operation
from .operation_list_result import OperationListResult
from .location import Location
from .subscription_policies import SubscriptionPolicies
from .subscription import Subscription
from .tenant_id_description import TenantIdDescription
from .location_paged import LocationPaged
from .subscription_paged import SubscriptionPaged
from .tenant_id_description_paged import TenantIdDescriptionPaged
from .subscription_client_enums import (
OfferType,
SubscriptionState,
SpendingLimit,
)

__all__ = [
'SubscriptionDefinition',
'SubscriptionCreationResult',
'AdPrincipal',
'SubscriptionCreationParameters',
'ErrorResponse', 'ErrorResponseException',
'SubscriptionOperation',
'SubscriptionOperationListResult',
'OperationDisplay',
'Operation',
'ErrorResponse', 'ErrorResponseException',
'OperationListResult',
'Location',
'SubscriptionPolicies',
'Subscription',
'TenantIdDescription',
'OperationPaged',
'SubscriptionDefinitionPaged',
'LocationPaged',
'SubscriptionPaged',
'TenantIdDescriptionPaged',
'OfferType',
'SubscriptionState',
'SpendingLimit',
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from msrest.serialization import Model


class AdPrincipal(Model):
"""Active Directory Principal for subscription creation delegated permission.

All required parameters must be populated in order to send to Azure.

:param object_id: Required. Object id of the Principal
:type object_id: str
"""

_validation = {
'object_id': {'required': True},
}

_attribute_map = {
'object_id': {'key': 'objectId', 'type': 'str'},
}

def __init__(self, **kwargs):
super(AdPrincipal, self).__init__(**kwargs)
self.object_id = kwargs.get('object_id', None)
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from msrest.serialization import Model


class AdPrincipal(Model):
"""Active Directory Principal for subscription creation delegated permission.

All required parameters must be populated in order to send to Azure.

:param object_id: Required. Object id of the Principal
:type object_id: str
"""

_validation = {
'object_id': {'required': True},
}

_attribute_map = {
'object_id': {'key': 'objectId', 'type': 'str'},
}

def __init__(self, *, object_id: str, **kwargs) -> None:
super(AdPrincipal, self).__init__(**kwargs)
self.object_id = object_id
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ class ErrorResponse(Model):
'message': {'key': 'message', 'type': 'str'},
}

def __init__(self, code=None, message=None):
self.code = code
self.message = message
def __init__(self, **kwargs):
super(ErrorResponse, self).__init__(**kwargs)
self.code = kwargs.get('code', None)
self.message = kwargs.get('message', None)


class ErrorResponseException(HttpOperationError):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# coding=utf-8
# --------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for
# license information.
#
# Code generated by Microsoft (R) AutoRest Code Generator.
# Changes may cause incorrect behavior and will be lost if the code is
# regenerated.
# --------------------------------------------------------------------------

from msrest.serialization import Model
from msrest.exceptions import HttpOperationError


class ErrorResponse(Model):
"""Describes the format of Error response.

:param code: Error code
:type code: str
:param message: Error message indicating why the operation failed.
:type message: str
"""

_attribute_map = {
'code': {'key': 'code', 'type': 'str'},
'message': {'key': 'message', 'type': 'str'},
}

def __init__(self, *, code: str=None, message: str=None, **kwargs) -> None:
super(ErrorResponse, self).__init__(**kwargs)
self.code = code
self.message = message


class ErrorResponseException(HttpOperationError):
"""Server responsed with exception of type: 'ErrorResponse'.

:param deserialize: A deserializer
:param response: Server response to be deserialized.
"""

def __init__(self, deserialize, response, *args):

super(ErrorResponseException, self).__init__(deserialize, response, 'ErrorResponse', *args)
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ class Location(Model):
'longitude': {'key': 'longitude', 'type': 'str'},
}

def __init__(self):
def __init__(self, **kwargs):
super(Location, self).__init__(**kwargs)
self.id = None
self.subscription_id = None
self.name = None
Expand Down
Loading