azurerm_synapse_workspace - remove sql password requirement for synapse
#18850
Conversation
There was a problem hiding this comment.
Thanks for this PR @RaphHaddad! When properties are not required rather than setting the property Required to false we set Optional to true.
We would also still need to add a test to validate that this change works, you can probably just copy one of the existing tests and remove the values for sql_administrator_login and sql_administrator_login_password in the test config. Once that's done we can run the tests and get this merged.
Thanks!
| @@ -73,14 +73,14 @@ func resourceSynapseWorkspace() *pluginsdk.Resource { | |||
|
|
|||
| "sql_administrator_login": { | |||
| Type: pluginsdk.TypeString, | |||
| Required: true, | |||
| Required: false, | |||
There was a problem hiding this comment.
| Required: false, | |
| Optional: true, |
| ForceNew: true, | ||
| ValidateFunc: validate.SqlAdministratorLoginName, | ||
| }, | ||
|
|
||
| "sql_administrator_login_password": { | ||
| Type: pluginsdk.TypeString, | ||
| Required: true, | ||
| Required: false, |
There was a problem hiding this comment.
| Required: false, | |
| Optional: true, |
RaphHaddad
changed the title
azurerm_synapse_workspace - remove sql password requirement for synapse
|
Hello @stephybun, thanks for the guidance. I have made the changes necessary and added a new test. Can you confirm that the naming is correct for the test? |
There was a problem hiding this comment.
Thanks @RaphHaddad the tests are passing 🎉 . I forgot to mention the docs need to be updated to reflect this change too, but once that's done this will be good to go!
|
@stephybun , done! |
|
This functionality has been released in v3.28.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Please excuse any major mistakes. This is my first PR.
This PR removes the requirement for a local SQL User on Synapse as per Azure's constraints.
I am not sure how to add a validation to check either AAD auth or local SQL user enabled. Any direction is appreciated.