Provide a way to configure own SerializerAdapter instance while building XxxBlob clients #24374
Conversation
reta
requested review from
alzimmermsft,
amishra-dev,
jaschrep-msft,
kasobol-msft and
rickle-msft
as code owners
ghost
added
Storage
|
Thank you for your contribution reta! We will review the pull request and get back to you soon. |
…ing XxxBlob clients Signed-off-by: Andriy Redko <[email protected]>
|
Thanks for submitting the issues and filing this PR @reta! Just have a few questions around specifics you were seeing which resulted in
|
|
Thanks a lot for looking, @alzimmermsft ! It would be my pleasure to share more details.
This is correct, the kind of exceptions we are seeing: (I cut stacktraces to the relevant parts, please let me know if you need full ones, I think the picture is more or less clear).
That would be my expectations as well, it seems like certain kind of model classes do trigger accessibility "correction" path in
Yes, the thread pools are aware of the
A number of the APIs to be fair, upload probably pops up more often than others (primarily chunked, commit / stage / ...), also list by prefix / hierarchy. Thank you for asking these questions. Although we run into some specific limitations, I think the ability to supply own serialization / deserialization laeyr or / and instrumentation comes up all the time, and Azure SDK has full support for that under the hood. Thank you! |
|
@alzimmermsft went down the |
|
@alzimmermsft @alzimmermsft apologies, any concerns left unaddressed? Thank you! |
I'd actually want to take a crack at this by changing the default serialization configurations in JacksonAdapter to play better with SecurityManger before exposing additional APIs. There's a lot of configuration in JacksonAdapter that is explicitly done to work with the model types used in the SDKs, which makes it brittle and could break runtime behavior easily. Would you be willing to share your SecurityManager configuration being used that results in these exceptions being thrown. Or, at least, a simple configurations that results in a replication of the exceptions. |
|
Thanks @alzimmermsft
Valid concerns, it has at least 3 different serializers, only one is somewhat configurable, not others.
Will try to provide minimal example shortly |
|
@alzimmermsft here we are So what this example does:
To run the example: The output (below) demonstrates the regular (unpriviledged) and priviledged calls. The issue we are dealing with is that This is basically it. If you have any further questions, please let me know. Alternative solution for this particular issues would be the ability to configure |
|
Thank you so much for the reproduction @reta. I've been testing out what would happen if the mentioned configuration is disabled and it appears to trigger a new issue where Jackson isn't able to access a private field on a class. Also, appears there is a second configuration which allows for I'll continue looking at other ways this issue can be remediated. |
|
@reta, this is still being looked into, and unfortunately this has opened a bigger "can of worms" than initially expected. As mentioned previously, preventing Jackson from Looking forward, we're looking into ways to begin remedying this through a few different avenues:
Unfortunately, both of these resolutions are much, much more comprehensive and will likely take a few months to implement and verify that there are no regressions. Given that, I would love to explore other potential avenues we could take in the meantime to unblock the issue you're running into. In the replication for this issue you gave you had a code path which leveraged |
|
Thanks a lot for the detailed investigation @alzimmermsft , I certainly understand the complexity involved.
That is right, the cleanest way to do that is to use the But the showstopper is this case is the inability to provide own Thank you @alzimmermsft ! |
|
Apologies on the slower reply @reta. I've filed this draft PR which begins looking into offering an way for an application to pass what I call an "access helper". The access helpers purpose is a method that can help |
|
Thanks a lot, @alzimmermsft , really appreciate you guys looking deeply into the issue. The draft suggests another viable option, no objections from my side, please feel free to close this PR in favor of access helper. Thank you! |
|
Thank you so much @alzimmermsft ! I am closing this one :) |
Use conditional compile (Azure#24374) * Add conditional compile of client.tsp * Always execute from main.tsp, always check client.tsp * Fix script * Remove yaml reference * Update specification/cognitiveservices/HealthInsights/healthinsights.oncophenotype/tspconfig.yaml --------- Co-authored-by: Mike Harder <[email protected]>
At the moment,
XxxBlobclient builders (BlobClientBuilder,BlobContainerClientBuilder,BlobServiceClientBuilder, ...) do not provide a way to supply ownSerializerAdapterinstance and rely onJacksonAdapter::createDefaultSerializerAdapter. It works in most cases but sometimes, fe. when application runs underSecurityManagerand needs to use reflection, it is not sufficient and using customSerializerAdapterwould be very helpful. The underlyingAzureBlobStorageImplalready supports that however the ability to pass theSerializerAdapterinstance down the chain of builders is not supported.The PR provides a way for the
XxxBlobclient builders to allow using user-provided SerializerAdapter instance. The change is low risk (arguably) and does not break an existing APIs.Fixes #24373.
Thank you!