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

Libgssapi #472

Merged
merged 6 commits into from
Apr 7, 2023
Merged

Libgssapi #472

merged 6 commits into from
Apr 7, 2023

Conversation

kotval
Copy link
Contributor

@kotval kotval commented Mar 16, 2023

The Tiberius connector's AuthMethod::Integrated means AD on windows, but it means kerberos on *nix. If libgssapi is installed, then you might want to use this auth method. This is a common set up in orgs which mostly use Windows and disallow SA auth. Kerberos is the only way to connect to the MSSQL server from linux. The #[cfg(windows)] meant that if you tried to use kerberos from a *nix client to connect to a MSSQL db with tiberius, you'd get a rather cryptic error message. For instance


con_str = f"mssql://some.db.url:1433/encrypt=true&trusted_execution=true"

query = "SELECT * FROM Table

cx.read_sql(con_str,query)

errors out with

Login failed for user ''. code=18456

when run on linux.

This PR fixes that issue.

@kotval kotval closed this Mar 16, 2023
@kotval kotval reopened this Mar 16, 2023
@kotval
Copy link
Contributor Author

kotval commented Mar 18, 2023

I missed the other PR #342, but even if that one is approved, this issue will remain as the #[cfg(windows)] decorator is still not correct for *nix. If that other PR is merged, I can open another PR with only the decorator change. Alternatively, that other one seems stalled for some reason. I can update this PR instead. Please pick one of these options.

@wangxiaoying
Copy link
Contributor

Hi @kotval , thanks for the PR.

I think #342 stalled due to the test of building on all platforms. Basically we need to make sure the release workflow can run correctly on linux, mac and windows (not including the final upload task).

It would be great if you can update this PR with modifications from #342 and try to pass both ci and release workflow. Please let me know if you need my help in running the workflow (I'm not sure whether you can run release it on your forked repo locally).

@kotval
Copy link
Contributor Author

kotval commented Mar 22, 2023

Also, apparently this addresses #334

maxb2 and others added 2 commits March 24, 2023 16:18
This updates the manylinux version because manylinux2014 only supports clang@3.4.2 where libgssapi requires clang>=3.9. manylinux_2_28 supports clang@14 solving a lot of issues. This also updates maturin to fix a panic while auditing the built wheel.

FWIW, manylinux2014 is EOL June 30th, 2024 so this change needs to happen soon anyway.


---------

Co-authored-by: david.kotval <david.kotval@veteransunited.com>
@kotval
Copy link
Contributor Author

kotval commented Mar 24, 2023

Go ahead and run the workflows that require approval. @maxb2 fixed the workflows to get them to build. If you have any questions about the changes, ask him. Thanks @maxb2 !

@maxb2
Copy link
Contributor

maxb2 commented Mar 24, 2023

An important change to consider is that I had to upgrade from manylinux2014 to manylinux_2_28. It was the only way to get a recent enough version of clang so that libgssapi could build (other than compiling clang from source in manylinux2014). For what it's worth, manylinux2014 will be EOL on June 30th, 2024, so you'll have to upgrade in the near-ish future anyway.

@kotval
Copy link
Contributor Author

kotval commented Mar 28, 2023

There is an issue that I discovered. I'm not sure how to get the integrated-auth-gssapi feature to be available from python. There is something I don't understand about how maturin builds the connectorx crate. I'll try to add a test to the python package that will illustrate the issue.

fix: pipeline

---------

Co-authored-by: matt.anderson <matt.anderson@veteransunited.com>
@maxb2
Copy link
Contributor

maxb2 commented Apr 4, 2023

@wangxiaoying this is ready for review! Here is a successful prerelease build on my fork.

The latest commit creates the integrated-auth-gssapi feature in the connectorx and connectorx-python rust libraries. At the lowest level, it just enables the tiberius/ntegrated-auth-gssapi feature which uses the libgssapi library to handle integrated authentication for MSSQL servers. It also enables the feature for all Unix (and NOT Windows) based python wheels built in .github/workflows/release.yml. @kotval and I assumed that you wouldn't want to use Kerberos on Windows rather than the built-in Windows authentication.

I'm not sure how to get the integrated-auth-gssapi feature to be available from python. There is something I don't understand about how maturin builds the connectorx crate.

We found that using pip install --editable connectorx-python/ doesn't enable features properly when it invokes maturin build. However, we verified that the wheels built directly with maturin using the --features flag work as intended.

@wangxiaoying wangxiaoying merged commit b94f7d6 into sfu-db:main Apr 7, 2023
@wangxiaoying
Copy link
Contributor

Thanks @kotval @maxb2 for working this out! I will try to release an alpha version this weekend.

@wangxiaoying
Copy link
Contributor

0.3.2-alpha.3 has been released.

@maxb2
Copy link
Contributor

maxb2 commented Apr 10, 2023

I double checked that the 0.3.2a3 pypi package works as expected. Thanks for the quick turnaround!

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.

3 participants