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.

Sign up for GitHub

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

[SQL DB, SQL MI] Add minimal_tls_version property for MI and SQL DB #12414

Merged
merged 19 commits into from
Mar 6, 2020
Merged

[SQL DB, SQL MI] Add minimal_tls_version property for MI and SQL DB #12414

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
35b5e3c
Managed Instance commands updated with new property
cicovica Mar 2, 2020
9662889
Added enum defs for input parameter and updated Sql Server arg
ziwa-msft Mar 3, 2020
d8a09da
fix blank line style error
ziwa-msft Mar 3, 2020
0914271
Add minimal_tls_version for sql db server
cicovica Mar 4, 2020
1d0dd71
adding tests
ziwa-msft Mar 5, 2020
0a221dd
Merge branch 'min-tls-version-prop' of https://github.com/cicovica/az…
ziwa-msft Mar 5, 2020
11c5c0e
Adding recordings for tests
ziwa-msft Mar 5, 2020
632fe0e
Re-record mi db test
ziwa-msft Mar 5, 2020
6683435
Bump dependency version on azure-mgmt-sql, fix lint errors
cicovica Mar 5, 2020
c595747
Fix code style/lint errors
cicovica Mar 5, 2020
7fcc7f7
re-recorded tests
ziwa-msft Mar 5, 2020
c8f0e1f
Merge branch 'min-tls-version-prop' of https://github.com/cicovica/az…
ziwa-msft Mar 5, 2020
6e396c1
Merge branch 'dev' of https://github.com/Azure/azure-cli into min-tls…
ziwa-msft Mar 5, 2020
24915a9
record tets
ziwa-msft Mar 6, 2020
218de31
reset some of the test fixes and added recordings
ziwa-msft Mar 6, 2020
0a06276
reset some of the test fixes and added recordings
ziwa-msft Mar 6, 2020
4f53792
retry
ziwa-msft Mar 6, 2020
f9e82ae
style
ziwa-msft Mar 6, 2020
859cec2
resolving merge conflict
ziwa-msft Mar 6, 2020
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
13 changes: 12 additions & 1 deletion src/azure-cli/azure/cli/command_modules/sql/_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@
ComputeModelType,
DatabaseCapabilitiesAdditionalDetails,
ElasticPoolCapabilitiesAdditionalDetails,
FailoverPolicyType
FailoverPolicyType,
SqlServerMinimalTlsVersionType,
SqlManagedInstanceMinimalTlsVersionType
)

from ._validators import (
Expand Down Expand Up @@ -1039,6 +1041,10 @@ def _configure_security_policy_storage_params(arg_ctx):
help='Generate and assign an Azure Active Directory Identity for this server'
'for use with key management services like Azure KeyVault.')

c.argument('minimal_tls_version',
arg_type=get_enum_type(SqlServerMinimalTlsVersionType),
Copy link
Contributor

@Juliehzl Juliehzl Mar 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Can we mark the parameter as preview by adding is_preview=True to give a sprint time to test the parameter behavior with the following flag?
  2. Do you have a default value for the parameter?

help='The minimal TLS version enforced by the sql server for inbound connections.')

with self.argument_context('sql server create') as c:
c.argument('location',
arg_type=get_location_type_with_default_from_resource_group(self.cli_ctx))
Expand Down Expand Up @@ -1246,6 +1252,10 @@ def _configure_security_policy_storage_params(arg_ctx):
arg_type=get_enum_type(ServerConnectionType),
help='The connection type used for connecting to the instance.')

c.argument('minimal_tls_version',
arg_type=get_enum_type(SqlManagedInstanceMinimalTlsVersionType),
help='The minimal TLS version enforced by the managed instance for inbound connections.')
Copy link
Contributor

Choose a reason for hiding this comment

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

You already defined this in sql server scope, why do you need to repeat this in sql server create scope?

Copy link
Contributor Author

@cicovica cicovica Mar 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is the sql mi scope, please correct me if I'm wrong, we need to add the argument for both sql server and managed instance


c.argument('public_data_endpoint_enabled',
arg_type=get_three_state_flag(),
help='Whether or not the public data endpoint is enabled for the instance.')
Expand All @@ -1264,6 +1274,7 @@ def _configure_security_policy_storage_params(arg_ctx):
'administrator_login',
'administrator_login_password',
'license_type',
'minimal_tls_version',
'virtual_network_subnet_id',
'vcores',
'storage_size_in_gb',
Expand Down
18 changes: 17 additions & 1 deletion src/azure-cli/azure/cli/command_modules/sql/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,19 @@ class FailoverPolicyType(Enum):
manual = 'Manual'


class SqlServerMinimalTlsVersionType(Enum):
Copy link
Contributor

Choose a reason for hiding this comment

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

You should have defined this enum in swagger so that it showed up in generated sdk. Then you wouldn't need to define it here. This is what I was asking about on your swagger review. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sorry about that, we have plans to fix it and do it the right way :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you, please do :)

tls_1_0 = "1.0"
tls_1_1 = "1.1"
tls_1_2 = "1.2"


class SqlManagedInstanceMinimalTlsVersionType(Enum):
tls_1_0 = "1.0"
tls_1_1 = "1.1"
tls_1_2 = "1.2"
no_tls = "None"


class ComputeModelType(str, Enum):

provisioned = "Provisioned"
Expand Down Expand Up @@ -2294,7 +2307,8 @@ def managed_instance_update(
proxy_override=None,
public_data_endpoint_enabled=None,
tier=None,
family=None):
family=None,
minimal_tls_version=None):
'''
Updates a managed instance. Custom update function to apply parameters to instance.
'''
Expand All @@ -2314,6 +2328,8 @@ def managed_instance_update(
storage_size_in_gb or instance.storage_size_in_gb)
instance.proxy_override = (
proxy_override or instance.proxy_override)
instance.minimal_tls_version = (
minimal_tls_version or instance.minimal_tls_version)

instance.sku.name = None
instance.sku.tier = (
Expand Down