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.

Sign up for GitHub

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

Jump to bottom

Enable DefaultAzureCredential authentication for Azure filesystem block #7513

Merged
merged 4 commits into from
Feb 24, 2023
Merged

Enable DefaultAzureCredential authentication for Azure filesystem block #7513

merged 4 commits into from
Feb 24, 2023

Conversation

samdyzon
Copy link
Contributor

Background

Recent updates to the Azure filesystem block introduced the ability to authenticate with Azure via a service principal (#6838). This short PR extends the parameters of the Azure filesystem block by exposing the anon parameter of the ALDFS filesystem class which is the main parameter required to tell ALDFS to use the DefaultAzureCredential authentication mechanism.

DefaultAzureCredential attempts to automatically determine what type of credentials to use based on the execution environment, and enables authentication via the following:

  • A service principal configured by environment variables.
  • An Azure managed identity.
  • The user currently signed in to Visual Studio Code.
  • The identity currently logged in to the Azure CLI.
  • The identity currently logged in to Azure PowerShell.

Problem Statement

ALDFS exposes a single parameter, anon, with a default value of True. If a user provides no credential, sas_token, and account_key, and anon is False then ADLFS will use DefaultAzureCredential. See the ADLFS spec.py file for details.

Solution

By exposing the anon parameter and leaving it default to True, this change is a backwards-compatible method for enabling additional authentication mechanisms that are currently impossible with the Azure filesystem block, including:

  • Deploying flows to Azure Blob storage with an appropriate Azure AD user logged in to the CLI
  • Running flows in Kubernetes using the Azure AD Workload Identity operator
  • Deploying/running flows on an Azure VM backed by a Managed Identity

Notes

I've looked at PR #6838 as a reference for the changes in this PR, the only thing I don't know how to do is to update the block test hash. I was also unable to test this change locally because attempting to run prefect block register -m prefect.filesystems throws an error about modifying fields.

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • This pull request includes tests or only affects documentation.
  • This pull request includes a label categorizing the change e.g. fix, feature, enhancement

@netlify
Copy link

netlify bot commented Nov 11, 2022 •
edited
Loading

✅ Deploy Preview for prefect-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 97177a2
🔍 Latest deploy log https://app.netlify.com/sites/prefect-docs/deploys/63f935897bc8c2000843ac94
😎 Deploy Preview https://deploy-preview-7513--prefect-docs.netlify.app/api-ref/prefect/filesystems
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

tpdorsey
tpdorsey approved these changes Nov 17, 2022
View reviewed changes
docs/concepts/filesystems.md Outdated Show resolved Hide resolved
@samdyzon samdyzon force-pushed the enable-azure-default-credentials branch 3 times, most recently from 7cf604c to 3172e2a Compare November 23, 2022 01:07
@samdyzon
Copy link
Contributor Author

Hi folks, is there anything you need me to do to have this merged?

@samdyzon samdyzon force-pushed the enable-azure-default-credentials branch from 3172e2a to 4caf7c3 Compare November 25, 2022 11:23
@OliverKleinBST
Copy link

Hi @madkinsz, @samdyzon

we are also very much interested to using service principal, so would be very much interested in getting this ticket integrated.
Is there anything we could help with to make it happen?

@zanieb zanieb added the enhancement An improvement of an existing feature label Feb 24, 2023
@zanieb
Copy link
Contributor

zanieb commented Feb 24, 2023

Hi @samdyzon! Looks like this got a little lost.

Is it feasible to add a test covering this change? Can you resolve the merge conflicts with main or do you need help?

@desertaxle desertaxle requested review from cicdw and a team as code owners February 24, 2023 19:26
@desertaxle desertaxle merged commit 15ed6e3 into PrefectHQ:main Feb 24, 2023
@samdyzon
Copy link
Contributor Author

Hi @madkinsz, apologies - all of these notifications came through in the middle of the night for me, but I'm glad this has been merged. Thanks!

ddelange added a commit to ddelange/prefect that referenced this pull request Feb 27, 2023
@ddelange
Merge branch 'main' of https://github.com/prefecthq/prefect into patch-1
d8a61ed 
* 'main' of https://github.com/prefecthq/prefect:
  Bump @playwright/test from 1.30.0 to 1.31.1 in /ui (PrefectHQ#8663)
  Bump @prefecthq/prefect-ui-library from 1.1.9 to 1.1.10 in /ui (PrefectHQ#8661)
  Bump vite from 4.1.2 to 4.1.4 in /ui (PrefectHQ#8662)
  Bump eslint from 8.34.0 to 8.35.0 in /ui (PrefectHQ#8664)
  Fix of a typo - removed repeated word (PrefectHQ#8654)
  Enable DefaultAzureCredential authentication for Azure filesystem block (PrefectHQ#7513)
  Improve supervisor repr for debugging (PrefectHQ#8633)
ddelange added a commit to ddelange/prefect that referenced this pull request Mar 1, 2023
@ddelange
Merge branch 'patch-1' of https://github.com/ddelange/prefect into gr…
80b403f 
…aceful-agent

* 'patch-1' of https://github.com/ddelange/prefect: (25 commits)
  Enhancement: Track flow run id when generating task run results (PrefectHQ#8674)
  Fix loading of existing deployment descriptions from the server (PrefectHQ#8675)
  Update Dask logo (PrefectHQ#8669)
  Update `send_call` to `send_call_to_supervisor` (PrefectHQ#8653)
  Add Netlify Edge function to proxy to Segment (PrefectHQ#8657)
  Add security headers for docs (PrefectHQ#8655)
  Override analytics block instead of partial (PrefectHQ#8656)
  Add description flag to prefect deployment build CLI command (PrefectHQ#8603)
  Add support for yaml config strings to `KubernetesClusterConfig` (PrefectHQ#8643)
  Change lazy loads from joined to selectin (PrefectHQ#8659)
  Add timeout support to supervisors (PrefectHQ#8649)
  Minor markdown link fix in orchestration docs (PrefectHQ#8660)
  Allow more ports
  Bump @playwright/test from 1.30.0 to 1.31.1 in /ui (PrefectHQ#8663)
  Bump @prefecthq/prefect-ui-library from 1.1.9 to 1.1.10 in /ui (PrefectHQ#8661)
  Bump vite from 4.1.2 to 4.1.4 in /ui (PrefectHQ#8662)
  Bump eslint from 8.34.0 to 8.35.0 in /ui (PrefectHQ#8664)
  Fix of a typo - removed repeated word (PrefectHQ#8654)
  Enable DefaultAzureCredential authentication for Azure filesystem block (PrefectHQ#7513)
  Improve supervisor repr for debugging (PrefectHQ#8633)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@desertaxle desertaxle desertaxle approved these changes

@zanieb zanieb zanieb approved these changes

@tpdorsey tpdorsey tpdorsey approved these changes

@cicdw cicdw Awaiting requested review from cicdw cicdw is a code owner

Assignees
No one assigned
Labels
enhancement An improvement of an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@samdyzon @OliverKleinBST @zanieb @tpdorsey @desertaxle