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

[C++][FS][Azure] Test CopyFile() with non account key credential #41315

Open
kou opened this issue Apr 21, 2024 · 0 comments
Open

[C++][FS][Azure] Test CopyFile() with non account key credential #41315

kou opened this issue Apr 21, 2024 · 0 comments
Labels
Component: C++ Type: enhancement

Comments

@kou
Copy link
Member

kou commented Apr 21, 2024

Describe the enhancement requested

We may be able to use Azure CLI authentication (GH-39344) for this.

See also: #41276 (comment)

Component(s)

C++
@kou kou mentioned this issue Apr 21, 2024
Merged
This was referenced Dec 12, 2024
Closed
Merged
kou pushed a commit that referenced this issue Dec 17, 2024
@Tom-Newton
GH-44308: [C++][FS][Azure] Implement SAS token authentication (#45021)
ba2b9e5 
### Rationale for this change
SAS token auth is sometimes useful and it the last one we haven't implemented. 

### What changes are included in this PR?
- Implement `ConfigureSasCredential`
- Update `AzureOptions::FromUri` so that simply appending a SAS token to a blob storage URI works. e.g. `AzureOptions::FromUri("abfs://file_system@ account.dfs.core.windows.net/?se=2024-12-12T18:57:47Z&sig=pAs7qEBdI6sjUhqX1nrhNAKsTY%2B1SqLxPK%2BbAxLiopw%3D&sp=racwdxylti&spr=https,http&sr=c&sv=2024-08-04")`
  - SAS tokens are made up of a bunch of URI query parameters that I'm not sure we can exhaustively list.
  - Therefore we now assume that any unrecognised URI query parameters are assumed to be part of a SAS token, instead of returning an error status. 
- Update `CopyFile` to use StartCopyFromUri instead of CopyFromUri
  - This avoids the need to generate SAS tokens. 
  - Supports blobs bigger than 256MiB
  - This makes #41315 redundant
 
### Are these changes tested?
Yes
- Added new tests for authenticating with SAS and doing some operations including `CopyFile`
- Added new tests for `AzureOptions::FromUri` with a SAS token. 

I also made sure to run the tests which connect to real blob storage. 

### Are there any user-facing changes?
- SAS token in now supported
- Unrecognised URI query parameters are ignored by `AzureOptions::FromUri` instead of failing fast. IMO this is a regression but still the best option to support SAS token. 

* GitHub Issue: #44308

Authored-by: Thomas Newton <thomas.w.newton@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: C++ Type: enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@kou