From b6a917291ab3f8ad27060f7810d5c1e68707015b Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 10:09:50 +0100 Subject: [PATCH 01/29] Add OPA permission calls to all applicable coprocessor hooks with test coverage for most of them --- .../OpenPolicyAgentAccessController.java | 474 ++++++++----- .../stackable/hbase/opa/OpaAclChecker.java | 9 +- .../TestOpenPolicyAgentAccessController.java | 660 +++++++++++++++--- ...OpenPolicyAgentAccessControllerRegion.java | 407 +++++++++++ ...enPolicyAgentAccessControllerVariants.java | 91 +++ .../java/tech/stackable/hbase/TestUtils.java | 38 + 6 files changed, 1410 insertions(+), 269 deletions(-) create mode 100644 src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java create mode 100644 src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 2e02c31..bf5f207 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -397,7 +397,7 @@ public Result preAppend(ObserverContext c, Append TableName tableName = c.getEnvironment().getRegionInfo().getTable(); LOG.trace("preAppend: user [{}] on table [{}] with append [{}]", user, tableName, append); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); // as per default access controller return null; @@ -560,7 +560,7 @@ public boolean preCheckAndPut( TableName tableName = c.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); return result; } @@ -580,7 +580,7 @@ public boolean preCheckAndPutAfterRowLock( LOG.trace( "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); return result; } @@ -600,7 +600,7 @@ public boolean preCheckAndDelete( LOG.trace( "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); return result; } @@ -623,7 +623,7 @@ public boolean preCheckAndDeleteAfterRowLock( tableName, delete); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); return result; } @@ -683,7 +683,7 @@ public Result preIncrement( TableName tableName = c.getEnvironment().getRegionInfo().getTable(); LOG.trace("preIncrement: user [{}] on table [{}]", user, tableName); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); // as per default controller return null; } @@ -771,11 +771,10 @@ public Optional getRegionServerObserver() { return Optional.of(this); } - /*********************************** Not implemented (yet) ***********************************/ - @Override public String preModifyTableStoreFileTracker( - ObserverContext c, TableName tableName, String dstSFT) { + ObserverContext c, TableName tableName, String dstSFT) + throws IOException { requirePermission( c, "modifyTableStoreFileTracker", tableName, null, null, Action.ADMIN, Action.CREATE); return dstSFT; @@ -786,7 +785,8 @@ public String preModifyColumnFamilyStoreFileTracker( ObserverContext c, TableName tableName, byte[] family, - String dstSFT) { + String dstSFT) + throws IOException { requirePermission( c, "modifyColumnFamilyStoreFileTracker", @@ -803,23 +803,26 @@ public void preMove( ObserverContext c, RegionInfo region, ServerName srcServer, - ServerName destServer) { + ServerName destServer) + throws IOException { requirePermission(c, "move", region.getTable(), null, null, Action.ADMIN); } @Override - public void preAssign(ObserverContext c, RegionInfo regionInfo) { + public void preAssign(ObserverContext c, RegionInfo regionInfo) + throws IOException { requirePermission(c, "assign", regionInfo.getTable(), null, null, Action.ADMIN); } @Override - public void preUnassign(ObserverContext c, RegionInfo regionInfo) { + public void preUnassign(ObserverContext c, RegionInfo regionInfo) + throws IOException { requirePermission(c, "unassign", regionInfo.getTable(), null, null, Action.ADMIN); } @Override public void preRegionOffline( - ObserverContext c, RegionInfo regionInfo) { + ObserverContext c, RegionInfo regionInfo) throws IOException { requirePermission(c, "regionOffline", regionInfo.getTable(), null, null, Action.ADMIN); } @@ -827,7 +830,8 @@ public void preRegionOffline( public void preSnapshot( final ObserverContext ctx, final SnapshotDescription snapshot, - final TableDescriptor hTableDescriptor) { + final TableDescriptor hTableDescriptor) + throws IOException { requirePermission( ctx, "snapshot " + snapshot.getName(), @@ -839,90 +843,89 @@ public void preSnapshot( @Override public void preListSnapshot( - ObserverContext ctx, final SnapshotDescription snapshot) { - LOG.debug("preListSnapshot not yet implemented! Snapshot: {}", snapshot); + ObserverContext ctx, final SnapshotDescription snapshot) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preListSnapshot: user [{}] snapshot[{}]", user, snapshot); + opaAclChecker.checkPermissionInfo(user, snapshot.getTableName(), Action.ADMIN); } @Override public void preCloneSnapshot( final ObserverContext ctx, final SnapshotDescription snapshot, - final TableDescriptor hTableDescriptor) { - LOG.debug("preCloneSnapshot not yet implemented! Snapshot: {}", snapshot); + final TableDescriptor hTableDescriptor) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = hTableDescriptor.getTableName(); + LOG.debug("preCloneSnapshot: user [{}] snapshot[{}] table [{}]", user, snapshot, tableName); + opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); } @Override public void preRestoreSnapshot( final ObserverContext ctx, final SnapshotDescription snapshot, - final TableDescriptor hTableDescriptor) { - LOG.debug("preRestoreSnapshot not yet implemented! Snapshot: {}", snapshot); + final TableDescriptor hTableDescriptor) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = hTableDescriptor.getTableName(); + LOG.debug("preRestoreSnapshot: user [{}] snapshot[{}] table [{}]", user, snapshot, tableName); + opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); } @Override public void preDeleteSnapshot( - final ObserverContext ctx, final SnapshotDescription snapshot) { - LOG.debug("preDeleteSnapshot not yet implemented! Snapshot: {}", snapshot); + final ObserverContext ctx, final SnapshotDescription snapshot) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preDeleteSnapshot: user [{}] snapshot[{}]", user, snapshot); + opaAclChecker.checkPermissionInfo(user, snapshot.getTableName(), Action.ADMIN); } @Override public void preSplitRegion( final ObserverContext ctx, final TableName tableName, - final byte[] splitRow) { + final byte[] splitRow) + throws IOException { requirePermission(ctx, "split", tableName, null, null, Action.ADMIN); } @Override public void preBulkLoadHFile( - ObserverContext ctx, List> familyPaths) { - LOG.debug("preBulkLoadHFile not implemented!"); - } - - @Override - public void prePrepareBulkLoad(ObserverContext ctx) { - LOG.debug("prePrepareBulkLoad not implemented!"); - } - - @Override - public void preCleanupBulkLoad(ObserverContext ctx) { - LOG.debug("preCleanupBulkLoad not implemented!"); - } - - @Override - public Message preEndpointInvocation( - ObserverContext ctx, - Service service, - String methodName, - Message request) { - LOG.debug("preEndpointInvocation not implemented! {}/{}", methodName, request); - return request; - } - - @Override - public void postEndpointInvocation( - ObserverContext ctx, - Service service, - String methodName, - Message request, - Message.Builder responseBuilder) { - LOG.debug("postEndpointInvocation not implemented! {}/{}", methodName, request); + ObserverContext ctx, List> familyPaths) + throws IOException { + User user = getActiveUser(ctx); + var tableName = ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(); + LOG.debug("preBulkLoadHFile: user [{}] on table [{}]", user, tableName); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); } @Override - public void preRequestLock( - ObserverContext ctx, - String namespace, - TableName tableName, - RegionInfo[] regionInfos, - String description) { - LOG.debug("preRequestLock not implemented! {}/{}", tableName, regionInfos); + public void prePrepareBulkLoad(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, + "prePrepareBulkLoad", + ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(), + null, + null, + Action.ADMIN, + Action.CREATE); } @Override - public void preLockHeartbeat( - ObserverContext ctx, TableName tableName, String description) { - LOG.debug("preLockHeartbeat not implemented! {}/{}", tableName, description); + public void preCleanupBulkLoad(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, + "preCleanupBulkLoad", + ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(), + null, + null, + Action.ADMIN, + Action.CREATE); } @Override @@ -930,7 +933,8 @@ public void preSetUserQuota( final ObserverContext ctx, final String userName, final TableName tableName, - final GlobalQuotaSettings quotas) { + final GlobalQuotaSettings quotas) + throws IOException { requirePermission(ctx, "setUserTableQuota", tableName, null, null, Action.ADMIN); } @@ -939,15 +943,17 @@ public void preSetUserQuota( final ObserverContext ctx, final String userName, final String namespace, - final GlobalQuotaSettings quotas) { - requirePermission(ctx, "setUserNamespaceQuota", Action.ADMIN); + final GlobalQuotaSettings quotas) + throws IOException { + requirePermission(ctx, namespace, "setUserNamespaceQuota", Action.ADMIN); } @Override public void preSetTableQuota( final ObserverContext ctx, final TableName tableName, - final GlobalQuotaSettings quotas) { + final GlobalQuotaSettings quotas) + throws IOException { requirePermission(ctx, "setTableQuota", tableName, null, null, Action.ADMIN); } @@ -955,13 +961,15 @@ public void preSetTableQuota( public void preSetNamespaceQuota( final ObserverContext ctx, final String namespace, - final GlobalQuotaSettings quotas) { - requirePermission(ctx, "setNamespaceQuota", Action.ADMIN); + final GlobalQuotaSettings quotas) + throws IOException { + requirePermission(ctx, namespace, "setNamespaceQuota", Action.ADMIN); } @Override public void preMergeRegions( - final ObserverContext ctx, final RegionInfo[] regionsToMerge) { + final ObserverContext ctx, final RegionInfo[] regionsToMerge) + throws IOException { requirePermission(ctx, "mergeRegions", regionsToMerge[0].getTable(), null, null, Action.ADMIN); } @@ -972,12 +980,33 @@ public void preGetUserPermissions( String namespace, TableName tableName, byte[] family, - byte[] qualifier) { - LOG.debug("preGetUserPermissions not implemented! {}/{}", userName, tableName); + byte[] qualifier) + throws IOException { + User user = getActiveUser(ctx); + if (tableName != null) { + LOG.debug("preGetUserPermissions: user [{}] on table [{}]", user, tableName); + requirePermission(ctx, "getUserPermissions", tableName, family, qualifier, Action.ADMIN); + } else if (namespace != null) { + LOG.debug("preGetUserPermissions: user [{}] on namespace [{}]", user, namespace); + requirePermission(ctx, namespace, "getUserPermissions", Action.ADMIN); + } else { + LOG.debug("preGetUserPermissions: user [{}] global", user); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "getUserPermissions", Action.ADMIN); + } } - private void requirePermission(ObserverContext ctx, String request, Action perm) { - LOG.debug("requirePermission not implemented! {}/{}", request, perm); + private void requirePermission( + final ObserverContext ctx, final String namespace, String request, Action perm) + throws IOException { + User user = getActiveUser(ctx); + LOG.trace( + "requirePermission: user [{}] namespace[{}] request [{}] permission [{}]", + user, + namespace, + request, + perm); + opaAclChecker.checkPermissionInfo(user, namespace, perm); } private void requirePermission( @@ -986,101 +1015,178 @@ private void requirePermission( TableName tableName, byte[] family, byte[] qualifier, - Action... permissions) { - LOG.debug("requirePermission for table not implemented! {}/{}", tableName, permissions); + Action... permissions) + throws IOException { + User user = getActiveUser(ctx); + for (Action perm : permissions) { + LOG.trace( + "requirePermission: user [{}] tableName[{}] permission [{}]", user, tableName, perm); + opaAclChecker.checkPermissionInfo(user, tableName, perm); + } } - /*********** Not implemented (admin tasks coming from the Master or RegionServer) *************************/ - @Override - public void preAbortProcedure( - ObserverContext ctx, final long procId) { - LOG.debug("preAbortProcedure not implemented!"); + public void preBalance(ObserverContext ctx, BalanceRequest request) + throws IOException { + requirePermission(ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "balance", Action.ADMIN); } @Override - public void postAbortProcedure(ObserverContext ctx) { - // There is nothing to do at this time after the procedure abort request was sent. + public void preBalanceSwitch(ObserverContext ctx, boolean newValue) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "balanceSwitch", Action.ADMIN); } @Override - public void preGetProcedures(ObserverContext ctx) { - LOG.debug("preGetProcedures not implemented!"); + public void preShutdown(ObserverContext ctx) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "shutdown", Action.ADMIN); } @Override - public void preGetLocks(ObserverContext ctx) { - LOG.trace("preGetLocks not implemented!"); + public void preStopMaster(ObserverContext ctx) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "stopMaster", Action.ADMIN); } @Override - public void preSetSplitOrMergeEnabled( - final ObserverContext ctx, - final boolean newValue, - final MasterSwitchType switchType) { - LOG.debug("preSetSplitOrMergeEnabled not implemented!"); + public void preClearDeadServers(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "clearDeadServers", Action.ADMIN); } @Override - public void preBalance(ObserverContext c, BalanceRequest request) { - LOG.debug("preBalance not implemented!"); + public void preDecommissionRegionServers( + ObserverContext ctx, List servers, boolean offload) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "decommissionRegionServers", + Action.ADMIN); } @Override - public void preBalanceSwitch(ObserverContext c, boolean newValue) { - LOG.debug("preBalanceSwitch not implemented!"); + public void preListDecommissionedRegionServers(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "listDecommissionedRegionServers", + Action.READ); } @Override - public void preShutdown(ObserverContext c) { - LOG.debug("preShutdown not implemented!"); + public void preRecommissionRegionServer( + ObserverContext ctx, + ServerName server, + List encodedRegionNames) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "recommissionRegionServers", + Action.ADMIN); } @Override - public void preStopMaster(ObserverContext c) { - LOG.debug("preStopMaster not implemented!"); + public void preStopRegionServer(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "preStopRegionServer", Action.ADMIN); } + /*********************************** Not implemented (yet) ***********************************/ + @Override - public void postStartMaster(ObserverContext ctx) { - LOG.debug("postStartMaster not implemented!"); + public Message preEndpointInvocation( + ObserverContext ctx, + Service service, + String methodName, + Message request) { + LOG.debug("preEndpointInvocation not implemented! {}/{}", methodName, request); + return request; } @Override - public void preClearDeadServers(ObserverContext ctx) { - LOG.debug("preClearDeadServers not implemented!"); + public void postEndpointInvocation( + ObserverContext ctx, + Service service, + String methodName, + Message request, + Message.Builder responseBuilder) { + LOG.debug("postEndpointInvocation not implemented! {}/{}", methodName, request); } @Override - public void preDecommissionRegionServers( + public void preRequestLock( ObserverContext ctx, - List servers, - boolean offload) { - LOG.debug("preDecommissionRegionServers not implemented!"); + String namespace, + TableName tableName, + RegionInfo[] regionInfos, + String description) { + LOG.debug("preRequestLock not implemented! {}/{}", tableName, regionInfos); } @Override - public void preListDecommissionedRegionServers( - ObserverContext ctx) { - LOG.debug("preListDecommissionedRegionServers not implemented!"); + public void preLockHeartbeat( + ObserverContext ctx, TableName tableName, String description) { + LOG.debug("preLockHeartbeat not implemented! {}/{}", tableName, description); } + /*********************************** Global admin operations ***********************************/ + @Override - public void preRecommissionRegionServer( - ObserverContext ctx, - ServerName server, - List encodedRegionNames) { - LOG.debug("preRecommissionRegionServer not implemented!"); + public void preAbortProcedure( + ObserverContext ctx, final long procId) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "abortProcedure", Action.ADMIN); + } + + @Override + public void postAbortProcedure(ObserverContext ctx) { + // There is nothing to do at this time after the procedure abort request was sent. } @Override - public void preStopRegionServer(ObserverContext ctx) { - LOG.debug("preStopRegionServer not implemented!"); + public void preGetProcedures(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "getProcedures", Action.ADMIN); } @Override - public void preRollWALWriterRequest(ObserverContext ctx) { - LOG.debug("preRollWALWriterRequest not implemented!"); + public void preGetLocks(ObserverContext ctx) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "getLocks", Action.ADMIN); + } + + @Override + public void preSetSplitOrMergeEnabled( + final ObserverContext ctx, + final boolean newValue, + final MasterSwitchType switchType) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "setSplitOrMergeEnabled", + Action.ADMIN); + } + + @Override + public void postStartMaster(ObserverContext ctx) { + // This would be used to create an ACL table if it does not already exist. + // We do not use an ACL table as all checks are routed to OPA. + } + + @Override + public void preRollWALWriterRequest(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "rollWALWriterRequest", Action.ADMIN); } @Override @@ -1092,16 +1198,20 @@ public void postRollWALWriterRequest(ObserverContext ctx, final String userName, - final GlobalQuotaSettings quotas) { - LOG.debug("preSetUserQuota not implemented!"); + final GlobalQuotaSettings quotas) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setUserQuota", Action.ADMIN); } @Override public void preSetRegionServerQuota( ObserverContext ctx, final String regionServer, - GlobalQuotaSettings quotas) { - LOG.debug("preSetRegionServerQuota not implemented!"); + GlobalQuotaSettings quotas) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setRegionServerQuota", Action.ADMIN); } @Override @@ -1111,81 +1221,111 @@ public ReplicationEndpoint postCreateReplicationEndPoint( } @Override - public void preReplicateLogEntries(ObserverContext ctx) { - LOG.debug("preReplicateLogEntries not implemented!"); + public void preReplicateLogEntries(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "replicateLogEntries", Action.WRITE); } @Override - public void preClearCompactionQueues(ObserverContext ctx) { - LOG.debug("preClearCompactionQueues not implemented!"); + public void preClearCompactionQueues(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "clearCompactionQueues", Action.ADMIN); } @Override public void preAddReplicationPeer( final ObserverContext ctx, String peerId, - ReplicationPeerConfig peerConfig) { - LOG.debug("preAddReplicationPeer not implemented!"); + ReplicationPeerConfig peerConfig) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "addReplicationPeer", Action.ADMIN); } @Override public void preRemoveReplicationPeer( - final ObserverContext ctx, String peerId) { - LOG.debug("preRemoveReplicationPeer not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "removeReplicationPeer", Action.ADMIN); } @Override public void preEnableReplicationPeer( - final ObserverContext ctx, String peerId) { - LOG.debug("preEnableReplicationPeer not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "enableReplicationPeer", Action.ADMIN); } @Override public void preDisableReplicationPeer( - final ObserverContext ctx, String peerId) { - LOG.debug("preDisableReplicationPeer not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "disableReplicationPeer", + Action.ADMIN); } @Override public void preGetReplicationPeerConfig( - final ObserverContext ctx, String peerId) { - LOG.debug("preGetReplicationPeerConfig not implemented!"); + final ObserverContext ctx, String peerId) throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "getReplicationPeerConfig", + Action.ADMIN); } @Override public void preUpdateReplicationPeerConfig( final ObserverContext ctx, String peerId, - ReplicationPeerConfig peerConfig) { - LOG.debug("preUpdateReplicationPeerConfig not implemented!"); + ReplicationPeerConfig peerConfig) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "updateReplicationPeerConfig", + Action.ADMIN); } @Override public void preListReplicationPeers( - final ObserverContext ctx, String regex) { - LOG.debug("preListReplicationPeers not implemented!"); + final ObserverContext ctx, String regex) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "listReplicationPeers", Action.ADMIN); } @Override public void preExecuteProcedures(ObserverContext ctx) { - LOG.debug("preExecuteProcedures not implemented!"); + // Not implemented: reference AC uses checkSystemOrSuperUser, a superuser mechanism + // not applicable to OPA-based authorization. } @Override public void preSwitchRpcThrottle( - ObserverContext ctx, boolean enable) { - LOG.debug("preSwitchRpcThrottle not implemented!"); + ObserverContext ctx, boolean enable) throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "switchRpcThrottle", Action.ADMIN); } @Override - public void preIsRpcThrottleEnabled(ObserverContext ctx) { - LOG.debug("preIsRpcThrottleEnabled not implemented!"); + public void preIsRpcThrottleEnabled(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "isRpcThrottleEnabled", Action.ADMIN); } @Override public void preSwitchExceedThrottleQuota( - ObserverContext ctx, boolean enable) { - LOG.debug("preSwitchExceedThrottleQuota not implemented!"); + ObserverContext ctx, boolean enable) throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "switchExceedThrottleQuota", + Action.ADMIN); } @Override @@ -1193,13 +1333,13 @@ public void preGrant( ObserverContext ctx, UserPermission userPermission, boolean mergeExistingPermissions) { - LOG.debug("preGrant not implemented!"); + // Not implemented: permissions are managed in OPA, not via HBase ACL table operations. } @Override public void preRevoke( ObserverContext ctx, UserPermission userPermission) { - LOG.debug("preRevoke not implemented!"); + // Not implemented: permissions are managed in OPA, not via HBase ACL table operations. } @Override @@ -1207,23 +1347,35 @@ public void preHasUserPermissions( ObserverContext ctx, String userName, List permissions) { - LOG.debug("preHasUserPermissions not implemented!"); + // Not implemented: permission checks are routed to OPA directly. } @Override - public void preClearRegionBlockCache(ObserverContext ctx) { - LOG.debug("preClearRegionBlockCache not implemented!"); + public void preClearRegionBlockCache(ObserverContext ctx) + throws IOException { + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "clearRegionBlockCache", Action.ADMIN); } @Override public void preUpdateRegionServerConfiguration( - ObserverContext ctx, Configuration preReloadConf) { - LOG.debug("preUpdateRegionServerConfiguration not implemented!"); + ObserverContext ctx, Configuration preReloadConf) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "updateRegionServerConfiguration", + Action.ADMIN); } @Override public void preUpdateMasterConfiguration( - ObserverContext ctx, Configuration preReloadConf) { - LOG.debug("preUpdateMasterConfiguration not implemented!"); + ObserverContext ctx, Configuration preReloadConf) + throws IOException { + requirePermission( + ctx, + NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, + "updateMasterConfiguration", + Action.ADMIN); } } diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java index 9dac7f2..6f516c5 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java @@ -80,13 +80,20 @@ public OpaAclChecker( } } - public void checkPermissionInfo(User user, TableName table, Permission.Action action) + private void checkPermissionInfo(User user, TableName table, Permission.Action action) throws AccessControlException { OpaAllowQuery query = new OpaAllowQuery(new OpaAllowQuery.OpaAllowQueryInput(user.getUGI(), table, action)); this.checkPermissionInfo(query); } + public void checkPermissionInfo(User user, TableName table, Permission.Action... actions) + throws AccessControlException { + for (Permission.Action action : actions) { + checkPermissionInfo(user, table, action); + } + } + public void checkPermissionInfo(User user, String namespace, Permission.Action action) throws AccessControlException { OpaAllowQuery query = diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java index 7b69db0..bfabe2c 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java @@ -6,44 +6,81 @@ import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.createTable; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.deleteTable; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; import java.util.ArrayList; import java.util.List; -import java.util.Optional; -import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.NamespaceDescriptor; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.RegionInfoBuilder; +import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; import org.apache.hadoop.hbase.master.MasterCoprocessorHost; +import org.apache.hadoop.hbase.quotas.GlobalQuotaSettings; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.security.AccessControlException; -import org.junit.Rule; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; import org.junit.Test; public class TestOpenPolicyAgentAccessController extends TestUtils { public static final String OPA_URL = "http://localhost:8089"; - @Rule public WireMockRule wireMockRule = new WireMockRule(8089); + @ClassRule public static WireMockRule wireMockRule = new WireMockRule(8089); + + @BeforeClass + public static void setUpClass() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL); + } + + @Before + public void resetStubs() { + WireMock.reset(); + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + } + + @AfterClass + public static void tearDownClass() throws Exception { + tearDown(); + } + + // --- helpers --- + + private ObserverContext ctx() { + return ObserverContextImpl.createAndPrepare(CP_ENV); + } + + private OpenPolicyAgentAccessController getOpaController() { + MasterCoprocessorHost masterCpHost = + TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); + return masterCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + } + + // --- original tests (non-standard allow/deny patterns) --- @Test public void testCreateAndPut() throws Exception { LOG.info("testCreateAndPut - start"); - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL); - HTableDescriptor htd = getHTableDescriptor(); - createTable(TEST_UTIL, TEST_UTIL.getAdmin(), htd, new byte[][] {Bytes.toBytes("s")}); - // put some test data List puts = new ArrayList<>(100); for (int i = 0; i < 100; i++) { Put p = new Put(Bytes.toBytes(i)); @@ -54,21 +91,13 @@ public void testCreateAndPut() throws Exception { table.put(puts); deleteTable(TEST_UTIL, TEST_TABLE); - - tearDown(); LOG.info("testCreateAndPut - complete"); } @Test public void testDeniedCreate() throws Exception { LOG.info("testDeniedCreate - start"); - - // let all set-up calls succeed - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL); - try { - // re-stub so that any subsequent calls will fail stubFor(post("/").willReturn(ok().withBody("{\"result\": \"false\"}"))); HTableDescriptor htd = getHTableDescriptor(); createTable(TEST_UTIL, TEST_UTIL.getAdmin(), htd, new byte[][] {Bytes.toBytes("s")}); @@ -76,157 +105,574 @@ public void testDeniedCreate() throws Exception { } catch (AccessControlException e) { logOk(e); } - - tearDown(); LOG.info("testDeniedCreate - complete"); } @Test public void testDeniedCreateByUser() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL); - User userDenied = User.createUserForTesting(conf, "cannotCreateTables", new String[0]); - SecureTestUtil.AccessTestAction createTable = () -> { - HTableDescriptor htd = getHTableDescriptor(); - getOpaController() - .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + getOpaController().preCreateTable(ctx(), getHTableDescriptor(), null); return null; }; - - // re-stub so that the call fails for the given user stubFor( post("/") .withRequestBody( matchingJsonPath("$.input.callerUgi[?(@.userName == 'cannotCreateTables')]")) .willReturn(ok().withBody("{\"result\": \"false\"}"))); - try { userDenied.runAs(createTable); fail("AccessControlException should have been thrown"); } catch (AccessControlException e) { logOk(e); } - - tearDown(); } @Test - public void testDryRun() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL, true, false); - - User userDenied = User.createUserForTesting(conf, "cannotCreateTables", new String[0]); - - SecureTestUtil.AccessTestAction createTable = + public void testCreateNamespace() throws Exception { + User userCreater = User.createUserForTesting(conf, "nsCreator", new String[0]); + User userDenied = User.createUserForTesting(conf, "nsNonCreator", new String[0]); + SecureTestUtil.AccessTestAction createNamespace = () -> { - HTableDescriptor htd = getHTableDescriptor(); - getOpaController() - .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + NamespaceDescriptor nsd = NamespaceDescriptor.create("new_ns").build(); + getOpaController().preCreateNamespace(ctx(), nsd); return null; }; - - // re-stub so that the call would fail for the given user in *non*-dryRun mode + try { + userCreater.runAs(createNamespace); + } catch (AccessControlException e) { + throw new AssertionError("AccessControlException should not have been thrown", e); + } stubFor( post("/") - .withRequestBody( - matchingJsonPath("$.input.callerUgi[?(@.userName == 'cannotCreateTables')]")) + .withRequestBody(matchingJsonPath("$.input.callerUgi[?(@.userName == 'nsNonCreator')]")) .willReturn(ok().withBody("{\"result\": \"false\"}"))); - try { - userDenied.runAs(createTable); - LOG.info("Action runs as expected due to being in dryRun mode"); + userDenied.runAs(createNamespace); + fail("AccessControlException should have been thrown"); } catch (AccessControlException e) { - throw new AssertionError("AccessControlException should not have been thrown", e); + logOk(e); } + } - tearDown(); + // --- namespace hooks --- + + @Test + public void testPreDeleteNamespace() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDeleteNamespace(ctx(), "default"); + return null; + }); } @Test - public void testUseCache() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL, false, true); + public void testPreModifyNamespace() throws Exception { + NamespaceDescriptor nsd = NamespaceDescriptor.create("default").build(); + assertAllowedThenDenied( + () -> { + getOpaController().preModifyNamespace(ctx(), nsd); + return null; + }); + } - User userDenied = User.createUserForTesting(conf, "useCacheUser", new String[0]); + @Test + public void testPreGetNamespaceDescriptor() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetNamespaceDescriptor(ctx(), "default"); + return null; + }); + } - // create a table explicitly using the cache from the cp-processor on the master... - SecureTestUtil.AccessTestAction createTable = + // --- table DDL hooks --- + + @Test + public void testPreDeleteTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDeleteTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreEnableTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preEnableTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreDisableTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDisableTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreTruncateTable() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preTruncateTable(ctx(), TEST_TABLE); + return null; + }); + } + + @Test + public void testPreModifyTable() throws Exception { + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preModifyTable(ctx(), TEST_TABLE, td, td); + return null; + }); + } + + @Test + public void testPreModifyColumnFamilyStoreFileTracker() throws Exception { + assertAllowedThenDenied( () -> { - HTableDescriptor htd = getHTableDescriptor(); getOpaController() - .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + .preModifyColumnFamilyStoreFileTracker(ctx(), TEST_TABLE, TEST_FAMILY, "FILE"); return null; - }; + }); + } - try { - userDenied.runAs(createTable); - } catch (AccessControlException e) { - throw new AssertionError("AccessControlException should not have been thrown", e); - } + // --- flush / quota hooks --- - // we should have only a single entry for this user as subsequent calls will hit the cache - assertEquals(Optional.of(1L), getOpaController().getAclCacheSize()); + @Test + public void testPreTableFlush() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preTableFlush(ctx(), TEST_TABLE); + return null; + }); + } - tearDown(); + @Test + public void testPreSetUserQuotaTableScope() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController().preSetUserQuota(ctx(), "u", TEST_TABLE, quotas); + return null; + }); } @Test - public void testCreateNamespace() throws Exception { - stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); - setup(OpenPolicyAgentAccessController.class, false, OPA_URL, false, false); + public void testPreSetUserQuotaNamespaceScope() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController() + .preSetUserQuota(ctx(), "u", NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, quotas); + return null; + }); + } - User userCreater = User.createUserForTesting(conf, "nsCreator", new String[0]); - User userDenied = User.createUserForTesting(conf, "nsNonCreator", new String[0]); + // --- region assignment / snapshot / quota hooks (existing) --- - SecureTestUtil.AccessTestAction createNamespace = + @Test + public void testPreMove() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( () -> { - NamespaceDescriptor nsd = NamespaceDescriptor.create("new_ns").build(); - getOpaController().preCreateNamespace(ObserverContextImpl.createAndPrepare(CP_ENV), nsd); + getOpaController().preMove(ctx(), regionInfo, null, null); return null; - }; + }); + } - try { - userCreater.runAs(createNamespace); - } catch (AccessControlException e) { - throw new AssertionError("AccessControlException should not have been thrown", e); - } + @Test + public void testPreAssign() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preAssign(ctx(), regionInfo); + return null; + }); + } - // re-stub so that the call would fail for the given user in *non*-dryRun mode - stubFor( - post("/") - .withRequestBody(matchingJsonPath("$.input.callerUgi[?(@.userName == 'nsNonCreator')]")) - .willReturn(ok().withBody("{\"result\": \"false\"}"))); + @Test + public void testPreUnassign() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preUnassign(ctx(), regionInfo); + return null; + }); + } - try { - userDenied.runAs(createNamespace); - fail("AccessControlException should have been thrown"); - } catch (AccessControlException e) { - logOk(e); - } + @Test + public void testPreRegionOffline() throws Exception { + RegionInfo regionInfo = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preRegionOffline(ctx(), regionInfo); + return null; + }); + } - tearDown(); + @Test + public void testPreSplitRegion() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSplitRegion(ctx(), TEST_TABLE, null); + return null; + }); + } + + @Test + public void testPreMergeRegions() throws Exception { + RegionInfo ri = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preMergeRegions(ctx(), new RegionInfo[] {ri, ri}); + return null; + }); + } + + @Test + public void testPreModifyTableStoreFileTracker() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preModifyTableStoreFileTracker(ctx(), TEST_TABLE, "FILE"); + return null; + }); } - private static void logOk(AccessControlException e) { - LOG.info("AccessControlException as expected: [{}]", e.getMessage()); + @Test + public void testPreSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preSnapshot(ctx(), snap, td); + return null; + }); } - private static HTableDescriptor getHTableDescriptor() { - HTableDescriptor htd = new HTableDescriptor(TEST_TABLE); - HColumnDescriptor hcd = new HColumnDescriptor(TEST_FAMILY); - hcd.setMaxVersions(100); - htd.addFamily(hcd); - htd.setOwner(USER_OWNER); + @Test + public void testPreListSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + assertAllowedThenDenied( + () -> { + getOpaController().preListSnapshot(ctx(), snap); + return null; + }); + } + + @Test + public void testPreCloneSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preCloneSnapshot(ctx(), snap, td); + return null; + }); + } - return htd; + @Test + public void testPreRestoreSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + TableDescriptor td = getHTableDescriptor(); + assertAllowedThenDenied( + () -> { + getOpaController().preRestoreSnapshot(ctx(), snap, td); + return null; + }); } - private OpenPolicyAgentAccessController getOpaController() { - MasterCoprocessorHost masterCpHost = - TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); - return masterCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + @Test + public void testPreDeleteSnapshot() throws Exception { + SnapshotDescription snap = new SnapshotDescription("snap", TEST_TABLE); + assertAllowedThenDenied( + () -> { + getOpaController().preDeleteSnapshot(ctx(), snap); + return null; + }); + } + + @Test + public void testPreSetTableQuota() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController().preSetTableQuota(ctx(), TEST_TABLE, quotas); + return null; + }); + } + + @Test + public void testPreSetNamespaceQuota() throws Exception { + GlobalQuotaSettings quotas = null; + assertAllowedThenDenied( + () -> { + getOpaController() + .preSetNamespaceQuota(ctx(), NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, quotas); + return null; + }); + } + + @Test + public void testPreGetUserPermissionsTableScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetUserPermissions(ctx(), "u", null, TEST_TABLE, null, null); + return null; + }); + } + + @Test + public void testPreGetUserPermissionsNamespaceScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController() + .preGetUserPermissions( + ctx(), "u", NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, null, null, null); + return null; + }); + } + + @Test + public void testPreBalance() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preBalance(ctx(), BalanceRequest.defaultInstance()); + return null; + }); + } + + @Test + public void testPreBalanceSwitch() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preBalanceSwitch(ctx(), true); + return null; + }); + } + + @Test + public void testPreShutdown() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preShutdown(ctx()); + return null; + }); + } + + @Test + public void testPreStopMaster() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preStopMaster(ctx()); + return null; + }); + } + + @Test + public void testPreClearDeadServers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preClearDeadServers(ctx()); + return null; + }); + } + + @Test + public void testPreDecommissionRegionServers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDecommissionRegionServers(ctx(), List.of(), false); + return null; + }); + } + + @Test + public void testPreListDecommissionedRegionServers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preListDecommissionedRegionServers(ctx()); + return null; + }); + } + + @Test + public void testPreRecommissionRegionServer() throws Exception { + ServerName serverName = ServerName.valueOf("localhost", 16010, 12345L); + assertAllowedThenDenied( + () -> { + getOpaController().preRecommissionRegionServer(ctx(), serverName, List.of()); + return null; + }); + } + + // --- procedure / lock hooks --- + + @Test + public void testPreAbortProcedure() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preAbortProcedure(ctx(), 1L); + return null; + }); + } + + @Test + public void testPreGetProcedures() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetProcedures(ctx()); + return null; + }); + } + + @Test + public void testPreGetLocks() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetLocks(ctx()); + return null; + }); + } + + @Test + public void testPreSetSplitOrMergeEnabled() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSetSplitOrMergeEnabled(ctx(), true, MasterSwitchType.SPLIT); + return null; + }); + } + + // --- quota hooks (global scope) --- + + @Test + public void testPreSetUserQuotaGlobalScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSetUserQuota(ctx(), "u", (GlobalQuotaSettings) null); + return null; + }); + } + + @Test + public void testPreSetRegionServerQuota() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSetRegionServerQuota(ctx(), "rs1", null); + return null; + }); + } + + // --- replication peer hooks --- + + @Test + public void testPreAddReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preAddReplicationPeer(ctx(), "peer1", null); + return null; + }); + } + + @Test + public void testPreRemoveReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preRemoveReplicationPeer(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreEnableReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preEnableReplicationPeer(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreDisableReplicationPeer() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preDisableReplicationPeer(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreGetReplicationPeerConfig() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preGetReplicationPeerConfig(ctx(), "peer1"); + return null; + }); + } + + @Test + public void testPreUpdateReplicationPeerConfig() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preUpdateReplicationPeerConfig(ctx(), "peer1", null); + return null; + }); + } + + @Test + public void testPreListReplicationPeers() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preListReplicationPeers(ctx(), ".*"); + return null; + }); + } + + // --- throttle hooks --- + + @Test + public void testPreSwitchRpcThrottle() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSwitchRpcThrottle(ctx(), true); + return null; + }); + } + + @Test + public void testPreIsRpcThrottleEnabled() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preIsRpcThrottleEnabled(ctx()); + return null; + }); + } + + @Test + public void testPreSwitchExceedThrottleQuota() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preSwitchExceedThrottleQuota(ctx(), true); + return null; + }); + } + + // --- configuration hooks --- + + @Test + public void testPreUpdateMasterConfiguration() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preUpdateMasterConfiguration(ctx(), conf); + return null; + }); } } diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java new file mode 100644 index 0000000..913ee71 --- /dev/null +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -0,0 +1,407 @@ +package tech.stackable.hbase; + +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static org.apache.hadoop.hbase.security.access.SecureTestUtil.createTable; +import static org.apache.hadoop.hbase.security.access.SecureTestUtil.deleteTable; +import static org.junit.Assert.fail; + +import com.github.tomakehurst.wiremock.client.WireMock; +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import org.apache.hadoop.hbase.CompareOperator; +import org.apache.hadoop.hbase.Coprocessor; +import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.Delete; +import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.client.Get; +import org.apache.hadoop.hbase.client.Increment; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.coprocessor.ObserverContext; +import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; +import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.RegionServerCoprocessorEnvironment; +import org.apache.hadoop.hbase.regionserver.HRegion; +import org.apache.hadoop.hbase.regionserver.HRegionServer; +import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; +import org.apache.hadoop.hbase.regionserver.RegionServerCoprocessorHost; +import org.apache.hadoop.hbase.regionserver.ScanType; +import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.SecureTestUtil; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.security.AccessControlException; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; + +public class TestOpenPolicyAgentAccessControllerRegion extends TestUtils { + public static final String OPA_URL = "http://localhost:8089"; + + private static final byte[] TEST_ROW = Bytes.toBytes("testRow"); + private static RegionCoprocessorEnvironment REGION_CP_ENV; + private static RegionServerCoprocessorEnvironment RS_CP_ENV; + + @ClassRule public static WireMockRule wireMockRule = new WireMockRule(8089); + + @BeforeClass + public static void setUpClass() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL); + + createTable( + TEST_UTIL, TEST_UTIL.getAdmin(), getHTableDescriptor(), new byte[][] {Bytes.toBytes("s")}); + + HRegion region = TEST_UTIL.getHBaseCluster().getRegions(TEST_TABLE).get(0); + RegionCoprocessorHost rcpHost = region.getCoprocessorHost(); + OpenPolicyAgentAccessController regionController = + rcpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + REGION_CP_ENV = + (RegionCoprocessorEnvironment) + rcpHost.createEnvironment(regionController, Coprocessor.PRIORITY_HIGHEST, 1, conf); + + HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0); + RegionServerCoprocessorHost rsCpHost = rs.getRegionServerCoprocessorHost(); + OpenPolicyAgentAccessController rsController = + rsCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + RS_CP_ENV = + (RegionServerCoprocessorEnvironment) + rsCpHost.createEnvironment(rsController, Coprocessor.PRIORITY_HIGHEST, 1, conf); + } + + @Before + public void resetStubs() { + WireMock.reset(); + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + } + + @AfterClass + public static void tearDownClass() throws Exception { + deleteTable(TEST_UTIL, TEST_TABLE); + tearDown(); + } + + // --- helpers --- + + private ObserverContext regionCtx() { + return ObserverContextImpl.createAndPrepare(REGION_CP_ENV); + } + + private ObserverContext rsCtx() { + return ObserverContextImpl.createAndPrepare(RS_CP_ENV); + } + + private OpenPolicyAgentAccessController getRegionController() { + HRegion region = TEST_UTIL.getHBaseCluster().getRegions(TEST_TABLE).get(0); + return region.getCoprocessorHost().findCoprocessor(OpenPolicyAgentAccessController.class); + } + + /** Stubs a deny for writeOnlyUser when action is READ, to verify READ is required. */ + private void stubDenyReadForWriteOnlyUser() { + stubFor( + post("/") + .withRequestBody( + WireMock.matchingJsonPath("$.input.callerUgi[?(@.userName == 'writeOnlyUser')]")) + .withRequestBody(WireMock.matchingJsonPath("$.input[?(@.action == 'READ')]")) + .willReturn(ok().withBody("{\"result\": \"false\"}"))); + } + + // --- read hooks --- + + @Test + public void testPreGetOp() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preGetOp(regionCtx(), new Get(TEST_ROW), null); + return null; + }); + } + + @Test + public void testPreExists() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preExists(regionCtx(), new Get(TEST_ROW), false); + return null; + }); + } + + @Test + public void testPreScannerOpen() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preScannerOpen(regionCtx(), new Scan()); + return null; + }); + } + + // --- write hooks --- + + @Test + public void testPrePut() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController() + .prePut(regionCtx(), new Put(TEST_ROW), null, Durability.USE_DEFAULT); + return null; + }); + } + + @Test + public void testPreDelete() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController() + .preDelete(regionCtx(), new Delete(TEST_ROW), null, Durability.USE_DEFAULT); + return null; + }); + } + + @Test + public void testPreBatchMutate() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preBatchMutate(regionCtx(), null); + return null; + }); + } + + @Test + public void testPreFlush() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preFlush(regionCtx(), null); + return null; + }); + } + + @Test + public void testPreCompact() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preCompact(regionCtx(), null, null, ScanType.USER_SCAN, null, null); + return null; + }); + } + + // --- read+write hooks (require both WRITE and READ) --- + + @Test + public void testPreAppend() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preAppend(regionCtx(), new Append(TEST_ROW)); + return null; + }); + } + + @Test + public void testPreAppendRequiresRead() throws Exception { + User writeOnlyUser = User.createUserForTesting(conf, "writeOnlyUser", new String[0]); + stubDenyReadForWriteOnlyUser(); + SecureTestUtil.AccessTestAction action = + () -> { + getRegionController().preAppend(regionCtx(), new Append(TEST_ROW)); + return null; + }; + try { + writeOnlyUser.runAs(action); + fail("AccessControlException should have been thrown"); + } catch (AccessControlException e) { + logOk(e); + } + } + + @Test + public void testPreIncrement() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preIncrement(regionCtx(), new Increment(TEST_ROW)); + return null; + }); + } + + @Test + public void testPreIncrementRequiresRead() throws Exception { + User writeOnlyUser = User.createUserForTesting(conf, "writeOnlyUser", new String[0]); + stubDenyReadForWriteOnlyUser(); + SecureTestUtil.AccessTestAction action = + () -> { + getRegionController().preIncrement(regionCtx(), new Increment(TEST_ROW)); + return null; + }; + try { + writeOnlyUser.runAs(action); + fail("AccessControlException should have been thrown"); + } catch (AccessControlException e) { + logOk(e); + } + } + + @Test + public void testPreCheckAndPut() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndPut( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + put, + false); + return null; + }); + } + + @Test + public void testPreCheckAndPutRequiresRead() throws Exception { + User writeOnlyUser = User.createUserForTesting(conf, "writeOnlyUser", new String[0]); + stubDenyReadForWriteOnlyUser(); + Put put = new Put(TEST_ROW); + SecureTestUtil.AccessTestAction action = + () -> { + getRegionController() + .preCheckAndPut( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + put, + false); + return null; + }; + try { + writeOnlyUser.runAs(action); + fail("AccessControlException should have been thrown"); + } catch (AccessControlException e) { + logOk(e); + } + } + + @Test + public void testPreCheckAndPutAfterRowLock() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndPutAfterRowLock( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + put, + false); + return null; + }); + } + + @Test + public void testPreCheckAndDelete() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDelete( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + delete, + false); + return null; + }); + } + + @Test + public void testPreCheckAndDeleteAfterRowLock() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDeleteAfterRowLock( + regionCtx(), + TEST_ROW, + TEST_FAMILY, + TEST_QUALIFIER, + CompareOperator.EQUAL, + null, + delete, + false); + return null; + }); + } + + // --- RegionServer hooks --- + + private OpenPolicyAgentAccessController getRsController() { + HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0); + return rs.getRegionServerCoprocessorHost() + .findCoprocessor(OpenPolicyAgentAccessController.class); + } + + @Test + public void testPreRollWALWriterRequest() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preRollWALWriterRequest(rsCtx()); + return null; + }); + } + + @Test + public void testPreReplicateLogEntries() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preReplicateLogEntries(rsCtx()); + return null; + }); + } + + @Test + public void testPreClearCompactionQueues() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preClearCompactionQueues(rsCtx()); + return null; + }); + } + + @Test + public void testPreClearRegionBlockCache() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preClearRegionBlockCache(rsCtx()); + return null; + }); + } + + @Test + public void testPreUpdateRegionServerConfiguration() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preUpdateRegionServerConfiguration(rsCtx(), conf); + return null; + }); + } + + @Test + public void testPreStopRegionServer() throws Exception { + assertAllowedThenDenied( + () -> { + getRsController().preStopRegionServer(rsCtx()); + return null; + }); + } +} diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java new file mode 100644 index 0000000..6d19bde --- /dev/null +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java @@ -0,0 +1,91 @@ +package tech.stackable.hbase; + +import static com.github.tomakehurst.wiremock.client.WireMock.matchingJsonPath; +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static org.junit.Assert.assertEquals; + +import com.github.tomakehurst.wiremock.junit.WireMockRule; +import java.util.Optional; +import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; +import org.apache.hadoop.hbase.master.MasterCoprocessorHost; +import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.SecureTestUtil; +import org.apache.hadoop.security.AccessControlException; +import org.junit.Rule; +import org.junit.Test; + +/** + * Tests for non-default coprocessor configurations (dryRun, cache). Each test manages its own + * mini-cluster lifecycle since the coprocessor config differs per test. + */ +public class TestOpenPolicyAgentAccessControllerVariants extends TestUtils { + public static final String OPA_URL = "http://localhost:8089"; + + @Rule public WireMockRule wireMockRule = new WireMockRule(8089); + + @Test + public void testDryRun() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL, true, false); + + User userDenied = User.createUserForTesting(conf, "cannotCreateTables", new String[0]); + + SecureTestUtil.AccessTestAction createTable = + () -> { + HTableDescriptor htd = getHTableDescriptor(); + getOpaController() + .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + return null; + }; + + stubFor( + post("/") + .withRequestBody( + matchingJsonPath("$.input.callerUgi[?(@.userName == 'cannotCreateTables')]")) + .willReturn(ok().withBody("{\"result\": \"false\"}"))); + + try { + userDenied.runAs(createTable); + LOG.info("Action runs as expected due to being in dryRun mode"); + } catch (AccessControlException e) { + throw new AssertionError("AccessControlException should not have been thrown", e); + } + + tearDown(); + } + + @Test + public void testUseCache() throws Exception { + stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); + setup(OpenPolicyAgentAccessController.class, false, OPA_URL, false, true); + + User userDenied = User.createUserForTesting(conf, "useCacheUser", new String[0]); + + SecureTestUtil.AccessTestAction createTable = + () -> { + HTableDescriptor htd = getHTableDescriptor(); + getOpaController() + .preCreateTable(ObserverContextImpl.createAndPrepare(CP_ENV), htd, null); + return null; + }; + + try { + userDenied.runAs(createTable); + } catch (AccessControlException e) { + throw new AssertionError("AccessControlException should not have been thrown", e); + } + + assertEquals(Optional.of(1L), getOpaController().getAclCacheSize()); + + tearDown(); + } + + private OpenPolicyAgentAccessController getOpaController() { + MasterCoprocessorHost masterCpHost = + TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost(); + return masterCpHost.findCoprocessor(OpenPolicyAgentAccessController.class); + } +} diff --git a/src/test/java/tech/stackable/hbase/TestUtils.java b/src/test/java/tech/stackable/hbase/TestUtils.java index 54e476d..2cf25a1 100644 --- a/src/test/java/tech/stackable/hbase/TestUtils.java +++ b/src/test/java/tech/stackable/hbase/TestUtils.java @@ -1,5 +1,9 @@ package tech.stackable.hbase; +import static com.github.tomakehurst.wiremock.client.WireMock.matchingJsonPath; +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static org.apache.hadoop.hbase.AuthUtil.toGroupEntry; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.*; import static org.junit.Assert.*; @@ -26,6 +30,7 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.*; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.security.AccessControlException; import org.apache.hadoop.security.UserGroupInformation; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -59,6 +64,9 @@ public class TestUtils { protected static final String GROUP_READ = "group_read"; protected static final String GROUP_WRITE = "group_write"; + protected static User ALLOWED_USER; + protected static User DENIED_USER; + protected static User USER_GROUP_ADMIN; protected static User USER_GROUP_CREATE; protected static User USER_GROUP_READ; @@ -158,6 +166,9 @@ protected static void setup( User.createUserForTesting(conf, "user_group_write", new String[] {GROUP_WRITE}); systemUserConnection = TEST_UTIL.getConnection(); + + ALLOWED_USER = User.createUserForTesting(conf, "allowedUser", new String[0]); + DENIED_USER = User.createUserForTesting(conf, "deniedUser", new String[0]); } protected static void setUpTables() throws Exception { @@ -232,6 +243,24 @@ protected static void setUpTables() throws Exception { assertEquals(5, size); } + protected static void logOk(AccessControlException e) { + LOG.info("AccessControlException as expected: [{}]", e.getMessage()); + } + + protected void assertAllowedThenDenied(SecureTestUtil.AccessTestAction action) throws Exception { + ALLOWED_USER.runAs(action); + stubFor( + post("/") + .withRequestBody(matchingJsonPath("$.input.callerUgi[?(@.userName == 'deniedUser')]")) + .willReturn(ok().withBody("{\"result\": \"false\"}"))); + try { + DENIED_USER.runAs(action); + fail("AccessControlException should have been thrown"); + } catch (AccessControlException e) { + logOk(e); + } + } + protected static void tearDown() throws Exception { TEST_UTIL.shutdownMiniCluster(); } @@ -537,4 +566,13 @@ protected void createTestTable(TableName tname, byte[] cf) throws Exception { htd.setOwner(USER_OWNER); createTable(TEST_UTIL, TEST_UTIL.getAdmin(), htd, new byte[][] {Bytes.toBytes("s")}); } + + protected static HTableDescriptor getHTableDescriptor() { + HTableDescriptor htd = new HTableDescriptor(TEST_TABLE); + HColumnDescriptor hcd = new HColumnDescriptor(TEST_FAMILY); + hcd.setMaxVersions(100); + htd.addFamily(hcd); + htd.setOwner(USER_OWNER); + return htd; + } } From 770172ca508ecb38fd6c4183da05ae3402d5abbc Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 10:42:31 +0100 Subject: [PATCH 02/29] add claude.md to gitignore and reduce log level to trace for high frequency calls --- .gitignore | 3 ++- .../stackable/hbase/OpenPolicyAgentAccessController.java | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 579d291..32ac77f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /target/ -/dependency-reduced-pom.xml \ No newline at end of file +/dependency-reduced-pom.xml +/CLAUDE.md \ No newline at end of file diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index bf5f207..a9fa78e 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -249,6 +249,7 @@ public void preGetOp( throws IOException { User user = getActiveUser(c); TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + LOG.trace("preGetOp: user [{}] on table [{}] with get [{}]", user, tableName, get); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { return; @@ -1106,7 +1107,7 @@ public Message preEndpointInvocation( Service service, String methodName, Message request) { - LOG.debug("preEndpointInvocation not implemented! {}/{}", methodName, request); + LOG.trace("preEndpointInvocation not implemented! {}/{}", methodName, request); return request; } @@ -1117,7 +1118,7 @@ public void postEndpointInvocation( String methodName, Message request, Message.Builder responseBuilder) { - LOG.debug("postEndpointInvocation not implemented! {}/{}", methodName, request); + LOG.trace("postEndpointInvocation not implemented! {}/{}", methodName, request); } @Override @@ -1127,13 +1128,13 @@ public void preRequestLock( TableName tableName, RegionInfo[] regionInfos, String description) { - LOG.debug("preRequestLock not implemented! {}/{}", tableName, regionInfos); + LOG.trace("preRequestLock not implemented! {}/{}", tableName, regionInfos); } @Override public void preLockHeartbeat( ObserverContext ctx, TableName tableName, String description) { - LOG.debug("preLockHeartbeat not implemented! {}/{}", tableName, description); + LOG.trace("preLockHeartbeat not implemented! {}/{}", tableName, description); } /*********************************** Global admin operations ***********************************/ From 64a3458042a660bbfb2234546b4acc5e39cb445b Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 10:59:19 +0100 Subject: [PATCH 03/29] bump to hbase 2.6.4 --- pom.xml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 29cb440..8954f58 100644 --- a/pom.xml +++ b/pom.xml @@ -52,9 +52,9 @@ 3.2.5 2.43.0 - 2.6.0 + 2.6.4 2.5.0 - 2.8.1 + 2.8.8 @@ -106,6 +106,12 @@ hbase-testing-util ${hbase.version} test + + + org.apache.directory.jdbm + apacheds-jdbm1 + + org.slf4j From dcae7d8d0d5fa88432546003acf39eaca7270b47 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 11:21:57 +0100 Subject: [PATCH 04/29] check default namespace permissions on namespace actions --- .../hbase/OpenPolicyAgentAccessController.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index a9fa78e..a8f477d 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -152,7 +152,8 @@ public void preCreateNamespace( ObserverContext c, NamespaceDescriptor ns) throws IOException { User user = getActiveUser(c); LOG.debug("preCreateNamespace: user [{}]", user); - opaAclChecker.checkPermissionInfo(user, ns.getName(), Action.ADMIN); + opaAclChecker.checkPermissionInfo( + user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); } @Override @@ -160,7 +161,8 @@ public void preDeleteNamespace(ObserverContext c, throws IOException { User user = getActiveUser(c); LOG.debug("preDeleteNamespace: user [{}]", user); - opaAclChecker.checkPermissionInfo(user, namespace, Action.ADMIN); + opaAclChecker.checkPermissionInfo( + user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); } @Override @@ -168,7 +170,8 @@ public void preModifyNamespace( ObserverContext c, NamespaceDescriptor ns) throws IOException { User user = getActiveUser(c); LOG.debug("preModifyNamespace: user [{}]", user); - opaAclChecker.checkPermissionInfo(user, ns.getName(), Action.ADMIN); + opaAclChecker.checkPermissionInfo( + user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); } @Override @@ -946,7 +949,8 @@ public void preSetUserQuota( final String namespace, final GlobalQuotaSettings quotas) throws IOException { - requirePermission(ctx, namespace, "setUserNamespaceQuota", Action.ADMIN); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setUserNamespaceQuota", Action.ADMIN); } @Override @@ -964,7 +968,8 @@ public void preSetNamespaceQuota( final String namespace, final GlobalQuotaSettings quotas) throws IOException { - requirePermission(ctx, namespace, "setNamespaceQuota", Action.ADMIN); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setNamespaceQuota", Action.ADMIN); } @Override From dae877b1ac7f7b5a86fddf09d61c51e0e42d911c Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 11:50:16 +0100 Subject: [PATCH 05/29] added reflection test to confirm coverage --- .../TestCoprocessorInterfaceCoverage.java | 209 ++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java diff --git a/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java new file mode 100644 index 0000000..2339113 --- /dev/null +++ b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java @@ -0,0 +1,209 @@ +package tech.stackable.hbase; + +import static org.junit.Assert.assertTrue; + +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.hadoop.hbase.coprocessor.BulkLoadObserver; +import org.apache.hadoop.hbase.coprocessor.EndpointObserver; +import org.apache.hadoop.hbase.coprocessor.MasterObserver; +import org.apache.hadoop.hbase.coprocessor.RegionObserver; +import org.apache.hadoop.hbase.coprocessor.RegionServerObserver; +import org.junit.Test; + +/** + * Verifies that every method in the coprocessor observer interfaces is either explicitly overridden + * in OpenPolicyAgentAccessController or listed in the known exclusion set below. + * + *

Because all HBase observer interface methods have default implementations, adding a new hook + * upstream would not cause a compile error. This test catches that: any new method that appears in + * an observer interface and is not in our class or in EXCLUDED will cause a test failure. + * + *

When upgrading HBase, if this test fails, review the new method(s) and either: + * + *

    + *
  • Implement them in OpenPolicyAgentAccessController, or + *
  • Add them to the appropriate section of EXCLUDED with a justification comment. + *
+ * + *

Note: all {@code post*} methods are excluded automatically — they fire after the operation has + * already been permitted and cannot block it, so OPA enforcement is never applicable. + */ +public class TestCoprocessorInterfaceCoverage { + + private static final Class[] OBSERVER_INTERFACES = { + MasterObserver.class, + RegionObserver.class, + RegionServerObserver.class, + EndpointObserver.class, + BulkLoadObserver.class, + }; + + /** + * Pre-hooks that are deliberately not overridden. Organised by reason. When a new HBase version + * adds a method that belongs here, add it with a comment explaining why. + */ + private static final Set EXCLUDED = + new HashSet<>( + Arrays.asList( + + // --- deprecated overloads superseded by variants we do override --- + // The WALEdit-carrying variants of data-write hooks were deprecated in HBase 2.x; + // the 2-arg versions (which we do override) are the current API. + "preAppend(ObserverContext, Append, WALEdit)", + "preDelete(ObserverContext, Delete, WALEdit)", + "preIncrement(ObserverContext, Increment, WALEdit)", + "prePut(ObserverContext, Put, WALEdit)", + // Old single-descriptor overload; we override the 4-arg (old + new) variant. + "preModifyTable(ObserverContext, TableName, TableDescriptor)", + // Old 2-descriptor namespace overload; we override the 2-arg (new descriptor only) + // variant. + "preModifyNamespace(ObserverContext, NamespaceDescriptor, NamespaceDescriptor)", + // Old 3-arg unassign with boolean; we override the 2-arg variant. + "preUnassign(ObserverContext, RegionInfo, boolean)", + + // --- after-row-lock variants where we check at the pre-lock level --- + // HBase calls the pre-lock hook before acquiring the row lock and the after-lock hook + // after. + // We enforce permissions at the pre-lock level (preAppend, preIncrement), so the + // after-lock + // variants are redundant for OPA authorization. + "preAppendAfterRowLock(ObserverContext, Append)", + "preIncrementAfterRowLock(ObserverContext, Increment)", + + // --- internal HBase multi-step DDL action hooks --- + // These are called internally by the HBase master during multi-step DDL procedures. + // They are not triggered by direct client calls; the public pre* hooks (e.g. + // preCreateTable) + // are already checked before these fire. + "preCreateTableAction(ObserverContext, TableDescriptor, RegionInfo[])", + "preCreateTableRegionsInfos(ObserverContext, TableDescriptor)", + "preDeleteTableAction(ObserverContext, TableName)", + "preEnableTableAction(ObserverContext, TableName)", + "preDisableTableAction(ObserverContext, TableName)", + "preTruncateTableAction(ObserverContext, TableName)", + "preTruncateRegion(ObserverContext, RegionInfo)", + "preTruncateRegionAction(ObserverContext, RegionInfo)", + "preModifyTableAction(ObserverContext, TableName, TableDescriptor)", + "preModifyTableAction(ObserverContext, TableName, TableDescriptor, TableDescriptor)", + "preMergeRegionsAction(ObserverContext, RegionInfo[])", + "preMergeRegionsCommitAction(ObserverContext, RegionInfo[], List)", + "preSplitRegionAction(ObserverContext, TableName, byte[])", + "preSplitRegionBeforeMETAAction(ObserverContext, byte[], List)", + "preSplitRegionAfterMETAAction(ObserverContext)", + + // --- internal storage, compaction, and scan hooks --- + // These are called by HBase's internal storage engine for compaction, flush, and scan + // operations. They are not user-initiated and carry no meaningful authorization + // context. + "preClose(ObserverContext, boolean)", + "preCommitStoreFile(ObserverContext, byte[], List)", + "preCompactScannerOpen(ObserverContext, Store, ScanType, ScanOptions, CompactionLifeCycleTracker, CompactionRequest)", + "preCompactSelection(ObserverContext, Store, List, CompactionLifeCycleTracker)", + "preFlush(ObserverContext, Store, InternalScanner, FlushLifeCycleTracker)", + "preFlushScannerOpen(ObserverContext, Store, ScanOptions, FlushLifeCycleTracker)", + "preMemStoreCompaction(ObserverContext, Store)", + "preMemStoreCompactionCompact(ObserverContext, Store, InternalScanner)", + "preMemStoreCompactionCompactScannerOpen(ObserverContext, Store, ScanOptions)", + "prePrepareTimeStampForDeleteVersion(ObserverContext, Mutation, Cell, byte[], Get)", + "preStoreFileReaderOpen(ObserverContext, FileSystem, Path, FSDataInputStreamWrapper, long, CacheConfig, Reference, StoreFileReader)", + "preStoreScannerOpen(ObserverContext, Store, ScanOptions)", + + // --- WAL, replication, and master lifecycle hooks --- + // Triggered by HBase internals (WAL writers, replication pipeline, master startup), + // not by user requests. + "preMasterInitialization(ObserverContext)", + "preMasterStoreFlush(ObserverContext)", + "preReplayWALs(ObserverContext, RegionInfo, Path)", + "preWALAppend(ObserverContext, WALKey, WALEdit)", + "preWALRestore(ObserverContext, RegionInfo, WALKey, WALEdit)", + "preReplicationSinkBatchMutate(ObserverContext, WALEntry, Mutation)", + + // --- RSGroup management --- + // RSGroups are an optional HBase feature for grouping RegionServers. Not yet + // implemented. + // All RSGroup operations should require ADMIN when implemented. + "preAddRSGroup(ObserverContext, String)", + "preRemoveRSGroup(ObserverContext, String)", + "preBalanceRSGroup(ObserverContext, String, BalanceRequest)", + "preGetRSGroupInfo(ObserverContext, String)", + "preGetRSGroupInfoOfServer(ObserverContext, Address)", + "preGetRSGroupInfoOfTable(ObserverContext, TableName)", + "preListRSGroups(ObserverContext)", + "preMoveServers(ObserverContext, Set, String)", + "preMoveServersAndTables(ObserverContext, Set, Set, String)", + "preMoveTables(ObserverContext, Set, String)", + "preRemoveServers(ObserverContext, Set)", + "preRenameRSGroup(ObserverContext, String, String)", + "preUpdateRSGroupConfig(ObserverContext, String, Map)", + + // --- TODO: genuine gaps that need OPA implementation --- + // These pre-hooks are user-facing and should enforce OPA permissions, but are not yet + // implemented. They are excluded here to keep this test focused on detecting new + // upstream + // methods; the implementation gaps are tracked separately in plan.md (Step 2+). + // + // preCheckAndMutate / preCheckAndMutateAfterRowLock: generalisation of + // checkAndPut/Delete; + // should require WRITE + READ (same as other check-and-* operations). + "preCheckAndMutate(ObserverContext, CheckAndMutate, CheckAndMutateResult)", + "preCheckAndMutateAfterRowLock(ObserverContext, CheckAndMutate, CheckAndMutateResult)", + // Filter-based overloads of checkAndPut/Delete: we check the byte[]-based overloads + // but + // not these Filter-based variants, which were added alongside the byte[] ones. + "preCheckAndPut(ObserverContext, byte[], Filter, Put, boolean)", + "preCheckAndPutAfterRowLock(ObserverContext, byte[], Filter, Put, boolean)", + "preCheckAndDelete(ObserverContext, byte[], Filter, Delete, boolean)", + "preCheckAndDeleteAfterRowLock(ObserverContext, byte[], Filter, Delete, boolean)", + // Metadata listing hooks: getTableNames is covered post-hoc by postGetTableNames + // filtering; + // listNamespace* hooks are not currently enforced. + "preGetTableNames(ObserverContext, List, String)", + "preListNamespaceDescriptors(ObserverContext, List)", + "preListNamespaces(ObserverContext, List)", + // Cluster metrics: currently unenforced; reference AC requires ADMIN. + "preGetClusterMetrics(ObserverContext)")); + + @Test + public void testAllObserverMethodsAreExplicitlyOverridden() { + Set interfaceMethodSigs = + Arrays.stream(OBSERVER_INTERFACES) + .flatMap(iface -> Arrays.stream(iface.getDeclaredMethods())) + .filter(m -> !m.isSynthetic() && !Modifier.isStatic(m.getModifiers())) + .map(TestCoprocessorInterfaceCoverage::signature) + .collect(Collectors.toSet()); + + Set ourMethodSigs = + Arrays.stream(OpenPolicyAgentAccessController.class.getDeclaredMethods()) + .filter(m -> !m.isSynthetic()) + .map(TestCoprocessorInterfaceCoverage::signature) + .collect(Collectors.toSet()); + + List unhandled = + interfaceMethodSigs.stream() + .filter(sig -> !sig.startsWith("post")) // post hooks can never block operations + .filter(sig -> !EXCLUDED.contains(sig)) + .filter(sig -> !ourMethodSigs.contains(sig)) + .sorted() + .collect(Collectors.toList()); + + assertTrue( + "Observer interface pre-hooks found that are neither overridden nor in the exclusion list" + + " — review each and either implement it or add it to EXCLUDED with a justification:\n" + + String.join("\n", unhandled), + unhandled.isEmpty()); + } + + private static String signature(Method m) { + String params = + Arrays.stream(m.getParameterTypes()) + .map(Class::getSimpleName) + .collect(Collectors.joining(", ")); + return m.getName() + "(" + params + ")"; + } +} From 379e495c2b6fcd3613874be81d9700e75772fff7 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 13:03:11 +0100 Subject: [PATCH 06/29] renamed ctx variable consistently and refactored requirePermission --- .gitignore | 3 +- .../OpenPolicyAgentAccessController.java | 348 ++++++++++-------- ...OpenPolicyAgentAccessControllerRegion.java | 84 +---- 3 files changed, 192 insertions(+), 243 deletions(-) diff --git a/.gitignore b/.gitignore index 32ac77f..cd975cc 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /target/ /dependency-reduced-pom.xml -/CLAUDE.md \ No newline at end of file +/CLAUDE.md +.claude/ diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index a8f477d..5f6b9ea 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -6,6 +6,7 @@ import com.google.protobuf.RpcController; import com.google.protobuf.Service; import java.io.IOException; +import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -149,17 +150,18 @@ private User getActiveUser(ObserverContext ctx) throws IOException { @Override public void preCreateNamespace( - ObserverContext c, NamespaceDescriptor ns) throws IOException { - User user = getActiveUser(c); + ObserverContext ctx, NamespaceDescriptor ns) + throws IOException { + User user = getActiveUser(ctx); LOG.debug("preCreateNamespace: user [{}]", user); opaAclChecker.checkPermissionInfo( user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); } @Override - public void preDeleteNamespace(ObserverContext c, String namespace) - throws IOException { - User user = getActiveUser(c); + public void preDeleteNamespace( + ObserverContext ctx, String namespace) throws IOException { + User user = getActiveUser(ctx); LOG.debug("preDeleteNamespace: user [{}]", user); opaAclChecker.checkPermissionInfo( user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); @@ -167,8 +169,9 @@ public void preDeleteNamespace(ObserverContext c, @Override public void preModifyNamespace( - ObserverContext c, NamespaceDescriptor ns) throws IOException { - User user = getActiveUser(c); + ObserverContext ctx, NamespaceDescriptor ns) + throws IOException { + User user = getActiveUser(ctx); LOG.debug("preModifyNamespace: user [{}]", user); opaAclChecker.checkPermissionInfo( user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); @@ -176,31 +179,36 @@ public void preModifyNamespace( @Override public void preGetNamespaceDescriptor( - ObserverContext c, String namespace) throws IOException { - User user = getActiveUser(c); + ObserverContext ctx, String namespace) throws IOException { + User user = getActiveUser(ctx); LOG.debug("preGetNamespaceDescriptor: user [{}]", user); opaAclChecker.checkPermissionInfo(user, namespace, Action.ADMIN); } @Override public void postListNamespaces( - ObserverContext c, List namespaces) throws IOException { + ObserverContext ctx, List namespaces) + throws IOException { /* always allow namespace listing */ } @Override public void preCreateTable( - ObserverContext c, TableDescriptor desc, RegionInfo[] regions) + ObserverContext ctx, TableDescriptor desc, RegionInfo[] regions) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preCreateTable: user [{}]", user); - - opaAclChecker.checkPermissionInfo(user, desc.getTableName(), Action.CREATE); + requirePermission( + ctx, + desc.getTableName().getNamespaceAsString(), + "createTable", + Action.ADMIN, + Action.CREATE); } @Override public void postCompletedCreateTableAction( - final ObserverContext c, + final ObserverContext ctx, final TableDescriptor desc, final RegionInfo[] regions) { /* @@ -210,18 +218,16 @@ public void postCompletedCreateTableAction( } @Override - public void preDeleteTable(ObserverContext c, TableName tableName) + public void preDeleteTable(ObserverContext ctx, TableName tableName) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preDeleteTable: user [{}]", user); - - // the default access controller treats create/delete as requiring the same permissions. - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "deleteTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override public void postDeleteTable( - ObserverContext c, final TableName tableName) { + ObserverContext ctx, final TableName tableName) { /* The default AccessController uses this method to remove the permissions for the deleted table in the internal ACL table. We do not need this as we are managing permissions in OPA. @@ -229,29 +235,29 @@ public void postDeleteTable( } @Override - public void preEnableTable(ObserverContext c, TableName tableName) + public void preEnableTable(ObserverContext ctx, TableName tableName) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preEnableTable: user [{}]", user); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "enableTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override - public void preDisableTable(ObserverContext c, TableName tableName) - throws IOException { - User user = getActiveUser(c); + public void preDisableTable( + ObserverContext ctx, TableName tableName) throws IOException { + User user = getActiveUser(ctx); LOG.debug("preDisableTable: user [{}]", user); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "disableTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override public void preGetOp( - final ObserverContext c, final Get get, final List result) + final ObserverContext ctx, + final Get get, + final List result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preGetOp: user [{}] on table [{}] with get [{}]", user, tableName, get); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { @@ -262,10 +268,10 @@ public void preGetOp( @Override public boolean preExists( - final ObserverContext c, final Get get, final boolean exists) + final ObserverContext ctx, final Get get, final boolean exists) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { return exists; @@ -277,10 +283,10 @@ public boolean preExists( } @Override - public void preScannerOpen(final ObserverContext c, final Scan scan) - throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + public void preScannerOpen( + final ObserverContext ctx, final Scan scan) throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { return; @@ -292,9 +298,11 @@ public void preScannerOpen(final ObserverContext c @Override public RegionScanner postScannerOpen( - final ObserverContext c, final Scan scan, final RegionScanner s) + final ObserverContext ctx, + final Scan scan, + final RegionScanner s) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); if (user != null && user.getShortName() != null) { // TODO this uses the shortName. Is it possible for the same scanner to be used by // different users across principals who nevertheless have the same shortName? This @@ -307,14 +315,14 @@ public RegionScanner postScannerOpen( @Override public boolean preScannerNext( - final ObserverContext c, + final ObserverContext ctx, final InternalScanner s, final List result, final int limit, final boolean hasNext) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preScannerNext: user [{}] on table [{}] with scan [{}]", user, tableName, s); requireScannerOwner(s); @@ -323,10 +331,10 @@ public boolean preScannerNext( @Override public void preScannerClose( - final ObserverContext c, final InternalScanner s) + final ObserverContext ctx, final InternalScanner s) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preScannerClose: user [{}] on table [{}] with scan [{}]", user, tableName, s); requireScannerOwner(s); @@ -334,10 +342,10 @@ public void preScannerClose( @Override public void postScannerClose( - final ObserverContext c, final InternalScanner s) + final ObserverContext ctx, final InternalScanner s) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("postScannerClose: user [{}] on table [{}] with scan [{}]", user, tableName, s); scannerOwners.remove(s); @@ -357,13 +365,13 @@ private void requireScannerOwner(InternalScanner s) throws AccessDeniedException @Override public void prePut( - final ObserverContext c, + final ObserverContext ctx, final Put put, final WALEdit edit, final Durability durability) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("prePut: user [{}] on table [{}] with put [{}]", user, tableName, put); opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); @@ -371,13 +379,13 @@ public void prePut( @Override public void preDelete( - final ObserverContext c, + final ObserverContext ctx, final Delete delete, final WALEdit edit, final Durability durability) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preDelete: user [{}] on table [{}] with delete [{}]", user, tableName, delete); // the default access controller uses a second enum - OpType - to distinguish between @@ -387,7 +395,7 @@ public void preDelete( @Override public void postDelete( - final ObserverContext c, + final ObserverContext ctx, final Delete delete, final WALEdit edit, final Durability durability) { @@ -395,13 +403,13 @@ public void postDelete( } @Override - public Result preAppend(ObserverContext c, Append append) + public Result preAppend(ObserverContext ctx, Append append) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preAppend: user [{}] on table [{}] with append [{}]", user, tableName, append); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); // as per default access controller return null; @@ -409,11 +417,11 @@ public Result preAppend(ObserverContext c, Append @Override public void preBatchMutate( - ObserverContext c, + ObserverContext ctx, MiniBatchOperationInProgress miniBatchOp) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preBatchMutate: user [{}] on table [{}] with miniBatchOp [{}]", user, @@ -424,9 +432,9 @@ public void preBatchMutate( } @Override - public void preOpen(ObserverContext c) throws IOException { - User user = getActiveUser(c); - final Region region = c.getEnvironment().getRegion(); + public void preOpen(ObserverContext ctx) throws IOException { + User user = getActiveUser(ctx); + final Region region = ctx.getEnvironment().getRegion(); if (region == null) { LOG.error("NULL region from RegionCoprocessorEnvironment in preOpen()"); } else { @@ -437,7 +445,7 @@ public void preOpen(ObserverContext c) throws IOEx } @Override - public void postOpen(ObserverContext c) { + public void postOpen(ObserverContext ctx) { // not needed as the ACL table is not used } @@ -446,37 +454,30 @@ public void preTableFlush( final ObserverContext ctx, final TableName tableName) throws IOException { User user = getActiveUser(ctx); - LOG.trace("preTableFlush: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + LOG.debug("preTableFlush: user [{}] on table [{}]", user, tableName); + requirePermission(ctx, "flushTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override public void preFlush( - ObserverContext c, FlushLifeCycleTracker tracker) + ObserverContext ctx, FlushLifeCycleTracker tracker) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); - LOG.trace("preFlush: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + // Internal storage engine flush — not a user-initiated operation, no authorization needed. } @Override public InternalScanner preCompact( - ObserverContext c, + ObserverContext ctx, Store store, InternalScanner scanner, ScanType scanType, CompactionLifeCycleTracker tracker, CompactionRequest request) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCompact: user [{}] on table [{}] for scanner [{}]", user, tableName, scanner); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); - + requirePermission(ctx, "compact", tableName, null, null, Action.ADMIN, Action.CREATE); return scanner; } @@ -491,15 +492,13 @@ public void preGetTableDescriptors( // We are delegating the authorization check to postGetTableDescriptors as we don't have // any concrete set of table names when a regex is present or the full list is requested. if (regex == null && tableNamesList != null && !tableNamesList.isEmpty()) { - User user = getActiveUser(ctx); - TableName[] sns = null; try (Admin admin = ctx.getEnvironment().getConnection().getAdmin()) { - sns = admin.listTableNames(); - if (sns == null) return; + if (admin.listTableNames() == null) return; for (TableName tableName : tableNamesList) { // Skip checks for a table that does not exist if (!admin.tableExists(tableName)) continue; - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission( + ctx, "getTableDescriptors", tableName, null, null, Action.ADMIN, Action.CREATE); } } } @@ -516,14 +515,20 @@ public void postGetTableDescriptors( if (regex == null && tableNamesList != null && !tableNamesList.isEmpty()) { return; } - User user = getActiveUser(ctx); // Retains only those which passes authorization checks, as the checks weren't done as part // of preGetTableDescriptors. Iterator itr = descriptors.iterator(); while (itr.hasNext()) { TableDescriptor htd = itr.next(); try { - opaAclChecker.checkPermissionInfo(user, htd.getTableName(), Action.CREATE); + requirePermission( + ctx, + "getTableDescriptors", + htd.getTableName(), + null, + null, + Action.ADMIN, + Action.CREATE); } catch (AccessControlException e) { itr.remove(); } @@ -536,13 +541,12 @@ public void postGetTableNames( List descriptors, String regex) throws IOException { - // Retains only those which passes authorization checks. - User user = getActiveUser(ctx); + // Retains only those on which the user has any permission (matches reference AC). Iterator itr = descriptors.iterator(); while (itr.hasNext()) { TableDescriptor htd = itr.next(); try { - opaAclChecker.checkPermissionInfo(user, htd.getTableName(), Action.CREATE); + requirePermission(ctx, "getTableNames", htd.getTableName(), null, null, Action.values()); } catch (AccessControlException e) { itr.remove(); } @@ -551,7 +555,7 @@ public void postGetTableNames( @Override public boolean preCheckAndPut( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -560,17 +564,16 @@ public boolean preCheckAndPut( final Put put, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); + requirePermission(ctx, "checkAndPut", tableName, null, null, Action.READ, Action.WRITE); return result; } @Override public boolean preCheckAndPutAfterRowLock( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -579,18 +582,17 @@ public boolean preCheckAndPutAfterRowLock( final Put put, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); + requirePermission(ctx, "checkAndPut", tableName, null, null, Action.READ, Action.WRITE); return result; } @Override public boolean preCheckAndDelete( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -599,18 +601,17 @@ public boolean preCheckAndDelete( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); + requirePermission(ctx, "checkAndDelete", tableName, null, null, Action.READ, Action.WRITE); return result; } @Override public boolean preCheckAndDeleteAfterRowLock( - final ObserverContext c, + final ObserverContext ctx, final byte[] row, final byte[] family, final byte[] qualifier, @@ -619,15 +620,14 @@ public boolean preCheckAndDeleteAfterRowLock( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDeleteAfterRowLock: user [{}] on table [{}] for delete [{}]", user, tableName, delete); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); + requirePermission(ctx, "checkAndDelete", tableName, null, null, Action.READ, Action.WRITE); return result; } @@ -639,12 +639,11 @@ public void postListNamespaceDescriptors( @Override public void preTruncateTable( - ObserverContext c, final TableName tableName) + ObserverContext ctx, final TableName tableName) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preTruncateTable: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "truncateTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override @@ -657,37 +656,35 @@ public void postTruncateTable( @Override public TableDescriptor preModifyTable( - ObserverContext c, + ObserverContext ctx, TableName tableName, TableDescriptor currentDesc, TableDescriptor newDesc) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.debug("preModifyTable: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, "modifyTable", tableName, null, null, Action.ADMIN, Action.CREATE); return currentDesc; } @Override public void postModifyTable( - ObserverContext c, + ObserverContext ctx, TableName tableName, final TableDescriptor htd) throws IOException { - User user = getActiveUser(c); + User user = getActiveUser(ctx); LOG.trace("postModifyTable: user [{}] on table [{}]", user, tableName); } @Override public Result preIncrement( - final ObserverContext c, final Increment increment) + final ObserverContext ctx, final Increment increment) throws IOException { - User user = getActiveUser(c); - TableName tableName = c.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preIncrement: user [{}] on table [{}]", user, tableName); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE, Action.READ); + opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); // as per default controller return null; } @@ -777,22 +774,22 @@ public Optional getRegionServerObserver() { @Override public String preModifyTableStoreFileTracker( - ObserverContext c, TableName tableName, String dstSFT) + ObserverContext ctx, TableName tableName, String dstSFT) throws IOException { requirePermission( - c, "modifyTableStoreFileTracker", tableName, null, null, Action.ADMIN, Action.CREATE); + ctx, "modifyTableStoreFileTracker", tableName, null, null, Action.ADMIN, Action.CREATE); return dstSFT; } @Override public String preModifyColumnFamilyStoreFileTracker( - ObserverContext c, + ObserverContext ctx, TableName tableName, byte[] family, String dstSFT) throws IOException { requirePermission( - c, + ctx, "modifyColumnFamilyStoreFileTracker", tableName, family, @@ -804,30 +801,30 @@ public String preModifyColumnFamilyStoreFileTracker( @Override public void preMove( - ObserverContext c, + ObserverContext ctx, RegionInfo region, ServerName srcServer, ServerName destServer) throws IOException { - requirePermission(c, "move", region.getTable(), null, null, Action.ADMIN); + requirePermission(ctx, "move", region.getTable(), null, null, Action.ADMIN); } @Override - public void preAssign(ObserverContext c, RegionInfo regionInfo) + public void preAssign(ObserverContext ctx, RegionInfo regionInfo) throws IOException { - requirePermission(c, "assign", regionInfo.getTable(), null, null, Action.ADMIN); + requirePermission(ctx, "assign", regionInfo.getTable(), null, null, Action.ADMIN); } @Override - public void preUnassign(ObserverContext c, RegionInfo regionInfo) + public void preUnassign(ObserverContext ctx, RegionInfo regionInfo) throws IOException { - requirePermission(c, "unassign", regionInfo.getTable(), null, null, Action.ADMIN); + requirePermission(ctx, "unassign", regionInfo.getTable(), null, null, Action.ADMIN); } @Override public void preRegionOffline( - ObserverContext c, RegionInfo regionInfo) throws IOException { - requirePermission(c, "regionOffline", regionInfo.getTable(), null, null, Action.ADMIN); + ObserverContext ctx, RegionInfo regionInfo) throws IOException { + requirePermission(ctx, "regionOffline", regionInfo.getTable(), null, null, Action.ADMIN); } @Override @@ -851,7 +848,8 @@ public void preListSnapshot( throws IOException { User user = getActiveUser(ctx); LOG.debug("preListSnapshot: user [{}] snapshot[{}]", user, snapshot); - opaAclChecker.checkPermissionInfo(user, snapshot.getTableName(), Action.ADMIN); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "listSnapshot", Action.ADMIN); } @Override @@ -863,7 +861,7 @@ public void preCloneSnapshot( User user = getActiveUser(ctx); TableName tableName = hTableDescriptor.getTableName(); LOG.debug("preCloneSnapshot: user [{}] snapshot[{}] table [{}]", user, snapshot, tableName); - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + requirePermission(ctx, tableName.getNamespaceAsString(), "cloneSnapshot", Action.ADMIN); } @Override @@ -873,9 +871,9 @@ public void preRestoreSnapshot( final TableDescriptor hTableDescriptor) throws IOException { User user = getActiveUser(ctx); - TableName tableName = hTableDescriptor.getTableName(); - LOG.debug("preRestoreSnapshot: user [{}] snapshot[{}] table [{}]", user, snapshot, tableName); - opaAclChecker.checkPermissionInfo(user, tableName, Action.CREATE); + LOG.debug("preRestoreSnapshot: user [{}] snapshot[{}]", user, snapshot); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "restoreSnapshot", Action.ADMIN); } @Override @@ -884,7 +882,8 @@ public void preDeleteSnapshot( throws IOException { User user = getActiveUser(ctx); LOG.debug("preDeleteSnapshot: user [{}] snapshot[{}]", user, snapshot); - opaAclChecker.checkPermissionInfo(user, snapshot.getTableName(), Action.ADMIN); + requirePermission( + ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "deleteSnapshot", Action.ADMIN); } @Override @@ -903,7 +902,7 @@ public void preBulkLoadHFile( User user = getActiveUser(ctx); var tableName = ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(); LOG.debug("preBulkLoadHFile: user [{}] on table [{}]", user, tableName); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + requirePermission(ctx, "preBulkLoadHFile", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override @@ -1003,16 +1002,29 @@ public void preGetUserPermissions( } private void requirePermission( - final ObserverContext ctx, final String namespace, String request, Action perm) + final ObserverContext ctx, final String namespace, String request, Action... permissions) throws IOException { User user = getActiveUser(ctx); - LOG.trace( - "requirePermission: user [{}] namespace[{}] request [{}] permission [{}]", - user, - namespace, - request, - perm); - opaAclChecker.checkPermissionInfo(user, namespace, perm); + AccessControlException[] last = {null}; + boolean allowed = + Arrays.stream(permissions) + .anyMatch( + perm -> { + LOG.trace( + "requirePermission: user [{}] namespace[{}] request [{}] permission [{}]", + user, + namespace, + request, + perm); + try { + opaAclChecker.checkPermissionInfo(user, namespace, perm); + return true; + } catch (AccessControlException e) { + last[0] = e; + return false; + } + }); + if (!allowed) throw last[0]; } private void requirePermission( @@ -1024,11 +1036,25 @@ private void requirePermission( Action... permissions) throws IOException { User user = getActiveUser(ctx); - for (Action perm : permissions) { - LOG.trace( - "requirePermission: user [{}] tableName[{}] permission [{}]", user, tableName, perm); - opaAclChecker.checkPermissionInfo(user, tableName, perm); - } + AccessControlException[] last = {null}; + boolean allowed = + Arrays.stream(permissions) + .anyMatch( + perm -> { + LOG.trace( + "requirePermission: user [{}] tableName[{}] permission [{}]", + user, + tableName, + perm); + try { + opaAclChecker.checkPermissionInfo(user, tableName, perm); + return true; + } catch (AccessControlException e) { + last[0] = e; + return false; + } + }); + if (!allowed) throw last[0]; } @Override diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java index 913ee71..a2327c2 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -5,7 +5,6 @@ import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.createTable; import static org.apache.hadoop.hbase.security.access.SecureTestUtil.deleteTable; -import static org.junit.Assert.fail; import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; @@ -27,10 +26,7 @@ import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; import org.apache.hadoop.hbase.regionserver.RegionServerCoprocessorHost; import org.apache.hadoop.hbase.regionserver.ScanType; -import org.apache.hadoop.hbase.security.User; -import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.security.AccessControlException; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -98,16 +94,6 @@ private OpenPolicyAgentAccessController getRegionController() { return region.getCoprocessorHost().findCoprocessor(OpenPolicyAgentAccessController.class); } - /** Stubs a deny for writeOnlyUser when action is READ, to verify READ is required. */ - private void stubDenyReadForWriteOnlyUser() { - stubFor( - post("/") - .withRequestBody( - WireMock.matchingJsonPath("$.input.callerUgi[?(@.userName == 'writeOnlyUser')]")) - .withRequestBody(WireMock.matchingJsonPath("$.input[?(@.action == 'READ')]")) - .willReturn(ok().withBody("{\"result\": \"false\"}"))); - } - // --- read hooks --- @Test @@ -170,11 +156,8 @@ public void testPreBatchMutate() throws Exception { @Test public void testPreFlush() throws Exception { - assertAllowedThenDenied( - () -> { - getRegionController().preFlush(regionCtx(), null); - return null; - }); + // preFlush is an internal storage engine hook; no authorization check is applied. + getRegionController().preFlush(regionCtx(), null); } @Test @@ -186,7 +169,7 @@ public void testPreCompact() throws Exception { }); } - // --- read+write hooks (require both WRITE and READ) --- + // --- read+write hooks (require WRITE or READ) --- @Test public void testPreAppend() throws Exception { @@ -197,23 +180,6 @@ public void testPreAppend() throws Exception { }); } - @Test - public void testPreAppendRequiresRead() throws Exception { - User writeOnlyUser = User.createUserForTesting(conf, "writeOnlyUser", new String[0]); - stubDenyReadForWriteOnlyUser(); - SecureTestUtil.AccessTestAction action = - () -> { - getRegionController().preAppend(regionCtx(), new Append(TEST_ROW)); - return null; - }; - try { - writeOnlyUser.runAs(action); - fail("AccessControlException should have been thrown"); - } catch (AccessControlException e) { - logOk(e); - } - } - @Test public void testPreIncrement() throws Exception { assertAllowedThenDenied( @@ -223,23 +189,6 @@ public void testPreIncrement() throws Exception { }); } - @Test - public void testPreIncrementRequiresRead() throws Exception { - User writeOnlyUser = User.createUserForTesting(conf, "writeOnlyUser", new String[0]); - stubDenyReadForWriteOnlyUser(); - SecureTestUtil.AccessTestAction action = - () -> { - getRegionController().preIncrement(regionCtx(), new Increment(TEST_ROW)); - return null; - }; - try { - writeOnlyUser.runAs(action); - fail("AccessControlException should have been thrown"); - } catch (AccessControlException e) { - logOk(e); - } - } - @Test public void testPreCheckAndPut() throws Exception { Put put = new Put(TEST_ROW); @@ -259,33 +208,6 @@ public void testPreCheckAndPut() throws Exception { }); } - @Test - public void testPreCheckAndPutRequiresRead() throws Exception { - User writeOnlyUser = User.createUserForTesting(conf, "writeOnlyUser", new String[0]); - stubDenyReadForWriteOnlyUser(); - Put put = new Put(TEST_ROW); - SecureTestUtil.AccessTestAction action = - () -> { - getRegionController() - .preCheckAndPut( - regionCtx(), - TEST_ROW, - TEST_FAMILY, - TEST_QUALIFIER, - CompareOperator.EQUAL, - null, - put, - false); - return null; - }; - try { - writeOnlyUser.runAs(action); - fail("AccessControlException should have been thrown"); - } catch (AccessControlException e) { - logOk(e); - } - } - @Test public void testPreCheckAndPutAfterRowLock() throws Exception { Put put = new Put(TEST_ROW); From 14f4a7cf7aeddd56a6467549b3093039a901958a Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 13:41:36 +0100 Subject: [PATCH 07/29] introduce OpType to add granularity --- .../OpenPolicyAgentAccessController.java | 62 +++++++++++++------ .../java/tech/stackable/hbase/opa/OpType.java | 28 +++++++++ .../stackable/hbase/opa/OpaAclChecker.java | 9 ++- .../stackable/hbase/opa/OpaAllowQuery.java | 10 ++- 4 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 src/main/java/tech/stackable/hbase/opa/OpType.java diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 5f6b9ea..2f0eeb0 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -72,6 +72,7 @@ import org.apache.hadoop.security.AccessControlException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import tech.stackable.hbase.opa.OpType; import tech.stackable.hbase.opa.OpaAclChecker; public class OpenPolicyAgentAccessController @@ -263,7 +264,7 @@ public void preGetOp( if (TableName.META_TABLE_NAME.equals(tableName)) { return; } - opaAclChecker.checkPermissionInfo(user, tableName, Action.READ); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.READ, OpType.GET); } @Override @@ -277,8 +278,7 @@ public boolean preExists( return exists; } LOG.trace("preExists: user [{}] on table [{}] with get [{}]", user, tableName, get); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.READ); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.READ, OpType.EXISTS); return exists; } @@ -292,8 +292,7 @@ public void preScannerOpen( return; } LOG.trace("preScannerOpen: user [{}] on table [{}] with scan [{}]", user, tableName, scan); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.READ); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.READ, OpType.SCAN); } @Override @@ -373,8 +372,7 @@ public void prePut( User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("prePut: user [{}] on table [{}] with put [{}]", user, tableName, put); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.PUT); } @Override @@ -387,10 +385,7 @@ public void preDelete( User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preDelete: user [{}] on table [{}] with delete [{}]", user, tableName, delete); - - // the default access controller uses a second enum - OpType - to distinguish between - // different types of write action (e.g. write, delete) - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.DELETE); } @Override @@ -408,8 +403,7 @@ public Result preAppend(ObserverContext ctx, Appen User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preAppend: user [{}] on table [{}] with append [{}]", user, tableName, append); - - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.APPEND); // as per default access controller return null; @@ -567,7 +561,8 @@ public boolean preCheckAndPut( User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); - requirePermission(ctx, "checkAndPut", tableName, null, null, Action.READ, Action.WRITE); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); return result; } @@ -586,7 +581,8 @@ public boolean preCheckAndPutAfterRowLock( TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); - requirePermission(ctx, "checkAndPut", tableName, null, null, Action.READ, Action.WRITE); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); return result; } @@ -605,7 +601,15 @@ public boolean preCheckAndDelete( TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); - requirePermission(ctx, "checkAndDelete", tableName, null, null, Action.READ, Action.WRITE); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); return result; } @@ -627,7 +631,15 @@ public boolean preCheckAndDeleteAfterRowLock( user, tableName, delete); - requirePermission(ctx, "checkAndDelete", tableName, null, null, Action.READ, Action.WRITE); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); return result; } @@ -684,7 +696,7 @@ public Result preIncrement( User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preIncrement: user [{}] on table [{}]", user, tableName); - opaAclChecker.checkPermissionInfo(user, tableName, Action.WRITE); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.INCREMENT); // as per default controller return null; } @@ -1035,6 +1047,18 @@ private void requirePermission( byte[] qualifier, Action... permissions) throws IOException { + requirePermission(ctx, request, tableName, family, qualifier, OpType.NONE, permissions); + } + + private void requirePermission( + ObserverContext ctx, + String request, + TableName tableName, + byte[] family, + byte[] qualifier, + OpType opType, + Action... permissions) + throws IOException { User user = getActiveUser(ctx); AccessControlException[] last = {null}; boolean allowed = @@ -1047,7 +1071,7 @@ private void requirePermission( tableName, perm); try { - opaAclChecker.checkPermissionInfo(user, tableName, perm); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, perm, opType); return true; } catch (AccessControlException e) { last[0] = e; diff --git a/src/main/java/tech/stackable/hbase/opa/OpType.java b/src/main/java/tech/stackable/hbase/opa/OpType.java new file mode 100644 index 0000000..0a538c0 --- /dev/null +++ b/src/main/java/tech/stackable/hbase/opa/OpType.java @@ -0,0 +1,28 @@ +package tech.stackable.hbase.opa; + +/** + * Region-level operation types, mirroring the OpType enum in the HBase reference AccessController. + */ +public enum OpType { + NONE("none"), + GET("get"), + EXISTS("exists"), + SCAN("scan"), + PUT("put"), + DELETE("delete"), + CHECK_AND_PUT("checkAndPut"), + CHECK_AND_DELETE("checkAndDelete"), + APPEND("append"), + INCREMENT("increment"); + + private final String value; + + OpType(String value) { + this.value = value; + } + + @Override + public String toString() { + return value; + } +} diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java index 6f516c5..f9d4e74 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java @@ -82,8 +82,15 @@ public OpaAclChecker( private void checkPermissionInfo(User user, TableName table, Permission.Action action) throws AccessControlException { + checkPermissionInfoWithOp(user, table, action, OpType.NONE); + } + + public void checkPermissionInfoWithOp( + User user, TableName table, Permission.Action action, OpType operation) + throws AccessControlException { OpaAllowQuery query = - new OpaAllowQuery(new OpaAllowQuery.OpaAllowQueryInput(user.getUGI(), table, action)); + new OpaAllowQuery( + new OpaAllowQuery.OpaAllowQueryInput(user.getUGI(), table, action, operation)); this.checkPermissionInfo(query); } diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java index 1ca0abd..18d1847 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java @@ -16,22 +16,28 @@ public static class OpaAllowQueryInput { public final TableName table; public final String namespace; public final Permission.Action action; + public final OpType operation; public OpaAllowQueryInput(UserGroupInformation ugi, TableName table, Permission.Action action) { - this.callerUgi = new OpaQueryUgi(ugi); + this(ugi, table, action, null); + } + public OpaAllowQueryInput( + UserGroupInformation ugi, TableName table, Permission.Action action, OpType operation) { + this.callerUgi = new OpaQueryUgi(ugi); this.table = table; this.action = action; this.namespace = table.getNamespaceAsString(); + this.operation = operation; } public OpaAllowQueryInput( UserGroupInformation ugi, String namespace, Permission.Action action) { this.callerUgi = new OpaQueryUgi(ugi); - this.table = null; this.action = action; this.namespace = namespace; + this.operation = OpType.NONE; } } } From d888e99427771cfde6d0cc6bf7d9e6fa3e0f25e1 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 13:58:28 +0100 Subject: [PATCH 08/29] implement Filter-based preCheckAndPut/preCheckAndDelete overloads --- .../OpenPolicyAgentAccessController.java | 85 +++++++++++++++++++ ...OpenPolicyAgentAccessControllerRegion.java | 44 ++++++++++ 2 files changed, 129 insertions(+) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 2f0eeb0..4a9197c 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -46,6 +46,7 @@ import org.apache.hadoop.hbase.coprocessor.RegionServerCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.RegionServerObserver; import org.apache.hadoop.hbase.filter.ByteArrayComparable; +import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.quotas.GlobalQuotaSettings; @@ -643,6 +644,90 @@ public boolean preCheckAndDeleteAfterRowLock( return result; } + @Override + public boolean preCheckAndPut( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Put put, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); + return result; + } + + @Override + public boolean preCheckAndPutAfterRowLock( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Put put, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); + requirePermission( + ctx, "checkAndPut", tableName, null, null, OpType.CHECK_AND_PUT, Action.READ, Action.WRITE); + return result; + } + + @Override + public boolean preCheckAndDelete( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Delete delete, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); + return result; + } + + @Override + public boolean preCheckAndDeleteAfterRowLock( + final ObserverContext ctx, + final byte[] row, + final Filter filter, + final Delete delete, + final boolean result) + throws IOException { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndDeleteAfterRowLock: user [{}] on table [{}] for delete [{}]", + user, + tableName, + delete); + requirePermission( + ctx, + "checkAndDelete", + tableName, + null, + null, + OpType.CHECK_AND_DELETE, + Action.READ, + Action.WRITE); + return result; + } + @Override public void postListNamespaceDescriptors( ObserverContext ctx, List descriptors) { diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java index a2327c2..76787c5 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -21,6 +21,7 @@ import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.RegionServerCoprocessorEnvironment; +import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; @@ -265,6 +266,49 @@ public void testPreCheckAndDeleteAfterRowLock() throws Exception { }); } + @Test + public void testPreCheckAndPutWithFilter() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController().preCheckAndPut(regionCtx(), TEST_ROW, (Filter) null, put, false); + return null; + }); + } + + @Test + public void testPreCheckAndPutAfterRowLockWithFilter() throws Exception { + Put put = new Put(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndPutAfterRowLock(regionCtx(), TEST_ROW, (Filter) null, put, false); + return null; + }); + } + + @Test + public void testPreCheckAndDeleteWithFilter() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDelete(regionCtx(), TEST_ROW, (Filter) null, delete, false); + return null; + }); + } + + @Test + public void testPreCheckAndDeleteAfterRowLockWithFilter() throws Exception { + Delete delete = new Delete(TEST_ROW); + assertAllowedThenDenied( + () -> { + getRegionController() + .preCheckAndDeleteAfterRowLock(regionCtx(), TEST_ROW, (Filter) null, delete, false); + return null; + }); + } + // --- RegionServer hooks --- private OpenPolicyAgentAccessController getRsController() { From fc0ce0baa9fe6c18101df180a44d74b59aef120f Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 14:08:15 +0100 Subject: [PATCH 09/29] add mutation checks --- .../OpenPolicyAgentAccessController.java | 36 +++++++++++++++++++ .../java/tech/stackable/hbase/opa/OpType.java | 4 ++- ...OpenPolicyAgentAccessControllerRegion.java | 35 ++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 4a9197c..796c907 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -21,6 +21,8 @@ import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Append; import org.apache.hadoop.hbase.client.BalanceRequest; +import org.apache.hadoop.hbase.client.CheckAndMutate; +import org.apache.hadoop.hbase.client.CheckAndMutateResult; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Durability; import org.apache.hadoop.hbase.client.Get; @@ -30,6 +32,7 @@ import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.RowMutations; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.SnapshotDescription; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -728,6 +731,39 @@ public boolean preCheckAndDeleteAfterRowLock( return result; } + @Override + public CheckAndMutateResult preCheckAndMutate( + ObserverContext ctx, + CheckAndMutate checkAndMutate, + CheckAndMutateResult result) + throws IOException { + if (checkAndMutate.getAction() instanceof RowMutations) { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace("preCheckAndMutate (RowMutations): user [{}] on table [{}]", user, tableName); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.ROW_MUTATIONS); + return result; + } + return RegionObserver.super.preCheckAndMutate(ctx, checkAndMutate, result); + } + + @Override + public CheckAndMutateResult preCheckAndMutateAfterRowLock( + ObserverContext ctx, + CheckAndMutate checkAndMutate, + CheckAndMutateResult result) + throws IOException { + if (checkAndMutate.getAction() instanceof RowMutations) { + User user = getActiveUser(ctx); + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + LOG.trace( + "preCheckAndMutateAfterRowLock (RowMutations): user [{}] on table [{}]", user, tableName); + opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.ROW_MUTATIONS); + return result; + } + return RegionObserver.super.preCheckAndMutateAfterRowLock(ctx, checkAndMutate, result); + } + @Override public void postListNamespaceDescriptors( ObserverContext ctx, List descriptors) { diff --git a/src/main/java/tech/stackable/hbase/opa/OpType.java b/src/main/java/tech/stackable/hbase/opa/OpType.java index 0a538c0..125cd98 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpType.java +++ b/src/main/java/tech/stackable/hbase/opa/OpType.java @@ -2,6 +2,7 @@ /** * Region-level operation types, mirroring the OpType enum in the HBase reference AccessController. + * {@code NONE} and {@code ROW_MUTATIONS} are extensions not present in the reference. */ public enum OpType { NONE("none"), @@ -13,7 +14,8 @@ public enum OpType { CHECK_AND_PUT("checkAndPut"), CHECK_AND_DELETE("checkAndDelete"), APPEND("append"), - INCREMENT("increment"); + INCREMENT("increment"), + ROW_MUTATIONS("rowMutations"); private final String value; diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java index 76787c5..cccadbe 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -11,11 +11,14 @@ import org.apache.hadoop.hbase.CompareOperator; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.client.Append; +import org.apache.hadoop.hbase.client.CheckAndMutate; +import org.apache.hadoop.hbase.client.CheckAndMutateResult; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.Durability; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.Increment; import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.RowMutations; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.coprocessor.ObserverContextImpl; @@ -309,6 +312,38 @@ public void testPreCheckAndDeleteAfterRowLockWithFilter() throws Exception { }); } + @Test + public void testPreCheckAndMutateWithRowMutations() throws Exception { + RowMutations rowMutations = new RowMutations(TEST_ROW); + rowMutations.add(new Put(TEST_ROW)); + CheckAndMutate checkAndMutate = + CheckAndMutate.newBuilder(TEST_ROW) + .ifNotExists(TEST_FAMILY, TEST_QUALIFIER) + .build(rowMutations); + CheckAndMutateResult result = new CheckAndMutateResult(true, null); + assertAllowedThenDenied( + () -> { + getRegionController().preCheckAndMutate(regionCtx(), checkAndMutate, result); + return null; + }); + } + + @Test + public void testPreCheckAndMutateAfterRowLockWithRowMutations() throws Exception { + RowMutations rowMutations = new RowMutations(TEST_ROW); + rowMutations.add(new Put(TEST_ROW)); + CheckAndMutate checkAndMutate = + CheckAndMutate.newBuilder(TEST_ROW) + .ifNotExists(TEST_FAMILY, TEST_QUALIFIER) + .build(rowMutations); + CheckAndMutateResult result = new CheckAndMutateResult(true, null); + assertAllowedThenDenied( + () -> { + getRegionController().preCheckAndMutateAfterRowLock(regionCtx(), checkAndMutate, result); + return null; + }); + } + // --- RegionServer hooks --- private OpenPolicyAgentAccessController getRsController() { From a1fd0a6ea810650d3cff378c3d1cdc6d6c2ddae9 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 14:28:40 +0100 Subject: [PATCH 10/29] pass families to permission checker --- .../OpenPolicyAgentAccessController.java | 31 ++++++++++++++----- .../stackable/hbase/opa/OpaAclChecker.java | 15 ++++++++- .../stackable/hbase/opa/OpaAllowQuery.java | 17 ++++++++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 796c907..d76f620 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -7,6 +7,7 @@ import com.google.protobuf.Service; import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -268,7 +269,8 @@ public void preGetOp( if (TableName.META_TABLE_NAME.equals(tableName)) { return; } - opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.READ, OpType.GET); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.READ, OpType.GET, get.getFamilyMap().keySet()); } @Override @@ -282,7 +284,8 @@ public boolean preExists( return exists; } LOG.trace("preExists: user [{}] on table [{}] with get [{}]", user, tableName, get); - opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.READ, OpType.EXISTS); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.READ, OpType.EXISTS, get.getFamilyMap().keySet()); return exists; } @@ -296,7 +299,8 @@ public void preScannerOpen( return; } LOG.trace("preScannerOpen: user [{}] on table [{}] with scan [{}]", user, tableName, scan); - opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.READ, OpType.SCAN); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.READ, OpType.SCAN, scan.getFamilyMap().keySet()); } @Override @@ -376,7 +380,8 @@ public void prePut( User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("prePut: user [{}] on table [{}] with put [{}]", user, tableName, put); - opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.PUT); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.WRITE, OpType.PUT, put.getFamilyCellMap().keySet()); } @Override @@ -389,7 +394,8 @@ public void preDelete( User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preDelete: user [{}] on table [{}] with delete [{}]", user, tableName, delete); - opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.DELETE); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.WRITE, OpType.DELETE, delete.getFamilyCellMap().keySet()); } @Override @@ -407,7 +413,8 @@ public Result preAppend(ObserverContext ctx, Appen User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preAppend: user [{}] on table [{}] with append [{}]", user, tableName, append); - opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.APPEND); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.WRITE, OpType.APPEND, append.getFamilyCellMap().keySet()); // as per default access controller return null; @@ -817,7 +824,8 @@ public Result preIncrement( User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preIncrement: user [{}] on table [{}]", user, tableName); - opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.INCREMENT); + opaAclChecker.checkPermissionInfoWithOp( + user, tableName, Action.WRITE, OpType.INCREMENT, increment.getFamilyCellMap().keySet()); // as per default controller return null; } @@ -1192,7 +1200,14 @@ private void requirePermission( tableName, perm); try { - opaAclChecker.checkPermissionInfoWithOp(user, tableName, perm, opType); + opaAclChecker.checkPermissionInfoWithOp( + user, + tableName, + perm, + opType, + family != null + ? Collections.singletonList(family) + : Collections.emptyList()); return true; } catch (AccessControlException e) { last[0] = e; diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java index f9d4e74..1719df9 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java @@ -12,6 +12,8 @@ import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.util.Collection; +import java.util.Collections; import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -88,9 +90,20 @@ private void checkPermissionInfo(User user, TableName table, Permission.Action a public void checkPermissionInfoWithOp( User user, TableName table, Permission.Action action, OpType operation) throws AccessControlException { + checkPermissionInfoWithOp(user, table, action, operation, Collections.emptyList()); + } + + public void checkPermissionInfoWithOp( + User user, + TableName table, + Permission.Action action, + OpType operation, + Collection families) + throws AccessControlException { OpaAllowQuery query = new OpaAllowQuery( - new OpaAllowQuery.OpaAllowQueryInput(user.getUGI(), table, action, operation)); + new OpaAllowQuery.OpaAllowQueryInput( + user.getUGI(), table, action, operation, families)); this.checkPermissionInfo(query); } diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java index 18d1847..3089888 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java @@ -1,7 +1,12 @@ package tech.stackable.hbase.opa; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.security.access.Permission; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.security.UserGroupInformation; public class OpaAllowQuery { @@ -17,6 +22,7 @@ public static class OpaAllowQueryInput { public final String namespace; public final Permission.Action action; public final OpType operation; + public final List families; public OpaAllowQueryInput(UserGroupInformation ugi, TableName table, Permission.Action action) { this(ugi, table, action, null); @@ -24,11 +30,21 @@ public OpaAllowQueryInput(UserGroupInformation ugi, TableName table, Permission. public OpaAllowQueryInput( UserGroupInformation ugi, TableName table, Permission.Action action, OpType operation) { + this(ugi, table, action, operation, Collections.emptyList()); + } + + public OpaAllowQueryInput( + UserGroupInformation ugi, + TableName table, + Permission.Action action, + OpType operation, + Collection families) { this.callerUgi = new OpaQueryUgi(ugi); this.table = table; this.action = action; this.namespace = table.getNamespaceAsString(); this.operation = operation; + this.families = families.stream().map(Bytes::toString).collect(Collectors.toList()); } public OpaAllowQueryInput( @@ -38,6 +54,7 @@ public OpaAllowQueryInput( this.action = action; this.namespace = namespace; this.operation = OpType.NONE; + this.families = Collections.emptyList(); } } } From 949148027621d905d7751de1bd15b11942613aeb Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 15:04:58 +0100 Subject: [PATCH 11/29] pass map of key + families to permission checker --- .../OpenPolicyAgentAccessController.java | 74 +++++++++++++++---- .../stackable/hbase/opa/OpaAclChecker.java | 7 +- .../stackable/hbase/opa/OpaAllowQuery.java | 19 +++-- 3 files changed, 75 insertions(+), 25 deletions(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index d76f620..3099259 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -11,9 +11,13 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.NavigableSet; import java.util.Optional; +import java.util.TreeMap; +import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.CompareOperator; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.NamespaceDescriptor; @@ -72,6 +76,7 @@ import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.Permission.Action; import org.apache.hadoop.hbase.security.access.UserPermission; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.wal.WALEdit; import org.apache.hadoop.security.AccessControlException; @@ -270,7 +275,7 @@ public void preGetOp( return; } opaAclChecker.checkPermissionInfoWithOp( - user, tableName, Action.READ, OpType.GET, get.getFamilyMap().keySet()); + user, tableName, Action.READ, OpType.GET, familiesFromQualifiers(get.getFamilyMap())); } @Override @@ -285,7 +290,7 @@ public boolean preExists( } LOG.trace("preExists: user [{}] on table [{}] with get [{}]", user, tableName, get); opaAclChecker.checkPermissionInfoWithOp( - user, tableName, Action.READ, OpType.EXISTS, get.getFamilyMap().keySet()); + user, tableName, Action.READ, OpType.EXISTS, familiesFromQualifiers(get.getFamilyMap())); return exists; } @@ -300,7 +305,7 @@ public void preScannerOpen( } LOG.trace("preScannerOpen: user [{}] on table [{}] with scan [{}]", user, tableName, scan); opaAclChecker.checkPermissionInfoWithOp( - user, tableName, Action.READ, OpType.SCAN, scan.getFamilyMap().keySet()); + user, tableName, Action.READ, OpType.SCAN, familiesFromQualifiers(scan.getFamilyMap())); } @Override @@ -381,7 +386,7 @@ public void prePut( TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("prePut: user [{}] on table [{}] with put [{}]", user, tableName, put); opaAclChecker.checkPermissionInfoWithOp( - user, tableName, Action.WRITE, OpType.PUT, put.getFamilyCellMap().keySet()); + user, tableName, Action.WRITE, OpType.PUT, familiesFromCells(put.getFamilyCellMap())); } @Override @@ -395,7 +400,7 @@ public void preDelete( TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preDelete: user [{}] on table [{}] with delete [{}]", user, tableName, delete); opaAclChecker.checkPermissionInfoWithOp( - user, tableName, Action.WRITE, OpType.DELETE, delete.getFamilyCellMap().keySet()); + user, tableName, Action.WRITE, OpType.DELETE, familiesFromCells(delete.getFamilyCellMap())); } @Override @@ -414,7 +419,7 @@ public Result preAppend(ObserverContext ctx, Appen TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preAppend: user [{}] on table [{}] with append [{}]", user, tableName, append); opaAclChecker.checkPermissionInfoWithOp( - user, tableName, Action.WRITE, OpType.APPEND, append.getFamilyCellMap().keySet()); + user, tableName, Action.WRITE, OpType.APPEND, familiesFromCells(append.getFamilyCellMap())); // as per default access controller return null; @@ -825,7 +830,11 @@ public Result preIncrement( TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preIncrement: user [{}] on table [{}]", user, tableName); opaAclChecker.checkPermissionInfoWithOp( - user, tableName, Action.WRITE, OpType.INCREMENT, increment.getFamilyCellMap().keySet()); + user, + tableName, + Action.WRITE, + OpType.INCREMENT, + familiesFromCells(increment.getFamilyCellMap())); // as per default controller return null; } @@ -1142,6 +1151,49 @@ public void preGetUserPermissions( } } + /** Converts a mutation's family→cell map to a family→qualifier-names map for OPA. */ + private static Map> familiesFromCells( + Map> familyCellMap) { + Map> result = new TreeMap<>(); + for (Map.Entry> entry : familyCellMap.entrySet()) { + String family = Bytes.toString(entry.getKey()); + List qualifiers = + entry.getValue().stream() + .map(cell -> Bytes.toString(CellUtil.cloneQualifier(cell))) + .distinct() + .collect(Collectors.toList()); + result.put(family, qualifiers); + } + return result; + } + + /** Converts a Get/Scan family→qualifier map to a family→qualifier-names map for OPA. */ + private static Map> familiesFromQualifiers( + Map> familyQualMap) { + Map> result = new TreeMap<>(); + for (Map.Entry> entry : familyQualMap.entrySet()) { + String family = Bytes.toString(entry.getKey()); + List qualifiers = + entry.getValue() == null + ? Collections.emptyList() + : entry.getValue().stream().map(Bytes::toString).collect(Collectors.toList()); + result.put(family, qualifiers); + } + return result; + } + + /** Builds a single-entry family map for OPA from explicit family/qualifier byte arrays. */ + private static Map> familyMap(byte[] family, byte[] qualifier) { + if (family == null) return Collections.emptyMap(); + Map> result = new TreeMap<>(); + result.put( + Bytes.toString(family), + qualifier != null + ? Collections.singletonList(Bytes.toString(qualifier)) + : Collections.emptyList()); + return result; + } + private void requirePermission( final ObserverContext ctx, final String namespace, String request, Action... permissions) throws IOException { @@ -1201,13 +1253,7 @@ private void requirePermission( perm); try { opaAclChecker.checkPermissionInfoWithOp( - user, - tableName, - perm, - opType, - family != null - ? Collections.singletonList(family) - : Collections.emptyList()); + user, tableName, perm, opType, familyMap(family, qualifier)); return true; } catch (AccessControlException e) { last[0] = e; diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java index 1719df9..ca7ac07 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAclChecker.java @@ -12,8 +12,9 @@ import java.net.http.HttpClient; import java.net.http.HttpRequest; import java.net.http.HttpResponse; -import java.util.Collection; import java.util.Collections; +import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -90,7 +91,7 @@ private void checkPermissionInfo(User user, TableName table, Permission.Action a public void checkPermissionInfoWithOp( User user, TableName table, Permission.Action action, OpType operation) throws AccessControlException { - checkPermissionInfoWithOp(user, table, action, operation, Collections.emptyList()); + checkPermissionInfoWithOp(user, table, action, operation, Collections.emptyMap()); } public void checkPermissionInfoWithOp( @@ -98,7 +99,7 @@ public void checkPermissionInfoWithOp( TableName table, Permission.Action action, OpType operation, - Collection families) + Map> families) throws AccessControlException { OpaAllowQuery query = new OpaAllowQuery( diff --git a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java index 3089888..b6abca6 100644 --- a/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java +++ b/src/main/java/tech/stackable/hbase/opa/OpaAllowQuery.java @@ -1,12 +1,10 @@ package tech.stackable.hbase.opa; -import java.util.Collection; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; +import java.util.Map; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.security.access.Permission; -import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.security.UserGroupInformation; public class OpaAllowQuery { @@ -22,7 +20,12 @@ public static class OpaAllowQueryInput { public final String namespace; public final Permission.Action action; public final OpType operation; - public final List families; + + /** + * Column families and their qualifiers being accessed. An empty qualifier list means CF-level + * access; a non-empty list means KV-level access to specific qualifiers within that family. + */ + public final Map> families; public OpaAllowQueryInput(UserGroupInformation ugi, TableName table, Permission.Action action) { this(ugi, table, action, null); @@ -30,7 +33,7 @@ public OpaAllowQueryInput(UserGroupInformation ugi, TableName table, Permission. public OpaAllowQueryInput( UserGroupInformation ugi, TableName table, Permission.Action action, OpType operation) { - this(ugi, table, action, operation, Collections.emptyList()); + this(ugi, table, action, operation, Collections.emptyMap()); } public OpaAllowQueryInput( @@ -38,13 +41,13 @@ public OpaAllowQueryInput( TableName table, Permission.Action action, OpType operation, - Collection families) { + Map> families) { this.callerUgi = new OpaQueryUgi(ugi); this.table = table; this.action = action; this.namespace = table.getNamespaceAsString(); this.operation = operation; - this.families = families.stream().map(Bytes::toString).collect(Collectors.toList()); + this.families = families; } public OpaAllowQueryInput( @@ -54,7 +57,7 @@ public OpaAllowQueryInput( this.action = action; this.namespace = namespace; this.operation = OpType.NONE; - this.families = Collections.emptyList(); + this.families = Collections.emptyMap(); } } } From d745ed3083f783e9b0921ff6b631f09fabbc2fc3 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 15:30:27 +0100 Subject: [PATCH 12/29] check pass-throughs and no-ops --- .../OpenPolicyAgentAccessController.java | 61 +++++++++++-------- .../TestCoprocessorInterfaceCoverage.java | 18 +----- 2 files changed, 37 insertions(+), 42 deletions(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 3099259..6b05184 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -67,7 +67,6 @@ import org.apache.hadoop.hbase.regionserver.Store; import org.apache.hadoop.hbase.regionserver.compactions.CompactionLifeCycleTracker; import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequest; -import org.apache.hadoop.hbase.replication.ReplicationEndpoint; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.security.AccessDeniedException; import org.apache.hadoop.hbase.security.User; @@ -1336,15 +1335,30 @@ public void preStopRegionServer(ObserverContext ctx, Service service, String methodName, - Message request) { - LOG.trace("preEndpointInvocation not implemented! {}/{}", methodName, request); + Message request) + throws IOException { + // Skip EXEC check for calls to the AccessControlService itself to avoid recursive checks. + if (!(service instanceof AccessControlProtos.AccessControlService)) { + TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); + User user = getActiveUser(ctx); + LOG.debug( + "preEndpointInvocation: user [{}] on table [{}] method [{}]", + user, + tableName, + methodName); + requirePermission( + ctx, + "invoke(" + service.getDescriptorForType().getName() + "." + methodName + ")", + tableName, + null, + null, + Action.EXEC); + } return request; } @@ -1355,7 +1369,7 @@ public void postEndpointInvocation( String methodName, Message request, Message.Builder responseBuilder) { - LOG.trace("postEndpointInvocation not implemented! {}/{}", methodName, request); + // as per reference AccessController } @Override @@ -1364,14 +1378,25 @@ public void preRequestLock( String namespace, TableName tableName, RegionInfo[] regionInfos, - String description) { - LOG.trace("preRequestLock not implemented! {}/{}", tableName, regionInfos); + String description) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preRequestLock: user [{}] namespace [{}] table [{}]", user, namespace, tableName); + if (namespace != null && !namespace.isEmpty()) { + requirePermission(ctx, namespace, "requestLock", Action.ADMIN, Action.CREATE); + } else { + TableName tn = tableName != null ? tableName : regionInfos[0].getTable(); + requirePermission(ctx, "requestLock", tn, null, null, Action.ADMIN, Action.CREATE); + } } @Override public void preLockHeartbeat( - ObserverContext ctx, TableName tableName, String description) { - LOG.trace("preLockHeartbeat not implemented! {}/{}", tableName, description); + ObserverContext ctx, TableName tableName, String description) + throws IOException { + User user = getActiveUser(ctx); + LOG.debug("preLockHeartbeat: user [{}] table [{}]", user, tableName); + requirePermission(ctx, "lockHeartbeat", tableName, null, null, Action.ADMIN, Action.CREATE); } /*********************************** Global admin operations ***********************************/ @@ -1383,11 +1408,6 @@ public void preAbortProcedure( ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "abortProcedure", Action.ADMIN); } - @Override - public void postAbortProcedure(ObserverContext ctx) { - // There is nothing to do at this time after the procedure abort request was sent. - } - @Override public void preGetProcedures(ObserverContext ctx) throws IOException { @@ -1427,11 +1447,6 @@ public void preRollWALWriterRequest(ObserverContext ctx) { - // as per default access controller - } - @Override public void preSetUserQuota( final ObserverContext ctx, @@ -1452,12 +1467,6 @@ public void preSetRegionServerQuota( ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "setRegionServerQuota", Action.ADMIN); } - @Override - public ReplicationEndpoint postCreateReplicationEndPoint( - ObserverContext ctx, ReplicationEndpoint endpoint) { - return endpoint; - } - @Override public void preReplicateLogEntries(ObserverContext ctx) throws IOException { diff --git a/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java index 2339113..c00959f 100644 --- a/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java +++ b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java @@ -145,24 +145,10 @@ public class TestCoprocessorInterfaceCoverage { // --- TODO: genuine gaps that need OPA implementation --- // These pre-hooks are user-facing and should enforce OPA permissions, but are not yet // implemented. They are excluded here to keep this test focused on detecting new - // upstream - // methods; the implementation gaps are tracked separately in plan.md (Step 2+). + // upstream methods; the implementation gaps are tracked separately in plan.md. // - // preCheckAndMutate / preCheckAndMutateAfterRowLock: generalisation of - // checkAndPut/Delete; - // should require WRITE + READ (same as other check-and-* operations). - "preCheckAndMutate(ObserverContext, CheckAndMutate, CheckAndMutateResult)", - "preCheckAndMutateAfterRowLock(ObserverContext, CheckAndMutate, CheckAndMutateResult)", - // Filter-based overloads of checkAndPut/Delete: we check the byte[]-based overloads - // but - // not these Filter-based variants, which were added alongside the byte[] ones. - "preCheckAndPut(ObserverContext, byte[], Filter, Put, boolean)", - "preCheckAndPutAfterRowLock(ObserverContext, byte[], Filter, Put, boolean)", - "preCheckAndDelete(ObserverContext, byte[], Filter, Delete, boolean)", - "preCheckAndDeleteAfterRowLock(ObserverContext, byte[], Filter, Delete, boolean)", // Metadata listing hooks: getTableNames is covered post-hoc by postGetTableNames - // filtering; - // listNamespace* hooks are not currently enforced. + // filtering; listNamespace* hooks are not currently enforced. "preGetTableNames(ObserverContext, List, String)", "preListNamespaceDescriptors(ObserverContext, List)", "preListNamespaces(ObserverContext, List)", From 809db410abb04082f02107972faca7b5cb4de300 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 17:41:26 +0100 Subject: [PATCH 13/29] fix 2 warnings --- .../hbase/OpenPolicyAgentAccessController.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 6b05184..f45569f 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -1,5 +1,6 @@ package tech.stackable.hbase; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.MapMaker; import com.google.protobuf.Message; import com.google.protobuf.RpcCallback; @@ -1182,15 +1183,13 @@ private static Map> familiesFromQualifiers( } /** Builds a single-entry family map for OPA from explicit family/qualifier byte arrays. */ - private static Map> familyMap(byte[] family, byte[] qualifier) { - if (family == null) return Collections.emptyMap(); - Map> result = new TreeMap<>(); - result.put( + private static ImmutableMap> familyMap(byte[] family, byte[] qualifier) { + if (family == null) return ImmutableMap.of(); + return ImmutableMap.of( Bytes.toString(family), qualifier != null ? Collections.singletonList(Bytes.toString(qualifier)) : Collections.emptyList()); - return result; } private void requirePermission( @@ -1246,9 +1245,10 @@ private void requirePermission( .anyMatch( perm -> { LOG.trace( - "requirePermission: user [{}] tableName[{}] permission [{}]", + "requirePermission: user [{}] tableName[{}] request [{}] permission [{}]", user, tableName, + request, perm); try { opaAclChecker.checkPermissionInfoWithOp( From 0ec94308e6fee3142f4831fe5dbea016edf3d0ae Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 17:59:54 +0100 Subject: [PATCH 14/29] Add OPA policy testing layer design doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Describes a fast intermediate test layer that captures unit-test WireMock payloads, remaps usernames to Rego-compatible principals, and validates the Rego policy logic using the opa test CLI — without integration test overhead. Co-Authored-By: Claude Sonnet 4.6 --- .../2026-03-06-opa-policy-testing-design.md | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 docs/plans/2026-03-06-opa-policy-testing-design.md diff --git a/docs/plans/2026-03-06-opa-policy-testing-design.md b/docs/plans/2026-03-06-opa-policy-testing-design.md new file mode 100644 index 0000000..e1c7b9f --- /dev/null +++ b/docs/plans/2026-03-06-opa-policy-testing-design.md @@ -0,0 +1,134 @@ +# OPA Policy Testing Layer — Design + +## Purpose + +Add a fast, intermediate test layer that validates the Rego policy logic against realistic +OPA inputs without the overhead of a full kuttl integration test setup. This augments (not +replaces) the integration tests. + +## Architecture + +``` +Unit tests run (Surefire) + │ + ▼ +WireMockRule ServeEventListener + │ collects (request body, response body) per serve event + ▼ +OpaFixtureWriter.flush() in TestUtils.tearDown() + │ remap usernames, split by result, deduplicate, write JSON + ▼ +src/test/rego/fixtures/ + allowed/ ← one .json per unique allow-path input + denied/ ← one .json per unique deny-path input + │ + ▼ (separate Maven phase) +exec-maven-plugin runs: opa test hbase.rego hbase_test.rego --data fixtures/ + │ + ▼ +Pass / Fail +``` + +## Components + +### 1. Fixture Capture (Java) + +**`OpaFixtureWriter`** — new test-only class: +- Thread-safe list accumulates `(requestBody, responseBody)` pairs via a `ServeEventListener` + registered on `WireMockRule` in `TestUtils` +- `flush(Path fixtureDir)` called from `TestUtils.tearDown()`: + 1. Remaps `callerUgi.userName` in raw JSON via string replacement: + - `allowedUser` → `admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL` + - `deniedUser` → `unknown@CLUSTER.LOCAL` + 2. Parses response body to determine `allowed/` vs `denied/` subdirectory + 3. Deduplicates — identical post-remap JSON written only once + 4. Writes files as `{index:04d}.json` + +Constants in `TestUtils`: +```java +static final String OPA_REMAP_ALLOWED = "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; +static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; +``` + +Fixtures are committed to source control. Running `mvn test` regenerates them in-place; +a `git diff` on the fixtures directory reveals any change in payload shape. + +### 2. OPA Test File (static, committed) + +**`src/test/rego/hbase_test.rego`**: +```rego +package hbase_test + +import data.hbase + +test_all_allowed_inputs if { + every inp in data.fixtures.allowed { + hbase.allow with input as inp + } +} + +test_all_denied_inputs if { + every inp in data.fixtures.denied { + not hbase.allow with input as inp + } +} +``` + +### 3. Rego Policy File (generated, not committed) + +**`target/generated-test-resources/rego/hbase.rego`** — generated during +`generate-test-resources` phase from the integration test source: + +- Source: `/home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2` +- Transformation: strip YAML ConfigMap wrapper, replace `$NAMESPACE` with `test-ns` +- Not committed (added to `.gitignore`) + +This ensures the Rego logic tested here is always identical to the integration test policy. +The `OPA_REMAP_ALLOWED` principal matches the generated `groups_for_user` entry for `admins`. + +### 4. Maven Setup + +Three plugin executions, all test-scoped: + +**Phase `generate-test-resources`** — `exec-maven-plugin`: +- Shell script strips YAML wrapper from `12-rego-rules.txt.j2` and substitutes `$NAMESPACE=test-ns` +- Output: `target/generated-test-resources/rego/hbase.rego` + +**Phase `process-test-resources`** — `download-maven-plugin`: +- Downloads pinned OPA binary to `target/opa` and chmods `+x` +- Version controlled via `` property in `pom.xml` +- OS/arch selected via Maven profiles: `linux_amd64`, `darwin_amd64`, `darwin_arm64` + +**Phase `test`** — `exec-maven-plugin` (after Surefire): +```bash +target/opa test \ + target/generated-test-resources/rego/hbase.rego \ + src/test/rego/hbase_test.rego \ + --data src/test/rego/fixtures/ +``` +- Skipped when `maven.test.skip=true` +- Fails the build on any OPA test failure + +## What This Tests + +- The JSON the Java coprocessor sends to OPA is parseable and has the expected field structure +- The Rego policy produces correct decisions (`allow`/`deny`) for those inputs +- The admin principal passes all hooks; an unknown principal fails all hooks + +## Future Extension + +The username mapping can be extended to a richer set of principals (developer, readonlyuser) +to exercise nuanced Rego branches (`matches_operation`, `matches_families`). This would +require either per-test-method mapping metadata or a separate `TestOpaInputCapture` class +with realistic principals. + +## Files Changed + +| File | Action | +|------|--------| +| `src/test/java/tech/stackable/hbase/TestUtils.java` | Add `ServeEventListener`, call `OpaFixtureWriter.flush()` | +| `src/test/java/tech/stackable/hbase/OpaFixtureWriter.java` | New class | +| `src/test/rego/hbase_test.rego` | New file (committed) | +| `src/test/rego/fixtures/` | New directory (committed, regenerated) | +| `pom.xml` | Add `download-maven-plugin`, two `exec-maven-plugin` executions, OS profiles, `opa.version` property | +| `.gitignore` | Add `target/generated-test-resources/` | From 89baf611104d467339f6a6b25ee83be6c51dd691 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 18:06:03 +0100 Subject: [PATCH 15/29] Add OPA policy testing implementation plan Five tasks: OpaFixtureWriter class, test wiring, Rego files, Maven plugins (download-maven-plugin + exec-maven-plugin), and end-to-end verification. Co-Authored-By: Claude Sonnet 4.6 --- docs/plans/2026-03-06-opa-policy-testing.md | 506 ++++++++++++++++++++ 1 file changed, 506 insertions(+) create mode 100644 docs/plans/2026-03-06-opa-policy-testing.md diff --git a/docs/plans/2026-03-06-opa-policy-testing.md b/docs/plans/2026-03-06-opa-policy-testing.md new file mode 100644 index 0000000..03d6f55 --- /dev/null +++ b/docs/plans/2026-03-06-opa-policy-testing.md @@ -0,0 +1,506 @@ +# OPA Policy Testing Layer — Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Capture OPA HTTP request payloads from unit tests, remap usernames to Rego-compatible principals, write them as fixture JSON files, and validate them against the real Rego policy using the `opa test` CLI — giving fast policy correctness feedback without a full integration test cluster. + +**Architecture:** WireMock's `RequestListener` captures every `(request, response)` pair during unit tests. After each test class, `OpaFixtureWriter.flush()` remaps `allowedUser` → admin Kerberos principal and `deniedUser` → unknown principal, deduplicates, and writes to `src/test/rego/fixtures/{allowed,denied}/`. Maven downloads a pinned OPA binary and runs `opa test` against the fixtures and a checked-in copy of the Rego rules (derived from the integration test template) in the `verify` phase. + +**Tech Stack:** WireMock 3.6.0 (`RequestListener`), Jackson (already in project), `download-maven-plugin`, `exec-maven-plugin`, OPA CLI `opa test`. + +--- + +### Task 1: `OpaFixtureWriter` class + +**Files:** +- Create: `src/test/java/tech/stackable/hbase/OpaFixtureWriter.java` + +**Step 1: Create the class** + +```java +package tech.stackable.hbase; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.github.tomakehurst.wiremock.http.Request; +import com.github.tomakehurst.wiremock.http.Response; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Captures WireMock OPA requests during unit tests and writes them as fixture JSON files for + * offline {@code opa test} validation. + * + *

Usernames are remapped so fixtures exercise real Rego policy logic: + *

    + *
  • {@code allowedUser} → admin Kerberos principal (member of "admins" group in Rego) + *
  • {@code deniedUser} → unknown principal (not in any Rego group) + *
+ */ +public class OpaFixtureWriter { + + static final String OPA_REMAP_ALLOWED = + "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; + static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; + + private static final Path FIXTURES_BASE = Paths.get("src/test/rego/fixtures"); + private static final Path ALLOWED_DIR = FIXTURES_BASE.resolve("allowed"); + private static final Path DENIED_DIR = FIXTURES_BASE.resolve("denied"); + + /** All captured (requestBody, responseBody) pairs across all test classes in this JVM run. */ + private static final List captured = new CopyOnWriteArrayList<>(); + + /** Tracks whether we have cleared old fixture files yet in this JVM run. */ + private static final AtomicBoolean clearedOnce = new AtomicBoolean(false); + + /** Sequential file counters, shared across all flush() calls in one JVM run. */ + private static final AtomicInteger allowedIdx = new AtomicInteger(0); + + private static final AtomicInteger deniedIdx = new AtomicInteger(0); + + /** Deduplication sets, shared across all flush() calls in one JVM run. */ + private static final Set seenAllowed = new HashSet<>(); + + private static final Set seenDenied = new HashSet<>(); + + /** + * Called by the WireMock RequestListener on each request. Thread-safe. + */ + public static void capture(Request request, Response response) { + captured.add(new String[] {request.getBodyAsString(), response.getBodyAsString()}); + } + + /** + * Remaps captured requests, deduplicates, and writes fixture JSON files. Called from {@link + * TestUtils#tearDown()} at the end of each test class. + */ + public static void flush() throws IOException { + if (clearedOnce.compareAndSet(false, true)) { + deleteDir(ALLOWED_DIR); + deleteDir(DENIED_DIR); + } + Files.createDirectories(ALLOWED_DIR); + Files.createDirectories(DENIED_DIR); + + List toProcess = List.copyOf(captured); + captured.clear(); + + for (String[] pair : toProcess) { + String requestBody = pair[0]; + String responseBody = pair[1]; + + String remapped = + requestBody + .replace("allowedUser", OPA_REMAP_ALLOWED) + .replace("deniedUser", OPA_REMAP_DENIED); + + boolean allowed = responseBody.contains("\"true\""); + + synchronized (OpaFixtureWriter.class) { + if (allowed) { + if (seenAllowed.add(remapped)) { + Files.writeString( + ALLOWED_DIR.resolve(String.format("%04d.json", allowedIdx.getAndIncrement())), + remapped); + } + } else { + if (seenDenied.add(remapped)) { + Files.writeString( + DENIED_DIR.resolve(String.format("%04d.json", deniedIdx.getAndIncrement())), + remapped); + } + } + } + } + } + + private static void deleteDir(Path dir) throws IOException { + if (Files.exists(dir)) { + try (var entries = Files.list(dir)) { + for (Path p : entries.toList()) { + Files.delete(p); + } + } + Files.delete(dir); + } + } +} +``` + +**Step 2: Run spotless and compile** + +```bash +JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn spotless:apply -q compile -q +``` + +Expected: BUILD SUCCESS, no errors. + +**Step 3: Commit** + +```bash +git add src/test/java/tech/stackable/hbase/OpaFixtureWriter.java +git commit -m "Add OpaFixtureWriter for capturing WireMock OPA fixtures" +``` + +--- + +### Task 2: Wire `OpaFixtureWriter` into the test infrastructure + +**Files:** +- Modify: `src/test/java/tech/stackable/hbase/TestUtils.java` +- Modify: `src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java` +- Modify: `src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java` +- Modify: `src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java` + +**Step 1: Add `flush()` call to `TestUtils.tearDown()`** + +In `TestUtils.java`, change: +```java +protected static void tearDown() throws Exception { + TEST_UTIL.shutdownMiniCluster(); +} +``` +to: +```java +protected static void tearDown() throws Exception { + OpaFixtureWriter.flush(); + TEST_UTIL.shutdownMiniCluster(); +} +``` + +**Step 2: Register the listener in each test class** + +In each of the three test classes, find the `@BeforeClass` / `setUpClass` method and add the listener registration as the FIRST line. The `WireMockRule` is a `@ClassRule`, so it is already started by the time `@BeforeClass` runs. + +`TestOpenPolicyAgentAccessController.java` — `setUpClass` currently starts with: +```java +stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); +setup(OpenPolicyAgentAccessController.class, false, OPA_URL); +``` +Change to: +```java +wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); +stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); +setup(OpenPolicyAgentAccessController.class, false, OPA_URL); +``` + +Apply the same one-line addition (`wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture);`) as the first line of `setUpClass` in: +- `TestOpenPolicyAgentAccessControllerRegion.java` +- `TestOpenPolicyAgentAccessControllerVariants.java` + +Add the import to each class that needs it: +```java +import com.github.tomakehurst.wiremock.http.RequestListener; +``` +(The method reference `OpaFixtureWriter::capture` satisfies the `RequestListener` functional interface.) + +**Step 3: Run spotless and the full test suite** + +```bash +JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn spotless:apply -q test -q 2>&1 | grep -E 'Tests run:|BUILD' +``` + +Expected: 87 tests, BUILD SUCCESS. Also check that fixture files were created: + +```bash +ls src/test/rego/fixtures/allowed/ | head -5 +ls src/test/rego/fixtures/denied/ | head -5 +``` + +Expected: several `.json` files in each directory. + +**Step 4: Inspect a fixture to verify remapping** + +```bash +cat src/test/rego/fixtures/allowed/0000.json | python3 -m json.tool | grep userName +``` + +Expected: output contains `"admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"`, not `"allowedUser"`. + +```bash +cat src/test/rego/fixtures/denied/0000.json | python3 -m json.tool | grep userName +``` + +Expected: output contains `"unknown@CLUSTER.LOCAL"`, not `"deniedUser"`. + +**Step 5: Commit** + +```bash +git add src/test/java/tech/stackable/hbase/TestUtils.java \ + src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java \ + src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java \ + src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java \ + src/test/rego/fixtures/ +git commit -m "Wire OpaFixtureWriter into test teardown and register WireMock listener" +``` + +--- + +### Task 3: Add Rego test files + +**Files:** +- Create: `src/test/rego/hbase.rego` +- Create: `src/test/rego/hbase_test.rego` + +**Step 1: Generate `hbase.rego` from the integration test template** + +The integration test Rego lives at: +`/home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2` + +It is a YAML ConfigMap wrapping the Rego with 4-space indentation. Strip the 9-line YAML header, remove the 4-space indent, and substitute `$NAMESPACE` with `test-ns`: + +```bash +tail -n +10 /home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 \ + | sed 's/^ //' \ + | sed 's/\$NAMESPACE/test-ns/g' \ + > src/test/rego/hbase.rego +``` + +Verify the first few lines look like valid Rego (no YAML, no leading spaces): +```bash +head -5 src/test/rego/hbase.rego +``` +Expected: +``` +package hbase + +default allow := false +default matches_identity(identity) := false +``` + +Add a comment at the top of the file noting its origin. Edit `src/test/rego/hbase.rego` to prepend: +```rego +# Derived from hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 +# with $NAMESPACE replaced by "test-ns". Regenerate with: +# tail -n +10 /12-rego-rules.txt.j2 | sed 's/^ //' | sed 's/\$NAMESPACE/test-ns/g' +# +``` + +**Step 2: Create `hbase_test.rego`** + +```bash +mkdir -p src/test/rego +``` + +Create `src/test/rego/hbase_test.rego`: + +```rego +package hbase_test + +import data.hbase + +# Every fixture in allowed/ must produce allow=true under the real Rego policy. +test_all_allowed_inputs if { + every key, fixture in data.fixtures.allowed { + hbase.allow with input as fixture.input + } +} + +# Every fixture in denied/ must produce allow=false under the real Rego policy. +test_all_denied_inputs if { + every key, fixture in data.fixtures.denied { + not hbase.allow with input as fixture.input + } +} +``` + +**Step 3: Commit** + +```bash +git add src/test/rego/hbase.rego src/test/rego/hbase_test.rego +git commit -m "Add Rego policy file and OPA test suite" +``` + +--- + +### Task 4: Maven — download OPA and run `opa test` + +**Files:** +- Modify: `pom.xml` +- Modify: `.gitignore` + +**Step 1: Add `opa.version` property and OS profiles to `pom.xml`** + +In the `` section, add: +```xml +0.63.0 +``` + +After the closing `` tag and before ``, add Maven profiles for OS detection: +```xml + + + opa-linux-amd64 + + Linuxamd64 + + linux_amd64_static + + + opa-mac-amd64 + + Mac OS Xx86_64 + + darwin_amd64 + + + opa-mac-arm64 + + Mac OS Xaarch64 + + darwin_arm64 + + +``` + +**Step 2: Add `download-maven-plugin` to ``** + +```xml + + com.googlecode.maven-download-plugin + download-maven-plugin + 1.8.1 + + + download-opa + process-test-resources + wget + + https://github.com/open-policy-agent/opa/releases/download/v${opa.version}/opa_${opa.classifier} + ${project.build.directory} + opa + false + + + + +``` + +**Step 3: Add `exec-maven-plugin` executions to ``** + +```xml + + org.codehaus.mojo + exec-maven-plugin + 3.1.0 + + + chmod-opa + process-test-resources + exec + + chmod + + +x + ${project.build.directory}/opa + + + + + opa-policy-test + verify + exec + + ${project.build.directory}/opa + + test + src/test/rego/hbase.rego + src/test/rego/hbase_test.rego + --data + src/test/rego/fixtures + -v + + ${maven.test.skip} + + + + +``` + +Note: `opa test` runs in the `verify` phase, which is AFTER the `test` phase (where Surefire generates the fixtures). Run with `mvn verify` to execute both unit tests and OPA tests. + +**Step 4: Add OPA binary to `.gitignore`** + +Append to `.gitignore`: +``` +/target/opa +``` + +**Step 5: Run spotless and verify compilation** + +```bash +JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn spotless:apply -q compile -q +``` + +Expected: BUILD SUCCESS. + +**Step 6: Commit** + +```bash +git add pom.xml .gitignore +git commit -m "Add download-maven-plugin and exec-maven-plugin for OPA policy testing" +``` + +--- + +### Task 5: End-to-end verification + +**Step 1: Run `mvn verify`** + +```bash +JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn verify 2>&1 | grep -E 'Tests run:|opa|PASS|FAIL|BUILD' +``` + +Expected output includes: +``` +Tests run: 87, Failures: 0, Errors: 0, Skipped: 1 +data.hbase_test.test_all_allowed_inputs: PASS (...) +data.hbase_test.test_all_denied_inputs: PASS (...) +BUILD SUCCESS +``` + +**Step 2: Verify fixture count is reasonable** + +```bash +echo "allowed: $(ls src/test/rego/fixtures/allowed/ | wc -l)" +echo "denied: $(ls src/test/rego/fixtures/denied/ | wc -l)" +``` + +Expected: at least 20 allowed fixtures and several denied fixtures (exact count will vary). + +**Step 3: Commit fixtures and final state** + +```bash +git add src/test/rego/fixtures/ +git commit -m "Add generated OPA fixtures from unit test capture" +``` + +--- + +## Running OPA tests standalone (no Maven) + +After the first `mvn test` has generated fixtures: + +```bash +target/opa test src/test/rego/hbase.rego src/test/rego/hbase_test.rego \ + --data src/test/rego/fixtures -v +``` + +## Keeping `hbase.rego` in sync + +When `12-rego-rules.txt.j2` changes in the hbase-operator repo, regenerate: + +```bash +tail -n +10 /home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 \ + | sed 's/^ //' \ + | sed 's/\$NAMESPACE/test-ns/g' \ + >> /tmp/hbase_new.rego +# Prepend the comment header, review diff, then replace src/test/rego/hbase.rego +``` From 62843aea7020dce073f12bef4e415e8140cb64d1 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 19:28:19 +0100 Subject: [PATCH 16/29] Add OpaFixtureWriter for capturing WireMock OPA fixtures Co-Authored-By: Claude Sonnet 4.6 --- .../stackable/hbase/OpaFixtureWriter.java | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 src/test/java/tech/stackable/hbase/OpaFixtureWriter.java diff --git a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java new file mode 100644 index 0000000..e053c10 --- /dev/null +++ b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java @@ -0,0 +1,112 @@ +package tech.stackable.hbase; + +import com.github.tomakehurst.wiremock.http.Request; +import com.github.tomakehurst.wiremock.http.Response; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Captures WireMock OPA requests during unit tests and writes them as fixture JSON files for + * offline {@code opa test} validation. + * + *

Usernames are remapped so fixtures exercise real Rego policy logic: + * + *

    + *
  • {@code allowedUser} → admin Kerberos principal (member of "admins" group in Rego) + *
  • {@code deniedUser} → unknown principal (not in any Rego group) + *
+ */ +public class OpaFixtureWriter { + + static final String OPA_REMAP_ALLOWED = + "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; + static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; + + private static final Path FIXTURES_BASE = Paths.get("src/test/rego/fixtures"); + private static final Path ALLOWED_DIR = FIXTURES_BASE.resolve("allowed"); + private static final Path DENIED_DIR = FIXTURES_BASE.resolve("denied"); + + /** All captured (requestBody, responseBody) pairs across all test classes in this JVM run. */ + private static final List captured = new CopyOnWriteArrayList<>(); + + /** Tracks whether we have cleared old fixture files yet in this JVM run. */ + private static final AtomicBoolean clearedOnce = new AtomicBoolean(false); + + /** Sequential file counters, shared across all flush() calls in one JVM run. */ + private static final AtomicInteger allowedIdx = new AtomicInteger(0); + + private static final AtomicInteger deniedIdx = new AtomicInteger(0); + + /** Deduplication sets, shared across all flush() calls in one JVM run. */ + private static final Set seenAllowed = new HashSet<>(); + + private static final Set seenDenied = new HashSet<>(); + + /** Called by the WireMock RequestListener on each request. Thread-safe. */ + public static void capture(Request request, Response response) { + captured.add(new String[] {request.getBodyAsString(), response.getBodyAsString()}); + } + + /** + * Remaps captured requests, deduplicates, and writes fixture JSON files. Called from {@link + * TestUtils#tearDown()} at the end of each test class. + */ + public static void flush() throws IOException { + if (clearedOnce.compareAndSet(false, true)) { + deleteDir(ALLOWED_DIR); + deleteDir(DENIED_DIR); + } + Files.createDirectories(ALLOWED_DIR); + Files.createDirectories(DENIED_DIR); + + List toProcess = List.copyOf(captured); + captured.clear(); + + for (String[] pair : toProcess) { + String requestBody = pair[0]; + String responseBody = pair[1]; + + String remapped = + requestBody + .replace("allowedUser", OPA_REMAP_ALLOWED) + .replace("deniedUser", OPA_REMAP_DENIED); + + boolean allowed = responseBody.contains("\"true\""); + + synchronized (OpaFixtureWriter.class) { + if (allowed) { + if (seenAllowed.add(remapped)) { + Files.writeString( + ALLOWED_DIR.resolve(String.format("%04d.json", allowedIdx.getAndIncrement())), + remapped); + } + } else { + if (seenDenied.add(remapped)) { + Files.writeString( + DENIED_DIR.resolve(String.format("%04d.json", deniedIdx.getAndIncrement())), + remapped); + } + } + } + } + } + + private static void deleteDir(Path dir) throws IOException { + if (Files.exists(dir)) { + try (var entries = Files.list(dir)) { + for (Path p : entries.toList()) { + Files.delete(p); + } + } + Files.delete(dir); + } + } +} From 9b7197574dc555511595e9af80fba06b490b532a Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 19:31:48 +0100 Subject: [PATCH 17/29] Fix OpaFixtureWriter concurrency: single monitor, plain int counters Co-Authored-By: Claude Sonnet 4.6 --- .../stackable/hbase/OpaFixtureWriter.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java index e053c10..d67bd47 100644 --- a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java +++ b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java @@ -6,12 +6,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; /** * Captures WireMock OPA requests during unit tests and writes them as fixture JSON files for @@ -35,15 +34,15 @@ public class OpaFixtureWriter { private static final Path DENIED_DIR = FIXTURES_BASE.resolve("denied"); /** All captured (requestBody, responseBody) pairs across all test classes in this JVM run. */ - private static final List captured = new CopyOnWriteArrayList<>(); + private static final List captured = new ArrayList<>(); /** Tracks whether we have cleared old fixture files yet in this JVM run. */ private static final AtomicBoolean clearedOnce = new AtomicBoolean(false); /** Sequential file counters, shared across all flush() calls in one JVM run. */ - private static final AtomicInteger allowedIdx = new AtomicInteger(0); + private static int allowedIdx = 0; - private static final AtomicInteger deniedIdx = new AtomicInteger(0); + private static int deniedIdx = 0; /** Deduplication sets, shared across all flush() calls in one JVM run. */ private static final Set seenAllowed = new HashSet<>(); @@ -51,7 +50,7 @@ public class OpaFixtureWriter { private static final Set seenDenied = new HashSet<>(); /** Called by the WireMock RequestListener on each request. Thread-safe. */ - public static void capture(Request request, Response response) { + public static synchronized void capture(Request request, Response response) { captured.add(new String[] {request.getBodyAsString(), response.getBodyAsString()}); } @@ -67,8 +66,11 @@ public static void flush() throws IOException { Files.createDirectories(ALLOWED_DIR); Files.createDirectories(DENIED_DIR); - List toProcess = List.copyOf(captured); - captured.clear(); + List toProcess; + synchronized (OpaFixtureWriter.class) { + toProcess = new ArrayList<>(captured); + captured.clear(); + } for (String[] pair : toProcess) { String requestBody = pair[0]; @@ -79,20 +81,19 @@ public static void flush() throws IOException { .replace("allowedUser", OPA_REMAP_ALLOWED) .replace("deniedUser", OPA_REMAP_DENIED); + // WireMock stubs in this test suite always return {"result": "true"} or {"result": "false"}. boolean allowed = responseBody.contains("\"true\""); synchronized (OpaFixtureWriter.class) { if (allowed) { if (seenAllowed.add(remapped)) { Files.writeString( - ALLOWED_DIR.resolve(String.format("%04d.json", allowedIdx.getAndIncrement())), - remapped); + ALLOWED_DIR.resolve(String.format("%04d.json", allowedIdx++)), remapped); } } else { if (seenDenied.add(remapped)) { Files.writeString( - DENIED_DIR.resolve(String.format("%04d.json", deniedIdx.getAndIncrement())), - remapped); + DENIED_DIR.resolve(String.format("%04d.json", deniedIdx++)), remapped); } } } From 748faae90bf6149221751d6e4ab6f6b46a7eb9f6 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 19:35:54 +0100 Subject: [PATCH 18/29] Wire OpaFixtureWriter into test teardown and WireMock listener Co-Authored-By: Claude Sonnet 4.6 --- src/test/java/tech/stackable/hbase/OpaFixtureWriter.java | 2 +- .../hbase/TestOpenPolicyAgentAccessController.java | 1 + .../hbase/TestOpenPolicyAgentAccessControllerRegion.java | 1 + .../hbase/TestOpenPolicyAgentAccessControllerVariants.java | 6 ++++++ src/test/java/tech/stackable/hbase/TestUtils.java | 1 + 5 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java index d67bd47..3f4d3b8 100644 --- a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java +++ b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java @@ -103,7 +103,7 @@ public static void flush() throws IOException { private static void deleteDir(Path dir) throws IOException { if (Files.exists(dir)) { try (var entries = Files.list(dir)) { - for (Path p : entries.toList()) { + for (Path p : entries.collect(java.util.stream.Collectors.toList())) { Files.delete(p); } } diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java index bfabe2c..c0ddfb4 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java @@ -45,6 +45,7 @@ public class TestOpenPolicyAgentAccessController extends TestUtils { @BeforeClass public static void setUpClass() throws Exception { + wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); setup(OpenPolicyAgentAccessController.class, false, OPA_URL); } diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java index cccadbe..fe13895 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -48,6 +48,7 @@ public class TestOpenPolicyAgentAccessControllerRegion extends TestUtils { @BeforeClass public static void setUpClass() throws Exception { + wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); setup(OpenPolicyAgentAccessController.class, false, OPA_URL); diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java index 6d19bde..b8841f5 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java @@ -14,6 +14,7 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.security.AccessControlException; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -26,6 +27,11 @@ public class TestOpenPolicyAgentAccessControllerVariants extends TestUtils { @Rule public WireMockRule wireMockRule = new WireMockRule(8089); + @Before + public void setUpListener() { + wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); + } + @Test public void testDryRun() throws Exception { stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); diff --git a/src/test/java/tech/stackable/hbase/TestUtils.java b/src/test/java/tech/stackable/hbase/TestUtils.java index 2cf25a1..b4e1157 100644 --- a/src/test/java/tech/stackable/hbase/TestUtils.java +++ b/src/test/java/tech/stackable/hbase/TestUtils.java @@ -262,6 +262,7 @@ protected void assertAllowedThenDenied(SecureTestUtil.AccessTestAction action) t } protected static void tearDown() throws Exception { + OpaFixtureWriter.flush(); TEST_UTIL.shutdownMiniCluster(); } From 0820e2995ea6e7a6a3c7541269f6a7ce8cff9d5d Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 19:41:56 +0100 Subject: [PATCH 19/29] Filter system-user fixtures; fix listener registration in Variants test Co-Authored-By: Claude Sonnet 4.6 --- .../tech/stackable/hbase/OpaFixtureWriter.java | 4 ++++ ...estOpenPolicyAgentAccessControllerVariants.java | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java index 3f4d3b8..79a6ae5 100644 --- a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java +++ b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java @@ -76,6 +76,10 @@ public static void flush() throws IOException { String requestBody = pair[0]; String responseBody = pair[1]; + // Only process requests that came from test users — skip cluster-internal traffic. + if (!requestBody.contains("allowedUser") && !requestBody.contains("deniedUser")) { + continue; + } String remapped = requestBody .replace("allowedUser", OPA_REMAP_ALLOWED) diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java index b8841f5..40c6174 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java @@ -14,7 +14,6 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.security.AccessControlException; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -25,12 +24,13 @@ public class TestOpenPolicyAgentAccessControllerVariants extends TestUtils { public static final String OPA_URL = "http://localhost:8089"; - @Rule public WireMockRule wireMockRule = new WireMockRule(8089); - - @Before - public void setUpListener() { - wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); - } + @Rule + public WireMockRule wireMockRule = + new WireMockRule(8089) { + { + addMockServiceRequestListener(OpaFixtureWriter::capture); + } + }; @Test public void testDryRun() throws Exception { From 1847ff64bd40a4438959b63032c21bda2f5f53fc Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 20:04:27 +0100 Subject: [PATCH 20/29] Use @Before for listener registration; document fixture filter Co-Authored-By: Claude Sonnet 4.6 --- .../2026-03-06-opa-policy-testing-design.md | 35 +++++++++++-------- docs/plans/2026-03-06-opa-policy-testing.md | 17 +++++++++ .../stackable/hbase/OpaFixtureWriter.java | 4 ++- ...enPolicyAgentAccessControllerVariants.java | 16 +++++---- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/docs/plans/2026-03-06-opa-policy-testing-design.md b/docs/plans/2026-03-06-opa-policy-testing-design.md index e1c7b9f..86722bc 100644 --- a/docs/plans/2026-03-06-opa-policy-testing-design.md +++ b/docs/plans/2026-03-06-opa-policy-testing-design.md @@ -35,16 +35,17 @@ Pass / Fail **`OpaFixtureWriter`** — new test-only class: - Thread-safe list accumulates `(requestBody, responseBody)` pairs via a `ServeEventListener` - registered on `WireMockRule` in `TestUtils` +registered on `WireMockRule` in `TestUtils` - `flush(Path fixtureDir)` called from `TestUtils.tearDown()`: - 1. Remaps `callerUgi.userName` in raw JSON via string replacement: - - `allowedUser` → `admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL` - - `deniedUser` → `unknown@CLUSTER.LOCAL` - 2. Parses response body to determine `allowed/` vs `denied/` subdirectory - 3. Deduplicates — identical post-remap JSON written only once - 4. Writes files as `{index:04d}.json` +1. Remaps `callerUgi.userName` in raw JSON via string replacement: +- `allowedUser` → `admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL` +- `deniedUser` → `unknown@CLUSTER.LOCAL` +2. Parses response body to determine `allowed/` vs `denied/` subdirectory +3. Deduplicates — identical post-remap JSON written only once +4. Writes files as `{index:04d}.json` Constants in `TestUtils`: + ```java static final String OPA_REMAP_ALLOWED = "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; @@ -56,6 +57,7 @@ a `git diff` on the fixtures directory reveals any change in payload shape. ### 2. OPA Test File (static, committed) **`src/test/rego/hbase_test.rego`**: + ```rego package hbase_test @@ -100,12 +102,14 @@ Three plugin executions, all test-scoped: - OS/arch selected via Maven profiles: `linux_amd64`, `darwin_amd64`, `darwin_arm64` **Phase `test`** — `exec-maven-plugin` (after Surefire): + ```bash target/opa test \ target/generated-test-resources/rego/hbase.rego \ src/test/rego/hbase_test.rego \ --data src/test/rego/fixtures/ ``` + - Skipped when `maven.test.skip=true` - Fails the build on any OPA test failure @@ -124,11 +128,12 @@ with realistic principals. ## Files Changed -| File | Action | -|------|--------| -| `src/test/java/tech/stackable/hbase/TestUtils.java` | Add `ServeEventListener`, call `OpaFixtureWriter.flush()` | -| `src/test/java/tech/stackable/hbase/OpaFixtureWriter.java` | New class | -| `src/test/rego/hbase_test.rego` | New file (committed) | -| `src/test/rego/fixtures/` | New directory (committed, regenerated) | -| `pom.xml` | Add `download-maven-plugin`, two `exec-maven-plugin` executions, OS profiles, `opa.version` property | -| `.gitignore` | Add `target/generated-test-resources/` | +| File | Action | +|------------------------------------------------------------|------------------------------------------------------------------------------------------------------| +| `src/test/java/tech/stackable/hbase/TestUtils.java` | Add `ServeEventListener`, call `OpaFixtureWriter.flush()` | +| `src/test/java/tech/stackable/hbase/OpaFixtureWriter.java` | New class | +| `src/test/rego/hbase_test.rego` | New file (committed) | +| `src/test/rego/fixtures/` | New directory (committed, regenerated) | +| `pom.xml` | Add `download-maven-plugin`, two `exec-maven-plugin` executions, OS profiles, `opa.version` property | +| `.gitignore` | Add `target/generated-test-resources/` | + diff --git a/docs/plans/2026-03-06-opa-policy-testing.md b/docs/plans/2026-03-06-opa-policy-testing.md index 03d6f55..126af48 100644 --- a/docs/plans/2026-03-06-opa-policy-testing.md +++ b/docs/plans/2026-03-06-opa-policy-testing.md @@ -162,12 +162,15 @@ git commit -m "Add OpaFixtureWriter for capturing WireMock OPA fixtures" **Step 1: Add `flush()` call to `TestUtils.tearDown()`** In `TestUtils.java`, change: + ```java protected static void tearDown() throws Exception { TEST_UTIL.shutdownMiniCluster(); } ``` + to: + ```java protected static void tearDown() throws Exception { OpaFixtureWriter.flush(); @@ -180,11 +183,14 @@ protected static void tearDown() throws Exception { In each of the three test classes, find the `@BeforeClass` / `setUpClass` method and add the listener registration as the FIRST line. The `WireMockRule` is a `@ClassRule`, so it is already started by the time `@BeforeClass` runs. `TestOpenPolicyAgentAccessController.java` — `setUpClass` currently starts with: + ```java stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); setup(OpenPolicyAgentAccessController.class, false, OPA_URL); ``` + Change to: + ```java wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); @@ -196,9 +202,11 @@ Apply the same one-line addition (`wireMockRule.addMockServiceRequestListener(Op - `TestOpenPolicyAgentAccessControllerVariants.java` Add the import to each class that needs it: + ```java import com.github.tomakehurst.wiremock.http.RequestListener; ``` + (The method reference `OpaFixtureWriter::capture` satisfies the `RequestListener` functional interface.) **Step 3: Run spotless and the full test suite** @@ -264,10 +272,13 @@ tail -n +10 /home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-re ``` Verify the first few lines look like valid Rego (no YAML, no leading spaces): + ```bash head -5 src/test/rego/hbase.rego ``` + Expected: + ``` package hbase @@ -276,6 +287,7 @@ default matches_identity(identity) := false ``` Add a comment at the top of the file noting its origin. Edit `src/test/rego/hbase.rego` to prepend: + ```rego # Derived from hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 # with $NAMESPACE replaced by "test-ns". Regenerate with: @@ -329,11 +341,13 @@ git commit -m "Add Rego policy file and OPA test suite" **Step 1: Add `opa.version` property and OS profiles to `pom.xml`** In the `` section, add: + ```xml 0.63.0 ``` After the closing `` tag and before ``, add Maven profiles for OS detection: + ```xml @@ -429,6 +443,7 @@ Note: `opa test` runs in the `verify` phase, which is AFTER the `test` phase (wh **Step 4: Add OPA binary to `.gitignore`** Append to `.gitignore`: + ``` /target/opa ``` @@ -459,6 +474,7 @@ JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn verify 2>&1 | grep -E 'Tests ru ``` Expected output includes: + ``` Tests run: 87, Failures: 0, Errors: 0, Skipped: 1 data.hbase_test.test_all_allowed_inputs: PASS (...) @@ -504,3 +520,4 @@ tail -n +10 /home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-re >> /tmp/hbase_new.rego # Prepend the comment header, review diff, then replace src/test/rego/hbase.rego ``` + diff --git a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java index 79a6ae5..88f98ed 100644 --- a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java +++ b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java @@ -76,7 +76,9 @@ public static void flush() throws IOException { String requestBody = pair[0]; String responseBody = pair[1]; - // Only process requests that came from test users — skip cluster-internal traffic. + // Only capture requests from the standard allow/deny test users defined in TestUtils. + // Requests from other named users (e.g. Variants-specific users) and cluster-internal + // traffic are intentionally skipped — they are not useful for Rego policy validation. if (!requestBody.contains("allowedUser") && !requestBody.contains("deniedUser")) { continue; } diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java index 40c6174..0911e6c 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java @@ -14,6 +14,7 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.SecureTestUtil; import org.apache.hadoop.security.AccessControlException; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -24,13 +25,14 @@ public class TestOpenPolicyAgentAccessControllerVariants extends TestUtils { public static final String OPA_URL = "http://localhost:8089"; - @Rule - public WireMockRule wireMockRule = - new WireMockRule(8089) { - { - addMockServiceRequestListener(OpaFixtureWriter::capture); - } - }; + // @Rule (not @ClassRule) because each test starts and tears down its own mini-cluster + // (setup/tearDown are called inside the test body, not in @BeforeClass/@AfterClass). + @Rule public WireMockRule wireMockRule = new WireMockRule(8089); + + @Before + public void registerOpaListener() { + wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); + } @Test public void testDryRun() throws Exception { From 8bb630259b73c6d7f07faa5595c7e8939479b02b Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 20:05:43 +0100 Subject: [PATCH 21/29] Add Rego policy file and OPA test suite Co-Authored-By: Claude Sonnet 4.6 --- src/test/rego/hbase.rego | 143 ++++++++++++++++++++++++++++++++++ src/test/rego/hbase_test.rego | 17 ++++ 2 files changed, 160 insertions(+) create mode 100644 src/test/rego/hbase.rego create mode 100644 src/test/rego/hbase_test.rego diff --git a/src/test/rego/hbase.rego b/src/test/rego/hbase.rego new file mode 100644 index 0000000..aef841d --- /dev/null +++ b/src/test/rego/hbase.rego @@ -0,0 +1,143 @@ +# Derived from hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 +# with $NAMESPACE replaced by "test-ns". Regenerate with: +# tail -n +10 /12-rego-rules.txt.j2 | sed 's/^ //' | sed 's/\$NAMESPACE/test-ns/g' +# +package hbase + +default allow := false +default matches_identity(identity) := false + +# table is null if the request is for namespace permissions, but as parameters cannot be +# undefined, we have to set it to something specific: +checked_table_name := input.table.qualifierAsString if {input.table.qualifierAsString} +checked_table_name := "__undefined__" if {not input.table.qualifierAsString} + +allow if { + some acl in acls + matches_identity(acl.identity) + matches_resource(input.namespace, checked_table_name, acl.resource) + action_sufficient_for_operation(acl.action, input.action) + matches_operation(acl, input.operation) + matches_families(acl, input.families) +} + +# Identity mentions the (long) userName explicitly +matches_identity(identity) if { + identity in { + concat("", ["user:", input.callerUgi.userName]) + } +} + +# Identity regex matches the (long) userName +matches_identity(identity) if { + match_entire(identity, concat("", ["userRegex:", input.callerUgi.userName])) +} + +# Identity mentions group the user is part of (by looking up using the (long) userName) +matches_identity(identity) if { + some group in groups_for_user[input.callerUgi.userName] + identity == concat("", ["group:", group]) +} + +# Allow all resources +matches_resource(namespace, table, resource) if { + resource == "hbase:" +} + +# Allow all namespaces +matches_resource(namespace, table, resource) if { + resource == "hbase:namespace:" +} + +# Resource mentions the namespace explicitly +matches_resource(namespace, table, resource) if { + resource == concat(":", ["hbase:namespace", namespace]) +} + +# Resource mentions the namespaced table explicitly +matches_resource(namespace, table, resource) if { + resource == concat("", ["hbase:table:", namespace, "/", table]) +} + +match_entire(pattern, value) if { + # Add the anchors ^ and $ + pattern_with_anchors := concat("", ["^", pattern, "$"]) + + regex.match(pattern_with_anchors, value) +} + +action_sufficient_for_operation(action, operation) if { + action_hierarchy[action][_] == action_for_operation[operation] +} + +action_hierarchy := { + "full": ["full", "rw", "ro"], + "rw": ["rw", "ro"], + "ro": ["ro"], +} + +action_for_operation := { + "ADMIN": "full", + "CREATE": "full", + "WRITE": "rw", + "READ": "ro", + "EXEC": "full", +} + +# If the ACL does not restrict operations, all operation types are permitted. +matches_operation(acl, _) if { + not acl.operations +} + +# If the ACL restricts operations, the requested OpType must be in the permitted set. +matches_operation(acl, operation) if { + acl.operations + operation in acl.operations +} + +# If the ACL does not restrict families, all column families are permitted. +matches_families(acl, _) if { + not acl.families +} + +# If the ACL restricts families, every family present in the request must be allowed. +# An empty families map (namespace-scoped or unfiltered scan) always passes. +matches_families(acl, families) if { + acl.families + count({f | f := object.keys(families)[_]; not f in acl.families}) == 0 +} + +groups_for_user := { + "hbase/hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL": ["admins"], + "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL": ["admins"], + "developer/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL": ["developers"], + "public/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL": ["public"], + "readonlyuser/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL": [], +} + +acls := [ + { + "identity": "group:admins", + "action": "full", + "resource": "hbase:", + }, + { + "identity": "group:developers", + "action": "full", + "resource": "hbase:namespace:developers", + }, + { + "identity": "group:public", + "action": "full", + "resource": "hbase:namespace:public", + }, + { + "identity": "user:readonlyuser/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL", + "action": "ro", + "resource": "hbase:namespace:", + # Restrict to read-only operation types; exercises matches_operation non-null branch. + "operations": ["exists", "get", "scan", "none"], + # Restrict to known column families; exercises matches_families non-null branch. + "families": ["cf1"], + }, +] diff --git a/src/test/rego/hbase_test.rego b/src/test/rego/hbase_test.rego new file mode 100644 index 0000000..161f8cb --- /dev/null +++ b/src/test/rego/hbase_test.rego @@ -0,0 +1,17 @@ +package hbase_test + +import data.hbase + +# Every fixture in allowed/ must produce allow=true under the real Rego policy. +test_all_allowed_inputs if { + every key, fixture in data.fixtures.allowed { + hbase.allow with input as fixture.input + } +} + +# Every fixture in denied/ must produce allow=false under the real Rego policy. +test_all_denied_inputs if { + every key, fixture in data.fixtures.denied { + not hbase.allow with input as fixture.input + } +} From 512e4a3086374b2fe14716066a9859a478ab8fdf Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Fri, 6 Mar 2026 20:08:33 +0100 Subject: [PATCH 22/29] Add download-maven-plugin and exec-maven-plugin for OPA policy testing Co-Authored-By: Claude Sonnet 4.6 --- .gitignore | 1 + pom.xml | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/.gitignore b/.gitignore index cd975cc..8ac713b 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /dependency-reduced-pom.xml /CLAUDE.md .claude/ +/target/opa diff --git a/pom.xml b/pom.xml index 8954f58..f6d228e 100644 --- a/pom.xml +++ b/pom.xml @@ -55,6 +55,7 @@ 2.6.4 2.5.0 2.8.8 + 0.63.0 @@ -139,6 +140,30 @@ + + + opa-linux-amd64 + + Linuxamd64 + + linux_amd64_static + + + opa-mac-amd64 + + Mac OS Xx86_64 + + darwin_amd64 + + + opa-mac-arm64 + + Mac OS Xaarch64 + + darwin_arm64 + + + @@ -240,6 +265,60 @@ maven-surefire-plugin ${maven-surefire-plugin.version} + + com.googlecode.maven-download-plugin + download-maven-plugin + 1.8.1 + + + download-opa + process-test-resources + wget + + https://github.com/open-policy-agent/opa/releases/download/v${opa.version}/opa_${opa.classifier} + ${project.build.directory} + opa + false + + + + + + org.codehaus.mojo + exec-maven-plugin + 3.1.0 + + + chmod-opa + process-test-resources + exec + + chmod + + +x + ${project.build.directory}/opa + + + + + opa-policy-test + verify + exec + + ${project.build.directory}/opa + + test + src/test/rego/hbase.rego + src/test/rego/hbase_test.rego + --data + src/test/rego/fixtures + -v + + ${maven.test.skip} + + + + com.diffplug.spotless spotless-maven-plugin From a885366cf7f3c44e68a5aa9b3f310112e40597c4 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 9 Mar 2026 11:03:30 +0100 Subject: [PATCH 23/29] Move OPA fixtures to target/, add print counts, untrack plan docs - OpaFixtureWriter writes to target/test-rego/fixtures.json (was src/test/rego/fixtures.json) so test runs no longer dirty the working tree - pom.xml opa-policy-test execution updated to match new path - hbase_test.rego: add print() calls so --explain notes reports fixture counts - .gitignore: add /docs/plans/ (plan docs are local-only, not committed) - Remove stale design doc and completed implementation plan from git Co-Authored-By: Claude Sonnet 4.6 --- .gitignore | 1 + .../2026-03-06-opa-policy-testing-design.md | 139 ----- docs/plans/2026-03-06-opa-policy-testing.md | 523 ------------------ pom.xml | 7 +- .../stackable/hbase/OpaFixtureWriter.java | 87 ++- src/test/rego/hbase_test.rego | 10 +- 6 files changed, 48 insertions(+), 719 deletions(-) delete mode 100644 docs/plans/2026-03-06-opa-policy-testing-design.md delete mode 100644 docs/plans/2026-03-06-opa-policy-testing.md diff --git a/.gitignore b/.gitignore index 8ac713b..b5d71d6 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /CLAUDE.md .claude/ /target/opa +/docs/plans/ diff --git a/docs/plans/2026-03-06-opa-policy-testing-design.md b/docs/plans/2026-03-06-opa-policy-testing-design.md deleted file mode 100644 index 86722bc..0000000 --- a/docs/plans/2026-03-06-opa-policy-testing-design.md +++ /dev/null @@ -1,139 +0,0 @@ -# OPA Policy Testing Layer — Design - -## Purpose - -Add a fast, intermediate test layer that validates the Rego policy logic against realistic -OPA inputs without the overhead of a full kuttl integration test setup. This augments (not -replaces) the integration tests. - -## Architecture - -``` -Unit tests run (Surefire) - │ - ▼ -WireMockRule ServeEventListener - │ collects (request body, response body) per serve event - ▼ -OpaFixtureWriter.flush() in TestUtils.tearDown() - │ remap usernames, split by result, deduplicate, write JSON - ▼ -src/test/rego/fixtures/ - allowed/ ← one .json per unique allow-path input - denied/ ← one .json per unique deny-path input - │ - ▼ (separate Maven phase) -exec-maven-plugin runs: opa test hbase.rego hbase_test.rego --data fixtures/ - │ - ▼ -Pass / Fail -``` - -## Components - -### 1. Fixture Capture (Java) - -**`OpaFixtureWriter`** — new test-only class: -- Thread-safe list accumulates `(requestBody, responseBody)` pairs via a `ServeEventListener` -registered on `WireMockRule` in `TestUtils` -- `flush(Path fixtureDir)` called from `TestUtils.tearDown()`: -1. Remaps `callerUgi.userName` in raw JSON via string replacement: -- `allowedUser` → `admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL` -- `deniedUser` → `unknown@CLUSTER.LOCAL` -2. Parses response body to determine `allowed/` vs `denied/` subdirectory -3. Deduplicates — identical post-remap JSON written only once -4. Writes files as `{index:04d}.json` - -Constants in `TestUtils`: - -```java -static final String OPA_REMAP_ALLOWED = "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; -static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; -``` - -Fixtures are committed to source control. Running `mvn test` regenerates them in-place; -a `git diff` on the fixtures directory reveals any change in payload shape. - -### 2. OPA Test File (static, committed) - -**`src/test/rego/hbase_test.rego`**: - -```rego -package hbase_test - -import data.hbase - -test_all_allowed_inputs if { - every inp in data.fixtures.allowed { - hbase.allow with input as inp - } -} - -test_all_denied_inputs if { - every inp in data.fixtures.denied { - not hbase.allow with input as inp - } -} -``` - -### 3. Rego Policy File (generated, not committed) - -**`target/generated-test-resources/rego/hbase.rego`** — generated during -`generate-test-resources` phase from the integration test source: - -- Source: `/home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2` -- Transformation: strip YAML ConfigMap wrapper, replace `$NAMESPACE` with `test-ns` -- Not committed (added to `.gitignore`) - -This ensures the Rego logic tested here is always identical to the integration test policy. -The `OPA_REMAP_ALLOWED` principal matches the generated `groups_for_user` entry for `admins`. - -### 4. Maven Setup - -Three plugin executions, all test-scoped: - -**Phase `generate-test-resources`** — `exec-maven-plugin`: -- Shell script strips YAML wrapper from `12-rego-rules.txt.j2` and substitutes `$NAMESPACE=test-ns` -- Output: `target/generated-test-resources/rego/hbase.rego` - -**Phase `process-test-resources`** — `download-maven-plugin`: -- Downloads pinned OPA binary to `target/opa` and chmods `+x` -- Version controlled via `` property in `pom.xml` -- OS/arch selected via Maven profiles: `linux_amd64`, `darwin_amd64`, `darwin_arm64` - -**Phase `test`** — `exec-maven-plugin` (after Surefire): - -```bash -target/opa test \ - target/generated-test-resources/rego/hbase.rego \ - src/test/rego/hbase_test.rego \ - --data src/test/rego/fixtures/ -``` - -- Skipped when `maven.test.skip=true` -- Fails the build on any OPA test failure - -## What This Tests - -- The JSON the Java coprocessor sends to OPA is parseable and has the expected field structure -- The Rego policy produces correct decisions (`allow`/`deny`) for those inputs -- The admin principal passes all hooks; an unknown principal fails all hooks - -## Future Extension - -The username mapping can be extended to a richer set of principals (developer, readonlyuser) -to exercise nuanced Rego branches (`matches_operation`, `matches_families`). This would -require either per-test-method mapping metadata or a separate `TestOpaInputCapture` class -with realistic principals. - -## Files Changed - -| File | Action | -|------------------------------------------------------------|------------------------------------------------------------------------------------------------------| -| `src/test/java/tech/stackable/hbase/TestUtils.java` | Add `ServeEventListener`, call `OpaFixtureWriter.flush()` | -| `src/test/java/tech/stackable/hbase/OpaFixtureWriter.java` | New class | -| `src/test/rego/hbase_test.rego` | New file (committed) | -| `src/test/rego/fixtures/` | New directory (committed, regenerated) | -| `pom.xml` | Add `download-maven-plugin`, two `exec-maven-plugin` executions, OS profiles, `opa.version` property | -| `.gitignore` | Add `target/generated-test-resources/` | - diff --git a/docs/plans/2026-03-06-opa-policy-testing.md b/docs/plans/2026-03-06-opa-policy-testing.md deleted file mode 100644 index 126af48..0000000 --- a/docs/plans/2026-03-06-opa-policy-testing.md +++ /dev/null @@ -1,523 +0,0 @@ -# OPA Policy Testing Layer — Implementation Plan - -> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. - -**Goal:** Capture OPA HTTP request payloads from unit tests, remap usernames to Rego-compatible principals, write them as fixture JSON files, and validate them against the real Rego policy using the `opa test` CLI — giving fast policy correctness feedback without a full integration test cluster. - -**Architecture:** WireMock's `RequestListener` captures every `(request, response)` pair during unit tests. After each test class, `OpaFixtureWriter.flush()` remaps `allowedUser` → admin Kerberos principal and `deniedUser` → unknown principal, deduplicates, and writes to `src/test/rego/fixtures/{allowed,denied}/`. Maven downloads a pinned OPA binary and runs `opa test` against the fixtures and a checked-in copy of the Rego rules (derived from the integration test template) in the `verify` phase. - -**Tech Stack:** WireMock 3.6.0 (`RequestListener`), Jackson (already in project), `download-maven-plugin`, `exec-maven-plugin`, OPA CLI `opa test`. - ---- - -### Task 1: `OpaFixtureWriter` class - -**Files:** -- Create: `src/test/java/tech/stackable/hbase/OpaFixtureWriter.java` - -**Step 1: Create the class** - -```java -package tech.stackable.hbase; - -import com.fasterxml.jackson.databind.ObjectMapper; -import com.github.tomakehurst.wiremock.http.Request; -import com.github.tomakehurst.wiremock.http.Response; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; - -/** - * Captures WireMock OPA requests during unit tests and writes them as fixture JSON files for - * offline {@code opa test} validation. - * - *

Usernames are remapped so fixtures exercise real Rego policy logic: - *

    - *
  • {@code allowedUser} → admin Kerberos principal (member of "admins" group in Rego) - *
  • {@code deniedUser} → unknown principal (not in any Rego group) - *
- */ -public class OpaFixtureWriter { - - static final String OPA_REMAP_ALLOWED = - "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; - static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; - - private static final Path FIXTURES_BASE = Paths.get("src/test/rego/fixtures"); - private static final Path ALLOWED_DIR = FIXTURES_BASE.resolve("allowed"); - private static final Path DENIED_DIR = FIXTURES_BASE.resolve("denied"); - - /** All captured (requestBody, responseBody) pairs across all test classes in this JVM run. */ - private static final List captured = new CopyOnWriteArrayList<>(); - - /** Tracks whether we have cleared old fixture files yet in this JVM run. */ - private static final AtomicBoolean clearedOnce = new AtomicBoolean(false); - - /** Sequential file counters, shared across all flush() calls in one JVM run. */ - private static final AtomicInteger allowedIdx = new AtomicInteger(0); - - private static final AtomicInteger deniedIdx = new AtomicInteger(0); - - /** Deduplication sets, shared across all flush() calls in one JVM run. */ - private static final Set seenAllowed = new HashSet<>(); - - private static final Set seenDenied = new HashSet<>(); - - /** - * Called by the WireMock RequestListener on each request. Thread-safe. - */ - public static void capture(Request request, Response response) { - captured.add(new String[] {request.getBodyAsString(), response.getBodyAsString()}); - } - - /** - * Remaps captured requests, deduplicates, and writes fixture JSON files. Called from {@link - * TestUtils#tearDown()} at the end of each test class. - */ - public static void flush() throws IOException { - if (clearedOnce.compareAndSet(false, true)) { - deleteDir(ALLOWED_DIR); - deleteDir(DENIED_DIR); - } - Files.createDirectories(ALLOWED_DIR); - Files.createDirectories(DENIED_DIR); - - List toProcess = List.copyOf(captured); - captured.clear(); - - for (String[] pair : toProcess) { - String requestBody = pair[0]; - String responseBody = pair[1]; - - String remapped = - requestBody - .replace("allowedUser", OPA_REMAP_ALLOWED) - .replace("deniedUser", OPA_REMAP_DENIED); - - boolean allowed = responseBody.contains("\"true\""); - - synchronized (OpaFixtureWriter.class) { - if (allowed) { - if (seenAllowed.add(remapped)) { - Files.writeString( - ALLOWED_DIR.resolve(String.format("%04d.json", allowedIdx.getAndIncrement())), - remapped); - } - } else { - if (seenDenied.add(remapped)) { - Files.writeString( - DENIED_DIR.resolve(String.format("%04d.json", deniedIdx.getAndIncrement())), - remapped); - } - } - } - } - } - - private static void deleteDir(Path dir) throws IOException { - if (Files.exists(dir)) { - try (var entries = Files.list(dir)) { - for (Path p : entries.toList()) { - Files.delete(p); - } - } - Files.delete(dir); - } - } -} -``` - -**Step 2: Run spotless and compile** - -```bash -JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn spotless:apply -q compile -q -``` - -Expected: BUILD SUCCESS, no errors. - -**Step 3: Commit** - -```bash -git add src/test/java/tech/stackable/hbase/OpaFixtureWriter.java -git commit -m "Add OpaFixtureWriter for capturing WireMock OPA fixtures" -``` - ---- - -### Task 2: Wire `OpaFixtureWriter` into the test infrastructure - -**Files:** -- Modify: `src/test/java/tech/stackable/hbase/TestUtils.java` -- Modify: `src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java` -- Modify: `src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java` -- Modify: `src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java` - -**Step 1: Add `flush()` call to `TestUtils.tearDown()`** - -In `TestUtils.java`, change: - -```java -protected static void tearDown() throws Exception { - TEST_UTIL.shutdownMiniCluster(); -} -``` - -to: - -```java -protected static void tearDown() throws Exception { - OpaFixtureWriter.flush(); - TEST_UTIL.shutdownMiniCluster(); -} -``` - -**Step 2: Register the listener in each test class** - -In each of the three test classes, find the `@BeforeClass` / `setUpClass` method and add the listener registration as the FIRST line. The `WireMockRule` is a `@ClassRule`, so it is already started by the time `@BeforeClass` runs. - -`TestOpenPolicyAgentAccessController.java` — `setUpClass` currently starts with: - -```java -stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); -setup(OpenPolicyAgentAccessController.class, false, OPA_URL); -``` - -Change to: - -```java -wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture); -stubFor(post("/").willReturn(ok().withBody("{\"result\": \"true\"}"))); -setup(OpenPolicyAgentAccessController.class, false, OPA_URL); -``` - -Apply the same one-line addition (`wireMockRule.addMockServiceRequestListener(OpaFixtureWriter::capture);`) as the first line of `setUpClass` in: -- `TestOpenPolicyAgentAccessControllerRegion.java` -- `TestOpenPolicyAgentAccessControllerVariants.java` - -Add the import to each class that needs it: - -```java -import com.github.tomakehurst.wiremock.http.RequestListener; -``` - -(The method reference `OpaFixtureWriter::capture` satisfies the `RequestListener` functional interface.) - -**Step 3: Run spotless and the full test suite** - -```bash -JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn spotless:apply -q test -q 2>&1 | grep -E 'Tests run:|BUILD' -``` - -Expected: 87 tests, BUILD SUCCESS. Also check that fixture files were created: - -```bash -ls src/test/rego/fixtures/allowed/ | head -5 -ls src/test/rego/fixtures/denied/ | head -5 -``` - -Expected: several `.json` files in each directory. - -**Step 4: Inspect a fixture to verify remapping** - -```bash -cat src/test/rego/fixtures/allowed/0000.json | python3 -m json.tool | grep userName -``` - -Expected: output contains `"admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"`, not `"allowedUser"`. - -```bash -cat src/test/rego/fixtures/denied/0000.json | python3 -m json.tool | grep userName -``` - -Expected: output contains `"unknown@CLUSTER.LOCAL"`, not `"deniedUser"`. - -**Step 5: Commit** - -```bash -git add src/test/java/tech/stackable/hbase/TestUtils.java \ - src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java \ - src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java \ - src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerVariants.java \ - src/test/rego/fixtures/ -git commit -m "Wire OpaFixtureWriter into test teardown and register WireMock listener" -``` - ---- - -### Task 3: Add Rego test files - -**Files:** -- Create: `src/test/rego/hbase.rego` -- Create: `src/test/rego/hbase_test.rego` - -**Step 1: Generate `hbase.rego` from the integration test template** - -The integration test Rego lives at: -`/home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2` - -It is a YAML ConfigMap wrapping the Rego with 4-space indentation. Strip the 9-line YAML header, remove the 4-space indent, and substitute `$NAMESPACE` with `test-ns`: - -```bash -tail -n +10 /home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 \ - | sed 's/^ //' \ - | sed 's/\$NAMESPACE/test-ns/g' \ - > src/test/rego/hbase.rego -``` - -Verify the first few lines look like valid Rego (no YAML, no leading spaces): - -```bash -head -5 src/test/rego/hbase.rego -``` - -Expected: - -``` -package hbase - -default allow := false -default matches_identity(identity) := false -``` - -Add a comment at the top of the file noting its origin. Edit `src/test/rego/hbase.rego` to prepend: - -```rego -# Derived from hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 -# with $NAMESPACE replaced by "test-ns". Regenerate with: -# tail -n +10 /12-rego-rules.txt.j2 | sed 's/^ //' | sed 's/\$NAMESPACE/test-ns/g' -# -``` - -**Step 2: Create `hbase_test.rego`** - -```bash -mkdir -p src/test/rego -``` - -Create `src/test/rego/hbase_test.rego`: - -```rego -package hbase_test - -import data.hbase - -# Every fixture in allowed/ must produce allow=true under the real Rego policy. -test_all_allowed_inputs if { - every key, fixture in data.fixtures.allowed { - hbase.allow with input as fixture.input - } -} - -# Every fixture in denied/ must produce allow=false under the real Rego policy. -test_all_denied_inputs if { - every key, fixture in data.fixtures.denied { - not hbase.allow with input as fixture.input - } -} -``` - -**Step 3: Commit** - -```bash -git add src/test/rego/hbase.rego src/test/rego/hbase_test.rego -git commit -m "Add Rego policy file and OPA test suite" -``` - ---- - -### Task 4: Maven — download OPA and run `opa test` - -**Files:** -- Modify: `pom.xml` -- Modify: `.gitignore` - -**Step 1: Add `opa.version` property and OS profiles to `pom.xml`** - -In the `` section, add: - -```xml -0.63.0 -``` - -After the closing `` tag and before ``, add Maven profiles for OS detection: - -```xml - - - opa-linux-amd64 - - Linuxamd64 - - linux_amd64_static - - - opa-mac-amd64 - - Mac OS Xx86_64 - - darwin_amd64 - - - opa-mac-arm64 - - Mac OS Xaarch64 - - darwin_arm64 - - -``` - -**Step 2: Add `download-maven-plugin` to ``** - -```xml - - com.googlecode.maven-download-plugin - download-maven-plugin - 1.8.1 - - - download-opa - process-test-resources - wget - - https://github.com/open-policy-agent/opa/releases/download/v${opa.version}/opa_${opa.classifier} - ${project.build.directory} - opa - false - - - - -``` - -**Step 3: Add `exec-maven-plugin` executions to ``** - -```xml - - org.codehaus.mojo - exec-maven-plugin - 3.1.0 - - - chmod-opa - process-test-resources - exec - - chmod - - +x - ${project.build.directory}/opa - - - - - opa-policy-test - verify - exec - - ${project.build.directory}/opa - - test - src/test/rego/hbase.rego - src/test/rego/hbase_test.rego - --data - src/test/rego/fixtures - -v - - ${maven.test.skip} - - - - -``` - -Note: `opa test` runs in the `verify` phase, which is AFTER the `test` phase (where Surefire generates the fixtures). Run with `mvn verify` to execute both unit tests and OPA tests. - -**Step 4: Add OPA binary to `.gitignore`** - -Append to `.gitignore`: - -``` -/target/opa -``` - -**Step 5: Run spotless and verify compilation** - -```bash -JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn spotless:apply -q compile -q -``` - -Expected: BUILD SUCCESS. - -**Step 6: Commit** - -```bash -git add pom.xml .gitignore -git commit -m "Add download-maven-plugin and exec-maven-plugin for OPA policy testing" -``` - ---- - -### Task 5: End-to-end verification - -**Step 1: Run `mvn verify`** - -```bash -JAVA_HOME=/usr/lib/jvm/java-11-openjdk-amd64 mvn verify 2>&1 | grep -E 'Tests run:|opa|PASS|FAIL|BUILD' -``` - -Expected output includes: - -``` -Tests run: 87, Failures: 0, Errors: 0, Skipped: 1 -data.hbase_test.test_all_allowed_inputs: PASS (...) -data.hbase_test.test_all_denied_inputs: PASS (...) -BUILD SUCCESS -``` - -**Step 2: Verify fixture count is reasonable** - -```bash -echo "allowed: $(ls src/test/rego/fixtures/allowed/ | wc -l)" -echo "denied: $(ls src/test/rego/fixtures/denied/ | wc -l)" -``` - -Expected: at least 20 allowed fixtures and several denied fixtures (exact count will vary). - -**Step 3: Commit fixtures and final state** - -```bash -git add src/test/rego/fixtures/ -git commit -m "Add generated OPA fixtures from unit test capture" -``` - ---- - -## Running OPA tests standalone (no Maven) - -After the first `mvn test` has generated fixtures: - -```bash -target/opa test src/test/rego/hbase.rego src/test/rego/hbase_test.rego \ - --data src/test/rego/fixtures -v -``` - -## Keeping `hbase.rego` in sync - -When `12-rego-rules.txt.j2` changes in the hbase-operator repo, regenerate: - -```bash -tail -n +10 /home/andrew/gitrepos/hbase-operator/tests/templates/kuttl/opa/12-rego-rules.txt.j2 \ - | sed 's/^ //' \ - | sed 's/\$NAMESPACE/test-ns/g' \ - >> /tmp/hbase_new.rego -# Prepend the comment header, review diff, then replace src/test/rego/hbase.rego -``` - diff --git a/pom.xml b/pom.xml index f6d228e..6a11fd7 100644 --- a/pom.xml +++ b/pom.xml @@ -306,12 +306,15 @@ exec ${project.build.directory}/opa + ${project.basedir} test src/test/rego/hbase.rego src/test/rego/hbase_test.rego - --data - src/test/rego/fixtures + target/test-rego/fixtures.json + --v1-compatible + --explain + notes -v ${maven.test.skip} diff --git a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java index 88f98ed..2e48e01 100644 --- a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java +++ b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java @@ -10,11 +10,11 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; /** - * Captures WireMock OPA requests during unit tests and writes them as fixture JSON files for - * offline {@code opa test} validation. + * Captures WireMock OPA requests during unit tests and writes them as a single {@code + * fixtures.json} file for offline {@code opa test} validation. * *

Usernames are remapped so fixtures exercise real Rego policy logic: * @@ -29,20 +29,15 @@ public class OpaFixtureWriter { "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; - private static final Path FIXTURES_BASE = Paths.get("src/test/rego/fixtures"); - private static final Path ALLOWED_DIR = FIXTURES_BASE.resolve("allowed"); - private static final Path DENIED_DIR = FIXTURES_BASE.resolve("denied"); + private static final Path FIXTURES_FILE = Paths.get("target/test-rego/fixtures.json"); /** All captured (requestBody, responseBody) pairs across all test classes in this JVM run. */ private static final List captured = new ArrayList<>(); - /** Tracks whether we have cleared old fixture files yet in this JVM run. */ - private static final AtomicBoolean clearedOnce = new AtomicBoolean(false); + /** Accumulated fixture bodies, shared across all flush() calls in one JVM run. */ + private static final List allowedFixtures = new ArrayList<>(); - /** Sequential file counters, shared across all flush() calls in one JVM run. */ - private static int allowedIdx = 0; - - private static int deniedIdx = 0; + private static final List deniedFixtures = new ArrayList<>(); /** Deduplication sets, shared across all flush() calls in one JVM run. */ private static final Set seenAllowed = new HashSet<>(); @@ -55,65 +50,55 @@ public static synchronized void capture(Request request, Response response) { } /** - * Remaps captured requests, deduplicates, and writes fixture JSON files. Called from {@link - * TestUtils#tearDown()} at the end of each test class. + * Remaps captured requests, deduplicates, and writes {@code src/test/rego/fixtures.json}. Called + * from {@link TestUtils#tearDown()} at the end of each test class. */ public static void flush() throws IOException { - if (clearedOnce.compareAndSet(false, true)) { - deleteDir(ALLOWED_DIR); - deleteDir(DENIED_DIR); - } - Files.createDirectories(ALLOWED_DIR); - Files.createDirectories(DENIED_DIR); - List toProcess; synchronized (OpaFixtureWriter.class) { toProcess = new ArrayList<>(captured); captured.clear(); } - for (String[] pair : toProcess) { - String requestBody = pair[0]; - String responseBody = pair[1]; - - // Only capture requests from the standard allow/deny test users defined in TestUtils. - // Requests from other named users (e.g. Variants-specific users) and cluster-internal - // traffic are intentionally skipped — they are not useful for Rego policy validation. - if (!requestBody.contains("allowedUser") && !requestBody.contains("deniedUser")) { - continue; - } - String remapped = - requestBody - .replace("allowedUser", OPA_REMAP_ALLOWED) - .replace("deniedUser", OPA_REMAP_DENIED); + synchronized (OpaFixtureWriter.class) { + for (String[] pair : toProcess) { + String requestBody = pair[0]; + String responseBody = pair[1]; + + // Only capture requests from the standard allow/deny test users defined in TestUtils. + // Requests from other named users (e.g. Variants-specific users) and cluster-internal + // traffic are intentionally skipped — they are not useful for Rego policy validation. + if (!requestBody.contains("allowedUser") && !requestBody.contains("deniedUser")) { + continue; + } + String remapped = + requestBody + .replace("allowedUser", OPA_REMAP_ALLOWED) + .replace("deniedUser", OPA_REMAP_DENIED); - // WireMock stubs in this test suite always return {"result": "true"} or {"result": "false"}. - boolean allowed = responseBody.contains("\"true\""); + // WireMock stubs in this test suite always return {"result": "true"} or {"result": + // "false"}. + boolean allowed = responseBody.contains("\"true\""); - synchronized (OpaFixtureWriter.class) { if (allowed) { if (seenAllowed.add(remapped)) { - Files.writeString( - ALLOWED_DIR.resolve(String.format("%04d.json", allowedIdx++)), remapped); + allowedFixtures.add(remapped); } } else { if (seenDenied.add(remapped)) { - Files.writeString( - DENIED_DIR.resolve(String.format("%04d.json", deniedIdx++)), remapped); + deniedFixtures.add(remapped); } } } + + Files.createDirectories(FIXTURES_FILE.getParent()); + Files.writeString(FIXTURES_FILE, buildFixturesJson()); } } - private static void deleteDir(Path dir) throws IOException { - if (Files.exists(dir)) { - try (var entries = Files.list(dir)) { - for (Path p : entries.collect(java.util.stream.Collectors.toList())) { - Files.delete(p); - } - } - Files.delete(dir); - } + private static String buildFixturesJson() { + String allowed = allowedFixtures.stream().collect(Collectors.joining(",")); + String denied = deniedFixtures.stream().collect(Collectors.joining(",")); + return "{\"fixtures\":{\"allowed\":[" + allowed + "],\"denied\":[" + denied + "]}}"; } } diff --git a/src/test/rego/hbase_test.rego b/src/test/rego/hbase_test.rego index 161f8cb..ae4c40d 100644 --- a/src/test/rego/hbase_test.rego +++ b/src/test/rego/hbase_test.rego @@ -2,16 +2,18 @@ package hbase_test import data.hbase -# Every fixture in allowed/ must produce allow=true under the real Rego policy. +# Every fixture in allowed must produce allow=true under the real Rego policy. test_all_allowed_inputs if { - every key, fixture in data.fixtures.allowed { + print("allowed fixtures:", count(data.fixtures.allowed)) + every fixture in data.fixtures.allowed { hbase.allow with input as fixture.input } } -# Every fixture in denied/ must produce allow=false under the real Rego policy. +# Every fixture in denied must produce allow=false under the real Rego policy. test_all_denied_inputs if { - every key, fixture in data.fixtures.denied { + print("denied fixtures:", count(data.fixtures.denied)) + every fixture in data.fixtures.denied { not hbase.allow with input as fixture.input } } From f798ec710c707a4df2ca035ed2d844589786a70a Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 9 Mar 2026 11:29:53 +0100 Subject: [PATCH 24/29] Add unit tests for bulk load and lock hooks prePrepareBulkLoad, preCleanupBulkLoad, preBulkLoadHFile (region-level), preLockHeartbeat, and preRequestLock (table, namespace, and region-scope branches) were implemented but had no unit tests. Brings coverage from 83 to 90 passing tests. Co-Authored-By: Claude Sonnet 4.6 --- .../TestOpenPolicyAgentAccessController.java | 39 +++++++++++++++++++ ...OpenPolicyAgentAccessControllerRegion.java | 30 ++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java index c0ddfb4..6b62cdd 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessController.java @@ -543,6 +543,45 @@ public void testPreGetLocks() throws Exception { }); } + @Test + public void testPreRequestLockTableScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preRequestLock(ctx(), null, TEST_TABLE, null, "desc"); + return null; + }); + } + + @Test + public void testPreRequestLockNamespaceScope() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController() + .preRequestLock( + ctx(), NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, null, null, "desc"); + return null; + }); + } + + @Test + public void testPreRequestLockRegionScope() throws Exception { + RegionInfo ri = RegionInfoBuilder.newBuilder(TEST_TABLE).build(); + assertAllowedThenDenied( + () -> { + getOpaController().preRequestLock(ctx(), null, null, new RegionInfo[] {ri}, "desc"); + return null; + }); + } + + @Test + public void testPreLockHeartbeat() throws Exception { + assertAllowedThenDenied( + () -> { + getOpaController().preLockHeartbeat(ctx(), TEST_TABLE, "desc"); + return null; + }); + } + @Test public void testPreSetSplitOrMergeEnabled() throws Exception { assertAllowedThenDenied( diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java index fe13895..5b9eb5a 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -8,6 +8,7 @@ import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit.WireMockRule; +import java.util.Collections; import org.apache.hadoop.hbase.CompareOperator; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.client.Append; @@ -345,6 +346,35 @@ public void testPreCheckAndMutateAfterRowLockWithRowMutations() throws Exception }); } + // --- bulk load hooks --- + + @Test + public void testPreBulkLoadHFile() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preBulkLoadHFile(regionCtx(), Collections.emptyList()); + return null; + }); + } + + @Test + public void testPrePrepareBulkLoad() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().prePrepareBulkLoad(regionCtx()); + return null; + }); + } + + @Test + public void testPreCleanupBulkLoad() throws Exception { + assertAllowedThenDenied( + () -> { + getRegionController().preCleanupBulkLoad(regionCtx()); + return null; + }); + } + // --- RegionServer hooks --- private OpenPolicyAgentAccessController getRsController() { From b81dfdb831d18c8cf5cacd397690cf97d4d4fdcc Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 9 Mar 2026 11:45:44 +0100 Subject: [PATCH 25/29] add final --- .../OpenPolicyAgentAccessController.java | 98 +++++++++---------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index f45569f..477a4bb 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -163,7 +163,7 @@ private User getActiveUser(ObserverContext ctx) throws IOException { public void preCreateNamespace( ObserverContext ctx, NamespaceDescriptor ns) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preCreateNamespace: user [{}]", user); opaAclChecker.checkPermissionInfo( user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); @@ -172,7 +172,7 @@ public void preCreateNamespace( @Override public void preDeleteNamespace( ObserverContext ctx, String namespace) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preDeleteNamespace: user [{}]", user); opaAclChecker.checkPermissionInfo( user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); @@ -182,7 +182,7 @@ public void preDeleteNamespace( public void preModifyNamespace( ObserverContext ctx, NamespaceDescriptor ns) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preModifyNamespace: user [{}]", user); opaAclChecker.checkPermissionInfo( user, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, Action.ADMIN); @@ -191,7 +191,7 @@ public void preModifyNamespace( @Override public void preGetNamespaceDescriptor( ObserverContext ctx, String namespace) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preGetNamespaceDescriptor: user [{}]", user); opaAclChecker.checkPermissionInfo(user, namespace, Action.ADMIN); } @@ -207,7 +207,7 @@ public void postListNamespaces( public void preCreateTable( ObserverContext ctx, TableDescriptor desc, RegionInfo[] regions) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preCreateTable: user [{}]", user); requirePermission( ctx, @@ -231,7 +231,7 @@ public void postCompletedCreateTableAction( @Override public void preDeleteTable(ObserverContext ctx, TableName tableName) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preDeleteTable: user [{}]", user); requirePermission(ctx, "deleteTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @@ -248,7 +248,7 @@ public void postDeleteTable( @Override public void preEnableTable(ObserverContext ctx, TableName tableName) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preEnableTable: user [{}]", user); requirePermission(ctx, "enableTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @@ -256,7 +256,7 @@ public void preEnableTable(ObserverContext ctx, Ta @Override public void preDisableTable( ObserverContext ctx, TableName tableName) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preDisableTable: user [{}]", user); requirePermission(ctx, "disableTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @@ -267,7 +267,7 @@ public void preGetOp( final Get get, final List result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preGetOp: user [{}] on table [{}] with get [{}]", user, tableName, get); // All users need read access to hbase:meta table. @@ -282,7 +282,7 @@ public void preGetOp( public boolean preExists( final ObserverContext ctx, final Get get, final boolean exists) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { @@ -297,7 +297,7 @@ public boolean preExists( @Override public void preScannerOpen( final ObserverContext ctx, final Scan scan) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); // All users need read access to hbase:meta table. if (TableName.META_TABLE_NAME.equals(tableName)) { @@ -314,7 +314,7 @@ public RegionScanner postScannerOpen( final Scan scan, final RegionScanner s) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); if (user != null && user.getShortName() != null) { // TODO this uses the shortName. Is it possible for the same scanner to be used by // different users across principals who nevertheless have the same shortName? This @@ -333,7 +333,7 @@ public boolean preScannerNext( final int limit, final boolean hasNext) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preScannerNext: user [{}] on table [{}] with scan [{}]", user, tableName, s); @@ -345,7 +345,7 @@ public boolean preScannerNext( public void preScannerClose( final ObserverContext ctx, final InternalScanner s) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preScannerClose: user [{}] on table [{}] with scan [{}]", user, tableName, s); @@ -356,7 +356,7 @@ public void preScannerClose( public void postScannerClose( final ObserverContext ctx, final InternalScanner s) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("postScannerClose: user [{}] on table [{}] with scan [{}]", user, tableName, s); @@ -382,7 +382,7 @@ public void prePut( final WALEdit edit, final Durability durability) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("prePut: user [{}] on table [{}] with put [{}]", user, tableName, put); opaAclChecker.checkPermissionInfoWithOp( @@ -396,7 +396,7 @@ public void preDelete( final WALEdit edit, final Durability durability) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preDelete: user [{}] on table [{}] with delete [{}]", user, tableName, delete); opaAclChecker.checkPermissionInfoWithOp( @@ -415,7 +415,7 @@ public void postDelete( @Override public Result preAppend(ObserverContext ctx, Append append) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preAppend: user [{}] on table [{}] with append [{}]", user, tableName, append); opaAclChecker.checkPermissionInfoWithOp( @@ -430,7 +430,7 @@ public void preBatchMutate( ObserverContext ctx, MiniBatchOperationInProgress miniBatchOp) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preBatchMutate: user [{}] on table [{}] with miniBatchOp [{}]", @@ -443,7 +443,7 @@ public void preBatchMutate( @Override public void preOpen(ObserverContext ctx) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); final Region region = ctx.getEnvironment().getRegion(); if (region == null) { LOG.error("NULL region from RegionCoprocessorEnvironment in preOpen()"); @@ -463,7 +463,7 @@ public void postOpen(ObserverContext ctx) { public void preTableFlush( final ObserverContext ctx, final TableName tableName) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preTableFlush: user [{}] on table [{}]", user, tableName); requirePermission(ctx, "flushTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @@ -484,7 +484,7 @@ public InternalScanner preCompact( CompactionLifeCycleTracker tracker, CompactionRequest request) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCompact: user [{}] on table [{}] for scanner [{}]", user, tableName, scanner); requirePermission(ctx, "compact", tableName, null, null, Action.ADMIN, Action.CREATE); @@ -574,7 +574,7 @@ public boolean preCheckAndPut( final Put put, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); requirePermission( @@ -593,7 +593,7 @@ public boolean preCheckAndPutAfterRowLock( final Put put, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); @@ -613,7 +613,7 @@ public boolean preCheckAndDelete( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); @@ -640,7 +640,7 @@ public boolean preCheckAndDeleteAfterRowLock( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDeleteAfterRowLock: user [{}] on table [{}] for delete [{}]", @@ -667,7 +667,7 @@ public boolean preCheckAndPut( final Put put, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCheckAndPut: user [{}] on table [{}] for put [{}]", user, tableName, put); requirePermission( @@ -683,7 +683,7 @@ public boolean preCheckAndPutAfterRowLock( final Put put, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndPutAfterRowLock: user [{}] on table [{}] for put [{}]", user, tableName, put); @@ -700,7 +700,7 @@ public boolean preCheckAndDelete( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDelete: user [{}] on table [{}] for delete [{}]", user, tableName, delete); @@ -724,7 +724,7 @@ public boolean preCheckAndDeleteAfterRowLock( final Delete delete, final boolean result) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndDeleteAfterRowLock: user [{}] on table [{}] for delete [{}]", @@ -750,7 +750,7 @@ public CheckAndMutateResult preCheckAndMutate( CheckAndMutateResult result) throws IOException { if (checkAndMutate.getAction() instanceof RowMutations) { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preCheckAndMutate (RowMutations): user [{}] on table [{}]", user, tableName); opaAclChecker.checkPermissionInfoWithOp(user, tableName, Action.WRITE, OpType.ROW_MUTATIONS); @@ -766,7 +766,7 @@ public CheckAndMutateResult preCheckAndMutateAfterRowLock( CheckAndMutateResult result) throws IOException { if (checkAndMutate.getAction() instanceof RowMutations) { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace( "preCheckAndMutateAfterRowLock (RowMutations): user [{}] on table [{}]", user, tableName); @@ -786,7 +786,7 @@ public void postListNamespaceDescriptors( public void preTruncateTable( ObserverContext ctx, final TableName tableName) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preTruncateTable: user [{}] on table [{}]", user, tableName); requirePermission(ctx, "truncateTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @@ -795,7 +795,7 @@ public void preTruncateTable( public void postTruncateTable( ObserverContext ctx, final TableName tableName) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.trace("postTruncateTable: user [{}] on table [{}]", user, tableName); } @@ -806,7 +806,7 @@ public TableDescriptor preModifyTable( TableDescriptor currentDesc, TableDescriptor newDesc) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preModifyTable: user [{}] on table [{}]", user, tableName); requirePermission(ctx, "modifyTable", tableName, null, null, Action.ADMIN, Action.CREATE); return currentDesc; @@ -818,7 +818,7 @@ public void postModifyTable( TableName tableName, final TableDescriptor htd) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.trace("postModifyTable: user [{}] on table [{}]", user, tableName); } @@ -826,7 +826,7 @@ public void postModifyTable( public Result preIncrement( final ObserverContext ctx, final Increment increment) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); LOG.trace("preIncrement: user [{}] on table [{}]", user, tableName); opaAclChecker.checkPermissionInfoWithOp( @@ -996,7 +996,7 @@ public void preSnapshot( public void preListSnapshot( ObserverContext ctx, final SnapshotDescription snapshot) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preListSnapshot: user [{}] snapshot[{}]", user, snapshot); requirePermission( ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "listSnapshot", Action.ADMIN); @@ -1008,7 +1008,7 @@ public void preCloneSnapshot( final SnapshotDescription snapshot, final TableDescriptor hTableDescriptor) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); TableName tableName = hTableDescriptor.getTableName(); LOG.debug("preCloneSnapshot: user [{}] snapshot[{}] table [{}]", user, snapshot, tableName); requirePermission(ctx, tableName.getNamespaceAsString(), "cloneSnapshot", Action.ADMIN); @@ -1020,7 +1020,7 @@ public void preRestoreSnapshot( final SnapshotDescription snapshot, final TableDescriptor hTableDescriptor) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preRestoreSnapshot: user [{}] snapshot[{}]", user, snapshot); requirePermission( ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "restoreSnapshot", Action.ADMIN); @@ -1030,7 +1030,7 @@ public void preRestoreSnapshot( public void preDeleteSnapshot( final ObserverContext ctx, final SnapshotDescription snapshot) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preDeleteSnapshot: user [{}] snapshot[{}]", user, snapshot); requirePermission( ctx, NamespaceDescriptor.DEFAULT_NAMESPACE_NAME_STR, "deleteSnapshot", Action.ADMIN); @@ -1049,8 +1049,8 @@ public void preSplitRegion( public void preBulkLoadHFile( ObserverContext ctx, List> familyPaths) throws IOException { - User user = getActiveUser(ctx); - var tableName = ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(); + final User user = getActiveUser(ctx); + final var tableName = ctx.getEnvironment().getRegion().getTableDescriptor().getTableName(); LOG.debug("preBulkLoadHFile: user [{}] on table [{}]", user, tableName); requirePermission(ctx, "preBulkLoadHFile", tableName, null, null, Action.ADMIN, Action.CREATE); } @@ -1137,7 +1137,7 @@ public void preGetUserPermissions( byte[] family, byte[] qualifier) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); if (tableName != null) { LOG.debug("preGetUserPermissions: user [{}] on table [{}]", user, tableName); requirePermission(ctx, "getUserPermissions", tableName, family, qualifier, Action.ADMIN); @@ -1195,7 +1195,7 @@ private static ImmutableMap> familyMap(byte[] family, byte[ private void requirePermission( final ObserverContext ctx, final String namespace, String request, Action... permissions) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); AccessControlException[] last = {null}; boolean allowed = Arrays.stream(permissions) @@ -1238,7 +1238,7 @@ private void requirePermission( OpType opType, Action... permissions) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); AccessControlException[] last = {null}; boolean allowed = Arrays.stream(permissions) @@ -1345,7 +1345,7 @@ public Message preEndpointInvocation( // Skip EXEC check for calls to the AccessControlService itself to avoid recursive checks. if (!(service instanceof AccessControlProtos.AccessControlService)) { TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug( "preEndpointInvocation: user [{}] on table [{}] method [{}]", user, @@ -1380,7 +1380,7 @@ public void preRequestLock( RegionInfo[] regionInfos, String description) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preRequestLock: user [{}] namespace [{}] table [{}]", user, namespace, tableName); if (namespace != null && !namespace.isEmpty()) { requirePermission(ctx, namespace, "requestLock", Action.ADMIN, Action.CREATE); @@ -1394,7 +1394,7 @@ public void preRequestLock( public void preLockHeartbeat( ObserverContext ctx, TableName tableName, String description) throws IOException { - User user = getActiveUser(ctx); + final User user = getActiveUser(ctx); LOG.debug("preLockHeartbeat: user [{}] table [{}]", user, tableName); requirePermission(ctx, "lockHeartbeat", tableName, null, null, Action.ADMIN, Action.CREATE); } From bcb293f9bff21e24fffe79905d64213f8099590c Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 9 Mar 2026 12:37:34 +0100 Subject: [PATCH 26/29] clarified comment --- .../stackable/hbase/OpenPolicyAgentAccessController.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java index 477a4bb..7895875 100644 --- a/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java +++ b/src/main/java/tech/stackable/hbase/OpenPolicyAgentAccessController.java @@ -1342,7 +1342,12 @@ public Message preEndpointInvocation( String methodName, Message request) throws IOException { - // Skip EXEC check for calls to the AccessControlService itself to avoid recursive checks. + // AccessControlService is the HBase ACL management RPC service (grant, revoke, etc.). + // Clients will not call it when permissions are managed in OPA rather than the HBase ACL + // table, so this branch is dead code in practice. The guard is retained from the reference + // AccessController, where omitting it would cause infinite recursion: the controller + // implements AccessControlService itself, so an EXEC check on an incoming ACL call would + // re-enter preEndpointInvocation. if (!(service instanceof AccessControlProtos.AccessControlService)) { TableName tableName = ctx.getEnvironment().getRegionInfo().getTable(); final User user = getActiveUser(ctx); From 9fc4f4ab41d33c203719557a8f6f8d92c4719ba7 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 9 Mar 2026 12:51:38 +0100 Subject: [PATCH 27/29] updated interface check test --- .../tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java index c00959f..7ee2f32 100644 --- a/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java +++ b/src/test/java/tech/stackable/hbase/TestCoprocessorInterfaceCoverage.java @@ -145,7 +145,7 @@ public class TestCoprocessorInterfaceCoverage { // --- TODO: genuine gaps that need OPA implementation --- // These pre-hooks are user-facing and should enforce OPA permissions, but are not yet // implemented. They are excluded here to keep this test focused on detecting new - // upstream methods; the implementation gaps are tracked separately in plan.md. + // upstream methods. // // Metadata listing hooks: getTableNames is covered post-hoc by postGetTableNames // filtering; listNamespace* hooks are not currently enforced. From bf5d181f130005f16905795775c95802dd38c0b7 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 9 Mar 2026 16:23:31 +0100 Subject: [PATCH 28/29] make casing consistent with rego rules in integration tests --- src/test/rego/hbase.rego | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rego/hbase.rego b/src/test/rego/hbase.rego index aef841d..14eba41 100644 --- a/src/test/rego/hbase.rego +++ b/src/test/rego/hbase.rego @@ -136,7 +136,7 @@ acls := [ "action": "ro", "resource": "hbase:namespace:", # Restrict to read-only operation types; exercises matches_operation non-null branch. - "operations": ["exists", "get", "scan", "none"], + "operations": ["EXISTS", "GET", "SCAN", "NONE"], # Restrict to known column families; exercises matches_families non-null branch. "families": ["cf1"], }, From 88fca27be17879e3bb0727cc973090062861e961 Mon Sep 17 00:00:00 2001 From: Andrew Kenworthy Date: Mon, 9 Mar 2026 16:45:44 +0100 Subject: [PATCH 29/29] added readonly tests to match integration test coverage --- .../stackable/hbase/OpaFixtureWriter.java | 12 +++++- ...OpenPolicyAgentAccessControllerRegion.java | 42 +++++++++++++++++++ .../java/tech/stackable/hbase/TestUtils.java | 20 +++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java index 2e48e01..6da36dc 100644 --- a/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java +++ b/src/test/java/tech/stackable/hbase/OpaFixtureWriter.java @@ -21,6 +21,9 @@ *

    *
  • {@code allowedUser} → admin Kerberos principal (member of "admins" group in Rego) *
  • {@code deniedUser} → unknown principal (not in any Rego group) + *
  • {@code readonlyUser} → readonlyuser Kerberos principal (ro ACL with operation/family + * restrictions in Rego; exercises the {@code matches_operation} and {@code matches_families} + * non-null branches) *
*/ public class OpaFixtureWriter { @@ -28,6 +31,8 @@ public class OpaFixtureWriter { static final String OPA_REMAP_ALLOWED = "admin/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; static final String OPA_REMAP_DENIED = "unknown@CLUSTER.LOCAL"; + static final String OPA_REMAP_READONLY = + "readonlyuser/access-hbase.test-ns.svc.cluster.local@CLUSTER.LOCAL"; private static final Path FIXTURES_FILE = Paths.get("target/test-rego/fixtures.json"); @@ -68,13 +73,16 @@ public static void flush() throws IOException { // Only capture requests from the standard allow/deny test users defined in TestUtils. // Requests from other named users (e.g. Variants-specific users) and cluster-internal // traffic are intentionally skipped — they are not useful for Rego policy validation. - if (!requestBody.contains("allowedUser") && !requestBody.contains("deniedUser")) { + if (!requestBody.contains("allowedUser") + && !requestBody.contains("deniedUser") + && !requestBody.contains("readonlyUser")) { continue; } String remapped = requestBody .replace("allowedUser", OPA_REMAP_ALLOWED) - .replace("deniedUser", OPA_REMAP_DENIED); + .replace("deniedUser", OPA_REMAP_DENIED) + .replace("readonlyUser", OPA_REMAP_READONLY); // WireMock stubs in this test suite always return {"result": "true"} or {"result": // "false"}. diff --git a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java index 5b9eb5a..01f3690 100644 --- a/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java +++ b/src/test/java/tech/stackable/hbase/TestOpenPolicyAgentAccessControllerRegion.java @@ -375,6 +375,48 @@ public void testPreCleanupBulkLoad() throws Exception { }); } + // --- OPA fixture coverage: readonlyUser principal --- + // These tests generate fixtures for the readonlyuser Kerberos principal (which has an ACL with + // operations and families restrictions in the Rego policy), exercising the matches_operation and + // matches_families non-null branches that the allowedUser/deniedUser fixtures never reach. + + @Test + public void testReadonlyUserScanAllowed() throws Exception { + assertReadonlyUserAllowed( + () -> { + getRegionController().preScannerOpen(regionCtx(), new Scan()); + return null; + }); + } + + @Test + public void testReadonlyUserGetAllowed() throws Exception { + assertReadonlyUserAllowed( + () -> { + getRegionController().preGetOp(regionCtx(), new Get(TEST_ROW), null); + return null; + }); + } + + @Test + public void testReadonlyUserExistsAllowed() throws Exception { + assertReadonlyUserAllowed( + () -> { + getRegionController().preExists(regionCtx(), new Get(TEST_ROW), false); + return null; + }); + } + + @Test + public void testReadonlyUserPutDenied() throws Exception { + assertReadonlyUserDenied( + () -> { + getRegionController() + .prePut(regionCtx(), new Put(TEST_ROW), null, Durability.USE_DEFAULT); + return null; + }); + } + // --- RegionServer hooks --- private OpenPolicyAgentAccessController getRsController() { diff --git a/src/test/java/tech/stackable/hbase/TestUtils.java b/src/test/java/tech/stackable/hbase/TestUtils.java index b4e1157..8011c1c 100644 --- a/src/test/java/tech/stackable/hbase/TestUtils.java +++ b/src/test/java/tech/stackable/hbase/TestUtils.java @@ -66,6 +66,7 @@ public class TestUtils { protected static User ALLOWED_USER; protected static User DENIED_USER; + protected static User READONLY_USER; protected static User USER_GROUP_ADMIN; protected static User USER_GROUP_CREATE; @@ -169,6 +170,7 @@ protected static void setup( ALLOWED_USER = User.createUserForTesting(conf, "allowedUser", new String[0]); DENIED_USER = User.createUserForTesting(conf, "deniedUser", new String[0]); + READONLY_USER = User.createUserForTesting(conf, "readonlyUser", new String[0]); } protected static void setUpTables() throws Exception { @@ -247,6 +249,24 @@ protected static void logOk(AccessControlException e) { LOG.info("AccessControlException as expected: [{}]", e.getMessage()); } + protected void assertReadonlyUserAllowed(SecureTestUtil.AccessTestAction action) + throws Exception { + READONLY_USER.runAs(action); + } + + protected void assertReadonlyUserDenied(SecureTestUtil.AccessTestAction action) throws Exception { + stubFor( + post("/") + .withRequestBody(matchingJsonPath("$.input.callerUgi[?(@.userName == 'readonlyUser')]")) + .willReturn(ok().withBody("{\"result\": \"false\"}"))); + try { + READONLY_USER.runAs(action); + fail("AccessControlException should have been thrown"); + } catch (AccessControlException e) { + logOk(e); + } + } + protected void assertAllowedThenDenied(SecureTestUtil.AccessTestAction action) throws Exception { ALLOWED_USER.runAs(action); stubFor(