Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import jakarta.annotation.Nullable;
import java.util.List;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.entity.PolarisBaseEntity;
import org.apache.polaris.core.entity.PolarisEntityCore;
import org.apache.polaris.core.entity.PolarisPrivilege;
import org.apache.polaris.core.persistence.dao.entity.LoadGrantsResult;
Expand Down Expand Up @@ -119,23 +118,8 @@ PrivilegeResult revokePrivilegeOnSecurableFromRole(
* ENTITY_NOT_FOUND if the securable cannot be found
*/
@Nonnull
default LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, PolarisBaseEntity securable) {
return loadGrantsOnSecurable(callCtx, securable.getCatalogId(), securable.getId());
}

/**
* This method should be used by the Polaris app to cache all grant records on a securable.
*
* @param callCtx call context
* @param securableCatalogId id of the catalog this securable belongs to
* @param securableId id of the securable
* @return the list of grants and the version of the grant records. We will return
* ENTITY_NOT_FOUND if the securable cannot be found
*/
Comment on lines -127 to -135
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, the doc itself was removed. Can we re-introduce a javadoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole method that hosted this javadoc was removed from the interface

@Nonnull
LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId);
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable);

/**
* This method should be used by the Polaris app to load all grants made to a grantee, either a
Expand All @@ -147,22 +131,6 @@ LoadGrantsResult loadGrantsOnSecurable(
* grantee does not exist
*/
@Nonnull
default LoadGrantsResult loadGrantsToGrantee(
@Nonnull PolarisCallContext callCtx, PolarisBaseEntity grantee) {
return loadGrantsToGrantee(callCtx, grantee.getCatalogId(), grantee.getId());
}

/**
* This method should be used by the Polaris app to load all grants made to a grantee, either a
* role or a principal.
*
* @param callCtx call context
* @param granteeCatalogId id of the catalog this grantee belongs to
* @param granteeId id of the grantee
* @return the list of grants and the version of the grant records. We will return NULL if the
* grantee does not exist
*/
@Nonnull
LoadGrantsResult loadGrantsToGrantee(
PolarisCallContext callCtx, long granteeCatalogId, long granteeId);
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee);
}
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,11 @@ private void revokeGrantRecord(

/** {@inheritDoc} */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we copy over the doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restored

@Override
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable) {
return loadGrantsOnSecurable(callCtx, securable.getCatalogId(), securable.getId());
}

