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.

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

fix: Azure fragment store with multiple tenants #354

Merged
merged 3 commits into from
Dec 8, 2023

Conversation

jshearer
Copy link
Contributor

@jshearer jshearer commented Dec 7, 2023

I knew that the service credentials would be scoped to tenant IDs, but I did not realize that the service.Clients and UserDelegationCredentials would also be tenant-scoped. This was causing problems where the credentials of whatever tenant was first seen would be used to sign all SignGet URLs, resulting in signature mismatch errors when the blob was owned by a different tenant.

This updates the cache logic to keep track of both the service clients and the user delegation credentials by tenant ID, which should solve the problem.


This change is Reviewable

I knew that the service credentials would be scoped to tenant IDs, but I did not realize that the `service.Client`s and `UserDelegationCredential`s would also be tenant-scoped. This was causing problems where the credentials of whatever tenant was first seen would be used to sign _all_ `SignGet` URLs, resulting in signature mismatch errors when the blob was owned by a different tenant.

This updates the cache logic to keep track of both the service clients and the user delegation credentials by tenant ID, which should solve the problem.
broker/fragment/store_azure.go Outdated Show resolved Hide resolved
broker/fragment/store_azure.go Outdated Show resolved Hide resolved
broker/fragment/store_azure.go Outdated Show resolved Hide resolved
broker/fragment/store_azure.go Show resolved Hide resolved
@jshearer jshearer force-pushed the jshearer/fix_azure_storage_multiple_tenants branch from f55be63 to b897637 Compare December 7, 2023 22:27
broker/fragment/store_azure.go Outdated Show resolved Hide resolved
broker/fragment/store_azure.go Outdated Show resolved Hide resolved
broker/fragment/store_azure.go Outdated Show resolved Hide resolved
@jshearer jshearer force-pushed the jshearer/fix_azure_storage_multiple_tenants branch from 278c802 to 800c8cd Compare December 8, 2023 17:10
Copy link
Contributor

@jgraettinger jgraettinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚢

@jshearer jshearer merged commit 79690d5 into master Dec 8, 2023
1 check passed
@jgraettinger jgraettinger deleted the jshearer/fix_azure_storage_multiple_tenants branch December 8, 2023 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants