Feature: Expose resetCredentials via a new reset api to allow root user to reset credentials for an existing principal with custom values

[fivetran-arunsuri]

Background:
See Issue -#1929 and based on Dev email discussion
WHAT

1. Exposes the resetCredentials operation via the api
2. Only Root user can reset the credentials of the existing principal
3. Requires the custom clientId and custom secret via root user to reset the random credentials already created via CreatePrincipal API

Local Testing:
Happy Scenario

Failure scenario:

If clientId and clientSecret not passed, we retain the previous clientId and rotate the creds:

[dimas-b]

Thanks for your contribution, @fivetran-arunsuri !

As far as I understand, principal secrets rotation is implemented only for JDBC persistence... I think EclipseLink (being deprecated) is ok to exclude from this feature, but the tree map (in-memory) persistence should probably have an implementation too (to support full API with in-memory servers).

Could you also add integration tests exercising both the happy path and authorization errors?

[dimas-b]

@fivetran-arunsuri : please check CI errors.

[fivetran-arunsuri]

Thanks for your contribution, @fivetran-arunsuri !

As far as I understand, principal secrets rotation is implemented only for JDBC persistence... I think EclipseLink (being deprecated) is ok to exclude from this feature, but the tree map (in-memory) persistence should probably have an implementation too (to support full API with in-memory servers).

Could you also add integration tests exercising both the happy path and authorization errors?

@dimas-b I’ve added the in-memory TreeMap implementation as well as the corresponding integration tests. However, I’m currently having trouble running the integration tests locally. I couldn’t find much detail around this in the README either.

Could you please guide me on how to set them up or point me to any relevant documentation?

The error I am seeing is as follows:
`org.junit.jupiter.api.extension.ParameterResolutionException: Failed to resolve parameter [org.apache.polaris.service.it.env.PolarisApiEndpoints arg0] in method [public static void org.apache.polaris.service.it.test.PolarisManagementServiceIntegrationTest.setup(org.apache.polaris.service.it.env.PolarisApiEndpoints,org.apache.polaris.service.it.env.ClientCredentials)]

at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Suppressed: java.lang.NullPointerException: Cannot invoke "org.apache.polaris.service.it.env.PolarisClient.close()" because "org.apache.polaris.service.it.test.PolarisManagementServiceIntegrationTest.client" is null
	at org.apache.polaris.service.it.test.PolarisManagementServiceIntegrationTest.close(PolarisManagementServiceIntegrationTest.java:127)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1116)
	... 1 more

Caused by: java.lang.ExceptionInInitializerError
at org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension.lambda$env$0(PolarisIntegrationTestExtension.java:72)
at org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension.env(PolarisIntegrationTestExtension.java:71)
at org.apache.polaris.service.it.ext.PolarisIntegrationTestExtension.resolveParameter(PolarisIntegrationTestExtension.java:56)
... 1 more
Caused by: java.lang.IllegalStateException: PolarisServerManager not found
at org.apache.polaris.service.it.ext.PolarisServerManagerLoader.lambda$static$0(PolarisServerManagerLoader.java:28)
at java.base/java.util.Optional.orElseThrow(Optional.java:403)
at org.apache.polaris.service.it.ext.PolarisServerManagerLoader.(PolarisServerManagerLoader.java:28)
... 4 more`

[fivetran-arunsuri]

• Helm testsExpected — Waiting for status to be reported

@dimas-b Also , I don't have access to run the ci checks by myself

[fivetran-arunsuri]

@dimas-b 've addressed most of the comments on the PR and would appreciate your suggestions on a few remaining points. Looking forward to your review and hoping to get this merged soon. Thanks!

[fivetran-arunsuri]

@dimas-b 've addressed most of the comments on the PR and would appreciate your suggestions on a few remaining points. Looking forward to your review and hoping to get this merged soon. Thanks!

@dimas-b Following up on this

[dimas-b]

Caused by: java.lang.IllegalStateException: PolarisServerManager not found

This looks like a class path issue to me. I'd guess you attempted to run the test(s) via IDE's own unit test runner. I'd suggest to use the gradle-based test runner. The overhead is really small, but the classpath is certain to be set according to module dependencies.

Side note: CI has some failures too.

[fivetran-arunsuri]

I have fixed the CI failures

[fivetran-arunsuri]

@dimas-b Sorry for the delay, I was tied up with some high-priority incidents. Could you please take a look? I’m planning to merge this week

[fivetran-arunsuri]

@dimas-b Please let me know if more changes are needed in this flow?

[fivetran-arunsuri]

Bumping up for a re review

[dimas-b]

Sorry about the delay. I have some more comments, though.

[fivetran-arunsuri]

@dimas-b I’ve addressed most comments. Could you clarify the remaining points so we can ensure everything is ready for merge

[dimas-b]

@fivetran-arunsuri : please resolve merge conflicts, otherwise CI will not run 🤷

[fivetran-arunsuri]

Fixed merge conflicts

[fivetran-arunsuri]

@dimas-b Please take a look at the current approach

[dimas-b]

@dennishuo: FYI PolarisSecretsManager interface changes

