Skip to content

Commit c7b3a45

Browse files
XN137XN137
committed
Fix REST responses for failed Admin operations
1 parent d753e3d commit c7b3a45

File tree

8 files changed

+445
-520
lines changed

8 files changed

+445
-520
lines changed

‎runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java

Lines changed: 81 additions & 108 deletions
Large diffs are not rendered by default.

‎runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

Lines changed: 73 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@
7171
import org.apache.polaris.core.entity.PrincipalRoleEntity;
7272
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
7373
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
74+
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
75+
import org.apache.polaris.core.persistence.dao.entity.PrivilegeResult;
7476
import org.apache.polaris.core.persistence.resolver.ResolutionManifestFactory;
7577
import org.apache.polaris.core.secrets.UserSecretsManager;
7678
import org.apache.polaris.core.secrets.UserSecretsManagerFactory;
@@ -134,6 +136,13 @@ private PolarisAdminService newAdminService(
134136
reservedProperties);
135137
}
136138

139+
private static Response toResponse(BaseResult result, Response.Status successStatus) {
140+
if (!result.isSuccess()) {
141+
return Response.status(Response.Status.INTERNAL_SERVER_ERROR).build();
142+
}
143+
return Response.status(successStatus).build();
144+
}
145+
137146
/** From PolarisCatalogsApiService */
138147
@Override
139148
public Response createCatalog(
@@ -457,8 +466,9 @@ public Response assignPrincipalRole(
457466
request.getPrincipalRole().getName(),
458467
principalName);
459468
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
460-
adminService.assignPrincipalRole(principalName, request.getPrincipalRole().getName());
461-
return Response.status(Response.Status.CREATED).build();
469+
PrivilegeResult result =
470+
adminService.assignPrincipalRole(principalName, request.getPrincipalRole().getName());
471+
return toResponse(result, Response.Status.CREATED);
462472
}
463473

464474
/** From PolarisPrincipalsApiService */
@@ -470,8 +480,8 @@ public Response revokePrincipalRole(
470480
SecurityContext securityContext) {
471481
LOGGER.info("Revoking principalRole {} from principal {}", principalRoleName, principalName);
472482
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
473-
adminService.revokePrincipalRole(principalName, principalRoleName);
474-
return Response.status(Response.Status.NO_CONTENT).build();
483+
PrivilegeResult result = adminService.revokePrincipalRole(principalName, principalRoleName);
484+
return toResponse(result, Response.Status.NO_CONTENT);
475485
}
476486

477487
/** From PolarisPrincipalsApiService */
@@ -503,9 +513,10 @@ public Response assignCatalogRoleToPrincipalRole(
503513
catalogName,
504514
principalRoleName);
505515
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
506-
adminService.assignCatalogRoleToPrincipalRole(
507-
principalRoleName, catalogName, request.getCatalogRole().getName());
508-
return Response.status(Response.Status.CREATED).build();
516+
PrivilegeResult result =
517+
adminService.assignCatalogRoleToPrincipalRole(
518+
principalRoleName, catalogName, request.getCatalogRole().getName());
519+
return toResponse(result, Response.Status.CREATED);
509520
}
510521

511522
/** From PolarisPrincipalRolesApiService */
@@ -522,9 +533,10 @@ public Response revokeCatalogRoleFromPrincipalRole(
522533
catalogName,
523534
principalRoleName);
524535
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
525-
adminService.revokeCatalogRoleFromPrincipalRole(
526-
principalRoleName, catalogName, catalogRoleName);
527-
return Response.status(Response.Status.NO_CONTENT).build();
536+
PrivilegeResult result =
537+
adminService.revokeCatalogRoleFromPrincipalRole(
538+
principalRoleName, catalogName, catalogRoleName);
539+
return toResponse(result, Response.Status.NO_CONTENT);
528540
}
529541

