Skip to content

Remove unused PolarisCredentialVendor.validateAccessToLocations() #1480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 29, 2025
Merged

Remove unused PolarisCredentialVendor.validateAccessToLocations() #1480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29709ab
Remove unused PolarisCredentialVendor.validateAccessToLocations()
dimas-b Apr 28, 2025
2ccc9e8
review: remove ValidateAccessResult and comments
dimas-b Apr 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 0 additions & 55 deletions ...re/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,10 @@
import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult;
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult;
import org.apache.polaris.core.policy.PolarisPolicyMappingRecord;
import org.apache.polaris.core.policy.PolicyEntity;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.storage.PolarisCredentialProperty;
import org.apache.polaris.core.storage.PolarisStorageActions;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageIntegration;
import org.slf4j.Logger;
Expand Down Expand Up @@ -1587,59 +1585,6 @@ private void revokeGrantRecord(
}
}

/** {@inheritDoc} */
@Override
public @Nonnull ValidateAccessResult validateAccessToLocations(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long entityId,
PolarisEntityType entityType,
@Nonnull Set<PolarisStorageActions> actions,
@Nonnull Set<String> locations) {
// get meta store we should be using
BasePersistence ms = callCtx.getMetaStore();
callCtx
.getDiagServices()
.check(
!actions.isEmpty() && !locations.isEmpty(),
"locations_and_operations_privileges_are_required");
// reload the entity, error out if not found
EntityResult reloadedEntity = loadEntity(callCtx, catalogId, entityId, entityType);
if (reloadedEntity.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) {
return new ValidateAccessResult(
reloadedEntity.getReturnStatus(), reloadedEntity.getExtraInformation());
}

// get storage integration, expect not null
PolarisStorageIntegration<PolarisStorageConfigurationInfo> storageIntegration =
((IntegrationPersistence) ms)
.loadPolarisStorageIntegration(callCtx, reloadedEntity.getEntity());
callCtx
.getDiagServices()
.checkNotNull(
storageIntegration,
"storage_integration_not_exists",
"catalogId={}, entityId={}",
catalogId,
entityId);

// validate access
PolarisStorageConfigurationInfo storageConfigurationInfo =
BaseMetaStoreManager.extractStorageConfiguration(callCtx, reloadedEntity.getEntity());
Map<String, String> validateLocationAccess =
storageIntegration
.validateAccessToLocations(storageConfigurationInfo, actions, locations)
.entrySet()
.stream()
.collect(
Collectors.toMap(
Map.Entry::getKey,
e -> PolarisObjectMapperUtil.serialize(callCtx, e.getValue())));

// done, return result
return new ValidateAccessResult(validateLocationAccess);
}

/**
* Get the internal property map for an entity
*
Expand Down
16 changes: 0 additions & 16 deletions ...c/main/java/org/apache/polaris/core/persistence/TransactionWorkspaceMetaStoreManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@
import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult;
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult;
import org.apache.polaris.core.policy.PolicyEntity;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.storage.PolarisStorageActions;

/**
* Wraps an existing impl of PolarisMetaStoreManager and delegates expected "read" operations
Expand Down Expand Up @@ -346,20 +344,6 @@ public ScopedCredentialsResult getSubscopedCredsForEntity(
allowedWriteLocations);
}

@Override
public ValidateAccessResult validateAccessToLocations(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long entityId,
PolarisEntityType entityType,
@Nonnull Set<PolarisStorageActions> actions,
@Nonnull Set<String> locations) {
callCtx
.getDiagServices()
.fail("illegal_method_in_transaction_workspace", "validateAccessToLocations");
return null;
}

@Override
public ResolvedEntityResult loadResolvedEntityById(
@Nonnull PolarisCallContext callCtx,
Expand Down
67 changes: 0 additions & 67 deletions ...re/src/main/java/org/apache/polaris/core/persistence/dao/entity/ValidateAccessResult.java
View file
Open in desktop

This file was deleted.

54 changes: 0 additions & 54 deletions .../org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,10 @@
import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult;
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult;
import org.apache.polaris.core.policy.PolarisPolicyMappingRecord;
import org.apache.polaris.core.policy.PolicyEntity;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.storage.PolarisCredentialProperty;
import org.apache.polaris.core.storage.PolarisStorageActions;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageIntegration;
import org.slf4j.Logger;
Expand Down Expand Up @@ -2016,58 +2014,6 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(
}
}

/** {@inheritDoc} */
@Override
public @Nonnull ValidateAccessResult validateAccessToLocations(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long entityId,
PolarisEntityType entityType,
@Nonnull Set<PolarisStorageActions> actions,
@Nonnull Set<String> locations) {
// get meta store we should be using
TransactionalPersistence ms = ((TransactionalPersistence) callCtx.getMetaStore());
callCtx
.getDiagServices()
.check(
!actions.isEmpty() && !locations.isEmpty(),
"locations_and_operations_privileges_are_required");
// reload the entity, error out if not found
EntityResult reloadedEntity = loadEntity(callCtx, catalogId, entityId, entityType);
if (reloadedEntity.getReturnStatus() != BaseResult.ReturnStatus.SUCCESS) {
return new ValidateAccessResult(
reloadedEntity.getReturnStatus(), reloadedEntity.getExtraInformation());
}

// get storage integration, expect not null
PolarisStorageIntegration<PolarisStorageConfigurationInfo> storageIntegration =
ms.loadPolarisStorageIntegrationInCurrentTxn(callCtx, reloadedEntity.getEntity());
callCtx
.getDiagServices()
.checkNotNull(
storageIntegration,
"storage_integration_not_exists",
"catalogId={}, entityId={}",
catalogId,
entityId);

// validate access
PolarisStorageConfigurationInfo storageConfigurationInfo =
BaseMetaStoreManager.extractStorageConfiguration(callCtx, reloadedEntity.getEntity());
Map<String, String> validateLocationAccess =
storageIntegration
.validateAccessToLocations(storageConfigurationInfo, actions, locations)
.entrySet()
.stream()
.collect(
Collectors.toMap(
Map.Entry::getKey,
e -> PolarisObjectMapperUtil.serialize(callCtx, e.getValue())));

// done, return result
return new ValidateAccessResult(validateLocationAccess);
}

