You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2016/05/24 14:56:11 UTC

[2/2] incubator-geode git commit: GEODE-17: region access needed for destroy index on a specific region

GEODE-17: region access needed for destroy index on a specific region


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/caa1d150
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/caa1d150
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/caa1d150

Branch: refs/heads/develop
Commit: caa1d1509fbd57c4dc950163c307a6e615d31f04
Parents: 6483ae3
Author: Jinmei Liao <ji...@pivotal.io>
Authored: Mon May 23 09:24:18 2016 -0700
Committer: Jinmei Liao <ji...@pivotal.io>
Committed: Tue May 24 07:55:23 2016 -0700

----------------------------------------------------------------------
 .../internal/cli/commands/IndexCommands.java    | 163 ++++++++++---------
 .../security/CliCommandsSecurityTest.java       |   2 +-
 .../GeodeSecurityUtilWithIniFileJUnitTest.java  |   2 +-
 .../security/GfshCommandsSecurityTest.java      |   8 +-
 .../internal/security/TestCommand.java          |   1 +
 5 files changed, 91 insertions(+), 85 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java
index b863737..e744eea 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/commands/IndexCommands.java
@@ -322,8 +322,6 @@ public class IndexCommands extends AbstractCommandsSupport {
 
   @CliCommand(value = CliStrings.DESTROY_INDEX, help = CliStrings.DESTROY_INDEX__HELP)
   @CliMetaData(shellOnly = false, relatedTopic={CliStrings.TOPIC_GEMFIRE_REGION, CliStrings.TOPIC_GEMFIRE_DATA}, writesToSharedConfiguration=true)
-  @ResourceOperation(resource = Resource.DATA, operation = OperationCode.MANAGE)
-  //TODO : Add optioncontext for the index name.
   public Result destroyIndex(
       @CliOption(
       key = CliStrings.DESTROY_INDEX__NAME,
@@ -349,107 +347,114 @@ public class IndexCommands extends AbstractCommandsSupport {
     Result result = null;
     XmlEntity xmlEntity = null;
 
-    try {
+    if (StringUtils.isBlank(indexName)
+        && StringUtils.isBlank(regionPath)
+        && StringUtils.isBlank(memberNameOrID)
+        && StringUtils.isBlank(group)) {
+      return ResultBuilder.createUserErrorResult(CliStrings.format(CliStrings.PROVIDE_ATLEAST_ONE_OPTION, CliStrings.DESTROY_INDEX));
+    }
 
-      if (StringUtils.isBlank(indexName)
-          && StringUtils.isBlank(regionPath)
-          && StringUtils.isBlank(memberNameOrID)
-          && StringUtils.isBlank(group)) {
-        return ResultBuilder.createUserErrorResult(CliStrings.format(CliStrings.PROVIDE_ATLEAST_ONE_OPTION, CliStrings.DESTROY_INDEX));
-      }
-      String regionName = null;
-      final Cache cache = CacheFactory.getAnyInstance();
-      
-      if (!StringUtils.isBlank(regionPath)) {
-        regionName = regionPath.startsWith("/") ? regionPath.substring(1) : regionPath;
-      }
-      IndexInfo indexInfo = new IndexInfo(indexName, regionName);
+    String regionName = null;
+    final Cache cache = CacheFactory.getAnyInstance();
 
-      Set<DistributedMember> targetMembers = CliUtil.findAllMatchingMembers(group, memberNameOrID);
+    // If a regionName is specified, then authorize data manage on the regionName, otherwise, it requires data manage permission on all regions
+    if (!StringUtils.isBlank(regionPath)) {
+      regionName = regionPath.startsWith("/") ? regionPath.substring(1) : regionPath;
+      GeodeSecurityUtil.authorizeRegionManage(regionName);
+    }
+    else{
+      GeodeSecurityUtil.authorizeDataManage();
+    }
 
-      ResultCollector rc = CliUtil.executeFunction(destroyIndexFunction, indexInfo, targetMembers);
-      List<Object> funcResults = (List<Object>) rc.getResult();
+    IndexInfo indexInfo = new IndexInfo(indexName, regionName);
+    Set<DistributedMember> targetMembers = null;
 
-      Set<String> successfulMembers = new TreeSet<String>();
-      Map<String, Set<String>> indexOpFailMap = new HashMap<String, Set<String>>();
+    try {
+      targetMembers = CliUtil.findAllMatchingMembers(group, memberNameOrID);
+    }
+    catch (CommandResultException e) {
+      return e.getResult();
+    }
 
-      for (Object funcResult : funcResults){
+    ResultCollector rc = CliUtil.executeFunction(destroyIndexFunction, indexInfo, targetMembers);
+    List<Object> funcResults = (List<Object>) rc.getResult();
 
-        if (funcResult instanceof CliFunctionResult) {
-          CliFunctionResult cliFunctionResult = (CliFunctionResult) funcResult;
+    Set<String> successfulMembers = new TreeSet<String>();
+    Map<String, Set<String>> indexOpFailMap = new HashMap<String, Set<String>>();
 
-          if (cliFunctionResult.isSuccessful()) {
-            successfulMembers.add(cliFunctionResult.getMemberIdOrName());
-            
-            if (xmlEntity == null) {
-              xmlEntity = cliFunctionResult.getXmlEntity();
-            }
-          } else {
-            String exceptionMessage  = cliFunctionResult.getMessage();
-            Set<String> failedMembers = indexOpFailMap.get(exceptionMessage);
-
-            if (failedMembers == null) {
-              failedMembers = new TreeSet<String>();
-            }
-            failedMembers.add(cliFunctionResult.getMemberIdOrName());
-            indexOpFailMap.put(exceptionMessage, failedMembers);
-          }
-        }
+    for (Object funcResult : funcResults){
+      if(!(funcResult instanceof CliFunctionResult)){
+        continue;
       }
 
-      if (!successfulMembers.isEmpty()) {
-        InfoResultData infoResult = ResultBuilder.createInfoResultData();
+      CliFunctionResult cliFunctionResult = (CliFunctionResult) funcResult;
+      if (cliFunctionResult.isSuccessful()) {
+        successfulMembers.add(cliFunctionResult.getMemberIdOrName());
 
-        if (!StringUtils.isBlank(indexName)) {
-          if (!StringUtils.isBlank(regionPath)) {
-            infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__SUCCESS__MSG, indexName, regionPath));
-          } else {
-            infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__SUCCESS__MSG, indexName));
-          }
-        } else {
-           if (!StringUtils.isBlank(regionPath)) {
-             infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__ONLY__SUCCESS__MSG, regionPath));
-           } else {
-             infoResult.addLine(CliStrings.DESTROY_INDEX__ON__MEMBERS__ONLY__SUCCESS__MSG);
-           }
+        if (xmlEntity == null) {
+          xmlEntity = cliFunctionResult.getXmlEntity();
         }
+      } else {
+        String exceptionMessage  = cliFunctionResult.getMessage();
+        Set<String> failedMembers = indexOpFailMap.get(exceptionMessage);
 
-        int num = 0;
-        for (String memberId : successfulMembers) {
-          infoResult.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId)));;
+        if (failedMembers == null) {
+          failedMembers = new TreeSet<String>();
         }
-        result = ResultBuilder.buildResult(infoResult);
+        failedMembers.add(cliFunctionResult.getMemberIdOrName());
+        indexOpFailMap.put(exceptionMessage, failedMembers);
+      }
+    }
 
