mssql_server: add minimal_tls_version property #8361
Conversation
manicminer
force-pushed
the
f/mssql-server-minimal-tls-version
branch
from
07e1781
to
f877513
Compare
manicminer
force-pushed
the
f/mssql-server-minimal-tls-version
branch
from
f877513
to
67bdffb
Compare
manicminer
changed the title
There was a problem hiding this comment.
Thanks @manicminer - looks good aside from a couple comments. I think we should change the property name to something that reads a bit better and matches redis frontdoor what we'd expect the property to be named.
Looks like postgres and mysql have the poorly named property added already :( might want to rename them for 3.0
| @@ -136,6 +137,16 @@ func resourceArmMsSqlServer() *schema.Resource { | |||
| }, | |||
| }, | |||
|
|
|||
| "minimal_tls_version": { | |||
There was a problem hiding this comment.
This might be better as minimum_tls_version? or even ssl_minimum_tls_version ?
There was a problem hiding this comment.
Agreed I wouldn't have chosen minimal but it seems to have pervaded. It did prompt me to check the dictionary though and technically it seems appropriate, though it feels a bit arcane to me.
There was a problem hiding this comment.
Yep, regardless of the api/sdk and existing properties i'd prefer we correct it (and the others) to minimum as technically right or not, to match what other resources/apis and the rest of the world uses
|
@katbyte Thanks for the review and catching my bad edits. Should all be fixed up now. |
| @@ -109,11 +112,11 @@ The following attributes are exported: | |||
|
|
|||
| * `tenant_id` - The Tenant ID for the Service Principal associated with the Identity of this SQL Server. | |||
|
|
|||
| -> You can access the Principal ID via `${azurerm_sql_server.example.identity.0.principal_id}` and the Tenant ID via `${azurerm_sql_server.example.identity.0.tenant_id}` | |||
| -> You can access the Principal ID via `${azurerm_mssql_server.example.identity.0.principal_id}` and the Tenant ID via `${azurerm_mssql_server.example.identity.0.tenant_id}` | |||
There was a problem hiding this comment.
Should we change this to .13 syntax?
| @@ -136,6 +137,16 @@ func resourceArmMsSqlServer() *schema.Resource { | |||
| }, | |||
| }, | |||
|
|
|||
| "minimal_tls_version": { | |||
There was a problem hiding this comment.
Yep, regardless of the api/sdk and existing properties i'd prefer we correct it (and the others) to minimum as technically right or not, to match what other resources/apis and the rest of the world uses
Co-authored-by: kt <kt@katbyte.me>
manicminer
force-pushed
the
f/mssql-server-minimal-tls-version
branch
from
b67be07
to
1e8e645
Compare
|
This has been released in version 2.31.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 2.31.0"
}
# ... other configuration ... |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked as resolved and limited conversation to collaborators
Closes: #7295
Closes: #7594