[fivetran-arunsuri]

@dimas-b @eric-maynard I rebased my branch, which removed your approval. Could you please take another look? If everything looks good, let’s move ahead with the merge — this feature is currently blocking our migration.

[flyrain]

Hi @fivetran-arunsuri , thanks a lot for working on it. Sorry for the late review. Appreciated if you can answer a few questions:

1. What's the use case of reset a client id of a principal? I understand that other systems like Keycloak support that. But Polaris doesn't have to mimic them.
2. Can we reuse the credential rotate endpoint(/principals/{principalName}/rotate) instead of having a new endpoint? We can still enforce only root can rotate other principal's credentials. It's also acceptable to add an optional field to allow client to input credentials.

[dimas-b]

@flyrain : FYI: https://lists.apache.org/thread/wc710p1xd25l9k9t9jplnnn8njf15lq0

[dimas-b]

Can we reuse the credential rotate endpoint(/principals/{principalName}/rotate) instead of having a new endpoint?

My reading of the current Polaris Management API spec is that the "rotate" endpoint has quite different functionality from "reset". Implementing the feature proposed in this PR via the old "reset" endpoint could technically be done (I guess), but I think it will overload the old "reset" functionality too much and make its logic complicated unnecessarily. I believe adding a new endpoint is preferable (as implemented in this PR).

[flyrain]

• Enable a safe, service-admin only way to inject known credentials
  via the API during the transition phase with validations of course

I didn't find a use case mentioned in the mail thread to reset the client id. Correct me if I'm wrong, @fivetran-arunsuri.

There is no old reset endpoint. Only rotate endpoint and principal creation endpoint return back the credentials.

My reading of the current Polaris Management API spec is that the "rotate" endpoint has quite different functionality from "reset".

How is that quite different? For credential refresh, the rotate endpoint can just accept an optional input to allow manual credential refresh.

[dimas-b]

There is no old reset endpoint.

Sorry, typo on my part. I meant "old rotate".

How is that quite different? For credential refresh, the rotate endpoint can just accept an optional input to allow manual credential refresh.

The "rotate" endpoint keeps old credentials effective (they become secondary).

The "rotate" endpoint implies that the client ID never changes.

If we're to allow completely replacing both the client ID and all associated secrets, that's no longer "rotation", IMHO.

[flyrain]

The new endpoint makes sense to me if we need update client id. Are there use cases for client id update now?

[fivetran-arunsuri]

Hi @flyrain, Based on the usecase explained in the email thread and as @dimas-b explained earlier.

Hi @fivetran-arunsuri , thanks a lot for working on it. Sorry for the late review. Appreciated if you can answer a few questions:

1. What's the use case of reset a client id of a principal? I understand that other systems like Keycloak support that. But Polaris doesn't have to mimic them.

This isn’t about mimicking Keycloak or expanding Polaris into a full IdP. The primary driver is our migration path from Polaris 0.9 → 1.0. Since we’ll be running both catalog servers in parallel during the cutover, we need to reuse the same clientId / clientSecret across both instances. This ensures:

• Clients can authenticate seamlessly without coordinating a mass credential rotation.
• We can safely switch traffic between catalogs and roll back instantly if needed.
• Zero downtime cutover is possible.

Without the ability to inject known credentials, we’d either have to rotate secrets across all clients (not feasible at scale) or risk schema/hash inconsistencies when trying to copy raw tables

Also this automatically answers your second question, As Rotate API keeps the clientId as same

[fivetran-arunsuri]

This change is mainly to help users migrate from 0.9 (EclipseLink) → 1.0 (JDBC) with schema changes, without risky manual DB manipulation by registering existing sets of users keeping creds as same. I believe it could be quite useful for adoption. Currently the functionality is only restricted to root user

[fivetran-arunsuri]

@flyrain Pasting what we discussed in DM around the reset vs rotate:

I see your point about client_id semantics and the flexibility of reusing the rotate endpoint.
That said, I’d still lean towards keeping a separate reset API, mainly for clarity of intent from a client perspective:

• rotate suggests replacing credentials while keeping the same client identity, whereas reset communicates a stronger action (e.g., provisioning a completely fresh set).

• users often expect reset when dealing with credentials (similar to password reset vs password rotate), so the explicit endpoint helps reduce confusion.

• Future flexibility: if Polaris later wants to evolve different behaviors (e.g., stricter validation or revoking old client IDs), having both endpoints gives us that room without introducing breaking changes.

• On top of that, the current implementation is limited to the root principal. rotate doesn’t work with the root principal (only with its own creds), so reset is the right fit here. It also provides a way to reset forgotten credentials for a particular principal without touching the existing implementation.

Finally, this direction was already discussed and agreed in the Dev email thread to support a separate endpoint, so keeping both aligns with that decision—rather than using the same API as createPrincipal by passing custom creds, as I initially proposed.

Would you be open to keeping both endpoints? Can we go ahead with the PR?

[flyrain]

Thanks @fivetran-arunsuri! I'm fine with new reset endpoint for the clarity of intent from a client perspective.