-
Notifications
You must be signed in to change notification settings - Fork 238
Add --disable-setaccesscontrol flag to prevent SetAccessControl calls on HNS accounts #1936
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
base: main
Are you sure you want to change the base?
Conversation
|
@vibhansa-msft 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot wasn't able to review any files in this pull request.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…n HNS accounts Co-authored-by: vibhansa-msft <[email protected]>
Co-authored-by: vibhansa-msft <[email protected]>
| dl.Config.blockSize = cfg.blockSize | ||
| dl.Config.maxConcurrency = cfg.maxConcurrency | ||
| dl.Config.defaultTier = cfg.defaultTier | ||
| dl.Config.ignoreAccessModifiers = cfg.ignoreAccessModifiers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ignoreAccessModifiers is also doing someting similar, lets consolidate both the flags with "preserve-permissions" name. By default flag will be set to 'default'. In block_blob.go it shall ignore both chmod and chown operations while in case of datalake chown shall be ignored. If flag is set explicitly to false then datalake shall ignore chmod as well, rest remains same as default. If flag is set explicitly to true then we shall return back ENOSUP from block_blob and chown in datalake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vibhansa-msft , there is already a flag preserve-acl that was introduced recently which might help here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve-acl flag is to only preserve ACLs while overwriting a file. Also, ACLs and permissions are different things so tommorow we may need a differentiation between these two for when to preserve what
When mounting an HNS account using
accountTypeset to ADLS, the system preserves permissions for files by default. When creating a file, the kernel implicitly calls a chmod operation which results in a REST call to the SetAccessControl API indatalake.go. Users whose authentication doesn't have permissions to change the mode of a blob experience failures on the backend.This PR adds a new CLI flag
--disable-setaccesscontrolthat allows users to disable the SetAccessControl functionality for chmod operations while maintaining all other HNS account functionality.Changes
--disable-setaccesscontrol(boolean, defaults tofalse)DisableSetAccessControlto config options and internal configChangeModfunction: Added early return when flag is enabled, skipping SetAccessControl API callsBehavior
--disable-setaccesscontrol=false): Normal behavior, SetAccessControl is called during chmod operations--disable-setaccesscontrol=true): Chmod operations succeed without calling SetAccessControl APIUsage
This maintains full backward compatibility while providing a solution for users with limited Azure permissions to successfully mount and use HNS accounts.
Fixes #1935.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.