Auth Manager API part 3: OAuth2 Manager

[adutra]

3rd PR for the Auth Manager API. Previous ones:

• Auth Manager API part 2: AuthManager #11809
• Auth Manager API part 1: HTTPRequest, HTTPHeader #11769

This PR introduces the OAuth2Manager along with session caching and credentials refreshing. It is still "unplugged" though. Most of the OAuth2Manager code reflects one-to-one what's currently hard-coded in RESTSessionCatalog (and it's still there).

\cc @nastra @danielcweeks

[johnnysohn]

can we also define something like AUTH_IMPL = "rest.auth.type"; that could be used here for any custom auth manager/provider we want to bring in?

[adutra]

That's precisely the intent here; if you have a custom manager, you would activate it with rest.auth.type=my.custom.AuthManager. Is that OK for you?

[johnnysohn]

Yes that works! I wasn't sure whether you were planning on having authType for as a name of the type

[adutra]

There will be aliases for built-in managers, e.g. you can activate the built-in oauth2 manager with either:

rest.auth.type = oauth2
rest.auth.type = org.apache.iceberg.rest.auth.OAuth2Manager

But for custom ones, you must provide the FQDN of your implementation.

[danielcweeks]

Is there a reason we would need to expose this?

[adutra]

No, not really, I can turn this constructor into package-private. It's meant only for tests.

[nastra]

+1

[nastra]

LGTM

[danielcweeks]

It looks like we should also make this package private. The other issue is that we shouldn't be using the common pool by default. This should probably be provided by the OAuth2Manager or if we expect there to be only one per manager, then just create a named pool for handling refreshes. We definitely don't want to rely on or possibly tie up the common pool.

[adutra]

@danielcweeks we are already using the common pool. The session cache currently is created as follows:

iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java

Lines 1144 to 1154 in a2b8008

	return Caffeine.newBuilder()
	.expireAfterAccess(Duration.ofMillis(expirationIntervalMs))
	.removalListener(
	(RemovalListener<String, AuthSession>)
	(id, auth, cause) -> {
	if (auth != null) {
	auth.stopRefreshing();
	}
	})
	.build();
	}

As you can see, we are not providing an executor explicitly, so evictions are being done on the common pool.

[adutra]

Re: turning this constructor package-private: this won't fly, as SigV4 will also need to create caches, so this constructor needs to be public.

[danielcweeks]

@adutra Just one last comment on the thread pool, but other than that this look good to go.