-
Notifications
You must be signed in to change notification settings - Fork 74
Feat: Add HostNameInCertificate for strict encryption #589
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.
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
Conversation
Change of plans....next iteration, I will change |
i will generate new localization files after merging the driver update change when it goes to main, just to avoid dealing with merge conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for specifying the expected host name in the server’s TLS certificate when using strict encryption, and replaces the -F <format>
flag for vertical output with a dedicated --vertical
switch.
- Introduce
HostNameInCertificate
field inConnectSettings
and include it in the connection string when non-empty - Change CLI: repurpose
-F
as the short flag for host-name-in-certificate, remove-F <format>
, and add--vertical
- Update tests and documentation to reflect the new flag behaviors
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
pkg/sqlcmd/connect.go | Added HostNameInCertificate field and query parameter logic |
pkg/sqlcmd/sqlcmd_test.go | Updated connection-string tests to expect the new hostname param |
cmd/sqlcmd/sqlcmd.go | Added CLI binding for host-name-in-certificate, removed format flag, added vertical flag |
cmd/sqlcmd/sqlcmd_test.go | Updated tests for --vertical , repurposed -F in argument parsing, adjusted invalid-value lists |
README.md | Documented usage of -F for hostname in certificate and --vertical |
Comments suppressed due to low confidence (1)
pkg/sqlcmd/sqlcmd_test.go:54
- Add a complementary test case where HostNameInCertificate is empty to verify that the connection string does not include the hostnameincertificate parameter when it’s unset.
&ConnectSettings{ServerName: `tcp:someserver,1045`, Encrypt: "strict", HostNameInCertificate: "*.mydomain.com"},
…ueybubbles/hostnameincertificate
Fixes #504
Since ODBC sqlcmd uses
-F
for the host name, we are going to follow suit and switch to--vertical
as the parameter to turn on vertical output formatting.sqlcmd -S someserver -Nstrict -F *.mydomain.com