-      } else {
+    if (!successfulMembers.isEmpty()) {
+      InfoResultData infoResult = ResultBuilder.createInfoResultData();
 
-        ErrorResultData erd = ResultBuilder.createErrorResultData();
-        if (!StringUtils.isBlank(indexName)) {
-          erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__FAILURE__MSG, indexName));
+      if (!StringUtils.isBlank(indexName)) {
+        if (!StringUtils.isBlank(regionPath)) {
+          infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__SUCCESS__MSG, indexName, regionPath));
         } else {
-          erd.addLine("Indexes could not be destroyed for following reasons");
+          infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__SUCCESS__MSG, indexName));
         }
+      } else {
+         if (!StringUtils.isBlank(regionPath)) {
+           infoResult.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__ON__REGION__ONLY__SUCCESS__MSG, regionPath));
+         } else {
+           infoResult.addLine(CliStrings.DESTROY_INDEX__ON__MEMBERS__ONLY__SUCCESS__MSG);
+         }
+      }
+
+      int num = 0;
+      for (String memberId : successfulMembers) {
+        infoResult.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId)));;
+      }
+      result = ResultBuilder.buildResult(infoResult);
 
-        Set<String> exceptionMessages = indexOpFailMap.keySet();
+    } else {
 
-        for (String exceptionMessage : exceptionMessages) {
-          erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__REASON_MESSAGE, exceptionMessage));
-          erd.addLine(CliStrings.DESTROY_INDEX__EXCEPTION__OCCURRED__ON);
+      ErrorResultData erd = ResultBuilder.createErrorResultData();
+      if (!StringUtils.isBlank(indexName)) {
+        erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__FAILURE__MSG, indexName));
+      } else {
+        erd.addLine("Indexes could not be destroyed for following reasons");
+      }
 