public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId) {
// get metastore we should be using
Expand Down Expand Up @@ -1371,6 +1376,11 @@ private void revokeGrantRecord(

/** {@inheritDoc} */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restored

@Override
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee) {
return loadGrantsToGrantee(callCtx, grantee.getCatalogId(), grantee.getId());
}

public @Nonnull LoadGrantsResult loadGrantsToGrantee(
@Nonnull PolarisCallContext callCtx, long granteeCatalogId, long granteeId) {
// get metastore we should be using
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,17 +280,17 @@ public PrivilegeResult revokePrivilegeOnSecurableFromRole(
}

@Override
public LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId) {
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable) {
callCtx
.getDiagServices()
.fail("illegal_method_in_transaction_workspace", "loadGrantsOnSecurable");
return null;
}

@Override
public LoadGrantsResult loadGrantsToGrantee(
PolarisCallContext callCtx, long granteeCatalogId, long granteeId) {
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee) {
callCtx
.getDiagServices()
.fail("illegal_method_in_transaction_workspace", "loadGrantsToGrantee");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1757,6 +1757,11 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(

/** {@inheritDoc} */
@Override
public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, PolarisEntityCore securable) {
return loadGrantsOnSecurable(callCtx, securable.getCatalogId(), securable.getId());
}

public @Nonnull LoadGrantsResult loadGrantsOnSecurable(
@Nonnull PolarisCallContext callCtx, long securableCatalogId, long securableId) {
// get metastore we should be using
Expand Down Expand Up @@ -1807,6 +1812,11 @@ private PolarisEntityResolver resolveSecurableToRoleGrant(

/** {@inheritDoc} */
@Override
public @Nonnull LoadGrantsResult loadGrantsToGrantee(
@Nonnull PolarisCallContext callCtx, PolarisEntityCore grantee) {
return loadGrantsToGrantee(callCtx, grantee.getCatalogId(), grantee.getId());
}

public @Nonnull LoadGrantsResult loadGrantsToGrantee(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[question] Does this still need to be public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but I did not want to expand the scope of changes unnecessarily.

@Nonnull PolarisCallContext callCtx, long granteeCatalogId, long granteeId) {
// get metastore we should be using
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,7 @@ void ensureGrantRecordExists(

// load all grant records on that securable, should not fail
LoadGrantsResult loadGrantsOnSecurable =
polarisMetaStoreManager.loadGrantsOnSecurable(
this.polarisCallContext, securable.getCatalogId(), securable.getId());
polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, securable);
// ensure entities for these grant records have been properly loaded
this.validateLoadedGrants(loadGrantsOnSecurable, false);

Expand All @@ -278,8 +277,7 @@ void ensureGrantRecordExists(

// load all grant records on that grantee, should not fail
LoadGrantsResult loadGrantsOnGrantee =
polarisMetaStoreManager.loadGrantsToGrantee(
this.polarisCallContext, grantee.getCatalogId(), grantee.getId());
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, grantee);
// ensure entities for these grant records have been properly loaded
this.validateLoadedGrants(loadGrantsOnGrantee, true);

Expand Down Expand Up @@ -355,8 +353,7 @@ void ensureGrantRecordRemoved(

// load all grant records on that securable, should not fail
LoadGrantsResult loadGrantsOnSecurable =
polarisMetaStoreManager.loadGrantsOnSecurable(
this.polarisCallContext, securable.getCatalogId(), securable.getId());
polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, securable);
// ensure entities for these grant records have been properly loaded
this.validateLoadedGrants(loadGrantsOnSecurable, false);

Expand All @@ -365,8 +362,7 @@ void ensureGrantRecordRemoved(

// load all grant records on that grantee, should not fail
LoadGrantsResult loadGrantsOnGrantee =
polarisMetaStoreManager.loadGrantsToGrantee(
this.polarisCallContext, grantee.getCatalogId(), grantee.getId());
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, grantee);
this.validateLoadedGrants(loadGrantsOnGrantee, true);

// check that the grant record has been removed
Expand Down Expand Up @@ -734,14 +730,12 @@ void dropEntity(List<PolarisEntityCore> catalogPath, PolarisBaseEntity entityToD
granteeEntities =
new ArrayList<>(
polarisMetaStoreManager
.loadGrantsOnSecurable(
this.polarisCallContext, entity.getCatalogId(), entity.getId())
.loadGrantsOnSecurable(this.polarisCallContext, entity)
.getEntities());
securableEntities =
new ArrayList<>(
polarisMetaStoreManager
.loadGrantsToGrantee(
this.polarisCallContext, entity.getCatalogId(), entity.getId())
.loadGrantsToGrantee(this.polarisCallContext, entity)
.getEntities());
} else {
granteeEntities = List.of();
Expand Down Expand Up @@ -831,8 +825,7 @@ void dropEntity(List<PolarisEntityCore> catalogPath, PolarisBaseEntity entityToD
// of the entity it was connected with before being dropped
for (PolarisBaseEntity connectedEntity : granteeEntities) {
LoadGrantsResult grantResult =
polarisMetaStoreManager.loadGrantsToGrantee(
this.polarisCallContext, connectedEntity.getCatalogId(), connectedEntity.getId());
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, connectedEntity);
if (grantResult.isSuccess()) {
long cnt =
grantResult.getGrantRecords().stream()
Expand All @@ -853,8 +846,7 @@ void dropEntity(List<PolarisEntityCore> catalogPath, PolarisBaseEntity entityToD
}
for (PolarisBaseEntity connectedEntity : securableEntities) {
LoadGrantsResult grantResult =
polarisMetaStoreManager.loadGrantsOnSecurable(
this.polarisCallContext, connectedEntity.getCatalogId(), connectedEntity.getId());
polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, connectedEntity);
long cnt =
grantResult.getGrantRecords().stream()
.filter(gr -> gr.getGranteeId() == entityToDrop.getId())
Expand Down Expand Up @@ -1294,8 +1286,7 @@ private void validateCacheEntryLoad(ResolvedEntityResult cacheEntry) {
List<PolarisGrantRecord> refGrantRecords = new ArrayList<>();
if (refEntity.getType().isGrantee()) {
LoadGrantsResult loadGrantResult =
this.polarisMetaStoreManager.loadGrantsToGrantee(
this.polarisCallContext, refEntity.getCatalogId(), refEntity.getId());
this.polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, refEntity);
this.validateLoadedGrants(loadGrantResult, true);

// same version
Expand All @@ -1306,8 +1297,7 @@ private void validateCacheEntryLoad(ResolvedEntityResult cacheEntry) {
}

LoadGrantsResult loadGrantResult =
this.polarisMetaStoreManager.loadGrantsOnSecurable(
this.polarisCallContext, refEntity.getCatalogId(), refEntity.getId());
this.polarisMetaStoreManager.loadGrantsOnSecurable(this.polarisCallContext, refEntity);
this.validateLoadedGrants(loadGrantResult, false);

// same version
Expand Down Expand Up @@ -1347,10 +1337,9 @@ private void validateCacheEntryRefresh(
// reload the grants
LoadGrantsResult loadGrantResult =
refEntity.getType().isGrantee()
? this.polarisMetaStoreManager.loadGrantsToGrantee(
this.polarisCallContext, catalogId, entityId)
? this.polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, refEntity)
: this.polarisMetaStoreManager.loadGrantsOnSecurable(
this.polarisCallContext, catalogId, entityId);
this.polarisCallContext, refEntity);
this.validateLoadedGrants(loadGrantResult, refEntity.getType().isGrantee());
Assertions.assertThat(cacheEntry.getGrantRecordsVersion())
.isEqualTo(loadGrantResult.getGrantsVersion());
Expand Down Expand Up @@ -2099,16 +2088,15 @@ public void testPrivileges() {
grantToGrantee(catalog, R1, PR9000, PolarisPrivilege.CATALOG_ROLE_USAGE);

LoadGrantsResult loadGrantsResult =
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, 0L, PR9000.getId());
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, PR9000);
this.validateLoadedGrants(loadGrantsResult, true);
Assertions.assertThat(loadGrantsResult.getGrantRecords()).hasSize(1);
Assertions.assertThat(loadGrantsResult.getGrantRecords().get(0).getSecurableCatalogId())
.isEqualTo(R1.getCatalogId());
Assertions.assertThat(loadGrantsResult.getGrantRecords().get(0).getSecurableId())
.isEqualTo(R1.getId());

loadGrantsResult =
polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, 0L, PR900.getId());
loadGrantsResult = polarisMetaStoreManager.loadGrantsToGrantee(this.polarisCallContext, PR900);
Assertions.assertThat(loadGrantsResult).isNotNull();
Assertions.assertThat(loadGrantsResult.getGrantRecords()).hasSize(0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,7 @@ public void after() {

protected @Nonnull Set<String> loadPrincipalRolesNames(AuthenticatedPolarisPrincipal p) {
return metaStoreManager
.loadGrantsToGrantee(
callContext.getPolarisCallContext(), 0L, p.getPrincipalEntity().getId())
.loadGrantsToGrantee(callContext.getPolarisCallContext(), p.getPrincipalEntity())
.getGrantRecords()
.stream()
.filter(gr -> gr.getPrivilegeCode() == PolarisPrivilege.PRINCIPAL_ROLE_USAGE.getCode())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1217,8 +1217,7 @@ public List<PolarisEntity> listPrincipalRolesAssigned(String principalName) {
findPrincipalByName(principalName)
.orElseThrow(() -> new NotFoundException("Principal %s not found", principalName));
LoadGrantsResult grantList =
metaStoreManager.loadGrantsToGrantee(
getCurrentPolarisContext(), principalEntity.getCatalogId(), principalEntity.getId());
metaStoreManager.loadGrantsToGrantee(getCurrentPolarisContext(), principalEntity);
return buildEntitiesFromGrantResults(grantList, false, PolarisEntityType.PRINCIPAL_ROLE, null);
}

Expand Down Expand Up @@ -1281,10 +1280,7 @@ public List<PolarisEntity> listAssigneePrincipalsForPrincipalRole(String princip
.orElseThrow(
() -> new NotFoundException("PrincipalRole %s not found", principalRoleName));
LoadGrantsResult grantList =
metaStoreManager.loadGrantsOnSecurable(
getCurrentPolarisContext(),
principalRoleEntity.getCatalogId(),
principalRoleEntity.getId());
metaStoreManager.loadGrantsOnSecurable(getCurrentPolarisContext(), principalRoleEntity);
return buildEntitiesFromGrantResults(grantList, true, PolarisEntityType.PRINCIPAL, null);
}

Expand Down Expand Up @@ -1336,10 +1332,7 @@ public List<PolarisEntity> listCatalogRolesForPrincipalRole(
.orElseThrow(
() -> new NotFoundException("PrincipalRole %s not found", principalRoleName));
LoadGrantsResult grantList =
metaStoreManager.loadGrantsToGrantee(
getCurrentPolarisContext(),
principalRoleEntity.getCatalogId(),
principalRoleEntity.getId());
metaStoreManager.loadGrantsToGrantee(getCurrentPolarisContext(), principalRoleEntity);
return buildEntitiesFromGrantResults(
grantList,
false,
Expand Down Expand Up @@ -1565,10 +1558,7 @@ public List<PolarisEntity> listAssigneePrincipalRolesForCatalogRole(
findCatalogRoleByName(catalogName, catalogRoleName)
.orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName));
LoadGrantsResult grantList =
metaStoreManager.loadGrantsOnSecurable(
getCurrentPolarisContext(),
catalogRoleEntity.getCatalogId(),
catalogRoleEntity.getId());
metaStoreManager.loadGrantsOnSecurable(getCurrentPolarisContext(), catalogRoleEntity);
return buildEntitiesFromGrantResults(grantList, true, PolarisEntityType.PRINCIPAL_ROLE, null);
}

Expand All @@ -1584,10 +1574,7 @@ public List<GrantResource> listGrantsForCatalogRole(String catalogName, String c
findCatalogRoleByName(catalogName, catalogRoleName)
.orElseThrow(() -> new NotFoundException("CatalogRole %s not found", catalogRoleName));
LoadGrantsResult grantList =
metaStoreManager.loadGrantsToGrantee(
getCurrentPolarisContext(),
catalogRoleEntity.getCatalogId(),
catalogRoleEntity.getId());
metaStoreManager.loadGrantsToGrantee(getCurrentPolarisContext(), catalogRoleEntity);
List<CatalogGrant> catalogGrants = new ArrayList<>();
List<NamespaceGrant> namespaceGrants = new ArrayList<>();
List<TableGrant> tableGrants = new ArrayList<>();
Expand Down