Skip to content

Enable Encryption by default + introduce HostNameInCertificate#17484

Merged
cheenamalhotra merged 30 commits intomainfrom
cheena/mds5
Dec 20, 2022
Merged

Enable Encryption by default + introduce HostNameInCertificate#17484
cheenamalhotra merged 30 commits intomainfrom
cheena/mds5

Conversation

@cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Nov 26, 2022

Bringing in Microsoft.Data.SqlClient 5.0.1 dependency with STS 4.4.0.7

  • Sets Encrypt to Mandatory by default.

  • Adds support for HostNameInCertificate SqlConnection property.

  • Adds welcome note for encryption behavior change (Updated)

    More information redirects to: https://aka.ms/vscodemssql-connection (placeholder link for blogpost)

  • Enables setting Trust Server Certificate to true with prompt for cert validation failure.

    Read more redirects to: https://aka.ms/vscodemssql-connection (placeholder link for blogpost)

Demo

VSCode-mssql
Captures double failure scenario as well

@dzsquared
Copy link
Contributor

I'd prefer both the dialogs link to our aka.ms link where we will have specific documentation on VS Code connectivity. From there, we can link to more information specifically on configuring SQL Server certificates but folks will also see info on the settings file and encryption options.

Copy link

@erinstellato-ms erinstellato-ms left a comment

Choose a reason for hiding this comment

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

I agree with Drew's comment - everything else looks great.

@Charles-Gagnon
Copy link
Contributor

Apologies for the large amount of nit comments - I mostly left them here to point out the different types of cleanup to be on the lookout for when you're working in this repo (most of the newer styles such as async/await weren't in use when this stuff was originally made). I'll tone it down in future PRs - and feel free to just mark those as follow up items if you want.

There are a couple of actual issues/comments to look at too though.

@cheenamalhotra
Copy link
Member Author

Apologies for the large amount of nit comments - I mostly left them here to point out the different types of cleanup to be on the lookout for when you're working in this repo (most of the newer styles such as async/await weren't in use when this stuff was originally made). I'll tone it down in future PRs - and feel free to just mark those as follow up items if you want.

There are a couple of actual issues/comments to look at too though.

I agree there's a lot of improvement scope in this repository, will address things that are in scope of the PR here.

Copy link
Contributor

@Charles-Gagnon Charles-Gagnon left a comment

Choose a reason for hiding this comment

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

ConnectionUI has an issue you should resolve, and there's some other stuff you should look at too.

cheenamalhotra and others added 3 commits December 5, 2022 17:25
Co-authored-by: Charles Gagnon <chgagnon@microsoft.com>
@cheenamalhotra cheenamalhotra dismissed Charles-Gagnon’s stale review December 20, 2022 01:09

Addressed all feedback

@cheenamalhotra cheenamalhotra merged commit 25283b4 into main Dec 20, 2022
@cheenamalhotra cheenamalhotra deleted the cheena/mds5 branch December 20, 2022 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants