diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java index 8f4bb294010e1..8cede59a14afa 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java @@ -542,6 +542,38 @@ public void testRemoveBucketAcl() Assert.assertTrue(!bucket.getAcls().contains(acls.get(0))); } + @Test + public void testRemoveBucketAclUsingRpcClientRemoveAcl() + throws IOException { + String volumeName = UUID.randomUUID().toString(); + String bucketName = UUID.randomUUID().toString(); + OzoneAcl userAcl = new OzoneAcl(USER, "test", + ACLType.ALL, ACCESS); + List acls = new ArrayList<>(); + acls.add(userAcl); + acls.add(new OzoneAcl(USER, "test1", + ACLType.ALL, ACCESS)); + store.createVolume(volumeName); + OzoneVolume volume = store.getVolume(volumeName); + BucketArgs.Builder builder = BucketArgs.newBuilder(); + builder.setAcls(acls); + volume.createBucket(bucketName, builder.build()); + OzoneObj ozoneObj = OzoneObjInfo.Builder.newBuilder() + .setBucketName(bucketName) + .setVolumeName(volumeName) + .setStoreType(OzoneObj.StoreType.OZONE) + .setResType(OzoneObj.ResourceType.BUCKET).build(); + + // Remove the 2nd acl added to the list. + boolean remove = store.removeAcl(ozoneObj, acls.get(1)); + Assert.assertTrue(remove); + Assert.assertFalse(store.getAcl(ozoneObj).contains(acls.get(1))); + + remove = store.removeAcl(ozoneObj, acls.get(0)); + Assert.assertTrue(remove); + Assert.assertFalse(store.getAcl(ozoneObj).contains(acls.get(0))); + } + @Test public void testSetBucketVersioning() throws IOException, OzoneException { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java index a20fd6a91eb67..530280b21cb61 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/BucketManagerImpl.java @@ -505,6 +505,7 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { BUCKET_NOT_FOUND); } + boolean removed = false; // When we are removing subset of rights from existing acl. for(OzoneAcl a: bucketInfo.getAcls()) { if(a.getName().equals(acl.getName()) && @@ -515,20 +516,21 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { if (bits.equals(ZERO_BITSET)) { return false; } - bits = (BitSet) acl.getAclBitSet().clone(); - bits.and(a.getAclBitSet()); + a.getAclBitSet().xor(bits); if(a.getAclBitSet().equals(ZERO_BITSET)) { bucketInfo.getAcls().remove(a); } + removed = true; break; - } else { - return false; } } - metadataManager.getBucketTable().put(dbBucketKey, bucketInfo); + if (removed) { + metadataManager.getBucketTable().put(dbBucketKey, bucketInfo); + } + return removed; } catch (IOException ex) { if (!(ex instanceof OMException)) { LOG.error("Remove acl operation failed for bucket:{}/{} acl:{}", @@ -538,8 +540,6 @@ public boolean removeAcl(OzoneObj obj, OzoneAcl acl) throws IOException { } finally { metadataManager.getLock().releaseLock(BUCKET_LOCK, volume, bucket); } - - return true; } /**