From 0f1768109159f95337e72505fbd094d9a1cf66d5 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Fri, 7 Feb 2025 08:56:14 +0100 Subject: [PATCH 1/6] add cluster for ACL tests --- .../java/org/apache/ozone/test/AclTests.java | 60 +++++++++++++++++++ .../test/TestOzoneNonHAWithNativeACL.java | 31 ++++++++++ 2 files changed, 91 insertions(+) create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACL.java diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java new file mode 100644 index 000000000000..fd8f3bd39a63 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ozone.test; + +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.hadoop.security.UserGroupInformation; +import org.junit.jupiter.api.TestInstance; + +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED; + +/** + * Tests with ACL enabled using Ozone Native Authorizer. + */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public abstract class AclTests extends ClusterForTests { + + public static final String ADMIN_USER = "om"; + public static final String ADMIN_GROUP = "ozone"; + public static final UserGroupInformation ADMIN_UGI = + UserGroupInformation.createUserForTesting(ADMIN_USER, new String[] {ADMIN_GROUP}); + + /** Hook method for subclasses. */ + MiniOzoneCluster.Builder newClusterBuilder() { + return MiniOzoneCluster.newBuilder(createOzoneConfig()) + .setNumDatanodes(3); + } + + @Override + protected OzoneConfiguration createOzoneConfig() { + UserGroupInformation.setLoginUser(ADMIN_UGI); + OzoneConfiguration conf = super.createOzoneConfig(); + conf.setBoolean(OZONE_ACL_ENABLED, true); + conf.set(OZONE_ACL_AUTHORIZER_CLASS, OZONE_ACL_AUTHORIZER_CLASS_NATIVE); + return conf; + } + + /** Test cases for non-HA cluster should implement this. */ + public interface TestCase { + MiniOzoneCluster cluster(); + } + +} diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACL.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACL.java new file mode 100644 index 000000000000..7f030e141e21 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACL.java @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ozone.test; + +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.junit.jupiter.api.TestInstance; + +/** Test Ozone with native ACLs in non-HA cluster. */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class TestOzoneNonHAWithNativeACL extends AclTests { + @Override + protected MiniOzoneCluster createCluster() throws Exception { + return newClusterBuilder() + .build(); + } +} From 22f7b13ccc273741d3419955749fc00e7dec3c83 Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Fri, 7 Feb 2025 08:58:13 +0100 Subject: [PATCH 2/6] convert TestBucketOwner --- .../hadoop/ozone/om/TestBucketOwner.java | 86 +++++++------------ .../java/org/apache/ozone/test/AclTests.java | 9 ++ 2 files changed, 42 insertions(+), 53 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java index 97512fec40c0..03c2b1288051 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java @@ -16,9 +16,7 @@ */ package org.apache.hadoop.ozone.om; -import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.StorageType; -import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.client.BucketArgs; import org.apache.hadoop.ozone.client.ObjectStore; @@ -31,19 +29,16 @@ import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; import org.apache.hadoop.security.UserGroupInformation; -import org.junit.jupiter.api.AfterAll; +import org.apache.ozone.test.AclTests; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.Timeout; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.UUID; import static org.apache.hadoop.ozone.OzoneAcl.AclScope.DEFAULT; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED; import static org.apache.hadoop.ozone.security.acl.IAccessAuthorizer.ACLIdentityType.USER; import static org.apache.hadoop.ozone.security.acl.OzoneObj.StoreType.OZONE; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -52,63 +47,48 @@ /** * Test for Ozone Bucket Owner. */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) @Timeout(120) -public class TestBucketOwner { +public abstract class TestBucketOwner implements AclTests.TestCase { - private static MiniOzoneCluster cluster; - private static final Logger LOG = - LoggerFactory.getLogger(TestBucketOwner.class); - private static UserGroupInformation adminUser = - UserGroupInformation.createUserForTesting("om", new String[]{"ozone"}); + private static final String UNIQUE = UUID.randomUUID().toString(); + private static final String VOLUME_NAME = "vol-" + UNIQUE; private static UserGroupInformation user1 = UserGroupInformation - .createUserForTesting("user1", new String[] {"test1"}); + .createUserForTesting("user-" + UNIQUE + 1, new String[] {"test1"}); private static UserGroupInformation user2 = UserGroupInformation - .createUserForTesting("user2", new String[] {"test2"}); + .createUserForTesting("user-" + UNIQUE + 2, new String[] {"test2"}); private static UserGroupInformation user3 = UserGroupInformation - .createUserForTesting("user3", new String[] {"test3"}); + .createUserForTesting("user-" + UNIQUE + 3, new String[] {"test3"}); @BeforeAll - public static void init() throws Exception { - // loginUser is the user running this test. - UserGroupInformation.setLoginUser(adminUser); - OzoneConfiguration conf = new OzoneConfiguration(); - conf.set(OZONE_ACL_AUTHORIZER_CLASS, OZONE_ACL_AUTHORIZER_CLASS_NATIVE); - conf.setBoolean(OZONE_ACL_ENABLED, true); - cluster = MiniOzoneCluster.newBuilder(conf).build(); - cluster.waitForClusterToBeReady(); - try (OzoneClient client = cluster.newClient()) { + void init() throws Exception { + UserGroupInformation.setLoginUser(AclTests.ADMIN_UGI); + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); /* r = READ, w = WRITE, c = CREATE, d = DELETE l = LIST, a = ALL, n = NONE, x = READ_ACL, y = WRITE_ACL */ String aclWorldAll = "world::a"; - createVolumeWithOwnerAndAcl(objectStore, "volume1", "user2", aclWorldAll); + createVolumeWithOwnerAndAcl(objectStore, VOLUME_NAME, user2.getShortUserName(), aclWorldAll); } UserGroupInformation.setLoginUser(user1); - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); - OzoneVolume volume = objectStore.getVolume("volume1"); + OzoneVolume volume = objectStore.getVolume(VOLUME_NAME); BucketArgs omBucketArgs = BucketArgs.newBuilder() - .setStorageType(StorageType.DISK).setOwner("user1").build(); + .setStorageType(StorageType.DISK).setOwner(user1.getShortUserName()).build(); volume.createBucket("bucket1", omBucketArgs); volume.createBucket("bucket2", omBucketArgs); volume.createBucket("bucket3", omBucketArgs); } } - @AfterAll - public static void stopCluster() { - if (cluster != null) { - cluster.shutdown(); - } - } - @Test public void testBucketOwner() throws Exception { // Test Key Operations as Bucket Owner, Non-Volume Owner UserGroupInformation.setLoginUser(user1); - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { OzoneVolume volume = client.getObjectStore() - .getVolume("volume1"); + .getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); //Key Create createKey(ozoneBucket, "key1", 10, new byte[10]); @@ -133,50 +113,50 @@ public void testNonBucketNonVolumeOwner() throws Exception { // Test Key Operations Non-Bucket Owner, Non-Volume Owner //Key Create UserGroupInformation.setLoginUser(user3); - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { assertThrows(Exception.class, () -> { - OzoneVolume volume = client.getObjectStore().getVolume("volume1"); + OzoneVolume volume = client.getObjectStore().getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); createKey(ozoneBucket, "key3", 10, new byte[10]); }, "Create key as non-volume and non-bucket owner should fail"); } //Key Delete - should fail - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { assertThrows(Exception.class, () -> { - OzoneVolume volume = client.getObjectStore().getVolume("volume1"); + OzoneVolume volume = client.getObjectStore().getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); ozoneBucket.deleteKey("key2"); }, "Delete key as non-volume and non-bucket owner should fail"); } //Key Rename - should fail - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { assertThrows(Exception.class, () -> { - OzoneVolume volume = client.getObjectStore().getVolume("volume1"); + OzoneVolume volume = client.getObjectStore().getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); ozoneBucket.renameKey("key2", "key4"); }, "Rename key as non-volume and non-bucket owner should fail"); } //List Keys - should fail - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { assertThrows(Exception.class, () -> { - OzoneVolume volume = client.getObjectStore().getVolume("volume1"); + OzoneVolume volume = client.getObjectStore().getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); ozoneBucket.listKeys("key"); }, "List keys as non-volume and non-bucket owner should fail"); } //Get Acls - should fail - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { assertThrows(Exception.class, () -> { - OzoneVolume volume = client.getObjectStore().getVolume("volume1"); + OzoneVolume volume = client.getObjectStore().getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); ozoneBucket.getAcls(); }, "Get Acls as non-volume and non-bucket owner should fail"); } //Add Acls - should fail - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { assertThrows(Exception.class, () -> { - OzoneVolume volume = client.getObjectStore().getVolume("volume1"); + OzoneVolume volume = client.getObjectStore().getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); OzoneAcl acl = new OzoneAcl(USER, "testuser1", DEFAULT, IAccessAuthorizer.ACLType.ALL); @@ -189,8 +169,8 @@ public void testNonBucketNonVolumeOwner() throws Exception { public void testVolumeOwner() throws Exception { //Test Key Operations for Volume Owner UserGroupInformation.setLoginUser(user2); - try (OzoneClient client = cluster.newClient()) { - OzoneVolume volume = client.getObjectStore().getVolume("volume1"); + try (OzoneClient client = cluster().newClient()) { + OzoneVolume volume = client.getObjectStore().getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); //Key Create createKey(ozoneBucket, "key2", 10, new byte[10]); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java index fd8f3bd39a63..8a52f8d749e6 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java @@ -20,6 +20,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.security.UserGroupInformation; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.TestInstance; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS; @@ -57,4 +58,12 @@ public interface TestCase { MiniOzoneCluster cluster(); } + @Nested + class BucketOwner extends org.apache.hadoop.ozone.om.TestBucketOwner { + @Override + public MiniOzoneCluster cluster() { + return getCluster(); + } + } + } From 865458e67cd0c1f0a0d19d30c4cdedb65100eb9a Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Fri, 7 Feb 2025 09:30:23 +0100 Subject: [PATCH 3/6] convert TestRecursiveAclWithFSO --- .../ozone/om/TestRecursiveAclWithFSO.java | 86 +++++-------------- .../java/org/apache/ozone/test/AclTests.java | 18 +++- 2 files changed, 38 insertions(+), 66 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java index 1a2e61b88005..fa6ba2fedd6f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java @@ -19,9 +19,7 @@ package org.apache.hadoop.ozone.om; import org.apache.commons.lang3.RandomStringUtils; -import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.StorageType; -import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.client.BucketArgs; import org.apache.hadoop.ozone.client.ObjectStore; @@ -31,13 +29,11 @@ import org.apache.hadoop.ozone.client.io.OzoneOutputStream; import org.apache.hadoop.ozone.client.protocol.ClientProtocol; import org.apache.hadoop.ozone.om.exceptions.OMException; -import org.apache.hadoop.ozone.om.request.OMRequestTestUtils; import org.apache.hadoop.ozone.security.acl.OzoneAclConfig; import org.apache.hadoop.ozone.security.acl.OzoneObj; import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; import org.apache.hadoop.security.UserGroupInformation; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; +import org.apache.ozone.test.AclTests; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -48,9 +44,6 @@ import java.util.List; import java.util.UUID; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED; import static org.apache.hadoop.ozone.security.acl.OzoneObj.StoreType.OZONE; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -61,28 +54,17 @@ * Test recursive acl checks for delete and rename for FSO Buckets. */ @Timeout(120) -public class TestRecursiveAclWithFSO { +public abstract class TestRecursiveAclWithFSO implements AclTests.TestCase { - private MiniOzoneCluster cluster; + private static final String UNIQUE = UUID.randomUUID().toString(); + private static final String VOLUME_NAME = "vol-" + UNIQUE; - private final UserGroupInformation adminUser = - UserGroupInformation.createUserForTesting("om", new String[] {"ozone"}); private final UserGroupInformation user1 = UserGroupInformation - .createUserForTesting("user1", new String[] {"test1"}); + .createUserForTesting("user1-" + UNIQUE, new String[] {"test1"}); private final UserGroupInformation user2 = UserGroupInformation - .createUserForTesting("user2", new String[] {"test2"}); + .createUserForTesting("user2-" + UNIQUE, new String[] {"test2"}); private final UserGroupInformation user3 = UserGroupInformation - .createUserForTesting("user3", new String[] {"test3, test4"}); - - @BeforeEach - public void init() throws Exception { - // loginUser is the user running this test. - // Implication: loginUser is automatically added to the OM admin list. - UserGroupInformation.setLoginUser(adminUser); - // ozone.acl.enabled = true - // start a cluster - startCluster(); - } + .createUserForTesting("user3-" + UNIQUE, new String[] {"test3, test4"}); @Test public void testKeyDeleteAndRenameWithoutPermission() throws Exception { @@ -91,16 +73,16 @@ public void testKeyDeleteAndRenameWithoutPermission() throws Exception { String aclWorldAll = "world::a"; List keys = new ArrayList<>(); // Create volumes with user1 - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); - createVolumeWithOwnerAndAcl(objectStore, "volume1", "user1", aclWorldAll); + createVolumeWithOwnerAndAcl(objectStore, VOLUME_NAME, user1.getShortUserName(), aclWorldAll); } // Login as user1, create directories and keys UserGroupInformation.setLoginUser(user1); - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); - OzoneVolume volume = objectStore.getVolume("volume1"); + OzoneVolume volume = objectStore.getVolume(VOLUME_NAME); BucketArgs omBucketArgs = BucketArgs.newBuilder().setStorageType(StorageType.DISK).build(); @@ -166,9 +148,9 @@ public void testKeyDeleteAndRenameWithoutPermission() throws Exception { } UserGroupInformation.setLoginUser(user2); - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); - OzoneVolume volume = objectStore.getVolume("volume1"); + OzoneVolume volume = objectStore.getVolume(VOLUME_NAME); OzoneBucket ozoneBucket = volume.getBucket("bucket1"); // perform delete @@ -188,9 +170,9 @@ public void testKeyDeleteAndRenameWithoutPermission() throws Exception { // Remove acl from directory c2, delete/rename a/b1 should throw // permission denied since c2 is a subdirectory user1.doAs((PrivilegedExceptionAction) () -> { - try (OzoneClient c = cluster.newClient()) { + try (OzoneClient c = cluster().newClient()) { ObjectStore o = c.getObjectStore(); - OzoneBucket b = o.getVolume("volume1").getBucket("bucket1"); + OzoneBucket b = o.getVolume(VOLUME_NAME).getBucket("bucket1"); removeAclsFromKey(o, b, "a/b1/c2"); } return null; @@ -221,7 +203,7 @@ public void testKeyDeleteAndRenameWithoutPermission() throws Exception { @Test public void testKeyDefaultACL() throws Exception { String volumeName = "vol1"; - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); objectStore.createVolume(volumeName); addVolumeAcl(objectStore, volumeName, "world::a"); @@ -232,10 +214,10 @@ public void testKeyDefaultACL() throws Exception { .setStoreType(OZONE).build(); List acls = objectStore.getAcl(obj); assertEquals(3, acls.size()); - assertEquals(adminUser.getShortUserName(), acls.get(0).getName()); - OzoneAclConfig aclConfig = cluster.getConf().getObject(OzoneAclConfig.class); + assertEquals(AclTests.ADMIN_UGI.getShortUserName(), acls.get(0).getName()); + OzoneAclConfig aclConfig = cluster().getConf().getObject(OzoneAclConfig.class); assertArrayEquals(aclConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray()); - assertEquals(adminUser.getPrimaryGroupName(), acls.get(1).getName()); + assertEquals(AclTests.ADMIN_UGI.getPrimaryGroupName(), acls.get(1).getName()); assertArrayEquals(aclConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray()); assertEquals("WORLD", acls.get(2).getName()); assertArrayEquals(aclConfig.getUserDefaultRights(), acls.get(2).getAclList().toArray()); @@ -243,7 +225,7 @@ public void testKeyDefaultACL() throws Exception { // set LoginUser as user3 UserGroupInformation.setLoginUser(user3); - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); OzoneVolume volume = objectStore.getVolume(volumeName); BucketArgs omBucketArgs = @@ -259,7 +241,7 @@ public void testKeyDefaultACL() throws Exception { List acls = objectStore.getAcl(obj); assertEquals(2, acls.size()); assertEquals(user3.getShortUserName(), acls.get(0).getName()); - OzoneAclConfig aclConfig = cluster.getConf().getObject(OzoneAclConfig.class); + OzoneAclConfig aclConfig = cluster().getConf().getObject(OzoneAclConfig.class); assertArrayEquals(aclConfig.getUserDefaultRights(), acls.get(0).getAclList().toArray()); assertEquals(user3.getPrimaryGroupName(), acls.get(1).getName()); assertArrayEquals(aclConfig.getGroupDefaultRights(), acls.get(1).getAclList().toArray()); @@ -295,32 +277,6 @@ private void removeAclsFromKey(ObjectStore objectStore, } } - /** - * Create a MiniOzoneCluster for testing. - */ - private void startCluster() throws Exception { - - OzoneConfiguration conf = new OzoneConfiguration(); - - // Use native impl here, default impl doesn't do actual checks - conf.set(OZONE_ACL_AUTHORIZER_CLASS, OZONE_ACL_AUTHORIZER_CLASS_NATIVE); - // Note: OM doesn't support live config reloading - conf.setBoolean(OZONE_ACL_ENABLED, true); - - OMRequestTestUtils.configureFSOptimizedPaths(conf, true); - - cluster = MiniOzoneCluster.newBuilder(conf).build(); - cluster.waitForClusterToBeReady(); - - } - - @AfterEach - public void stopCluster() { - if (cluster != null) { - cluster.shutdown(); - } - } - private void createVolumeWithOwnerAndAcl(ObjectStore objectStore, String volumeName, String ownerName, String aclString) throws IOException { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java index 8a52f8d749e6..fa44983e3d4c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java @@ -19,7 +19,9 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.hadoop.ozone.om.OMConfigKeys; import org.apache.hadoop.security.UserGroupInformation; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.TestInstance; @@ -46,13 +48,19 @@ MiniOzoneCluster.Builder newClusterBuilder() { @Override protected OzoneConfiguration createOzoneConfig() { - UserGroupInformation.setLoginUser(ADMIN_UGI); + loginAdmin(); OzoneConfiguration conf = super.createOzoneConfig(); conf.setBoolean(OZONE_ACL_ENABLED, true); conf.set(OZONE_ACL_AUTHORIZER_CLASS, OZONE_ACL_AUTHORIZER_CLASS_NATIVE); + conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS, true); return conf; } + @BeforeEach + void loginAdmin() { + UserGroupInformation.setLoginUser(ADMIN_UGI); + } + /** Test cases for non-HA cluster should implement this. */ public interface TestCase { MiniOzoneCluster cluster(); @@ -66,4 +74,12 @@ public MiniOzoneCluster cluster() { } } + @Nested + class RecursiveAclWithFSO extends org.apache.hadoop.ozone.om.TestRecursiveAclWithFSO { + @Override + public MiniOzoneCluster cluster() { + return getCluster(); + } + } + } From 335c548258835ffb2899de249f3bf53d61e081ad Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Fri, 7 Feb 2025 12:59:46 +0100 Subject: [PATCH 4/6] convert TestOzoneManagerListVolumes --- .../ozone/om/TestOzoneManagerListVolumes.java | 218 +++++++++--------- .../java/org/apache/ozone/test/AclTests.java | 8 + .../apache/ozone/test/ConfigAssumptions.java | 32 +++ .../org/apache/ozone/test/NonHATests.java | 8 + ...nHAWithNativeACLListAllVolumesFlipped.java | 43 ++++ 5 files changed, 195 insertions(+), 114 deletions(-) create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/ConfigAssumptions.java create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java index 1fcfa63d1a80..9efdb0a88e8a 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java @@ -25,10 +25,8 @@ import java.util.Iterator; import java.util.List; import java.util.Set; -import java.util.concurrent.TimeoutException; +import java.util.UUID; -import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.OzoneAcl; import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneClient; @@ -39,114 +37,97 @@ import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; import org.apache.hadoop.security.UserGroupInformation; -import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_RATIS_PIPELINE_LIMIT; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_AUTHORIZER_CLASS_NATIVE; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT; import static org.apache.hadoop.ozone.security.acl.OzoneObj.StoreType.OZONE; +import static org.apache.ozone.test.ConfigAssumptions.assumeConfig; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import org.junit.jupiter.api.AfterAll; +import org.apache.ozone.test.AclTests; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.Timeout; /** * Test OzoneManager list volume operation under combinations of configs. */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) @Timeout(120) -public class TestOzoneManagerListVolumes { +public abstract class TestOzoneManagerListVolumes implements AclTests.TestCase { - private static MiniOzoneCluster cluster; + private static final String UNIQUE = UUID.randomUUID().toString(); + private static final String VOL_PREFIX = "vol-" + UNIQUE; + private static final String VOLUME_1 = VOL_PREFIX + 1; + private static final String VOLUME_2 = VOL_PREFIX + 2; + private static final String VOLUME_3 = VOL_PREFIX + 3; + private static final String VOLUME_4 = VOL_PREFIX + 4; + private static final String VOLUME_5 = VOL_PREFIX + 5; - private static UserGroupInformation adminUser = - UserGroupInformation.createUserForTesting("om", new String[]{"ozone"}); + public static final String USER_1 = "user1-" + UNIQUE; private static UserGroupInformation user1 = - UserGroupInformation.createUserForTesting("user1", new String[]{"test"}); + UserGroupInformation.createUserForTesting(USER_1, new String[]{"test"}); + public static final String USER_2 = "user2-" + UNIQUE; private static UserGroupInformation user2 = - UserGroupInformation.createUserForTesting("user2", new String[]{"test"}); + UserGroupInformation.createUserForTesting(USER_2, new String[]{"test"}); // Typycal kerberos user, with shortname different from username. private static UserGroupInformation user3 = - UserGroupInformation.createUserForTesting("user3@example.com", + UserGroupInformation.createUserForTesting("user3-" + UNIQUE + "@example.com", new String[]{"test"}); @BeforeEach - public void init() throws Exception { + void loginAdmin() { // loginUser is the user running this test. // Implication: loginUser is automatically added to the OM admin list. - UserGroupInformation.setLoginUser(adminUser); + UserGroupInformation.setLoginUser(AclTests.ADMIN_UGI); } - /** - * Create a MiniDFSCluster for testing. - */ + @AfterEach + void logout() { + UserGroupInformation.setLoginUser(null); + } @BeforeAll - public static void setupClass() - throws InterruptedException, TimeoutException, IOException { - OzoneConfiguration conf = new OzoneConfiguration(); - UserGroupInformation.setLoginUser(adminUser); - conf.setInt(OZONE_SCM_RATIS_PIPELINE_LIMIT, 10); - - // Use native impl here, default impl doesn't do actual checks - conf.set(OZONE_ACL_AUTHORIZER_CLASS, OZONE_ACL_AUTHORIZER_CLASS_NATIVE); - - cluster = MiniOzoneCluster.newBuilder(conf) - .withoutDatanodes() - .build(); - cluster.waitForClusterToBeReady(); + void createVolumes() throws IOException { + loginAdmin(); // Create volumes with non-default owners and ACLs - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { ObjectStore objectStore = client.getObjectStore(); /* r = READ, w = WRITE, c = CREATE, d = DELETE l = LIST, a = ALL, n = NONE, x = READ_ACL, y = WRITE_ACL */ - String aclUser1All = "user:user1:a"; - String aclUser2All = "user:user2:a"; + String aclUser1All = userAllACL(USER_1); + String aclUser2All = userAllACL(USER_2); String aclWorldAll = "world::a"; - createVolumeWithOwnerAndAcl(objectStore, "volume1", "user1", aclUser1All); - createVolumeWithOwnerAndAcl(objectStore, "volume2", "user2", aclUser2All); - createVolumeWithOwnerAndAcl(objectStore, "volume3", "user1", aclUser2All); - createVolumeWithOwnerAndAcl(objectStore, "volume4", "user2", aclUser1All); - createVolumeWithOwnerAndAcl(objectStore, "volume5", "user1", aclWorldAll); + createVolumeWithOwnerAndAcl(objectStore, VOLUME_1, USER_1, aclUser1All); + createVolumeWithOwnerAndAcl(objectStore, VOLUME_2, USER_2, aclUser2All); + createVolumeWithOwnerAndAcl(objectStore, VOLUME_3, USER_1, aclUser2All); + createVolumeWithOwnerAndAcl(objectStore, VOLUME_4, USER_2, aclUser1All); + createVolumeWithOwnerAndAcl(objectStore, VOLUME_5, USER_1, aclWorldAll); } - - OzoneManager om = cluster.getOzoneManager(); - om.stop(); - om.join(); } - @AfterAll - public static void shutdownClass() { - if (cluster != null) { - cluster.shutdown(); - } + private static String userAllACL(String user) { + return "user:" + user + ":a"; } - private void startOM(boolean aclEnabled, - boolean volListAllAllowed) throws Exception { - - OzoneConfiguration conf = cluster.getOzoneManager().getConfiguration(); - // Note: OM doesn't support live config reloading - conf.setBoolean(OZONE_ACL_ENABLED, aclEnabled); - conf.setBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED, volListAllAllowed); - cluster.getOzoneManager().restart(); + private void assumeAclEnabled(boolean expected) { + assumeConfig(cluster().getConf(), + OZONE_ACL_ENABLED, OZONE_ACL_ENABLED_DEFAULT, expected); } - @AfterEach - public void stopOM() { - OzoneManager om = cluster.getOzoneManager(); - if (om != null) { - om.stop(); - om.join(); - } + private void assumeListAllVolumesAllowed(boolean expected) { + assumeConfig(cluster().getConf(), + OZONE_OM_VOLUME_LISTALL_ALLOWED, OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT, expected); } private static void createVolumeWithOwnerAndAcl(ObjectStore objectStore, @@ -181,7 +162,7 @@ private static void setVolumeAcl(ObjectStore objectStore, String volumeName, private void checkUser(UserGroupInformation user, List expectVol, boolean expectListAllSuccess, boolean expectListByUserSuccess) throws IOException { - try (OzoneClient client = cluster.newClient()) { + try (OzoneClient client = cluster().newClient()) { checkUser(client, user, expectVol, expectListAllSuccess, expectListByUserSuccess); } @@ -204,7 +185,8 @@ private static void checkUser(OzoneClient client, UserGroupInformation user, String volumeName = vol.getName(); accessibleVolumes.add(volumeName); } - assertEquals(new HashSet<>(expectVol), accessibleVolumes); + assertThat(accessibleVolumes) + .containsAll(expectVol); } catch (RuntimeException ex) { if (expectListByUserSuccess) { throw ex; @@ -224,7 +206,7 @@ private static void checkUser(OzoneClient client, UserGroupInformation user, // `ozone sh volume list --all` returns all volumes, // or throws exception (for non-admin if acl enabled & listall disallowed). if (expectListAllSuccess) { - it = objectStore.listVolumes("volume"); + it = objectStore.listVolumes(VOL_PREFIX); int count = 0; while (it.hasNext()) { it.next(); @@ -233,7 +215,7 @@ private static void checkUser(OzoneClient client, UserGroupInformation user, assertEquals(5, count); } else { RuntimeException ex = - assertThrows(RuntimeException.class, () -> objectStore.listVolumes("volume")); + assertThrows(RuntimeException.class, () -> objectStore.listVolumes(VOL_PREFIX)); // Current listAllVolumes throws RuntimeException if (ex.getCause() instanceof OMException) { // Expect PERMISSION_DENIED @@ -256,27 +238,28 @@ private static void checkUser(OzoneClient client, UserGroupInformation user, @Test public void testListVolumeWithOtherUsersListAllAllowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = true - startOM(true, true); + assumeAclEnabled(true); + assumeListAllVolumesAllowed(true); // Login as user1, list other users' volumes UserGroupInformation.setLoginUser(user1); - checkUser(user2, Arrays.asList("volume2", "volume3", "volume4", - "volume5"), true, false); + checkUser(user2, Arrays.asList(VOLUME_2, VOLUME_3, VOLUME_4, + VOLUME_5), true, false); // Add "s3v" created default by OM. - checkUser(adminUser, Arrays.asList("volume1", "volume2", "volume3", - "volume4", "volume5", "s3v"), true); + checkUser(AclTests.ADMIN_UGI, Arrays.asList(VOLUME_1, VOLUME_2, VOLUME_3, + VOLUME_4, VOLUME_5, "s3v"), true); UserGroupInformation.setLoginUser(user2); - checkUser(user1, Arrays.asList("volume1", "volume3", "volume4", - "volume5"), true); - checkUser(adminUser, Arrays.asList("volume1", "volume2", "volume3", - "volume4", "volume5", "s3v"), true); + checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_4, + VOLUME_5), true); + checkUser(AclTests.ADMIN_UGI, Arrays.asList(VOLUME_1, VOLUME_2, VOLUME_3, + VOLUME_4, VOLUME_5, "s3v"), true); // list volumes should success for user with shortname different from // full name. UserGroupInformation.setLoginUser(user3); - checkUser(user3, Collections.singletonList("volume5"), true, true); + checkUser(user3, Collections.singletonList(VOLUME_5), true, true); } /** @@ -287,76 +270,83 @@ public void testListVolumeWithOtherUsersListAllAllowed() throws Exception { @Test public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = false - startOM(true, false); + assumeAclEnabled(true); + assumeListAllVolumesAllowed(false); // Login as user1, list other users' volumes, expect failure UserGroupInformation.setLoginUser(user1); - checkUser(user2, Arrays.asList("volume2", "volume3", "volume4", - "volume5"), false, false); + checkUser(user2, Arrays.asList(VOLUME_2, VOLUME_3, VOLUME_4, + VOLUME_5), false, false); // Add "s3v" created default by OM. - checkUser(adminUser, Arrays.asList("volume1", "volume2", "volume3", - "volume4", "volume5", "s3v"), false, false); + checkUser(AclTests.ADMIN_UGI, Arrays.asList(VOLUME_1, VOLUME_2, VOLUME_3, + VOLUME_4, VOLUME_5, "s3v"), false, false); // While admin should be able to list volumes just fine. - UserGroupInformation.setLoginUser(adminUser); - checkUser(user1, Arrays.asList("volume1", "volume3", "volume4", - "volume5"), true); - checkUser(user2, Arrays.asList("volume2", "volume3", "volume4", - "volume5"), true); + UserGroupInformation.setLoginUser(AclTests.ADMIN_UGI); + checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_4, + VOLUME_5), true); + checkUser(user2, Arrays.asList(VOLUME_2, VOLUME_3, VOLUME_4, + VOLUME_5), true); } @Test public void testAclEnabledListAllAllowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = true - startOM(true, true); - checkUser(user1, Arrays.asList("volume1", "volume3", "volume4", - "volume5"), true); - checkUser(user2, Arrays.asList("volume2", "volume3", "volume4", - "volume5"), true); + assumeAclEnabled(true); + assumeListAllVolumesAllowed(true); + + checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_4, + VOLUME_5), true); + checkUser(user2, Arrays.asList(VOLUME_2, VOLUME_3, VOLUME_4, + VOLUME_5), true); // Add "s3v" created default by OM. - checkUser(adminUser, Arrays.asList("volume1", "volume2", "volume3", - "volume4", "volume5", "s3v"), true); + checkUser(AclTests.ADMIN_UGI, Arrays.asList(VOLUME_1, VOLUME_2, VOLUME_3, + VOLUME_4, VOLUME_5, "s3v"), true); } @Test public void testAclEnabledListAllDisallowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = false - startOM(true, false); - // The default user is adminUser as set in init(), + assumeAclEnabled(true); + assumeListAllVolumesAllowed(false); + + // The default user is AclTests.ADMIN_UGI as set in init(), // listall always succeeds if we use that UGI, we should use non-admin here UserGroupInformation.setLoginUser(user1); - checkUser(user1, Arrays.asList("volume1", "volume3", "volume4", - "volume5"), false); + checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_4, + VOLUME_5), false); UserGroupInformation.setLoginUser(user2); - checkUser(user2, Arrays.asList("volume2", "volume3", "volume4", - "volume5"), false); - UserGroupInformation.setLoginUser(adminUser); + checkUser(user2, Arrays.asList(VOLUME_2, VOLUME_3, VOLUME_4, + VOLUME_5), false); + UserGroupInformation.setLoginUser(AclTests.ADMIN_UGI); // Add "s3v" created default by OM. - checkUser(adminUser, Arrays.asList("volume1", "volume2", - "volume3", "volume4", "volume5", "s3v"), true); + checkUser(AclTests.ADMIN_UGI, Arrays.asList(VOLUME_1, VOLUME_2, + VOLUME_3, VOLUME_4, VOLUME_5, "s3v"), true); } @Test - public void testAclDisabledListAllAllowed() throws Exception { - // ozone.acl.enabled = false, ozone.om.volume.listall.allowed = true - startOM(false, true); - checkUser(user1, Arrays.asList("volume1", "volume3", "volume5"), + public void testAclDisabledAdmin() throws Exception { + // ozone.acl.enabled = false, ozone.om.volume.listall.allowed = don't care + assumeAclEnabled(false); + + checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_5), true); - checkUser(user2, Arrays.asList("volume2", "volume4"), + checkUser(user2, Arrays.asList(VOLUME_2, VOLUME_4), true); } @Test - public void testAclDisabledListAllDisallowed() throws Exception { - // ozone.acl.enabled = false, ozone.om.volume.listall.allowed = false - startOM(false, false); + public void testAclDisabled() throws Exception { + // ozone.acl.enabled = false, ozone.om.volume.listall.allowed = don't care + assumeAclEnabled(false); + // If ACL is disabled, all permission checks are disabled in Ozone by design UserGroupInformation.setLoginUser(user1); - checkUser(user1, Arrays.asList("volume1", "volume3", "volume5"), + checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_5), true); UserGroupInformation.setLoginUser(user2); - checkUser(user2, Arrays.asList("volume2", "volume4"), + checkUser(user2, Arrays.asList(VOLUME_2, VOLUME_4), true); // listall will succeed since acl is disabled } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java index fa44983e3d4c..6c88b62a088c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java @@ -74,6 +74,14 @@ public MiniOzoneCluster cluster() { } } + @Nested + class OzoneManagerListVolumes extends org.apache.hadoop.ozone.om.TestOzoneManagerListVolumes { + @Override + public MiniOzoneCluster cluster() { + return getCluster(); + } + } + @Nested class RecursiveAclWithFSO extends org.apache.hadoop.ozone.om.TestRecursiveAclWithFSO { @Override diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/ConfigAssumptions.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/ConfigAssumptions.java new file mode 100644 index 000000000000..6f2a5d5b8342 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/ConfigAssumptions.java @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ozone.test; + +import org.apache.hadoop.hdds.conf.ConfigurationSource; + +import static org.assertj.core.api.Assumptions.assumeThat; + +/** Assumptions for skipping tests based on configuration. */ +public interface ConfigAssumptions { + + static void assumeConfig(ConfigurationSource conf, String key, boolean defaultValue, boolean expected) { + boolean actual = conf.getBoolean(key, defaultValue); + assumeThat(actual).isEqualTo(expected); + } + +} diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/NonHATests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/NonHATests.java index 0566b356b301..b1ff1e32d2dd 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/NonHATests.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/NonHATests.java @@ -178,6 +178,14 @@ public MiniOzoneCluster cluster() { } } + @Nested + class OzoneManagerListVolumes extends org.apache.hadoop.ozone.om.TestOzoneManagerListVolumes { + @Override + public MiniOzoneCluster cluster() { + return getCluster(); + } + } + @Nested class OzoneManagerRestInterface extends org.apache.hadoop.ozone.om.TestOzoneManagerRestInterface { @Override diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java new file mode 100644 index 000000000000..d6cd1332c03b --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.ozone.test; + +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.junit.jupiter.api.TestInstance; + +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED; +import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT; + +/** Test Ozone with native ACLs in non-HA cluster, with non-default value for listing allow volumes. */ +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +public class TestOzoneNonHAWithNativeACLListAllVolumesFlipped extends AclTests { + + @Override + protected MiniOzoneCluster createCluster() throws Exception { + return newClusterBuilder() + .build(); + } + + @Override + protected OzoneConfiguration createOzoneConfig() { + OzoneConfiguration conf = super.createOzoneConfig(); + conf.setBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED, !OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT); + return conf; + } +} From 8dfe9af6fdce90c3f3747fd2dba8df45d82129ae Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Fri, 7 Feb 2025 14:52:09 +0100 Subject: [PATCH 5/6] remove redundant interface --- .../java/org/apache/hadoop/ozone/om/TestBucketOwner.java | 3 ++- .../apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java | 3 ++- .../org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java | 3 ++- .../src/test/java/org/apache/ozone/test/AclTests.java | 5 ----- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java index 03c2b1288051..a5d31ca6af7c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestBucketOwner.java @@ -30,6 +30,7 @@ import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; import org.apache.hadoop.security.UserGroupInformation; import org.apache.ozone.test.AclTests; +import org.apache.ozone.test.NonHATests; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; @@ -49,7 +50,7 @@ */ @TestInstance(TestInstance.Lifecycle.PER_CLASS) @Timeout(120) -public abstract class TestBucketOwner implements AclTests.TestCase { +public abstract class TestBucketOwner implements NonHATests.TestCase { private static final String UNIQUE = UUID.randomUUID().toString(); private static final String VOLUME_NAME = "vol-" + UNIQUE; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java index 9efdb0a88e8a..0fd1d19229a2 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java @@ -49,6 +49,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import org.apache.ozone.test.AclTests; +import org.apache.ozone.test.NonHATests; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -61,7 +62,7 @@ */ @TestInstance(TestInstance.Lifecycle.PER_CLASS) @Timeout(120) -public abstract class TestOzoneManagerListVolumes implements AclTests.TestCase { +public abstract class TestOzoneManagerListVolumes implements NonHATests.TestCase { private static final String UNIQUE = UUID.randomUUID().toString(); private static final String VOL_PREFIX = "vol-" + UNIQUE; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java index fa6ba2fedd6f..df3e66d5708c 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestRecursiveAclWithFSO.java @@ -34,6 +34,7 @@ import org.apache.hadoop.ozone.security.acl.OzoneObjInfo; import org.apache.hadoop.security.UserGroupInformation; import org.apache.ozone.test.AclTests; +import org.apache.ozone.test.NonHATests; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -54,7 +55,7 @@ * Test recursive acl checks for delete and rename for FSO Buckets. */ @Timeout(120) -public abstract class TestRecursiveAclWithFSO implements AclTests.TestCase { +public abstract class TestRecursiveAclWithFSO implements NonHATests.TestCase { private static final String UNIQUE = UUID.randomUUID().toString(); private static final String VOLUME_NAME = "vol-" + UNIQUE; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java index 6c88b62a088c..1633fa8443a4 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/AclTests.java @@ -61,11 +61,6 @@ void loginAdmin() { UserGroupInformation.setLoginUser(ADMIN_UGI); } - /** Test cases for non-HA cluster should implement this. */ - public interface TestCase { - MiniOzoneCluster cluster(); - } - @Nested class BucketOwner extends org.apache.hadoop.ozone.om.TestBucketOwner { @Override From b9597a01def19b938a7939adf5a8ee38e6e80eea Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" Date: Sat, 8 Feb 2025 07:32:53 +0100 Subject: [PATCH 6/6] set allowListAllVolumes dynamically in test --- .../ozone/om/TestOzoneManagerListVolumes.java | 30 ++++++++----- ...nHAWithNativeACLListAllVolumesFlipped.java | 43 ------------------- 2 files changed, 19 insertions(+), 54 deletions(-) delete mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java index 0fd1d19229a2..06e9c7e56262 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumes.java @@ -56,6 +56,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; /** * Test OzoneManager list volume operation under combinations of configs. @@ -94,6 +96,7 @@ void loginAdmin() { @AfterEach void logout() { UserGroupInformation.setLoginUser(null); + setListAllVolumesAllowed(OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT); } @BeforeAll @@ -126,9 +129,10 @@ private void assumeAclEnabled(boolean expected) { OZONE_ACL_ENABLED, OZONE_ACL_ENABLED_DEFAULT, expected); } - private void assumeListAllVolumesAllowed(boolean expected) { - assumeConfig(cluster().getConf(), - OZONE_OM_VOLUME_LISTALL_ALLOWED, OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT, expected); + private void setListAllVolumesAllowed(boolean newValue) { + OzoneManager om = cluster().getOzoneManager(); + om.getConfiguration().setBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED, newValue); + om.setAllowListAllVolumesFromConfig(); } private static void createVolumeWithOwnerAndAcl(ObjectStore objectStore, @@ -240,7 +244,7 @@ private static void checkUser(OzoneClient client, UserGroupInformation user, public void testListVolumeWithOtherUsersListAllAllowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = true assumeAclEnabled(true); - assumeListAllVolumesAllowed(true); + setListAllVolumesAllowed(true); // Login as user1, list other users' volumes UserGroupInformation.setLoginUser(user1); @@ -272,7 +276,7 @@ public void testListVolumeWithOtherUsersListAllAllowed() throws Exception { public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = false assumeAclEnabled(true); - assumeListAllVolumesAllowed(false); + setListAllVolumesAllowed(false); // Login as user1, list other users' volumes, expect failure UserGroupInformation.setLoginUser(user1); @@ -294,7 +298,7 @@ public void testListVolumeWithOtherUsersListAllDisallowed() throws Exception { public void testAclEnabledListAllAllowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = true assumeAclEnabled(true); - assumeListAllVolumesAllowed(true); + setListAllVolumesAllowed(true); checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_4, VOLUME_5), true); @@ -310,7 +314,7 @@ public void testAclEnabledListAllAllowed() throws Exception { public void testAclEnabledListAllDisallowed() throws Exception { // ozone.acl.enabled = true, ozone.om.volume.listall.allowed = false assumeAclEnabled(true); - assumeListAllVolumesAllowed(false); + setListAllVolumesAllowed(false); // The default user is AclTests.ADMIN_UGI as set in init(), // listall always succeeds if we use that UGI, we should use non-admin here @@ -326,10 +330,12 @@ public void testAclEnabledListAllDisallowed() throws Exception { VOLUME_3, VOLUME_4, VOLUME_5, "s3v"), true); } - @Test - public void testAclDisabledAdmin() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testAclDisabledAdmin(boolean allowListAllVolumes) throws Exception { // ozone.acl.enabled = false, ozone.om.volume.listall.allowed = don't care assumeAclEnabled(false); + setListAllVolumesAllowed(allowListAllVolumes); checkUser(user1, Arrays.asList(VOLUME_1, VOLUME_3, VOLUME_5), true); @@ -337,10 +343,12 @@ public void testAclDisabledAdmin() throws Exception { true); } - @Test - public void testAclDisabled() throws Exception { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void testAclDisabled(boolean allowListAllVolumes) throws Exception { // ozone.acl.enabled = false, ozone.om.volume.listall.allowed = don't care assumeAclEnabled(false); + setListAllVolumesAllowed(allowListAllVolumes); // If ACL is disabled, all permission checks are disabled in Ozone by design UserGroupInformation.setLoginUser(user1); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java b/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java deleted file mode 100644 index d6cd1332c03b..000000000000 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/ozone/test/TestOzoneNonHAWithNativeACLListAllVolumesFlipped.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.ozone.test; - -import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.apache.hadoop.ozone.MiniOzoneCluster; -import org.junit.jupiter.api.TestInstance; - -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED; -import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT; - -/** Test Ozone with native ACLs in non-HA cluster, with non-default value for listing allow volumes. */ -@TestInstance(TestInstance.Lifecycle.PER_CLASS) -public class TestOzoneNonHAWithNativeACLListAllVolumesFlipped extends AclTests { - - @Override - protected MiniOzoneCluster createCluster() throws Exception { - return newClusterBuilder() - .build(); - } - - @Override - protected OzoneConfiguration createOzoneConfig() { - OzoneConfiguration conf = super.createOzoneConfig(); - conf.setBoolean(OZONE_OM_VOLUME_LISTALL_ALLOWED, !OZONE_OM_VOLUME_LISTALL_ALLOWED_DEFAULT); - return conf; - } -}