keith-turner commented on code in PR #5864:
URL: https://github.com/apache/accumulo/pull/5864#discussion_r2323707725


##########
test/src/main/java/org/apache/accumulo/test/conf/ResourceGroupConfigIT.java:
##########
@@ -207,4 +212,132 @@ private void checkProperty(InstanceOperations iops, 
ResourceGroupOperations ops,
     System.clearProperty(TServerClient.DEBUG_HOST);
   }
 
+  @Test
+  public void testDefaultResourceGroup() throws Exception {
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      Set<ResourceGroupId> rgs = client.resourceGroupOperations().list();
+      assertEquals(1, rgs.size());
+      assertEquals(ResourceGroupId.DEFAULT, rgs.iterator().next());
+      client.resourceGroupOperations().create(ResourceGroupId.DEFAULT);
+      assertThrows(AccumuloException.class,
+          () -> 
client.resourceGroupOperations().remove(ResourceGroupId.DEFAULT));
+    }
+  }
+
+  @Test
+  public void testPermissions() throws Exception {
+
+    ClusterUser testUser = getUser(0);
+
+    String principal = testUser.getPrincipal();
+    AuthenticationToken token = testUser.getToken();
+    PasswordToken passwordToken = null;
+    if (token instanceof PasswordToken) {
+      passwordToken = (PasswordToken) token;
+    }
+
+    ResourceGroupId rgid = ResourceGroupId.of("TEST_GROUP");
+
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      client.securityOperations().createLocalUser(principal, passwordToken);
+    }
+
+    try (AccumuloClient test_user_client =
+        Accumulo.newClient().from(getClientProps()).as(principal, 
token).build()) {
+      assertThrows(AccumuloSecurityException.class,
+          () -> test_user_client.resourceGroupOperations().create(rgid));
+    }
+
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      client.resourceGroupOperations().create(rgid);
+    }
+
+    try (AccumuloClient test_user_client =
+        Accumulo.newClient().from(getClientProps()).as(principal, 
token).build()) {
+      assertThrows(AccumuloSecurityException.class,
+          () -> test_user_client.resourceGroupOperations().remove(rgid));
+    }
+  }
+
+  @Test
+  public void testMultipleConfigurations() throws Exception {
+
+    final String FIRST = "FIRST";
+    final String SECOND = "SECOND";
+    final String THIRD = "THIRD";
+
+    final ResourceGroupId first = ResourceGroupId.of(FIRST);
+    final ResourceGroupId second = ResourceGroupId.of(SECOND);
+    final ResourceGroupId third = ResourceGroupId.of(THIRD);
+
+    // @formatter:off
+    Map<String,String> firstProps = Map.of(
+        Property.COMPACTION_WARN_TIME.getKey(), "1m",
+        Property.SSERV_WAL_SORT_MAX_CONCURRENT.getKey(), "4",

Review Comment:
   Would be good to set the same prop w/ diff value in the three sets of props. 
 Could change the value of SSERV_WAL_SORT_MAX_CONCURRENT to something other 
than 4 across all., or could add another prop that has the same key and diff 
values across all.



##########
test/src/main/java/org/apache/accumulo/test/conf/ResourceGroupConfigIT.java:
##########
@@ -207,4 +212,132 @@ private void checkProperty(InstanceOperations iops, 
ResourceGroupOperations ops,
     System.clearProperty(TServerClient.DEBUG_HOST);
   }
 
+  @Test
+  public void testDefaultResourceGroup() throws Exception {
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      Set<ResourceGroupId> rgs = client.resourceGroupOperations().list();
+      assertEquals(1, rgs.size());
+      assertEquals(ResourceGroupId.DEFAULT, rgs.iterator().next());
+      client.resourceGroupOperations().create(ResourceGroupId.DEFAULT);
+      assertThrows(AccumuloException.class,
+          () -> 
client.resourceGroupOperations().remove(ResourceGroupId.DEFAULT));
+    }
+  }
+
+  @Test
+  public void testPermissions() throws Exception {
+
+    ClusterUser testUser = getUser(0);
+
+    String principal = testUser.getPrincipal();
+    AuthenticationToken token = testUser.getToken();
+    PasswordToken passwordToken = null;
+    if (token instanceof PasswordToken) {
+      passwordToken = (PasswordToken) token;
+    }
+
+    ResourceGroupId rgid = ResourceGroupId.of("TEST_GROUP");
+
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      client.securityOperations().createLocalUser(principal, passwordToken);
+    }
+
+    try (AccumuloClient test_user_client =
+        Accumulo.newClient().from(getClientProps()).as(principal, 
token).build()) {
+      assertThrows(AccumuloSecurityException.class,
+          () -> test_user_client.resourceGroupOperations().create(rgid));
+    }
+
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      client.resourceGroupOperations().create(rgid);
+    }
+
+    try (AccumuloClient test_user_client =
+        Accumulo.newClient().from(getClientProps()).as(principal, 
token).build()) {
+      assertThrows(AccumuloSecurityException.class,
+          () -> test_user_client.resourceGroupOperations().remove(rgid));
+    }
+  }
+
+  @Test
+  public void testMultipleConfigurations() throws Exception {
+
+    final String FIRST = "FIRST";
+    final String SECOND = "SECOND";
+    final String THIRD = "THIRD";
+
+    final ResourceGroupId first = ResourceGroupId.of(FIRST);
+    final ResourceGroupId second = ResourceGroupId.of(SECOND);
+    final ResourceGroupId third = ResourceGroupId.of(THIRD);
+

Review Comment:
   Would it be useful to also set some stuff at the system level to test the 
layering works as expected?  Could see props at the system level that are 
overridden and not some that are not overridden. 



##########
test/src/main/java/org/apache/accumulo/test/conf/ResourceGroupConfigIT.java:
##########
@@ -207,4 +212,132 @@ private void checkProperty(InstanceOperations iops, 
ResourceGroupOperations ops,
     System.clearProperty(TServerClient.DEBUG_HOST);
   }
 
+  @Test
+  public void testDefaultResourceGroup() throws Exception {
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      Set<ResourceGroupId> rgs = client.resourceGroupOperations().list();
+      assertEquals(1, rgs.size());
+      assertEquals(ResourceGroupId.DEFAULT, rgs.iterator().next());
+      client.resourceGroupOperations().create(ResourceGroupId.DEFAULT);
+      assertThrows(AccumuloException.class,
+          () -> 
client.resourceGroupOperations().remove(ResourceGroupId.DEFAULT));
+    }
+  }
+
+  @Test
+  public void testPermissions() throws Exception {
+
+    ClusterUser testUser = getUser(0);
+
+    String principal = testUser.getPrincipal();
+    AuthenticationToken token = testUser.getToken();
+    PasswordToken passwordToken = null;
+    if (token instanceof PasswordToken) {
+      passwordToken = (PasswordToken) token;
+    }
+
+    ResourceGroupId rgid = ResourceGroupId.of("TEST_GROUP");
+
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      client.securityOperations().createLocalUser(principal, passwordToken);
+    }
+
+    try (AccumuloClient test_user_client =
+        Accumulo.newClient().from(getClientProps()).as(principal, 
token).build()) {
+      assertThrows(AccumuloSecurityException.class,
+          () -> test_user_client.resourceGroupOperations().create(rgid));
+    }
+
+    try (var client = Accumulo.newClient().from(getClientProps()).build()) {
+      client.resourceGroupOperations().create(rgid);
+    }
+
+    try (AccumuloClient test_user_client =
+        Accumulo.newClient().from(getClientProps()).as(principal, 
token).build()) {
+      assertThrows(AccumuloSecurityException.class,
+          () -> test_user_client.resourceGroupOperations().remove(rgid));
+    }

Review Comment:
   Could also try granting the user a permission and then seeing if they can 
create/remove.  This would test what permission is being checked for the 
create/remove operations.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to