Skip to content
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

Workaround for Go 1.16 #642

Merged
merged 4 commits into from
Apr 11, 2021
Merged

Workaround for Go 1.16 #642

merged 4 commits into from
Apr 11, 2021

Conversation

robinknaapen
Copy link
Contributor

This PR should workaround the issues regarding to the tls changes in Go 1.16.

This is by no means a proper fix
I do not have enough knowledge about the foundation of the tls package. I can only confirm that the changes fixed my issues.

@robinknaapen
Copy link
Contributor Author

I noticed #640, this PR contains infinite recursion on tlsHandshakeConn

This should fix the issues in #639

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #642 (0643a28) into master (045585d) will increase coverage by 0.32%.
The diff coverage is 82.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #642      +/-   ##
==========================================
+ Coverage   71.98%   72.31%   +0.32%     
==========================================
  Files          24       24              
  Lines        5469     5469              
==========================================
+ Hits         3937     3955      +18     
+ Misses       1309     1289      -20     
- Partials      223      225       +2     
Impacted Files Coverage Δ
net.go 67.81% <82.35%> (+25.28%) ⬆️
token.go 60.28% <0.00%> (-0.64%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 045585d...0643a28. Read the comment docs.

@uvw
Copy link

uvw commented Mar 11, 2021

Any chance we could see this PR reviewed/merged soon?

@kedlas
Copy link

kedlas commented Mar 25, 2021

Hope to see this merged soon too.

@prochac
Copy link

prochac commented Mar 30, 2021

Author's GitHub activity seems pretty poor.

@robinknaapen
Copy link
Contributor Author

@prochac

You mean mine, or @denisenkom's?

@prochac
Copy link

prochac commented Mar 31, 2021

@prochac

You mean mine, or @denisenkom's?

Sorry, yes, the maintainer's, the author of the driver. That's the person we all waiting for.

@zapodot
Copy link

zapodot commented Apr 12, 2021

Any chance of a new release containing this fix?

@denisenkom
Copy link
Owner

Published https://github.com/denisenkom/go-mssqldb/releases/tag/v0.10.0

jsternberg added a commit to influxdata/flux that referenced this pull request May 4, 2021
jsternberg added a commit to influxdata/flux that referenced this pull request May 4, 2021
samfoxcode added a commit to samfoxcode/migrate that referenced this pull request Jun 18, 2021
update go-mssqldb to resolve panic issue referenced here: denisenkom/go-mssqldb#642
dhui pushed a commit to golang-migrate/migrate that referenced this pull request Jul 2, 2021
* MSI Auth for SQL Server

Add MSI Auth option to SQL Server connection. Default if no password is provided in connection string.

* Parse resource endpoint from server for msi

update host name parsing to get just the resource endpoint for msi

* Update go-mssqldb

update go-mssqldb to resolve panic issue referenced here: denisenkom/go-mssqldb#642

* Update sqlserver.go

switch from deprecated methods. NewServicePrincipalTokenFromManagedIdentity calls the two methods that are deprecated for us

* Update sqlserver.go

add useMsi param instead of looking for nil password

* Update sqlserver readme

Update sqlserver readme for msi auth. make useMsi a bit safer

* Update sqlserver.go

remove comment

* Update database/sqlserver/README.md

Co-authored-by: Keegan Campbell <[email protected]>

* Update sqlserver.go

refactor resource uri logic into its own method

* Update sqlserver_test.go

add tests for msi connection. can only test whether it fails with useMsi= true, or succeeds with useMsi=false

* Update sqlserver.go

check msi.EnsureFresh return value

* Return error for multiple auth and move query filter

move migrate.FilterCustomQuery(purl).String() into one line out of if/else. return error if both useMsi=true and password are passed

* Update README.md

update readme with warning for useMsi

* Update sqlserver_test.go

Update TestMsiFalse test case as now it should fail when useMsi=false and no password is provided

Co-authored-by: Keegan Campbell <[email protected]>
szh added a commit to cyberark/go-mssqldb that referenced this pull request Oct 29, 2021
szh added a commit to cyberark/go-mssqldb that referenced this pull request Nov 1, 2021
FPiety0521 pushed a commit to FPiety0521/Golang-SQL that referenced this pull request May 24, 2023
* MSI Auth for SQL Server

Add MSI Auth option to SQL Server connection. Default if no password is provided in connection string.

* Parse resource endpoint from server for msi

update host name parsing to get just the resource endpoint for msi

* Update go-mssqldb

update go-mssqldb to resolve panic issue referenced here: denisenkom/go-mssqldb#642

* Update sqlserver.go

switch from deprecated methods. NewServicePrincipalTokenFromManagedIdentity calls the two methods that are deprecated for us

* Update sqlserver.go

add useMsi param instead of looking for nil password

* Update sqlserver readme

Update sqlserver readme for msi auth. make useMsi a bit safer

* Update sqlserver.go

remove comment

* Update database/sqlserver/README.md

Co-authored-by: Keegan Campbell <[email protected]>

* Update sqlserver.go

refactor resource uri logic into its own method

* Update sqlserver_test.go

add tests for msi connection. can only test whether it fails with useMsi= true, or succeeds with useMsi=false

* Update sqlserver.go

check msi.EnsureFresh return value

* Return error for multiple auth and move query filter

move migrate.FilterCustomQuery(purl).String() into one line out of if/else. return error if both useMsi=true and password are passed

* Update README.md

update readme with warning for useMsi

* Update sqlserver_test.go

Update TestMsiFalse test case as now it should fail when useMsi=false and no password is provided

Co-authored-by: Keegan Campbell <[email protected]>
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.

6 participants