fivetran-arunsuri commented on code in PR #2197:
URL: https://github.com/apache/polaris/pull/2197#discussion_r2252738969
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/JdbcBasePersistenceImpl.java:
##########
@@ -774,6 +774,69 @@ public PolarisPrincipalSecrets generateNewPrincipalSecrets(
return principalSecrets;
}
+ @Nullable
+ @Override
+ public PolarisPrincipalSecrets resetPrincipalSecrets(
+ @Nonnull PolarisCallContext callCtx,
+ @Nonnull String clientId,
+ long principalId,
+ boolean reset,
+ @Nonnull String oldSecretHash,
+ String customClientId,
+ String customClientSecret) {
+ PolarisPrincipalSecrets principalSecrets = loadPrincipalSecrets(callCtx,
clientId);
+
+ // should be found
+ callCtx
+ .getDiagServices()
+ .checkNotNull(
+ principalSecrets,
+ "cannot_find_secrets",
+ "client_id={} principalId={}",
+ clientId,
+ principalId);
+
+ // ensure principal id is matching
+ callCtx
+ .getDiagServices()
+ .check(
+ principalId == principalSecrets.getPrincipalId(),
+ "principal_id_mismatch",
+ "expectedId={} id={}",
+ principalId,
+ principalSecrets.getPrincipalId());
+
+ principalSecrets.setPrincipalClientId(customClientId);
+ principalSecrets.resetSecrets(customClientSecret);
Review Comment:
Thanks for the suggestion — keeping principalSecrets immutable does make
sense in general.
However, we already have a rotateCredentials operation that modifies the
existing secret in place, so the pattern of mutating the current instance is
already established in this case.
Let me know if my thinking about the same is wrong?
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