530542
/** From PolarisPrincipalRolesApiService */
@@ -574,6 +586,7 @@ public Response addGrantToCatalogRole(
574586
catalogRoleName,
575587
catalogName);
576588
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
589+
PrivilegeResult result;
577590
switch (grantRequest.getGrant()) {
578591
// The per-securable-type Privilege enums must be exact String match for a subset of all
579592
// PolarisPrivilege values.
@@ -583,11 +596,12 @@ public Response addGrantToCatalogRole(
583596
PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString());
584597
String viewName = viewGrant.getViewName();
585598
String[] namespaceParts = viewGrant.getNamespace().toArray(new String[0]);
586-
adminService.grantPrivilegeOnViewToRole(
587-
catalogName,
588-
catalogRoleName,
589-
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
590-
privilege);
599+
result =
600+
adminService.grantPrivilegeOnViewToRole(
601+
catalogName,
602+
catalogRoleName,
603+
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
604+
privilege);
591605
break;
592606
}
593607
case TableGrant tableGrant:
@@ -596,27 +610,30 @@ public Response addGrantToCatalogRole(
596610
PolarisPrivilege.valueOf(tableGrant.getPrivilege().toString());
597611
String tableName = tableGrant.getTableName();
598612
String[] namespaceParts = tableGrant.getNamespace().toArray(new String[0]);
599-
adminService.grantPrivilegeOnTableToRole(
600-
catalogName,
601-
catalogRoleName,
602-
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
603-
privilege);
613+
result =
614+
adminService.grantPrivilegeOnTableToRole(
615+
catalogName,
616+
catalogRoleName,
617+
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
618+
privilege);
604619
break;
605620
}
606621
case NamespaceGrant namespaceGrant:
607622
{
608623
PolarisPrivilege privilege =
609624
PolarisPrivilege.valueOf(namespaceGrant.getPrivilege().toString());
610625
String[] namespaceParts = namespaceGrant.getNamespace().toArray(new String[0]);
611-
adminService.grantPrivilegeOnNamespaceToRole(
612-
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
626+
result =
627+
adminService.grantPrivilegeOnNamespaceToRole(
628+
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
613629
break;
614630
}
615631
case CatalogGrant catalogGrant:
616632
{
617633
PolarisPrivilege privilege =
618634
PolarisPrivilege.valueOf(catalogGrant.getPrivilege().toString());
619-
adminService.grantPrivilegeOnCatalogToRole(catalogName, catalogRoleName, privilege);
635+
result =
636+
adminService.grantPrivilegeOnCatalogToRole(catalogName, catalogRoleName, privilege);
620637
break;
621638
}
622639
case PolicyGrant policyGrant:
@@ -625,11 +642,12 @@ public Response addGrantToCatalogRole(
625642
PolarisPrivilege.valueOf(policyGrant.getPrivilege().toString());
626643
String policyName = policyGrant.getPolicyName();
627644
String[] namespaceParts = policyGrant.getNamespace().toArray(new String[0]);
628-
adminService.grantPrivilegeOnPolicyToRole(
629-
catalogName,
630-
catalogRoleName,
631-
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
632-
privilege);
645+
result =
646+
adminService.grantPrivilegeOnPolicyToRole(
647+
catalogName,
648+
catalogRoleName,
649+
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
650+
privilege);
633651
break;
634652
}
635653
default:
@@ -640,7 +658,7 @@ public Response addGrantToCatalogRole(
640658
.log("Don't know how to handle privilege grant: {}", grantRequest);
641659
return Response.status(Response.Status.BAD_REQUEST).build();
642660
}
643-
return Response.status(Response.Status.CREATED).build();
661+
return toResponse(result, Response.Status.CREATED);
644662
}
645663

646664
/** From PolarisCatalogsApiService */
@@ -663,6 +681,7 @@ public Response revokeGrantFromCatalogRole(
663681
}
664682

665683
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
684+
PrivilegeResult result;
666685
switch (grantRequest.getGrant()) {
667686
// The per-securable-type Privilege enums must be exact String match for a subset of all
668687
// PolarisPrivilege values.
@@ -672,11 +691,12 @@ public Response revokeGrantFromCatalogRole(
672691
PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString());
673692
String viewName = viewGrant.getViewName();
674693
String[] namespaceParts = viewGrant.getNamespace().toArray(new String[0]);
675-
adminService.revokePrivilegeOnViewFromRole(
676-
catalogName,
677-
catalogRoleName,
678-
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
679-
privilege);
694+
result =
695+
adminService.revokePrivilegeOnViewFromRole(
696+
catalogName,
697+
catalogRoleName,
698+
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
699+
privilege);
680700
break;
681701
}
682702
case TableGrant tableGrant:
@@ -685,27 +705,31 @@ public Response revokeGrantFromCatalogRole(
685705
PolarisPrivilege.valueOf(tableGrant.getPrivilege().toString());
686706
String tableName = tableGrant.getTableName();
687707
String[] namespaceParts = tableGrant.getNamespace().toArray(new String[0]);
688-
adminService.revokePrivilegeOnTableFromRole(
689-
catalogName,
690-
catalogRoleName,
691-
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
692-
privilege);
708+
result =
709+
adminService.revokePrivilegeOnTableFromRole(
710+
catalogName,
711+
catalogRoleName,
712+
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
713+
privilege);
693714
break;
694715
}
695716
case NamespaceGrant namespaceGrant:
696717
{
697718
PolarisPrivilege privilege =
698719
PolarisPrivilege.valueOf(namespaceGrant.getPrivilege().toString());
699720
String[] namespaceParts = namespaceGrant.getNamespace().toArray(new String[0]);
700-
adminService.revokePrivilegeOnNamespaceFromRole(
701-
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
721+
result =
722+
adminService.revokePrivilegeOnNamespaceFromRole(
723+
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
702724
break;
703725
}
704726
case CatalogGrant catalogGrant:
705727
{
706728
PolarisPrivilege privilege =
707729
PolarisPrivilege.valueOf(catalogGrant.getPrivilege().toString());
708-
adminService.revokePrivilegeOnCatalogFromRole(catalogName, catalogRoleName, privilege);
730+
result =
731+
adminService.revokePrivilegeOnCatalogFromRole(
732+
catalogName, catalogRoleName, privilege);
709733
break;
710734
}
711735
case PolicyGrant policyGrant:
@@ -714,11 +738,12 @@ public Response revokeGrantFromCatalogRole(
714738
PolarisPrivilege.valueOf(policyGrant.getPrivilege().toString());
715739
String policyName = policyGrant.getPolicyName();
716740
String[] namespaceParts = policyGrant.getNamespace().toArray(new String[0]);
717-
adminService.revokePrivilegeOnPolicyFromRole(
718-
catalogName,
719-
catalogRoleName,
720-
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
721-
privilege);
741+
result =
742+
adminService.revokePrivilegeOnPolicyFromRole(
743+
catalogName,
744+
catalogRoleName,
745+
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
746+
privilege);
722747
break;
723748
}
724749
default:
@@ -729,7 +754,7 @@ public Response revokeGrantFromCatalogRole(
729754
.log("Don't know how to handle privilege revocation: {}", grantRequest);
730755
return Response.status(Response.Status.BAD_REQUEST).build();
731756
}
732-
return Response.status(Response.Status.CREATED).build();
757+
return toResponse(result, Response.Status.CREATED);
733758
}
734759

735760
/** From PolarisCatalogsApiService */

0 commit comments

Comments
 (0)