Connect-DbaInstance - Change EncryptConnection to a switch with a default of $true

[andreasjordan]

Type of Change

• Bug fix (non-breaking change, fixes Connect-DbaInstance - ignores -Encrypt False #9113 )
• New feature (non-breaking change, adds functionality, fixes # )
• Breaking change (affects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (.\tests\manual.pester.ps1)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• Unit test is included
• Documentation
• Build system

See issue for details.

Up for discussion: Should we always use EncryptConnection = $true and only change TrustServerCertificate to $true within Set-DbatoolsInsecureConnection? I think it would be best practice to change the least number of default as possible. And switching off encryption is not needed if we only trust the (self-signed) server certificate.

[potatoqualitee]

Thank you so much, andreas! I've done this before and have had complaints. We talked about it here #9021

I'm open to merging this since the tests are passing. but I'm concerned about old documentation that mentions Optional. Is there no way for us to handle that? I remember it being SO weirdly picky in that it only wanted a $true or $false (but maybe its the bit thing you mentioned?_

[andreasjordan]

I think we must not mix two things:

• SqlConnectionEncryptOption is a string and is currently the related class to the config 'sql.connection.encrypt' and the parameter EncryptConnection of Connect-DbaInstance. But we don't use it that way!
• We use the property EncryptConnection of the type Microsoft.SqlServer.Management.Common.SqlConnectionInfo which is a bool. So we definitly use a bool and thus we need a bool to set this to one of the two possible values: $true or $false.

Currently we can only set this to $true.

We could change the code inside of Connect-DbaInstance to look at the value of the string and set the bool based on this rules: https://learn.microsoft.com/en-us/dotnet/api/microsoft.data.sqlclient.sqlconnectionencryptoption?view=sqlclient-dotnet-standard-5.1
But I personally think "the normal user" would see the parameter as a switch (as the current docs also tell) and vote for changing this to a switch so set encryption to $true or $false.

But we can let this pr sit here for a while and discuss the topic.

[potatoqualitee]

thank you andreas. do they mix well? that sqlclient is used within the command. if that still works, I'm happy to merge as this comes up regularly

[andreasjordan]

Do you mean "Microsoft.Data.SqlClient" when you talk about "sqlclient"? That is only used twice;

In the code path for $inputObjectType -in 'RegisteredServer', 'ConnectionString' (starting line 647) and when the parameter AzureToken is used (starting line 875). In the first place, SqlConnectionStringBuilder is used, in the second place a SqlConnection is created based on a connection string. In both places, the value of the parameter -EncryptConnection is not used.

[andreasjordan]

Please have a look at the yml files. Maybe you want to change the parameter to $true there to always test the new default: encrypted connection. I can update the pr if you like.

[potatoqualitee]

Let's do this and see how it goes. Thank you again 🙇🏼