Skip to content

Commit 7737ffa

Browse files
ChenSammiChenSammi
committed
address comments
1 parent 155d382 commit 7737ffa

5 files changed

Lines changed: 99 additions & 33 deletions

File tree

‎hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@
171171
import java.util.function.Function;
172172
import java.util.stream.Collectors;
173173

174+
import static org.apache.hadoop.ozone.OzoneAcl.LINK_BUCKET_DEFAULT_ACL;
174175
import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY;
175176
import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_KEY_PROVIDER_CACHE_EXPIRY_DEFAULT;
176177
import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY;
@@ -659,6 +660,12 @@ public void createBucket(
659660
builder.setAcls(bucketArgs.getAcls());
660661
}
661662

663+
// Link bucket default acl
664+
if (bucketArgs.getSourceVolume() != null
665+
&& bucketArgs.getSourceBucket() != null) {
666+
builder.addAcl(LINK_BUCKET_DEFAULT_ACL);
667+
}
668+
662669
if (bek != null) {
663670
builder.setBucketEncryptionKey(bek);
664671
}

‎hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OzoneAcl.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo;
2828
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclScope;
2929
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OzoneAclInfo.OzoneAclType;
30+
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
3031
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType;
3132
import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType;
3233
import org.apache.ratis.util.MemoizedSupplier;
@@ -41,8 +42,11 @@
4142
import java.util.function.IntFunction;
4243
import java.util.function.Supplier;
4344

45+
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
4446
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL;
4547
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.NONE;
48+
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ;
49+
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE;
4650

4751
/**
4852
* OzoneACL classes define bucket ACLs used in OZONE.
@@ -58,6 +62,13 @@
5862
public class OzoneAcl {
5963

6064
private static final String ACL_SCOPE_REGEX = ".*\\[(ACCESS|DEFAULT)\\]";
65+
/**
66+
* Link bucket default acl defined [world::rw]
67+
* which is similar to Linux POSIX symbolic.
68+
*/
69+
public static final OzoneAcl LINK_BUCKET_DEFAULT_ACL =
70+
new OzoneAcl(IAccessAuthorizer.ACLIdentityType.WORLD, "", ACCESS, READ, WRITE);
71+
6172
private final ACLIdentityType type;
6273
private final String name;
6374
@JsonIgnore

‎hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneAclUtil.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@
4040
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT;
4141
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP;
4242
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER;
43-
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ;
44-
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE;
4543

4644
/**
4745
* Helper class for ozone acls operations.
@@ -80,16 +78,6 @@ public static List<OzoneAcl> getDefaultAclList(UserGroupInformation ugi, OzoneCo
8078
return listOfAcls;
8179
}
8280

83-
/**
84-
* Link bucket default acl defined [world::rw]
85-
* which is similar to Linux POSIX symbolic.
86-
*
87-
* @return OzoneAclInfo
88-
*/
89-
public static OzoneAcl linkBucketDefaultAcl() {
90-
return new OzoneAcl(IAccessAuthorizer.ACLIdentityType.WORLD, "", ACCESS, READ, WRITE);
91-
}
92-
9381
public static List<OzoneAcl> getAclList(UserGroupInformation ugi, ACLType userPrivilege, ACLType groupPrivilege) {
9482
List<OzoneAcl> listOfAcls = new ArrayList<>();
9583
// User ACL.

‎hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/OzoneRpcClientTests.java‎

Lines changed: 79 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@
172172
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.PARTIAL_RENAME;
173173
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.GROUP;
174174
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER;
175+
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.ALL;
176+
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.LIST;
175177
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.READ;
176178

177179
import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLType.WRITE;
@@ -394,10 +396,10 @@ public void testBucketSetOwner() throws IOException {
394396
.setVolumeName(volumeName).setBucketName(bucketName)
395397
.setStoreType(OzoneObj.StoreType.OZONE)
396398
.setResType(OzoneObj.ResourceType.BUCKET).build();
397-
store.addAcl(volumeObj, new OzoneAcl(USER, "user1", ACCESS, ACLType.ALL));
398-
store.addAcl(volumeObj, new OzoneAcl(USER, "user2", ACCESS, ACLType.ALL));
399-
store.addAcl(bucketObj, new OzoneAcl(USER, "user1", ACCESS, ACLType.ALL));
400-
store.addAcl(bucketObj, new OzoneAcl(USER, "user2", ACCESS, ACLType.ALL));
399+
store.addAcl(volumeObj, new OzoneAcl(USER, "user1", ACCESS, ALL));
400+
store.addAcl(volumeObj, new OzoneAcl(USER, "user2", ACCESS, ALL));
401+
store.addAcl(bucketObj, new OzoneAcl(USER, "user1", ACCESS, ALL));
402+
store.addAcl(bucketObj, new OzoneAcl(USER, "user2", ACCESS, ALL));
401403

402404
createKeyForUser(volumeName, bucketName, key1, content, user1);
403405
createKeyForUser(volumeName, bucketName, key2, content, user2);
@@ -779,7 +781,7 @@ public void testCreateBucketWithAllArgument()
779781
String volumeName = UUID.randomUUID().toString();
780782
String bucketName = UUID.randomUUID().toString();
781783
OzoneAcl userAcl = new OzoneAcl(USER, "test",
782-
ACCESS, ACLType.ALL);
784+
ACCESS, ALL);
783785
ReplicationConfig repConfig = new ECReplicationConfig(3, 2);
784786
store.createVolume(volumeName);
785787
OzoneVolume volume = store.getVolume(volumeName);
@@ -818,7 +820,7 @@ public void testAddBucketAcl()
818820
OzoneVolume volume = store.getVolume(volumeName);
819821
volume.createBucket(bucketName);
820822
List<OzoneAcl> acls = new ArrayList<>();
821-
acls.add(new OzoneAcl(USER, "test", ACCESS, ACLType.ALL));
823+
acls.add(new OzoneAcl(USER, "test", ACCESS, ALL));
822824
OzoneBucket bucket = volume.getBucket(bucketName);
823825
for (OzoneAcl acl : acls) {
824826
assertTrue(bucket.addAcl(acl));
@@ -834,7 +836,7 @@ public void testRemoveBucketAcl()
834836
String volumeName = UUID.randomUUID().toString();
835837
String bucketName = UUID.randomUUID().toString();
836838
OzoneAcl userAcl = new OzoneAcl(USER, "test",
837-
ACCESS, ACLType.ALL);
839+
ACCESS, ALL);
838840
store.createVolume(volumeName);
839841
OzoneVolume volume = store.getVolume(volumeName);
840842
BucketArgs.Builder builder = BucketArgs.newBuilder()
@@ -853,9 +855,9 @@ public void testRemoveBucketAclUsingRpcClientRemoveAcl()
853855
String volumeName = UUID.randomUUID().toString();
854856
String bucketName = UUID.randomUUID().toString();
855857
OzoneAcl userAcl = new OzoneAcl(USER, "test",
856-
ACCESS, ACLType.ALL);
858+
ACCESS, ALL);
857859
OzoneAcl acl2 = new OzoneAcl(USER, "test1",
858-
ACCESS, ACLType.ALL);
860+
ACCESS, ALL);
859861
store.createVolume(volumeName);
860862
OzoneVolume volume = store.getVolume(volumeName);
861863
BucketArgs.Builder builder = BucketArgs.newBuilder()
@@ -913,6 +915,64 @@ public void testAclsAfterCallingSetBucketProperty() throws Exception {
913915

914916
}
915917

918+
@Test
919+
public void testAclDeDuplication()
920+
throws IOException {
921+
String volumeName = UUID.randomUUID().toString();
922+
String bucketName = UUID.randomUUID().toString();
923+
OzoneAcl userAcl1 = new OzoneAcl(USER, "test", DEFAULT, READ);
924+
UserGroupInformation currentUser = UserGroupInformation.getCurrentUser();
925+
OzoneAcl currentUserAcl = new OzoneAcl(USER, currentUser.getShortUserName(), ACCESS, ALL);
926+
OzoneAcl currentUserPrimaryGroupAcl = new OzoneAcl(GROUP, currentUser.getPrimaryGroupName(), ACCESS, READ, LIST);
927+
VolumeArgs createVolumeArgs = VolumeArgs.newBuilder()
928+
.setOwner(currentUser.getShortUserName())
929+
.setAdmin(currentUser.getShortUserName())
930+
.addAcl(userAcl1)
931+
.addAcl(currentUserAcl)
932+
.addAcl(currentUserPrimaryGroupAcl)
933+
.build();
934+
935+
store.createVolume(volumeName, createVolumeArgs);
936+
OzoneVolume volume = store.getVolume(volumeName);
937+
List<OzoneAcl> volumeAcls = volume.getAcls();
938+
assertEquals(3, volumeAcls.size());
939+
assertTrue(volumeAcls.contains(userAcl1));
940+
assertTrue(volumeAcls.contains(currentUserAcl));
941+
assertTrue(volumeAcls.contains(currentUserPrimaryGroupAcl));
942+
943+
// normal bucket
944+
BucketArgs.Builder builder = BucketArgs.newBuilder()
945+
.addAcl(currentUserAcl).addAcl(currentUserPrimaryGroupAcl);
946+
volume.createBucket(bucketName, builder.build());
947+
OzoneBucket bucket = volume.getBucket(bucketName);
948+
List<OzoneAcl> bucketAcls = bucket.getAcls();
949+
assertEquals(bucketName, bucket.getName());
950+
assertEquals(3, bucketAcls.size());
951+
assertTrue(bucketAcls.contains(currentUserAcl));
952+
assertTrue(bucketAcls.contains(currentUserPrimaryGroupAcl));
953+
assertTrue(bucketAcls.get(2).getName().equals(userAcl1.getName()));
954+
assertTrue(bucketAcls.get(2).getAclList().equals(userAcl1.getAclList()));
955+
assertTrue(bucketAcls.get(2).getAclScope().equals(ACCESS));
956+
957+
// link bucket
958+
OzoneAcl userAcl2 = new OzoneAcl(USER, "test-link", DEFAULT, READ);
959+
String linkBucketName = "link-" + bucketName;
960+
builder = BucketArgs.newBuilder().setSourceVolume(volumeName).setSourceBucket(bucketName)
961+
.addAcl(currentUserAcl).addAcl(currentUserPrimaryGroupAcl).addAcl(userAcl2);
962+
volume.createBucket(linkBucketName, builder.build());
963+
OzoneBucket linkBucket = volume.getBucket(linkBucketName);
964+
List<OzoneAcl> linkBucketAcls = linkBucket.getAcls();
965+
assertEquals(linkBucketName, linkBucket.getName());
966+
assertEquals(5, linkBucketAcls.size());
967+
assertTrue(linkBucketAcls.contains(currentUserAcl));
968+
assertTrue(linkBucketAcls.contains(currentUserPrimaryGroupAcl));
969+
assertTrue(linkBucketAcls.contains(userAcl2));
970+
assertTrue(linkBucketAcls.contains(OzoneAcl.LINK_BUCKET_DEFAULT_ACL));
971+
assertTrue(linkBucketAcls.get(4).getName().equals(userAcl1.getName()));
972+
assertTrue(linkBucketAcls.get(4).getAclList().equals(userAcl1.getAclList()));
973+
assertTrue(linkBucketAcls.get(4).getAclScope().equals(ACCESS));
974+
}
975+
916976
@Test
917977
public void testSetBucketStorageType()
918978
throws IOException {
@@ -3029,10 +3089,10 @@ public void testMultipartUploadWithACL() throws Exception {
30293089
OzoneBucket bucket = volume.getBucket(bucketName);
30303090

30313091
// Add ACL on Bucket
3032-
OzoneAcl acl1 = new OzoneAcl(USER, "Monday", DEFAULT, ACLType.ALL);
3033-
OzoneAcl acl2 = new OzoneAcl(USER, "Friday", DEFAULT, ACLType.ALL);
3034-
OzoneAcl acl3 = new OzoneAcl(USER, "Jan", ACCESS, ACLType.ALL);
3035-
OzoneAcl acl4 = new OzoneAcl(USER, "Feb", ACCESS, ACLType.ALL);
3092+
OzoneAcl acl1 = new OzoneAcl(USER, "Monday", DEFAULT, ALL);
3093+
OzoneAcl acl2 = new OzoneAcl(USER, "Friday", DEFAULT, ALL);
3094+
OzoneAcl acl3 = new OzoneAcl(USER, "Jan", ACCESS, ALL);
3095+
OzoneAcl acl4 = new OzoneAcl(USER, "Feb", ACCESS, ALL);
30363096
bucket.addAcl(acl1);
30373097
bucket.addAcl(acl2);
30383098
bucket.addAcl(acl3);
@@ -3203,10 +3263,10 @@ public void testMultipartUploadOwner() throws Exception {
32033263
.setVolumeName(volumeName).setBucketName(bucketName)
32043264
.setStoreType(OzoneObj.StoreType.OZONE)
32053265
.setResType(OzoneObj.ResourceType.BUCKET).build();
3206-
store.addAcl(volumeObj, new OzoneAcl(USER, "user1", ACCESS, ACLType.ALL));
3207-
store.addAcl(volumeObj, new OzoneAcl(USER, "awsUser1", ACCESS, ACLType.ALL));
3208-
store.addAcl(bucketObj, new OzoneAcl(USER, "user1", ACCESS, ACLType.ALL));
3209-
store.addAcl(bucketObj, new OzoneAcl(USER, "awsUser1", ACCESS, ACLType.ALL));
3266+
store.addAcl(volumeObj, new OzoneAcl(USER, "user1", ACCESS, ALL));
3267+
store.addAcl(volumeObj, new OzoneAcl(USER, "awsUser1", ACCESS, ALL));
3268+
store.addAcl(bucketObj, new OzoneAcl(USER, "user1", ACCESS, ALL));
3269+
store.addAcl(bucketObj, new OzoneAcl(USER, "awsUser1", ACCESS, ALL));
32103270

32113271
// user1 MultipartUpload a key
32123272
UserGroupInformation.setLoginUser(user1);
@@ -3943,7 +4003,7 @@ public void testNativeAclsForPrefix() throws Exception {
39434003
aclsGet = store.getAcl(prefixObj);
39444004
assertEquals(0, aclsGet.size());
39454005

3946-
OzoneAcl group1Acl = new OzoneAcl(GROUP, "group1", ACCESS, ACLType.ALL);
4006+
OzoneAcl group1Acl = new OzoneAcl(GROUP, "group1", ACCESS, ALL);
39474007
List<OzoneAcl> acls = new ArrayList<>();
39484008
acls.add(user1Acl);
39494009
acls.add(group1Acl);
@@ -4054,7 +4114,7 @@ private void validateOzoneAccessAcl(OzoneObj ozObj) throws IOException {
40544114
OzoneAcl ua = new OzoneAcl(USER, "userx",
40554115
ACCESS, ACLType.READ_ACL);
40564116
OzoneAcl ug = new OzoneAcl(GROUP, "userx",
4057-
ACCESS, ACLType.ALL);
4117+
ACCESS, ALL);
40584118
store.setAcl(ozObj, Arrays.asList(ua, ug));
40594119
newAcls = store.getAcl(ozObj);
40604120
assertEquals(2, newAcls.size());

‎hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@
6969
import java.util.stream.Collectors;
7070

7171
import static org.apache.hadoop.ozone.OzoneAcl.AclScope.ACCESS;
72+
import static org.apache.hadoop.ozone.OzoneAcl.LINK_BUCKET_DEFAULT_ACL;
7273
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_ALREADY_EXISTS;
7374
import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND;
7475
import static org.apache.hadoop.ozone.om.helpers.OzoneAclUtil.getDefaultAclList;
75-
import static org.apache.hadoop.ozone.om.helpers.OzoneAclUtil.linkBucketDefaultAcl;
7676
import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.BUCKET_LOCK;
7777
import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.Resource.VOLUME_LOCK;
7878

@@ -335,7 +335,7 @@ private void addDefaultAcls(OmBucketInfo omBucketInfo,
335335

336336
// Link bucket default acl
337337
if (omBucketInfo.getSourceBucket() != null && omBucketInfo.getSourceVolume() != null) {
338-
acls.add(linkBucketDefaultAcl());
338+
acls.add(LINK_BUCKET_DEFAULT_ACL);
339339
}
340340

341341
// Add default acls from volume.

0 commit comments

Comments
 (0)