tokoko commented on code in PR #3224:
URL: https://github.com/apache/polaris/pull/3224#discussion_r2595245447
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/io/StorageAccessConfigProvider.java:
##########
@@ -49,13 +50,16 @@ public class StorageAccessConfigProvider {
private final StorageCredentialCache storageCredentialCache;
private final StorageCredentialsVendor storageCredentialsVendor;
+ private final PolarisPrincipal polarisPrincipal;
@Inject
public StorageAccessConfigProvider(
StorageCredentialCache storageCredentialCache,
- StorageCredentialsVendor storageCredentialsVendor) {
+ StorageCredentialsVendor storageCredentialsVendor,
+ PolarisPrincipal polarisPrincipal) {
Review Comment:
After quite a bit of digging, looks like the core issue here is that a lot
of tests are set up and executed in a way that http request context is not
present during test execution. It's also not trivial to inject it in some way
because we're using quarkus augmentors for auth and seems like SecurityContext
injection doesn't really get along with that. In the end, I ended up switching
PolarisPrincipal bean to acquire context from CurrentIdentityAssociation
instead of SecurityContext. Added a testing Augmentor which injects root
principal in CurrentIdentityAssociation. I still have a couple of integration
test to pin down though (some use multiple principals in the span of a single
test which is hard to replicate by injection).
@adutra I saw most of auth stuff was your doing.. can you take a look at
this as well? If it seems like a worthwhile change, I will modify remaining
failing tests as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
