-
Notifications
You must be signed in to change notification settings - Fork 218
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(fs/azure): add support for Azure Blob Storage (FS Object Backend) #2538
Conversation
Uffizzi Ephemeral Environment
|
412990d
to
3029d5d
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2538 +/- ##
==========================================
+ Coverage 71.62% 71.74% +0.12%
==========================================
Files 85 87 +2
Lines 8066 8197 +131
==========================================
+ Hits 5777 5881 +104
- Misses 1940 1961 +21
- Partials 349 355 +6 ☔ View full report in Codecov by Sentry. |
2aac4ca
to
3850813
Compare
Will give this a thorough 👁️ in the morning @erka ! Apologies was out of town this weekend/today. Looks great at a glance though! much appreciated! |
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.
One minor tidy up comment, otherwise, this is super awesome. Thanks for putting this together @erka 💪 Solid.
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.
Minor JSON config changes
Thank you @erka!!
Co-authored-by: Mark Phelps <[email protected]>
I was looking at how our AWS S3 bucket config is authenticated and it seems it supports the default ENV vars supported by the AWS S3 go client:
Actually, I think it uses the aws s3 client default access credentials chain: https://docs.aws.amazon.com/sdk-for-go/v1/developer-guide/configuring-sdk.html#specifying-credentials I think we should do the same here for azure blob, and not pass access credentials via |
Azure Blob Storage is a bit more complex and provides few ways to auth. There is a default which you mentioned above but there are few others. Please read https://pkg.go.dev/[email protected]/blob/azureblob#hdr-URLs about it. I have no objectives with dropping |
👍🏻 so then we would remove the configuration options |
@markphelps I have excluded BTW maybe |
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.
amazing!! thank you @erka . one minor suggestion.
BTW maybe s3 bucket should be excluded as well as it's globally unique.
I agree, if you dont want to do this in this pr we can do it in another
Co-authored-by: Mark Phelps <[email protected]>
There is a strange error `Log in goroutine after Test_Store has completed` occurs randomly.
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.
looks great!! Thank you @erka ! I will update docs and this will go out in next release 🎉
This is a good description of the auth for Azure Blob Storage https://github.com/moby/buildkit?tab=readme-ov-file#azure-blob-storage-cache-experimental
fixes #2289