Skip to content

Commit 339ac46

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

File tree

9 files changed

+450
-522
lines changed

9 files changed

+450
-522
lines changed

‎integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisManagementServiceIntegrationTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,7 +1728,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRoles() {
17281728
.managementApi(catalogAdminToken)
17291729
.request(
17301730
"v1/principal-roles/"
1731-
+ principalRoleName
1731+
+ principalRoleName2
17321732
+ "/catalog-roles/"
17331733
+ catalogName
17341734
+ "/"
@@ -1743,7 +1743,7 @@ public void testCatalogAdminGrantAndRevokeCatalogRoles() {
17431743
managementApi
17441744
.request(
17451745
"v1/principal-roles/"
1746-
+ principalRoleName
1746+
+ principalRoleName2
17471747
+ "/catalog-roles/"
17481748
+ catalogName
17491749
+ "/"

‎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: 76 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,16 @@ 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(
142+
Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(),
143+
"Operation failed: " + result.getReturnStatus().toString())
144+
.build();
145+
}
146+
return Response.status(successStatus).build();
147+
}
148+
137149
/** From PolarisCatalogsApiService */
138150
@Override
139151
public Response createCatalog(
@@ -457,8 +469,9 @@ public Response assignPrincipalRole(
457469
request.getPrincipalRole().getName(),
458470
principalName);
459471
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
460-
adminService.assignPrincipalRole(principalName, request.getPrincipalRole().getName());
461-
return Response.status(Response.Status.CREATED).build();
472+
PrivilegeResult result =
473+
adminService.assignPrincipalRole(principalName, request.getPrincipalRole().getName());
474+
return toResponse(result, Response.Status.CREATED);
462475
}
463476

464477
/** From PolarisPrincipalsApiService */
@@ -470,8 +483,8 @@ public Response revokePrincipalRole(
470483
SecurityContext securityContext) {
471484
LOGGER.info("Revoking principalRole {} from principal {}", principalRoleName, principalName);
472485
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
473-
adminService.revokePrincipalRole(principalName, principalRoleName);
474-
return Response.status(Response.Status.NO_CONTENT).build();
486+
PrivilegeResult result = adminService.revokePrincipalRole(principalName, principalRoleName);
487+
return toResponse(result, Response.Status.NO_CONTENT);
475488
}
476489

477490
/** From PolarisPrincipalsApiService */
@@ -503,9 +516,10 @@ public Response assignCatalogRoleToPrincipalRole(
503516
catalogName,
504517
principalRoleName);
505518
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
506-
adminService.assignCatalogRoleToPrincipalRole(
507-
principalRoleName, catalogName, request.getCatalogRole().getName());
508-
return Response.status(Response.Status.CREATED).build();
519+
PrivilegeResult result =
520+
adminService.assignCatalogRoleToPrincipalRole(
521+
principalRoleName, catalogName, request.getCatalogRole().getName());
522+
return toResponse(result, Response.Status.CREATED);
509523
}
510524

511525
/** From PolarisPrincipalRolesApiService */
@@ -522,9 +536,10 @@ public Response revokeCatalogRoleFromPrincipalRole(
522536
catalogName,
523537
principalRoleName);
524538
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
525-
adminService.revokeCatalogRoleFromPrincipalRole(
526-
principalRoleName, catalogName, catalogRoleName);
527-
return Response.status(Response.Status.NO_CONTENT).build();
539+
PrivilegeResult result =
540+
adminService.revokeCatalogRoleFromPrincipalRole(
541+
principalRoleName, catalogName, catalogRoleName);
542+
return toResponse(result, Response.Status.NO_CONTENT);
528543
}
529544

530545
/** From PolarisPrincipalRolesApiService */
@@ -574,6 +589,7 @@ public Response addGrantToCatalogRole(
574589
catalogRoleName,
575590
catalogName);
576591
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
592+
PrivilegeResult result;
577593
switch (grantRequest.getGrant()) {
578594
// The per-securable-type Privilege enums must be exact String match for a subset of all
579595
// PolarisPrivilege values.
@@ -583,11 +599,12 @@ public Response addGrantToCatalogRole(
583599
PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString());
584600
String viewName = viewGrant.getViewName();
585601
String[] namespaceParts = viewGrant.getNamespace().toArray(new String[0]);
586-
adminService.grantPrivilegeOnViewToRole(
587-
catalogName,
588-
catalogRoleName,
589-
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
590-
privilege);
602+
result =
603+
adminService.grantPrivilegeOnViewToRole(
604+
catalogName,
605+
catalogRoleName,
606+
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
607+
privilege);
591608
break;
592609
}
593610
case TableGrant tableGrant:
@@ -596,27 +613,30 @@ public Response addGrantToCatalogRole(
596613
PolarisPrivilege.valueOf(tableGrant.getPrivilege().toString());
597614
String tableName = tableGrant.getTableName();
598615
String[] namespaceParts = tableGrant.getNamespace().toArray(new String[0]);
599-
adminService.grantPrivilegeOnTableToRole(
600-
catalogName,
601-
catalogRoleName,
602-
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
603-
privilege);
616+
result =
617+
adminService.grantPrivilegeOnTableToRole(
618+
catalogName,
619+
catalogRoleName,
620+
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
621+
privilege);
604622
break;
605623
}
606624
case NamespaceGrant namespaceGrant:
607625
{
608626
PolarisPrivilege privilege =
609627
PolarisPrivilege.valueOf(namespaceGrant.getPrivilege().toString());
610628
String[] namespaceParts = namespaceGrant.getNamespace().toArray(new String[0]);
611-
adminService.grantPrivilegeOnNamespaceToRole(
612-
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
629+
result =
630+
adminService.grantPrivilegeOnNamespaceToRole(
631+
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
613632
break;
614633
}
615634
case CatalogGrant catalogGrant:
616635
{
617636
PolarisPrivilege privilege =
618637
PolarisPrivilege.valueOf(catalogGrant.getPrivilege().toString());
619-
adminService.grantPrivilegeOnCatalogToRole(catalogName, catalogRoleName, privilege);
638+
result =
639+
adminService.grantPrivilegeOnCatalogToRole(catalogName, catalogRoleName, privilege);
620640
break;
621641
}
622642
case PolicyGrant policyGrant:
@@ -625,11 +645,12 @@ public Response addGrantToCatalogRole(
625645
PolarisPrivilege.valueOf(policyGrant.getPrivilege().toString());
626646
String policyName = policyGrant.getPolicyName();
627647
String[] namespaceParts = policyGrant.getNamespace().toArray(new String[0]);
628-
adminService.grantPrivilegeOnPolicyToRole(
629-
catalogName,
630-
catalogRoleName,
631-
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
632-
privilege);
648+
result =
649+
adminService.grantPrivilegeOnPolicyToRole(
650+
catalogName,
651+
catalogRoleName,
652+
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
653+
privilege);
633654
break;
634655
}
635656
default:
@@ -640,7 +661,7 @@ public Response addGrantToCatalogRole(
640661
.log("Don't know how to handle privilege grant: {}", grantRequest);
641662
return Response.status(Response.Status.BAD_REQUEST).build();
642663
}
643-
return Response.status(Response.Status.CREATED).build();
664+
return toResponse(result, Response.Status.CREATED);
644665
}
645666

646667
/** From PolarisCatalogsApiService */
@@ -663,6 +684,7 @@ public Response revokeGrantFromCatalogRole(
663684
}
664685

665686
PolarisAdminService adminService = newAdminService(realmContext, securityContext);
687+
PrivilegeResult result;
666688
switch (grantRequest.getGrant()) {
667689
// The per-securable-type Privilege enums must be exact String match for a subset of all
668690
// PolarisPrivilege values.
@@ -672,11 +694,12 @@ public Response revokeGrantFromCatalogRole(
672694
PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString());
673695
String viewName = viewGrant.getViewName();
674696
String[] namespaceParts = viewGrant.getNamespace().toArray(new String[0]);
675-
adminService.revokePrivilegeOnViewFromRole(
676-
catalogName,
677-
catalogRoleName,
678-
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
679-
privilege);
697+
result =
698+
adminService.revokePrivilegeOnViewFromRole(
699+
catalogName,
700+
catalogRoleName,
701+
TableIdentifier.of(Namespace.of(namespaceParts), viewName),
702+
privilege);
680703
break;
681704
}
682705
case TableGrant tableGrant:
@@ -685,27 +708,31 @@ public Response revokeGrantFromCatalogRole(
685708
PolarisPrivilege.valueOf(tableGrant.getPrivilege().toString());
686709
String tableName = tableGrant.getTableName();
687710
String[] namespaceParts = tableGrant.getNamespace().toArray(new String[0]);
688-
adminService.revokePrivilegeOnTableFromRole(
689-
catalogName,
690-
catalogRoleName,
691-
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
692-
privilege);
711+
result =
712+
adminService.revokePrivilegeOnTableFromRole(
713+
catalogName,
714+
catalogRoleName,
715+
TableIdentifier.of(Namespace.of(namespaceParts), tableName),
716+
privilege);
693717
break;
694718
}
695719
case NamespaceGrant namespaceGrant:
696720
{
697721
PolarisPrivilege privilege =
698722
PolarisPrivilege.valueOf(namespaceGrant.getPrivilege().toString());
699723
String[] namespaceParts = namespaceGrant.getNamespace().toArray(new String[0]);
700-
adminService.revokePrivilegeOnNamespaceFromRole(
701-
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
724+
result =
725+
adminService.revokePrivilegeOnNamespaceFromRole(
726+
catalogName, catalogRoleName, Namespace.of(namespaceParts), privilege);
702727
break;
703728
}
704729
case CatalogGrant catalogGrant:
705730
{
706731
PolarisPrivilege privilege =
707732
PolarisPrivilege.valueOf(catalogGrant.getPrivilege().toString());
708-
adminService.revokePrivilegeOnCatalogFromRole(catalogName, catalogRoleName, privilege);
733+
result =
734+
adminService.revokePrivilegeOnCatalogFromRole(
735+
catalogName, catalogRoleName, privilege);
709736
break;
710737
}
711738
case PolicyGrant policyGrant:
@@ -714,11 +741,12 @@ public Response revokeGrantFromCatalogRole(
714741
PolarisPrivilege.valueOf(policyGrant.getPrivilege().toString());
715742
String policyName = policyGrant.getPolicyName();
716743
String[] namespaceParts = policyGrant.getNamespace().toArray(new String[0]);
717-
adminService.revokePrivilegeOnPolicyFromRole(
718-
catalogName,
719-
catalogRoleName,
720-
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
721-
privilege);
744+
result =
745+
adminService.revokePrivilegeOnPolicyFromRole(
746+
catalogName,
747+
catalogRoleName,
748+
new PolicyIdentifier(Namespace.of(namespaceParts), policyName),
749+
privilege);
722750
break;
723751
}
724752
default:
@@ -729,7 +757,7 @@ public Response revokeGrantFromCatalogRole(
729757
.log("Don't know how to handle privilege revocation: {}", grantRequest);
730758
return Response.status(Response.Status.BAD_REQUEST).build();
731759
}
732-
return Response.status(Response.Status.CREATED).build();
760+
return toResponse(result, Response.Status.CREATED);
733761
}
734762

735763
/** From PolarisCatalogsApiService */

0 commit comments

Comments
 (0)