collado-mike commented on code in PR #511:
URL: https://github.com/apache/polaris/pull/511#discussion_r1879198859
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisSecretsManager.java:
##########
@@ -39,6 +41,17 @@ public interface PolarisSecretsManager {
PrincipalSecretsResult loadPrincipalSecrets(
@Nonnull PolarisCallContext callCtx, @Nonnull String clientId);
+ @Nonnull
+ SecretValidationResult validateSecret(
+ @Nonnull PolarisCallContext callCtx, @Nonnull String clientId, @Nonnull
String clientSecret);
+
+ @Nonnull
+ EntityResult loadPrincipal(
+ @Nonnull PolarisCallContext callCtx,
+ @Nullable String roleName,
+ @Nullable String clientId,
+ @Nullable Long principalId);
Review Comment:
Vault, Secrets Manager, and K8s all allow storing secret values as
structured types, like JSON. I think it's pretty easy to imagine a layout like
using the `clientId` as the secret key and storing the secret value as
something like `{"secret": "abcd", "principalId": 1}`. This allows full
decoupling of the PolarisSecretsManager from the PolarisMetaStoreManager,
whereas returning the `PrincipalEntity` from the `PolarisSecretsManager` means
it has to maintain a reference to the MetaStore and the MetaStore has to deal
with lookup keys beside name and id. No other entity has a lookup key beside
name or id, so I think keeping it consistent for Principal makes sense.
--
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]
