cube-js
feat(databricks-driver): Enable Azure AD authentication via service principal
Check List
- [X] Tests has been run in packages where changes made if available
- [X] Linter has been run for changed code
- [X] Tests for the changes have been added if not covered yet
- [X] Docs have been added / updated if required
Issue Reference this PR resolves
[For example #12]
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.
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.
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！
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.😁
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.
@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[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Ignored Deployments
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| examples-angular-dashboard | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am | |
| examples-react-d3 | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am | |
| examples-react-dashboard | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am | |
| examples-react-data-table | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am | |
| examples-react-highcharts | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am | |
| examples-react-material-ui | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am | |
| examples-react-pivot-table | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am | |
| examples-vue-query-builder | ⬜️ Ignored (Inspect) | Visit Preview | Jun 21, 2024 6:59am |
@MaggieZhang-01 Could you please also add docs to this PR? You can take this one as inspiration: https://github.com/cube-js/cube/pull/7116
@MaggieZhang-01 Could you please also add docs to this PR? You can take this one as inspiration: #7116
@igorlukanin Many thanks for your review, the docs have been added by referring to the above PR.
@MaggieZhang-01 Could you please rebase PR to fix conflicts? Then we should be good to merge. Great job!
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 47.97%. Comparing base (
815be2f) to head (794b68b). Report is 233 commits behind head on master.
:exclamation: Current head 794b68b differs from pull request most recent head bfbd8a6
Please upload reports for the commit bfbd8a6 to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## master #6763 +/- ##
==========================================
+ Coverage 47.90% 47.97% +0.06%
==========================================
Files 154 154
Lines 21034 20915 -119
Branches 5422 5238 -184
==========================================
- Hits 10077 10033 -44
- Misses 10203 10688 +485
+ Partials 754 194 -560
| Flag | Coverage Δ | |
|---|---|---|
| cube-backend | 47.97% <100.00%> (+0.06%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
@MaggieZhang-01 Could you please rebase PR to fix conflicts? Then we should be good to merge. Great job!
@paveltiunov Conflict has been fixed, thanks for your time!
@paveltiunov Sorry for bothering you, would you help to check and merge the PR? All conflicts have been fixed, and the doc has been moved to the new path too. Thx!
@MaggieZhang-01 It seems tsc build is failing:
#121 61.52 > @cubejs-backend/[email protected] tsc
#121 61.52 > tsc
#121 61.52
#121 61.52 src/DatabricksDriver.ts(21,40): error TS2307: Cannot find module '@azure/identity' or its corresponding type declarations.
Could you please fix it?
@MaggieZhang-01 It seems tsc build is failing:
#121 61.52 > @cubejs-backend/[email protected] tsc #121 61.52 > tsc #121 61.52 #121 61.52 src/DatabricksDriver.ts(21,40): error TS2307: Cannot find module '@azure/identity' or its corresponding type declarations.Could you please fix it?
@paveltiunov Thanks for the reminder. The version of the newly added module @azure/identity is deprecated, I have upgraded the version to fix the issue.
@paveltiunov I have fixed the issue you mentioned, please feel free to approach me if there are any concerns. Much appreciated!
@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 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.
Hello @paveltiunov , still need your review and approve, would you help on this PR, thanks.