-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add policy set definition commands #4515
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
Changes from 15 commits
df4c539
69ec6d6
98c4048
cf9fe45
9ba706e
e78019c
9988cc6
613f40b
bea8810
26b629d
25188b8
878fe46
0af53b1
dfc1e15
588b86f
c510185
52c9e91
b44f552
820856d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -172,7 +172,7 @@ | |
| } | ||
| } | ||
| } | ||
| """ | ||
| """ | ||
| helps['policy definition delete'] = """ | ||
| type: command | ||
| short-summary: Delete a policy definition. | ||
|
|
@@ -189,6 +189,43 @@ | |
| type: command | ||
| short-summary: List policy definitions. | ||
| """ | ||
| helps['policy set-definition'] = """ | ||
| type: group | ||
| short-summary: Manage resource policy set definitions. | ||
| """ | ||
| helps['policy set-definition create'] = """ | ||
| type: command | ||
| short-summary: Create a policy set definition. | ||
| parameters: | ||
| - name: --definitions | ||
| type: string | ||
| short-summary: Policy definitions in JSON format, or a path to a file containing JSON rules. | ||
| examples: | ||
| - name: Create a policy set definition. | ||
| text: | | ||
| az policy setdefinition create -n readOnlyStorage --definitions \\ | ||
| [ \\ | ||
| { \\ | ||
| "policyDefinitionId": "/subscriptions/mySubId/providers/Microsoft.Authorization/policyDefinitions/storagePolicy" \\ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is a policy set just a collection of IDs, or is there more to it? If it is, it seems you could get away from the JSON object approach and just accepts a list of names and/or IDs to assemble the set definition.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not just IDs, it can have the entire policy rule as json as well.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry I misspoke above. The policy set cannot contain entire policy rule, but it can have a multiples of policyDefinitionId and parameters (which is json object), so either way, this is a complex object that requires file handling |
||
| } \\ | ||
| ] | ||
| """ | ||
| helps['policy set-definition delete'] = """ | ||
| type: command | ||
| short-summary: Delete a policy set definition. | ||
| """ | ||
| helps['policy set-definition show'] = """ | ||
| type: command | ||
| short-summary: get a policy set definition. | ||
| """ | ||
| helps['policy set-definition update'] = """ | ||
| type: command | ||
| short-summary: Update a policy set definition. | ||
| """ | ||
| helps['policy set-definition list'] = """ | ||
| type: command | ||
| short-summary: List policy set definitions. | ||
| """ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no description for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| helps['policy assignment'] = """ | ||
| type: group | ||
| short-summary: Manage resource policy assignments. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,8 +14,9 @@ | |
| from azure.cli.core.commands.parameters import (ignore_type, resource_group_name_type, tag_type, | ||
| tags_type, get_resource_group_completion_list, | ||
| enum_choice_list, no_wait_type, file_type) | ||
| from .custom import (get_policy_completion_list, get_policy_assignment_completion_list, | ||
| get_resource_types_completion_list, get_providers_completion_list) | ||
| from .custom import (get_policy_completion_list, get_policy_set_completion_list, | ||
| get_policy_assignment_completion_list, get_resource_types_completion_list, | ||
| get_providers_completion_list) | ||
| from ._validators import process_deployment_create_namespace, validate_lock_parameters | ||
|
|
||
|
|
||
|
|
@@ -77,7 +78,6 @@ | |
| help='mode of the new policy definition.', | ||
| **enum_choice_list(PolicyMode)) | ||
|
|
||
|
|
||
| register_cli_argument('policy definition', 'display_name', help='display name of policy definition') | ||
| register_cli_argument('policy definition', 'description', help='description of policy definition') | ||
| register_cli_argument('policy assignment', 'name', options_list=('--name', '-n'), completer=get_policy_assignment_completion_list, help='name of the assignment') | ||
|
|
@@ -87,10 +87,28 @@ | |
| c.register_cli_argument('policy assignment create', 'params', options_list=('--params', '-p'), | ||
| help='JSON formatted string or path to file with parameter values of policy rule') | ||
|
|
||
| with VersionConstraint(ResourceType.MGMT_RESOURCE_POLICY, min_api='2017-06-01-preview') as c: | ||
| existing_policy_set_definition_name_type = CliArgumentType(options_list=('--name', '-n'), completer=get_policy_set_completion_list, help='The policy set definition name') | ||
| c.register_cli_argument('policy set-definition', 'policy_set_definition_name', arg_type=existing_policy_set_definition_name_type) | ||
| c.register_cli_argument('policy set-definition create', 'name', options_list=('--name', '-n'), help='name of the new policy set definition') | ||
| c.register_cli_argument('policy set-definition', 'display_name', help='display name of policy set definition') | ||
| c.register_cli_argument('policy set-definition', 'description', help='description of policy set definition') | ||
| c.register_cli_argument('policy set-definition', 'params', | ||
| help='JSON formatted string or a path to a file or uri with parameter definitions', | ||
| type=file_type, completer=FilesCompleter()) | ||
| c.register_cli_argument('policy set-definition', 'definitions', | ||
| help='JSON formatted string or a path to a file or uri with such content', | ||
| type=file_type, completer=FilesCompleter()) | ||
| c.register_cli_argument('policy assignment create', 'policy_set_definition', options_list=('--policy-set-definition', '-d'), | ||
| help='name or id of the policy set definition.') | ||
| c.register_cli_argument('policy assignment create', 'sku', options_list=('--sku', '-s'), | ||
| help='policy sku.', **enum_choice_list(['free', 'standard'])) | ||
| c.register_cli_argument('policy assignment create', 'notscopes', options_list=('--not-scopes'), nargs='+') | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no help for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will add, thanks
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| register_cli_argument('policy assignment', 'scope', help='scope at which this policy assignment applies to, e.g., /subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333, /subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup, or /subscriptions/0b1f6471-1bf0-4dda-aec3-111122223333/resourceGroups/myGroup/providers/Microsoft.Compute/virtualMachines/myVM') | ||
| register_cli_argument('policy assignment', 'disable_scope_strict_match', action='store_true', help='include assignment either inhertied from parent scope or at child scope') | ||
| register_cli_argument('policy assignment', 'display_name', help='display name of the assignment') | ||
| register_cli_argument('policy assignment', 'policy', help='policy name or fully qualified id', completer=get_policy_completion_list) | ||
| register_cli_argument('policy assignment', 'policy', help='name or id of the policy definition.', completer=get_policy_completion_list) | ||
|
|
||
| register_cli_argument('group', 'tag', tag_type) | ||
| register_cli_argument('group', 'tags', tags_type) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -690,12 +690,20 @@ def list_features(client, resource_provider_namespace=None): | |
| return client.list_all() | ||
|
|
||
|
|
||
| def create_policy_assignment(policy, name=None, display_name=None, params=None, | ||
| resource_group_name=None, scope=None): | ||
| def create_policy_assignment(policy=None, policy_set_definition=None, | ||
| name=None, display_name=None, params=None, | ||
| resource_group_name=None, scope=None, sku=None, | ||
| not_scopes=None): | ||
| """Creates a policy assignment | ||
| :param not_scopes: Space separated scopes where the policy assignment does not apply. | ||
| """ | ||
| if bool(policy) == bool(policy_set_definition): | ||
| raise CLIError('usage error: --policy NAME_OR_ID | \ | ||
| --policy-set-definition NAME_OR_ID') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: The formatting here is not quite right. When I run it, I see the following: You need to add a couple extra quotes to avoid the line-wrap from showing in the output. |
||
| policy_client = _resource_policy_client_factory() | ||
| scope = _build_policy_scope(policy_client.config.subscription_id, | ||
| resource_group_name, scope) | ||
| policy_id = _resolve_policy_id(policy, policy_client) | ||
| policy_id = _resolve_policy_id(policy, policy_set_definition, policy_client) | ||
|
|
||
| if params: | ||
| if os.path.exists(params): | ||
|
|
@@ -704,7 +712,26 @@ def create_policy_assignment(policy, name=None, display_name=None, params=None, | |
| params = shell_safe_json_parse(params) | ||
|
|
||
| PolicyAssignment = get_sdk(ResourceType.MGMT_RESOURCE_POLICY, 'PolicyAssignment', mod='models') | ||
| assignment = PolicyAssignment(display_name, policy_id, scope, params if params else None) | ||
| assignment = PolicyAssignment(display_name, policy_id, scope) | ||
| assignment.parameters = params if params else None | ||
|
|
||
| if supported_api_version(ResourceType.MGMT_RESOURCE_POLICY, min_api='2017-06-01-preview'): | ||
| if not_scopes: | ||
| kwargs_list = [] | ||
| for id_arg in not_scopes.split(' '): | ||
| if parse_resource_id(id_arg): | ||
| kwargs_list.append(id_arg) | ||
| else: | ||
| logger.error('az policy assignment create error: argument --not-scopes: \ | ||
| invalid notscopes value: \'%s\'' % id_arg) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here with the use of Right now, you are doing: Rather, it should be: |
||
| return | ||
| assignment.not_scopes = kwargs_list | ||
| PolicySku = get_sdk(ResourceType.MGMT_RESOURCE_POLICY, 'PolicySku', mod='models') | ||
| policySku = PolicySku('A0', 'Free') | ||
| if sku: | ||
| policySku = policySku if sku.lower() == 'free' else PolicySku('A1', 'Standard') | ||
| assignment.sku = policySku | ||
|
|
||
| return policy_client.policy_assignments.create(scope, | ||
| name or uuid.uuid4(), | ||
| assignment) | ||
|
|
@@ -774,11 +801,15 @@ def _build_policy_scope(subscription_id, resource_group_name, scope): | |
| return scope | ||
|
|
||
|
|
||
| def _resolve_policy_id(policy, client): | ||
| policy_id = policy | ||
| if not is_valid_resource_id(policy): | ||
| policy_def = client.policy_definitions.get(policy) | ||
| policy_id = policy_def.id | ||
| def _resolve_policy_id(policy, policy_set_definition, client): | ||
| policy_id = policy or policy_set_definition | ||
| if not is_valid_resource_id(policy_id): | ||
| if policy: | ||
| policy_def = client.policy_definitions.get(policy) | ||
| policy_id = policy_def.id | ||
| else: | ||
| policy_set_def = client.policy_set_definitions.get(policy_set_definition) | ||
| policy_id = policy_set_def.id | ||
| return policy_id | ||
|
|
||
|
|
||
|
|
@@ -812,6 +843,17 @@ def create_policy_definition(name, rules=None, params=None, display_name=None, d | |
| return policy_client.policy_definitions.create_or_update(name, parameters) | ||
|
|
||
|
|
||
| def create_policy_setdefinition(name, definitions, params=None, display_name=None, description=None): | ||
| definitions = _load_file_string_or_uri(definitions, 'definitions') | ||
| params = _load_file_string_or_uri(params, 'params', False) | ||
|
|
||
| policy_client = _resource_policy_client_factory() | ||
| PolicySetDefinition = get_sdk(ResourceType.MGMT_RESOURCE_POLICY, 'PolicySetDefinition', mod='models') | ||
| parameters = PolicySetDefinition(policy_definitions=definitions, parameters=params, description=description, | ||
| display_name=display_name) | ||
| return policy_client.policy_set_definitions.create_or_update(name, parameters) | ||
|
|
||
|
|
||
| def get_policy_definition(policy_definition_name): | ||
| from msrestazure.azure_exceptions import CloudError | ||
| policy_client = _resource_policy_client_factory() | ||
|
|
@@ -826,6 +868,20 @@ def get_policy_definition(policy_definition_name): | |
| raise | ||
|
|
||
|
|
||
| def get_policy_setdefinition(policy_set_definition_name): | ||
| from msrestazure.azure_exceptions import CloudError | ||
| policy_client = _resource_policy_client_factory() | ||
| try: | ||
| return policy_client.policy_set_definitions.get(policy_set_definition_name) | ||
| except CloudError as ex: | ||
| if ex.status_code == 404: | ||
| # work around for https://github.com/Azure/azure-cli/issues/692 | ||
| policy_id = '/providers/Microsoft.Authorization/policysetdefinitions/' + policy_set_definition_name | ||
| rcf = _resource_client_factory() | ||
| return rcf.resources.get_by_id(policy_id, policy_client.policy_set_definitions.api_version) | ||
| raise | ||
|
|
||
|
|
||
| def update_policy_definition(policy_definition_name, rules=None, params=None, | ||
| display_name=None, description=None): | ||
| if rules: | ||
|
|
@@ -852,12 +908,44 @@ def update_policy_definition(policy_definition_name, rules=None, params=None, | |
| return policy_client.policy_definitions.create_or_update(policy_definition_name, parameters) | ||
|
|
||
|
|
||
| def update_policy_setdefinition(policy_set_definition_name, definitions=None, params=None, | ||
| display_name=None, description=None): | ||
| if definitions: | ||
| if os.path.exists(definitions): | ||
| definitions = get_file_json(definitions) | ||
| else: | ||
| definitions = shell_safe_json_parse(definitions) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you just accept JSON and point of the @{file} syntax, you can eliminate this source of potential bugs.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading your above comment I understand the reason to keep these commands similar, so this is fine. |
||
|
|
||
| if params: | ||
| if os.path.exists(params): | ||
| params = get_file_json(params) | ||
| else: | ||
| params = shell_safe_json_parse(params) | ||
|
|
||
| policy_client = _resource_policy_client_factory() | ||
| definition = policy_client.policy_set_definitions.get(policy_set_definition_name) | ||
| # pylint: disable=line-too-long,no-member | ||
| PolicySetDefinition = get_sdk(ResourceType.MGMT_RESOURCE_POLICY, 'PolicySetDefinition', mod='models') | ||
| parameters = PolicySetDefinition( | ||
| policy_definitions=definitions if definitions is not None else definition.policy_definitions, | ||
| description=description if description is not None else definition.description, | ||
| display_name=display_name if display_name is not None else definition.display_name, | ||
| parameters=params if params is not None else definition.parameters) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Recommend you change "params" to "parameters" for consistency with the group deployment commands. Additionally, just like
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you are going for parity with the |
||
| return policy_client.policy_set_definitions.create_or_update(policy_set_definition_name, parameters) | ||
|
|
||
|
|
||
| def get_policy_completion_list(prefix, **kwargs): # pylint: disable=unused-argument | ||
| policy_client = _resource_policy_client_factory() | ||
| result = policy_client.policy_definitions.list() | ||
| return [i.name for i in result] | ||
|
|
||
|
|
||
| def get_policy_set_completion_list(prefix, **kwargs): # pylint: disable=unused-argument | ||
| policy_client = _resource_policy_client_factory() | ||
| result = policy_client.policy_set_definitions.list() | ||
| return [i.name for i in result] | ||
|
|
||
|
|
||
| def get_policy_assignment_completion_list(prefix, **kwargs): # pylint: disable=unused-argument | ||
| policy_client = _resource_policy_client_factory() | ||
| result = policy_client.policy_assignments.list() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendations: 1) don't accept JSON unless you truly have to. 2) If you do need this to be JSON, then just accept the JSON string. The help text would look like
JSON string containing the policy definition. Use @{file} to load from a file.We are trying to move away from sniffing whether an input is JSON or a path through the @ symbol (which loads the contents of the file).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a json, since it takes a json array which can have:
With regards to help, the definitions can also be a uri that points to a valid json file, just like the rules in policy definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are going for parity with
policy definition createthen disregard my comment. However, do make sure the help and behavior is consistent.