Skip to content

Conversation

vpipkt
Copy link
Contributor

@vpipkt vpipkt commented Jun 12, 2025

What

Allow Azure Entra authentication to SQL Server databases in this

Fix #20866

How

simply add the appropriate azure identity library for the JDBC driver version

Review guide

User Impact

enables Azure Entra (AD) authentication for MSSQL. Should be no negative side effects.

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

Copy link

vercel bot commented Jun 12, 2025

@vpipkt is attempting to deploy a commit to the Airbyte Growth Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

👋 Welcome to Airbyte!

Thank you for your contribution from vpipkt/airbyte! We're excited to have you in the Airbyte community.

Helpful Resources

PR Slash Commands

As needed or by request, Airbyte Maintainers can execute the following slash commands on your PR:

  • /format-fix - Fixes most formatting issues.
  • /bump-version - Bumps connector versions.
  • /run-connector-tests - Runs connector tests.
  • /run-cat-tests - Runs CAT tests.
  • /build-connector-images - Builds and publishes a pre-release docker image for the modified connector(s).

If you have any questions, feel free to ask in the PR comments or join our Slack community.

Tips for Working with CI

  1. "Vercel" CI failures. You can always ignore these. These are our docs builds but they can only be run by approval. A maintainer will approve the workflow upon request, but this failure should not be of concern.
  2. Pre-Release Checks. Please pay attention to these, as they contain standard checks on the metadata.yaml file, docs requirements, etc. If you need help resolving a pre-release check, please ask a maintainer.
    • Note: If you are creating a new connector, please be sure to replace the default logo.svg file with a suitable icon.
  3. Connector CI Tests. Some failures here may be expected if your tests require credentials. Please review these results to ensure (1) unit tests are passing, if applicable, and (2) integration tests pass to the degree possible and expected.
  4. (Optional.) BYO Connector Credentials for tests in your fork. You can optionally set up your fork with BYO credentials for your connector. This can significantly speed up your review, ensuring your changes are fully tested before the maintainers begin their review.

@vpipkt vpipkt marked this pull request as ready for review June 12, 2025 09:09
@vpipkt vpipkt requested a review from a team as a code owner June 12, 2025 09:09
@vpipkt
Copy link
Contributor Author

vpipkt commented Jun 13, 2025

I re-ran the unit tests locally and I had some similar but not exactly overlapping failures. On re-run I see all tests passing. Any other contributors on this have experience with flaky tests?

@marcosmarxm
Copy link
Contributor

marcosmarxm commented Jun 16, 2025

/run-connector-tests

Connector CI Tests Started

These tests will leverage Airbyte's integration test credentials.

Check job output.
Connector CI Tests job completed. See logs for details.

@burakku
Copy link
Contributor

burakku commented Aug 21, 2025

Hi @vpipkt do you mind posting a screenshot or something to show the new auth is working? Then I have no problem to merge this change

@vpipkt
Copy link
Contributor Author

vpipkt commented Aug 22, 2025

@burakku sounds great. I will resolve conflicts, build locally again and add some screenshots here showing auth. Similar to #57511

@vpipkt
Copy link
Contributor Author

vpipkt commented Aug 22, 2025

Based on my build of the latest commit 040e4d1 I have set up this connector using Azure Entra ServicePrincipal. The service principal's client id is used as the username, with a client secret as the database password. Works great. @burakku

image

@github-project-automation github-project-automation bot moved this from Backlog to Ready to Ship in 🧑‍🏭 Community Pull Requests Aug 27, 2025
@cgardens
Copy link
Contributor

cgardens commented Sep 7, 2025

/run-connector-tests

Connector CI Tests Started

These tests will leverage Airbyte's integration test credentials.

Check job output.
✅ Connector CI Tests job completed successfully. See logs for details.

@cgardens
Copy link
Contributor

cgardens commented Sep 7, 2025

/run-connector-tests

Connector CI Tests Started

These tests will leverage Airbyte's integration test credentials.

Check job output.
✅ Connector CI Tests job completed successfully. See logs for details.

@cgardens
Copy link
Contributor

cgardens commented Sep 7, 2025

@burakku I'm seeing a unit test failure. Ran it twice and got a different failure each time. I suspect it is unrelated to the change in this PR. Can you advise on whether you'd like me to go ahead and merge this or wait until we can get the tests to pass.

@vpipkt
Copy link
Contributor Author

vpipkt commented Sep 8, 2025

@cgardens ditto.. i noticed some flaky failures also #61547 (comment)

@burakku
Copy link
Contributor

burakku commented Sep 11, 2025

@vpipkt we are actually in the process of rolling out a new version of mssql connector in PR #63731
The old CDK will be deprecated so your change will have to be redo. I have included your change in the new PR so it will be rolled out in next 1-2 weeks

@vpipkt
Copy link
Contributor Author

vpipkt commented Sep 12, 2025

@burakku thank you so much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Ready to Ship

Development

Successfully merging this pull request may close these issues.

Source MSSQL - Failed to load MSAL4J Java library for performing ActiveDirectoryPassword authentication.

5 participants