/**
* Get the internal property map for an entity
*
Expand Down
43 changes: 0 additions & 43 deletions polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisCredentialVendor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.entity.PolarisEntityType;
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
import org.apache.polaris.core.persistence.dao.entity.ValidateAccessResult;

/** Manage credentials for storage locations. */
public interface PolarisCredentialVendor {
Expand All @@ -49,46 +48,4 @@ ScopedCredentialsResult getSubscopedCredsForEntity(
boolean allowListOperation,
@Nonnull Set<String> allowedReadLocations,
@Nonnull Set<String> allowedWriteLocations);

/**
* Validate whether the entity has access to the locations with the provided target operations
*
* @param callCtx the polaris call context
* @param catalogId the catalog id
* @param entityId the entity id
* @param actions a set of operation actions: READ/WRITE/LIST/DELETE/ALL
* @param locations a set of locations to verify
* @return a Map of {@code <location, validate result>}, a validate result value looks like this
* <pre>
* {
* "status" : "failure",
* "actions" : {
* "READ" : {
* "message" : "The specified file was not found",
* "status" : "failure"
* },
* "DELETE" : {
* "message" : "One or more objects could not be deleted (Status Code: 200; Error Code: null)",
* "status" : "failure"
* },
* "LIST" : {
* "status" : "success"
* },
* "WRITE" : {
* "message" : "Access Denied (Status Code: 403; Error Code: AccessDenied)",
* "status" : "failure"
* }
* },
* "message" : "Some of the integration checks failed. Check the Polaris documentation for more information."
* }
* </pre>
*/
@Nonnull
ValidateAccessResult validateAccessToLocations(
@Nonnull PolarisCallContext callCtx,
long catalogId,
long entityId,
PolarisEntityType entityType,
@Nonnull Set<PolarisStorageActions> actions,
@Nonnull Set<String> locations);
}
19 changes: 0 additions & 19 deletions service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,25 +989,6 @@ private void validateLocationsForTableLike(
identifier);
}
}));

// TODO: Consider exposing a property to control whether to use the explicit default
// in-memory PolarisStorageIntegration implementation to perform validation or
// whether to delegate to PolarisMetaStoreManager::validateAccessToLocations.
// Usually the validation is better to perform with local business logic, but if
// there are additional rules to be evaluated by a custom PolarisMetaStoreManager
// implementation, then the validation should go through that API instead as follows:
//
// PolarisMetaStoreManager.ValidateAccessResult validateResult =
// getMetaStoreManager().validateAccessToLocations(
// getCurrentPolarisContext(),
// storageInfoHolderEntity.getCatalogId(),
// storageInfoHolderEntity.getId(),
// Set.of(PolarisStorageActions.ALL),
// Set.of(location));
// if (!validateResult.isSuccess()) {
// throw new ForbiddenException("Invalid location '%s' for identifier '%s': %s",
// location, identifier, validateResult.getExtraInformation());
// }
},
() -> {
List<String> allowedStorageTypes =
Expand Down