HBASE-26001 When turn on access control, the cell level TTL of Increment and Append operations is invalid (#3385)
Signed-off-by: Reid Chan <reidchan@apache.org>
This commit is contained in:
YutSean
GitHub
parent
336d8464cc
commit
53f61ef8d5
|
|
@ -127,7 +127,6 @@ import org.slf4j.Logger;
|
|||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
|
||||
import org.apache.hbase.thirdparty.com.google.common.collect.ArrayListMultimap;
|
||||
import org.apache.hbase.thirdparty.com.google.common.collect.ImmutableSet;
|
||||
import org.apache.hbase.thirdparty.com.google.common.collect.ListMultimap;
|
||||
import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
|
||||
|
|
@ -1740,7 +1739,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
|
|||
List<Pair<Cell, Cell>> cellPairs) throws IOException {
|
||||
// If the HFile version is insufficient to persist tags, we won't have any
|
||||
// work to do here
|
||||
if (!cellFeaturesEnabled) {
|
||||
if (!cellFeaturesEnabled || mutation.getACL() == null) {
|
||||
return cellPairs;
|
||||
}
|
||||
return cellPairs.stream().map(pair -> new Pair<>(pair.getFirst(),
|
||||
|
|
@ -1754,7 +1753,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
|
|||
List<Pair<Cell, Cell>> cellPairs) throws IOException {
|
||||
// If the HFile version is insufficient to persist tags, we won't have any
|
||||
// work to do here
|
||||
if (!cellFeaturesEnabled) {
|
||||
if (!cellFeaturesEnabled || mutation.getACL() == null) {
|
||||
return cellPairs;
|
||||
}
|
||||
return cellPairs.stream().map(pair -> new Pair<>(pair.getFirst(),
|
||||
|
|
@ -1763,50 +1762,28 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
|
|||
}
|
||||
|
||||
private Cell createNewCellWithTags(Mutation mutation, Cell oldCell, Cell newCell) {
|
||||
// Collect any ACLs from the old cell
|
||||
// As Increment and Append operations have already copied the tags of oldCell to the newCell,
|
||||
// there is no need to rewrite them again. Just extract non-acl tags of newCell if we need to
|
||||
// add a new acl tag for the cell. Actually, oldCell is useless here.
|
||||
List<Tag> tags = Lists.newArrayList();
|
||||
List<Tag> aclTags = Lists.newArrayList();
|
||||
ListMultimap<String,Permission> perms = ArrayListMultimap.create();
|
||||
if (oldCell != null) {
|
||||
Iterator<Tag> tagIterator = PrivateCellUtil.tagsIterator(oldCell);
|
||||
if (newCell != null) {
|
||||
Iterator<Tag> tagIterator = PrivateCellUtil.tagsIterator(newCell);
|
||||
while (tagIterator.hasNext()) {
|
||||
Tag tag = tagIterator.next();
|
||||
if (tag.getType() != PermissionStorage.ACL_TAG_TYPE) {
|
||||
// Not an ACL tag, just carry it through
|
||||
if (LOG.isTraceEnabled()) {
|
||||
LOG.trace("Carrying forward tag from " + oldCell + ": type " + tag.getType()
|
||||
+ " length " + tag.getValueLength());
|
||||
LOG.trace("Carrying forward tag from " + newCell + ": type " + tag.getType()
|
||||
+ " length " + tag.getValueLength());
|
||||
}
|
||||
tags.add(tag);
|
||||
} else {
|
||||
aclTags.add(tag);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Do we have an ACL on the operation?
|
||||
byte[] aclBytes = mutation.getACL();
|
||||
if (aclBytes != null) {
|
||||
// Yes, use it
|
||||
tags.add(new ArrayBackedTag(PermissionStorage.ACL_TAG_TYPE, aclBytes));
|
||||
} else {
|
||||
// No, use what we carried forward
|
||||
if (perms != null) {
|
||||
// TODO: If we collected ACLs from more than one tag we may have a
|
||||
// List<Permission> of size > 1, this can be collapsed into a single
|
||||
// Permission
|
||||
if (LOG.isTraceEnabled()) {
|
||||
LOG.trace("Carrying forward ACLs from " + oldCell + ": " + perms);
|
||||
}
|
||||
tags.addAll(aclTags);
|
||||
}
|
||||
}
|
||||
|
||||
// If we have no tags to add, just return
|
||||
if (tags.isEmpty()) {
|
||||
return newCell;
|
||||
}
|
||||
|
||||
// We have checked the ACL tag of mutation is not null.
|
||||
// So that the tags could not be empty.
|
||||
tags.add(new ArrayBackedTag(PermissionStorage.ACL_TAG_TYPE, mutation.getACL()));
|
||||
return PrivateCellUtil.createCell(newCell, tags);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import static org.junit.Assert.assertTrue;
|
|||
import static org.junit.Assert.fail;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import java.util.stream.Collectors;
|
||||
|
|
@ -29,10 +30,15 @@ import java.util.stream.Collectors;
|
|||
import org.apache.hadoop.hbase.Cell;
|
||||
import org.apache.hadoop.hbase.CellBuilderType;
|
||||
import org.apache.hadoop.hbase.CellUtil;
|
||||
import org.apache.hadoop.hbase.DoNotRetryIOException;
|
||||
import org.apache.hadoop.hbase.ExtendedCellBuilderFactory;
|
||||
import org.apache.hadoop.hbase.HBaseClassTestRule;
|
||||
import org.apache.hadoop.hbase.HBaseTestingUtility;
|
||||
import org.apache.hadoop.hbase.PrivateCellUtil;
|
||||
import org.apache.hadoop.hbase.TableName;
|
||||
import org.apache.hadoop.hbase.Tag;
|
||||
import org.apache.hadoop.hbase.TagBuilderFactory;
|
||||
import org.apache.hadoop.hbase.TagType;
|
||||
import org.apache.hadoop.hbase.client.Append;
|
||||
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
|
||||
import org.apache.hadoop.hbase.client.Connection;
|
||||
|
|
@ -45,6 +51,8 @@ import org.apache.hadoop.hbase.client.TableDescriptor;
|
|||
import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
|
||||
import org.apache.hadoop.hbase.client.TestFromClientSide;
|
||||
import org.apache.hadoop.hbase.regionserver.NoSuchColumnFamilyException;
|
||||
import org.apache.hadoop.hbase.security.access.AccessController;
|
||||
import org.apache.hadoop.hbase.security.access.Permission;
|
||||
import org.apache.hadoop.hbase.testclassification.CoprocessorTests;
|
||||
import org.apache.hadoop.hbase.testclassification.MediumTests;
|
||||
import org.apache.hadoop.hbase.util.Bytes;
|
||||
|
|
@ -64,7 +72,7 @@ import org.slf4j.LoggerFactory;
|
|||
* {@link RegionObserver#postIncrementBeforeWAL(ObserverContext, Mutation, List)} and
|
||||
* {@link RegionObserver#postAppendBeforeWAL(ObserverContext, Mutation, List)}. These methods may
|
||||
* change the cells which will be applied to memstore and WAL. So add unit test for the case which
|
||||
* change the cell's column family.
|
||||
* change the cell's column family and tags.
|
||||
*/
|
||||
@Category({CoprocessorTests.class, MediumTests.class})
|
||||
public class TestPostIncrementAndAppendBeforeWAL {
|
||||
|
|
@ -92,6 +100,10 @@ public class TestPostIncrementAndAppendBeforeWAL {
|
|||
private static final byte[] CQ1 = Bytes.toBytes("cq1");
|
||||
private static final byte[] CQ2 = Bytes.toBytes("cq2");
|
||||
private static final byte[] VALUE = Bytes.toBytes("value");
|
||||
private static final byte[] VALUE2 = Bytes.toBytes("valuevalue");
|
||||
private static final String USER = "User";
|
||||
private static final Permission PERMS =
|
||||
Permission.newBuilder().withActions(Permission.Action.READ).build();
|
||||
|
||||
@BeforeClass
|
||||
public static void setupBeforeClass() throws Exception {
|
||||
|
|
@ -161,6 +173,94 @@ public class TestPostIncrementAndAppendBeforeWAL {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testIncrementTTLWithACLTag() throws Exception {
|
||||
TableName tableName = TableName.valueOf(name.getMethodName());
|
||||
createTableWithCoprocessor(tableName, ChangeCellWithACLTagObserver.class.getName());
|
||||
try (Table table = connection.getTable(tableName)) {
|
||||
// Increment without TTL
|
||||
Increment firstIncrement = new Increment(ROW).addColumn(CF1_BYTES, CQ1, 1)
|
||||
.setACL(USER, PERMS);
|
||||
Result result = table.increment(firstIncrement);
|
||||
assertEquals(1, result.size());
|
||||
assertEquals(1, Bytes.toLong(result.getValue(CF1_BYTES, CQ1)));
|
||||
|
||||
// Check if the new cell can be read
|
||||
Get get = new Get(ROW).addColumn(CF1_BYTES, CQ1);
|
||||
result = table.get(get);
|
||||
assertEquals(1, result.size());
|
||||
assertEquals(1, Bytes.toLong(result.getValue(CF1_BYTES, CQ1)));
|
||||
|
||||
// Increment with TTL
|
||||
Increment secondIncrement = new Increment(ROW).addColumn(CF1_BYTES, CQ1, 1).setTTL(1000)
|
||||
.setACL(USER, PERMS);
|
||||
result = table.increment(secondIncrement);
|
||||
|
||||
// We should get value 2 here
|
||||
assertEquals(1, result.size());
|
||||
assertEquals(2, Bytes.toLong(result.getValue(CF1_BYTES, CQ1)));
|
||||
|
||||
// Wait 4s to let the second increment expire
|
||||
Thread.sleep(4000);
|
||||
get = new Get(ROW).addColumn(CF1_BYTES, CQ1);
|
||||
result = table.get(get);
|
||||
|
||||
// The value should revert to 1
|
||||
assertEquals(1, result.size());
|
||||
assertEquals(1, Bytes.toLong(result.getValue(CF1_BYTES, CQ1)));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAppendTTLWithACLTag() throws Exception {
|
||||
TableName tableName = TableName.valueOf(name.getMethodName());
|
||||
createTableWithCoprocessor(tableName, ChangeCellWithACLTagObserver.class.getName());
|
||||
try (Table table = connection.getTable(tableName)) {
|
||||
// Append without TTL
|
||||
Append firstAppend = new Append(ROW).addColumn(CF1_BYTES, CQ2, VALUE).setACL(USER, PERMS);
|
||||
Result result = table.append(firstAppend);
|
||||
assertEquals(1, result.size());
|
||||
assertTrue(Bytes.equals(VALUE, result.getValue(CF1_BYTES, CQ2)));
|
||||
|
||||
// Check if the new cell can be read
|
||||
Get get = new Get(ROW).addColumn(CF1_BYTES, CQ2);
|
||||
result = table.get(get);
|
||||
assertEquals(1, result.size());
|
||||
assertTrue(Bytes.equals(VALUE, result.getValue(CF1_BYTES, CQ2)));
|
||||
|
||||
// Append with TTL
|
||||
Append secondAppend = new Append(ROW).addColumn(CF1_BYTES, CQ2, VALUE).setTTL(1000)
|
||||
.setACL(USER, PERMS);
|
||||
result = table.append(secondAppend);
|
||||
|
||||
// We should get "valuevalue""
|
||||
assertEquals(1, result.size());
|
||||
assertTrue(Bytes.equals(VALUE2, result.getValue(CF1_BYTES, CQ2)));
|
||||
|
||||
// Wait 4s to let the second append expire
|
||||
Thread.sleep(4000);
|
||||
get = new Get(ROW).addColumn(CF1_BYTES, CQ2);
|
||||
result = table.get(get);
|
||||
|
||||
// The value should revert to "value"
|
||||
assertEquals(1, result.size());
|
||||
assertTrue(Bytes.equals(VALUE, result.getValue(CF1_BYTES, CQ2)));
|
||||
}
|
||||
}
|
||||
|
||||
private static boolean checkAclTag(byte[] acl, Cell cell) {
|
||||
Iterator<Tag> iter = PrivateCellUtil.tagsIterator(cell);
|
||||
while (iter.hasNext()) {
|
||||
Tag tag = iter.next();
|
||||
if (tag.getType() == TagType.ACL_TAG_TYPE) {
|
||||
Tag temp = TagBuilderFactory.create().
|
||||
setTagType(TagType.ACL_TAG_TYPE).setTagValue(acl).build();
|
||||
return Tag.matchingValue(tag, temp);
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
public static class ChangeCellWithDifferntColumnFamilyObserver
|
||||
implements RegionCoprocessor, RegionObserver {
|
||||
@Override
|
||||
|
|
@ -232,4 +332,37 @@ public class TestPostIncrementAndAppendBeforeWAL {
|
|||
.collect(Collectors.toList());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public static class ChangeCellWithACLTagObserver extends AccessController {
|
||||
@Override
|
||||
public Optional<RegionObserver> getRegionObserver() {
|
||||
return Optional.of(this);
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<Pair<Cell, Cell>> postIncrementBeforeWAL(
|
||||
ObserverContext<RegionCoprocessorEnvironment> ctx, Mutation mutation,
|
||||
List<Pair<Cell, Cell>> cellPairs) throws IOException {
|
||||
List<Pair<Cell, Cell>> result = super.postIncrementBeforeWAL(ctx, mutation, cellPairs);
|
||||
for (Pair<Cell, Cell> pair : result) {
|
||||
if (mutation.getACL() != null && !checkAclTag(mutation.getACL(), pair.getSecond())) {
|
||||
throw new DoNotRetryIOException("Unmatched ACL tag.");
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<Pair<Cell, Cell>> postAppendBeforeWAL(
|
||||
ObserverContext<RegionCoprocessorEnvironment> ctx, Mutation mutation,
|
||||
List<Pair<Cell, Cell>> cellPairs) throws IOException {
|
||||
List<Pair<Cell, Cell>> result = super.postAppendBeforeWAL(ctx, mutation, cellPairs);
|
||||
for (Pair<Cell, Cell> pair : result) {
|
||||
if (mutation.getACL() != null && !checkAclTag(mutation.getACL(), pair.getSecond())) {
|
||||
throw new DoNotRetryIOException("Unmatched ACL tag.");
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue