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

feat(databricks-driver): Enable Azure AD authentication via service principal #6763

Open
wants to merge 53 commits into
base: master
Choose a base branch
from

Conversation

MaggieZhang-01
Copy link

@MaggieZhang-01 MaggieZhang-01 commented Jun 26, 2023

Check List

  • Tests has been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Description of Changes Made (if issue reference is not provided)

In the Databricks driver, the credential is generated by the Azure storage account access key now which provides full access to the storage account. For security best practice, Microsoft recommends using authorization with Azure Active Directory.

This PR is used to support Azure AD authentication via service principal which provides more fine-grained control over access to storage resources.

@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Jun 26, 2023
@MaggieZhang-01 MaggieZhang-01 changed the title feat(databricks-driver): Enable client secret credential of azure feat(databricks-driver): Enable Azure AD authentication via service principal Jun 27, 2023
@MaggieZhang-01 MaggieZhang-01 marked this pull request as ready for review June 27, 2023 02:35
@MaggieZhang-01 MaggieZhang-01 requested review from a team as code owners June 27, 2023 02:35
@MaggieZhang-01
Copy link
Author

Hello @paveltiunov, how are you doing? Thanks for providing this amazing product, our team depends on Cube to build our data analytics app, and we would like to support authentication via the Azure service principal. It will be highly appreciated if colleagues could help to review this PR. Thanks in advance and sorry for any inconvenience.

@MaggieZhang-01
Copy link
Author

Hello @pacofvf, would you be willing to help re-trigger the workflows? The last running failed, but it seems not related to my changes, many thanks and have a nice day!

@zhjuncai
Copy link

zhjuncai commented Jul 4, 2023

This is a cool enhancement feature on security perspective, we'd like to have the same. Looking forward to get this features merged into master branch.😁

@MaggieZhang-01
Copy link
Author

Hello @pacofvf Sorry for bothering you, would you please help to re-trigger the workflows again? And it would be highly appreciated if my changes could be reviewed.

@vercel
Copy link

vercel bot commented Jul 24, 2023

@MaggieZhang-01 is attempting to deploy a commit to the Cube Dev Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Jul 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-angular-dashboard ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am
examples-react-d3 ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am
examples-react-dashboard ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am
examples-react-data-table ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am
examples-react-highcharts ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am
examples-react-material-ui ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am
examples-react-pivot-table ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am
examples-vue-query-builder ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2024 3:28am

@MaggieZhang-01
Copy link
Author

@paveltiunov I have fixed the issue you mentioned, please feel free to approach me if there are any concerns. Much appreciated!

@igorlukanin
Copy link
Member

@MaggieZhang-01 Great job on this PR! Could you please kindly update this documentation page (https://github.com/cube-js/cube/blob/master/docs/pages/reference/configuration/environment-variables.mdx) with new environment variables as well? Thank you in advance!

@MaggieZhang-01
Copy link
Author

@MaggieZhang-01 Great job on this PR! Could you please kindly update this documentation page (https://github.com/cube-js/cube/blob/master/docs/pages/reference/configuration/environment-variables.mdx) with new environment variables as well? Thank you in advance!

@igorlukanin Thanks, I have updated the docs, please help to verify.

Copy link
Member

@igorlukanin igorlukanin left a comment

Choose a reason for hiding this comment

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

Docs changes LGTM. Thank you, @MaggieZhang-01!

@MaggieZhang-01
Copy link
Author

Hello @paveltiunov , still need your review and approve, would you help on this PR, thanks.

@zhjuncai
Copy link

I voted this feature, when this can be merged?

@KSDaemon
Copy link
Member

KSDaemon commented Dec 5, 2024

@MaggieZhang-01 Could you please resolve the conflicts and sync with the latest changes in master? I believe something related is already in place.

@KSDaemon
Copy link
Member

KSDaemon commented Jan 6, 2025

Gentle ping @MaggieZhang-01 ....

@KSDaemon
Copy link
Member

Okay... I took it into my own hands, rebased, and updated it. #9104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants