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

[FEATURE REQ] Make the process of using a CustomerProvidedKey with BlobClient more DI-friendly #20233

Closed
bkromhout opened this issue Apr 8, 2021 · 11 comments · Fixed by #20399
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)

Comments

@bkromhout
Copy link

Library or service name.
Azure.Storage.Blobs

Is your feature request related to a problem? Please describe.
Currently, the process of setting up a BlobClient instance to use a CustomerProvidedKey in projects that leverage DI (e.g., and ASP.NET Core project) is very annoying.

You can inject BlobServiceClient via DI, and obviously you can create BlobContainerClients and BlobClients using the injected BlobServiceClient instance. That's great for normal usage...but as soon as I need to create a BlobClient that uses a CustomerProvidedKey, it becomes frustrating, because:

  • CustomerProvidedKey must be passed via BlobClientOptions
  • The only way to pass BlobClientOptions to BlobClient is via its constructors, there's no other way via the "normal" BlobServiceClient -> BlobContainerClient -> BlobClient route.
  • To make things even more annoying, there's nothing I can "get" from the injected BlobServiceClient and pass to the BlobClient constructor to let me "inherit" the identity, URI, etc in that fashion.

So what you end up with is a situation where your DI-injected BlobServiceClient becomes useless, and you have to inject all of the information needed to set up a brand new BlobClient instance using IOptions<Foo> (or something similar).

For my specific use-case, we generate new encryption keys for each new blob we upload, so I've basically ended up creating a BlobClient factory and inject that via DI, but it would really be preferable if there were some way to just create the BlobClient via BlobServiceClient I'm already injecting for other reasons.

If there were an extra overload of BlobContainerClient.GetBlobClient() that took a second parameter of type BlobClientOptions (or perhaps simply CustomerProvidedKey), that would be excellent.

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Apr 8, 2021
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files) labels Apr 8, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Apr 8, 2021
@ghost
Copy link

ghost commented Apr 8, 2021

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

Issue Details

Library or service name.
Azure.Storage.Blobs

Is your feature request related to a problem? Please describe.
Currently, the process of setting up a BlobClient instance to use a CustomerProvidedKey in projects that leverage DI (e.g., and ASP.NET Core project) is very annoying.

You can inject BlobServiceClient via DI, and obviously you can create BlobContainerClients and BlobClients using the injected BlobServiceClient instance. That's great for normal usage...but as soon as I need to create a BlobClient that uses a CustomerProvidedKey, it becomes frustrating, because:

  • CustomerProvidedKey must be passed via BlobClientOptions
  • The only way to pass BlobClientOptions to BlobClient is via its constructors, there's no other way via the "normal" BlobServiceClient -> BlobContainerClient -> BlobClient route.
  • To make things even more annoying, there's nothing I can "get" from the injected BlobServiceClient and pass to the BlobClient constructor to let me "inherit" the identity, URI, etc in that fashion.

So what you end up with is a situation where your DI-injected BlobServiceClient becomes useless, and you have to inject all of the information needed to set up a brand new BlobClient instance using IOptions<Foo> (or something similar).

For my specific use-case, we generate new encryption keys for each new blob we upload, so I've basically ended up creating a BlobClient factory and inject that via DI, but it would really be preferable if there were some way to just create the BlobClient via BlobServiceClient I'm already injecting for other reasons.

If there were an extra overload of BlobContainerClient.GetBlobClient() that took a second parameter of type BlobClientOptions (or perhaps simply CustomerProvidedKey), that would be excellent.

Author: bkromhout
Assignees: -
Labels:

Client, Service Attention, Storage, customer-reported, needs-team-attention, needs-triage, question

Milestone: -

@amnguye
Copy link
Member

amnguye commented Apr 12, 2021

Hi,

Just to be clear BlobServiceClient.GetBlobClient(string containerName, string blobName) (and of course ensure that the Encryption info gets passed accordingly)?
I wonder if providing an API like BlobContainerClient.GetBlobClient(string blobName, BlobClientOptions? options = null) would be a good idea. I think there needs to be some discussion around if we should do this, and how to implement it. Do we take the entire options bag from the passed parameters and ignore any options set from the service client? Or do we assume that if they leave some values null in the option bags, but it's set in the parent client, then we still inherit from the parent class? @kasobol-msft @seanmcc-msft ?

@bkromhout
Copy link
Author

No, not on BlobServiceClient; your second suggestion of adding something to BlobContainerClient is really what I'm after.

As for how to handle the BlobClientOptions: I think the only reasonable approach is the latter one, where you default to taking the parent client's values, except for those explicitly passed (i.e., non-null) in the BlobClientOptions. (That's the whole point of adding a method like this after all; if you wanted to ignore the parent client's values, you'd just use a BlobClient constructor instead.)

@seanmcc-msft
Copy link
Member

@tg-msft, @KrzysztofCwalina, what do you think of this idea?

For Blobs, this would entail the following new overloads:

  • BlobServiceClient.GetContainerClient(string containerName, BlobClientOptions options)
  • BlobContainerClient.GetBlobClient(string blobName, BlobClientOptions options)
  • BlobContainerClient.GetBlockBlobClient(string blobName, BlobClientOptions options)
  • BlobContainerClient.GetAppendBlobClient(string blobName, BlobClientOptions options)
  • BlobContainerClient.GetPageBlobClient(string blobName, BlobClientOptions options)

We would also make this change in Data Lake, SMB Files, and Queues.

Any client options that aren't null would override the options on the parent client in the newly created client.

@tg-msft
Copy link
Member

tg-msft commented Apr 14, 2021

No, we can't take new options because that would require us to either recreate the pipeline or ignore most of the values. We should follow the WithSnapshot pattern for individual properties. In this case, I'd suggest a WithCustomerProvidedKey method that can be used to add or clear that key.

@seanmcc-msft
Copy link
Member

@bkromhout, would this solution above work for you?

@bkromhout
Copy link
Author

Sounds great to me!

@seanmcc-msft
Copy link
Member

Ok, I will work on this this week.

@seanmcc-msft
Copy link
Member

@tg-msft, just want to double check, we are cool with:

  • BlobServiceClient.WithCustomerProvidedKey(CustomerProvidedKey customerProvidedKey)
  • BlobContainerClient.WithCustomerProvidedKey(CustomerProvidedKey customerProvidedKey)
  • BlobBaseClient.WithCustomerProvidedKey(CustomerProvidedKey customerProvidedKey)

We could also consider adding WithEncryptionScope(), since the idea is essentially the same.

@seanmcc-msft
Copy link
Member

@bkromhout, I added WithCustomerProvidedKey() and WithEncryptionScope() to BaseBlobClient, BlockBlobClient, AppendBlobClient, and PageBlobClient. These methods will be present in our next beta release, which is scheduled for mid May.

@bkromhout
Copy link
Author

Awesome, thanks for the quick work! When do you expect it to be out in stable, roughly?

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
5 participants