-          Set<String> memberIds = indexOpFailMap.get(exceptionMessage);
-          int num = 0;
+      Set<String> exceptionMessages = indexOpFailMap.keySet();
 
-          for (String memberId : memberIds) {
-            erd.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId)));
-          }
-          erd.addLine("");
+      for (String exceptionMessage : exceptionMessages) {
+        erd.addLine(CliStrings.format(CliStrings.DESTROY_INDEX__REASON_MESSAGE, exceptionMessage));
+        erd.addLine(CliStrings.DESTROY_INDEX__EXCEPTION__OCCURRED__ON);
+
+        Set<String> memberIds = indexOpFailMap.get(exceptionMessage);
+        int num = 0;
+
+        for (String memberId : memberIds) {
+          erd.addLine(CliStrings.format(CliStrings.format(CliStrings.DESTROY_INDEX__NUMBER__AND__MEMBER, ++num, memberId)));
         }
-        result = ResultBuilder.buildResult(erd);
+        erd.addLine("");
       }
-    } catch (CommandResultException crex) {
-      result = crex.getResult();
-    } catch (Exception e) {
-      result = ResultBuilder.createGemFireErrorResult(e.getMessage());
+      result = ResultBuilder.buildResult(erd);
     }
+
     
     if (xmlEntity != null) {
       result.setCommandPersisted((new SharedConfigurationWriter()).deleteXmlEntity(xmlEntity, group !=null ? group.split(",") : null));

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java
index 3ccd71c..b4864e9 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java
@@ -72,7 +72,7 @@ public class CliCommandsSecurityTest {
         }
         catch(NotAuthorizedException e){
           assertTrue(e.getMessage()+" should contain "+command.getPermission(),
-            e.getMessage().contains(command.getPermission().toString()));
+            e.getMessage().contains("["+command.getPermission().toString()+"]"));
         }
       }
     }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java
index 63bf447..61c913b 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilWithIniFileJUnitTest.java
@@ -140,7 +140,7 @@ public class GeodeSecurityUtilWithIniFileJUnitTest {
   }
 
   private void assertNotAuthorized(OperationContext context){
-    assertThatThrownBy(()-> GeodeSecurityUtil.authorize(context)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining(context.toString());
+    assertThatThrownBy(()-> GeodeSecurityUtil.authorize(context)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining("["+context.toString()+"]");
   }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java
index 1a15367..9e24317 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java
@@ -112,19 +112,19 @@ public class GfshCommandsSecurityTest {
 
   @Test
   @JMXConnectionConfiguration(user = "regionA-reader", password = "1234567")
-  public void testregionAReader() throws Exception{
+  public void testRegionAReader() throws Exception{
     runCommandsWithAndWithout("DATA:READ:RegionA");
   }
 
   @Test
   @JMXConnectionConfiguration(user = "regionA-writer", password = "1234567")
-  public void testregionAWriter() throws Exception{
+  public void testRegionAWriter() throws Exception{
     runCommandsWithAndWithout("DATA:WRITE:RegionA");
   }
 
   @Test
   @JMXConnectionConfiguration(user = "regionA-manager", password = "1234567")
-  public void testregionAManager() throws Exception{
+  public void testRegionAManager() throws Exception{
     runCommandsWithAndWithout("DATA:MANAGE:RegionA");
   }
 
@@ -168,7 +168,7 @@ public class GfshCommandsSecurityTest {
       assertEquals(ResultBuilder.ERRORCODE_UNAUTHORIZED, ((ErrorResultData) result.getResultData()).getErrorCode());
       String resultMessage = result.getContent().toString();
       String permString = other.getPermission().toString();
-      assertTrue(resultMessage+" does not contain "+permString,resultMessage.contains(permString));
+      assertTrue(resultMessage+" does not contain "+permString,resultMessage.contains("["+permString+"]"));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/caa1d150/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java
index 4b482a9..2ddc6ee 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestCommand.java
@@ -151,6 +151,7 @@ public class TestCommand {
     createTestCommand("create index --name=myKeyIndex --expression=region1.Id --region=RegionA --type=key", regionAManage);
     createTestCommand("define index --name=myIndex1 --expression=exp1 --region=/RegionA", regionAManage);
     createTestCommand("destroy index --member=server2", dataManage);
+    createTestCommand("destroy index --region=RegionA --member=server2", regionAManage);
     createTestCommand("list indexes", clusterRead);
 
     //LauncherLifecycleCommands