-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[SQL] Support setting of AAD administrator on Managed Instance #10446
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 all commits
6c6441a
70b2e03
ee93a67
d8cc932
e2da8c5
c2afe5b
2463fba
5828b13
c05ae62
1762a83
7965e1f
856bbd7
6184aff
6e76c9a
229aaa7
6219076
b31e456
27f51f1
43d6b2d
3d6ceed
649c1ce
cd1d265
489daad
ec3b7cb
20ee134
e236f28
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 |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| ExportRequest, | ||
| ManagedDatabase, | ||
| ManagedInstance, | ||
| ManagedInstanceAdministrator, | ||
| Server, | ||
| ServerAzureADAdministrator, | ||
| Sku, | ||
|
|
@@ -195,6 +196,14 @@ def get_location_type_with_default_from_resource_group(cli_ctx): | |
| help='Complete the failover even if doing so may result in data loss. ' | ||
| 'This will allow the failover to proceed even if a primary database is unavailable.') | ||
|
|
||
| aad_admin_login_param_type = CLIArgumentType( | ||
| options_list=['--display-name', '-u'], | ||
| help='Display name of the Azure AD administrator user or group.') | ||
|
|
||
| aad_admin_sid_param_type = CLIArgumentType( | ||
| options_list=['--object-id', '-i'], | ||
| help='The unique ID of the Azure AD administrator.') | ||
|
|
||
| db_service_objective_examples = 'Basic, S0, P1, GP_Gen4_1, BC_Gen5_2.' | ||
| dw_service_objective_examples = 'DW100, DW1000c' | ||
|
|
||
|
|
@@ -937,12 +946,10 @@ def _configure_security_policy_storage_params(arg_ctx): | |
| options_list=['--server-name', '--server', '-s']) | ||
|
|
||
| c.argument('login', | ||
| options_list=['--display-name', '-u'], | ||
| help='Display name of the Azure AD administrator user or group.') | ||
| arg_type=aad_admin_login_param_type) | ||
|
|
||
| c.argument('sid', | ||
| options_list=['--object-id', '-i'], | ||
| help='The unique ID of the Azure AD administrator ') | ||
| arg_type=aad_admin_sid_param_type) | ||
|
|
||
| c.ignore('tenant_id') | ||
|
|
||
|
|
@@ -1199,6 +1206,35 @@ def _configure_security_policy_storage_params(arg_ctx): | |
| arg_type=kid_param_type, | ||
| required=True,) | ||
|
|
||
| ##### | ||
| # sql managed instance ad-admin | ||
| ###### | ||
| with self.argument_context('sql mi ad-admin') as c: | ||
| c.argument('managed_instance_name', | ||
| arg_type=managed_instance_param_type) | ||
|
|
||
| c.argument('login', | ||
| arg_type=aad_admin_login_param_type) | ||
|
|
||
| c.argument('sid', | ||
| arg_type=aad_admin_sid_param_type) | ||
|
|
||
| with self.argument_context('sql mi ad-admin create') as c: | ||
| # Create args that will be used to build up the ManagedInstanceAdministrator object | ||
| create_args_for_complex_type( | ||
|
Contributor
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 think you can put this in
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've tried to do this, but when executing get or delete command get the following error: ValueError: command authoring error: extra argument 'login' cannot be registered to a group-level scope 'sql mi ad-admin'. It must be registered to a specific command. |
||
| c, 'properties', ManagedInstanceAdministrator, [ | ||
| 'login', | ||
| 'sid', | ||
| ]) | ||
|
|
||
| with self.argument_context('sql mi ad-admin update') as c: | ||
| # Create args that will be used to build up the ManagedInstanceAdministrator object | ||
| create_args_for_complex_type( | ||
| c, 'properties', ManagedInstanceAdministrator, [ | ||
| 'login', | ||
| 'sid', | ||
| ]) | ||
|
|
||
| ##### | ||
| # sql server tde-key | ||
| ##### | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,6 @@ | |
| CapabilityStatus, | ||
| CreateMode, | ||
| DatabaseEdition, | ||
| EncryptionProtector, | ||
| FailoverGroup, | ||
| FailoverGroupReadOnlyEndpoint, | ||
| FailoverGroupReadWriteEndpoint, | ||
|
|
@@ -328,6 +327,17 @@ def _db_elastic_pool_update_sku( | |
| allow_reset_family=allow_reset_family) | ||
|
|
||
|
|
||
| def _get_tenant_id(): | ||
| ''' | ||
| Gets tenantId from current subscription. | ||
| ''' | ||
| from azure.cli.core._profile import Profile | ||
|
Contributor
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. Please add
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 |
||
|
|
||
| profile = Profile() | ||
| sub = profile.get_subscription() | ||
| return sub['tenantId'] | ||
|
|
||
|
|
||
| _DEFAULT_SERVER_VERSION = "12.0" | ||
|
|
||
|
|
||
|
|
@@ -1777,19 +1787,15 @@ def server_update( | |
|
|
||
|
|
||
| def server_ad_admin_set( | ||
| cmd, | ||
| client, | ||
| resource_group_name, | ||
| server_name, | ||
| **kwargs): | ||
| ''' | ||
| Sets a server's AD admin. | ||
| ''' | ||
| from azure.cli.core._profile import Profile | ||
|
|
||
| profile = Profile(cli_ctx=cmd.cli_ctx) | ||
| sub = profile.get_subscription() | ||
| kwargs['tenant_id'] = sub['tenantId'] | ||
| kwargs['tenant_id'] = _get_tenant_id() | ||
|
|
||
| return client.create_or_update( | ||
| server_name=server_name, | ||
|
|
@@ -2010,10 +2016,8 @@ def encryption_protector_update( | |
| return client.create_or_update( | ||
| resource_group_name=resource_group_name, | ||
| server_name=server_name, | ||
| parameters=EncryptionProtector( | ||
| server_key_type=server_key_type, | ||
| server_key_name=key_name | ||
| ) | ||
| server_key_type=server_key_type, | ||
| server_key_name=key_name | ||
|
Contributor
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. Why is this changed?
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. Build wasn't successful because of failed test test_sql_tdebyok (azure.cli.command_modules.sql.tests.latest.test_sql_commands.SqlTransparentDataEncryptionScenarioTest). You can find more on this link https://github.com/Azure/azure-cli/runs/214404678.
Contributor
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. Azure/azure-sdk-for-python@58857ae @zikalino , can you clarify why this breaking change happened? |
||
| ) | ||
|
|
||
| ############################################### | ||
|
|
@@ -2226,6 +2230,44 @@ def managed_instance_encryption_protector_update( | |
| server_key_name=key_name | ||
| ) | ||
|
|
||
|
|
||
| ##### | ||
| # sql managed instance ad-admin | ||
| ##### | ||
|
|
||
|
|
||
| def mi_ad_admin_set( | ||
| client, | ||
| resource_group_name, | ||
| managed_instance_name, | ||
| **kwargs): | ||
| ''' | ||
| Creates a managed instance active directory administrator. | ||
| ''' | ||
|
|
||
| kwargs['tenant_id'] = _get_tenant_id() | ||
|
|
||
| return client.create_or_update( | ||
| resource_group_name=resource_group_name, | ||
| managed_instance_name=managed_instance_name, | ||
| administrator_name="ActiveDirectory", | ||
|
Contributor
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. define constant, e.g.
Contributor
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. Also unclear why this is needed compared to SQL server admin
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. Skip this for now, because it will be removed after we change api specification and update the python client |
||
| parameters=kwargs | ||
| ) | ||
|
|
||
|
|
||
| def mi_ad_admin_delete( | ||
| client, | ||
| resource_group_name, | ||
| managed_instance_name): | ||
| ''' | ||
| Deletes a managed instance active directory administrator. | ||
| ''' | ||
| return client.delete( | ||
| resource_group_name=resource_group_name, | ||
| managed_instance_name=managed_instance_name, | ||
| administrator_name="ActiveDirectory" | ||
| ) | ||
|
|
||
| ############################################### | ||
| # sql managed db # | ||
| ############################################### | ||
|
|
||
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.
Please define arg types (at top of file) that can be used for both sql server and MI arguments, instead of copying
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.
Done